From aed190dc6d1b22233a405013d02fd1fe673a36d4 Mon Sep 17 00:00:00 2001
From: Raphael Sofaer <raphael@joindiaspora.com>
Date: Fri, 25 Mar 2011 16:09:02 -0700
Subject: [PATCH] cucumber for comment retractions, modified views

---
 app/controllers/comments_controller.rb        | 21 ++++++-
 app/helpers/sockets_helper.rb                 |  2 +-
 app/models/relayable_retraction.rb            |  5 ++
 app/views/comments/_comment.html.haml         |  3 +
 app/views/comments/_comment.mobile.haml       |  3 +
 app/views/comments/_new_comment.mobile.haml   |  2 +-
 app/views/shared/_stream_element.html.haml    |  4 +-
 config/routes.rb                              |  2 +-
 features/comments.feature                     | 14 +++++
 features/posts.feature                        |  2 +
 features/step_definitions/custom_web_steps.rb | 15 ++++-
 public/javascripts/stream.js                  | 22 ++++++-
 public/javascripts/web-socket-receiver.js     |  1 -
 public/stylesheets/sass/application.sass      | 61 +++++++-----------
 spec/controllers/comments_controller_spec.rb  | 63 +++++++++++++++----
 spec/models/retraction_spec.rb                | 10 ---
 16 files changed, 159 insertions(+), 71 deletions(-)

diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
index 9f66f2b15..97f9f5886 100644
--- a/app/controllers/comments_controller.rb
+++ b/app/controllers/comments_controller.rb
@@ -34,7 +34,7 @@ class CommentsController < ApplicationController
             render(:json => json, :status => 201)
           }
           format.html{ render :nothing => true, :status => 201 }
-          format.mobile{ redirect_to status_message_path(@comment.post_id) }
+          format.mobile{ redirect_to @comment.post }
         end
       else
         render :nothing => true, :status => 422
@@ -44,4 +44,23 @@ class CommentsController < ApplicationController
     end
   end
 
+  def destroy
+    if @comment = Comment.where(:id => params[:id]).first
+      if current_user.owns?(@comment) || current_user.owns?(@comment.parent)
+        current_user.retract(@comment)
+        respond_to do |format|
+          format.mobile{ redirect_to @comment.post }
+          format.js {render :nothing => true, :status => 204}
+        end
+      else
+        respond_to do |format|
+          format.mobile {redirect_to :back}
+          format.js {render :nothing => true, :status => 401}
+        end
+      end
+    else
+      render :nothing => true, :status => 404
+    end
+  end
+
 end
diff --git a/app/helpers/sockets_helper.rb b/app/helpers/sockets_helper.rb
index 92b5fa11f..e5d94977e 100644
--- a/app/helpers/sockets_helper.rb
+++ b/app/helpers/sockets_helper.rb
@@ -48,7 +48,7 @@ module SocketsHelper
         v = render_to_string(:partial => 'people/person', :locals => person_hash)
 
       elsif object.is_a? Comment
-        v = render_to_string(:partial => 'comments/comment', :locals => {:comment => object, :person => object.author})
+        v = render_to_string(:partial => 'comments/comment', :locals => {:comment => object, :person => object.author, :current_user => user})
 
       elsif object.is_a? Like
         v = render_to_string(:partial => 'likes/likes', :locals => {:likes => object.post.likes, :dislikes => object.post.dislikes})
diff --git a/app/models/relayable_retraction.rb b/app/models/relayable_retraction.rb
index a8c17ea3c..1f593f5cc 100644
--- a/app/models/relayable_retraction.rb
+++ b/app/models/relayable_retraction.rb
@@ -7,6 +7,7 @@ class RelayableRetraction
   include Diaspora::Webhooks
   include Diaspora::Encryptable
 
+  xml_name :relayable_retraction
   xml_attr :target_guid
   xml_attr :target_type
   xml_attr :sender_handle
@@ -37,6 +38,10 @@ class RelayableRetraction
     @sender.diaspora_handle
   end
 
+  def diaspora_handle
+    self.sender_handle
+  end
+
   def subscribers(user)
     self.target.subscribers(user)
   end
diff --git a/app/views/comments/_comment.html.haml b/app/views/comments/_comment.html.haml
index cd1b8c108..a0d8ae137 100644
--- a/app/views/comments/_comment.html.haml
+++ b/app/views/comments/_comment.html.haml
@@ -3,6 +3,9 @@
 -#   the COPYRIGHT file.
 
 %li.comment.posted{:data=>{:guid => comment.id}, :class => ("hidden" if(defined? hidden))}
+  - if current_user.owns?(comment) || current_user.owns?(comment.post)
+    .right.controls
+      = link_to image_tag('deletelabel.png'), comment_path(comment), :confirm => t('are_you_sure'), :method => :delete, :remote => true, :class => "delete comment_delete", :title => t('delete')
   = person_image_link(comment.author)
   .content
     .from
diff --git a/app/views/comments/_comment.mobile.haml b/app/views/comments/_comment.mobile.haml
index 1c0943bf6..0ef9736d0 100644
--- a/app/views/comments/_comment.mobile.haml
+++ b/app/views/comments/_comment.mobile.haml
@@ -6,6 +6,9 @@
   .right
     %span.time
       = comment.created_at ? time_ago_in_words(comment.created_at) : time_ago_in_words(Time.now)
+    .controls
+      = link_to image_tag('deletelabel.png'), comment, :confirm => t('are_you_sure'), :method => :delete, :class => "delete comment_delete", :title => t('delete')
+
 
   = person_image_link(comment.author)
   .content
diff --git a/app/views/comments/_new_comment.mobile.haml b/app/views/comments/_new_comment.mobile.haml
index a5e2d2bca..08ae44a00 100644
--- a/app/views/comments/_new_comment.mobile.haml
+++ b/app/views/comments/_new_comment.mobile.haml
@@ -2,7 +2,7 @@
 -#   licensed under the Affero General Public License version 3 or later.  See
 -#   the COPYRIGHT file.
 
-= form_tag( comments_path, :id => "new_comment_on_#{post_id}", :class => 'new_comment', :remote => true) do
+= form_tag( comments_path, :id => "new_comment_on_#{post_id}", :class => 'new_comment') do
   = hidden_field_tag :post_id,  post_id, :id => "post_id_on_#{post_id}"
   = text_area_tag :text, nil, :rows => 2, :class => "comment_box",:id => "comment_text_on_#{post_id}"
 
diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml
index f4946eb77..eada69d58 100644
--- a/app/views/shared/_stream_element.html.haml
+++ b/app/views/shared/_stream_element.html.haml
@@ -4,11 +4,11 @@
 
 .stream_element{:data=>{:guid=>post.id}}
   - if current_user && post.author.owner_id == current_user.id
-    .right.hidden.controls
+    .right.controls
       - reshare_aspects = aspects_without_post(all_aspects, post)
       - unless reshare_aspects.empty?
         = render 'shared/reshare', :aspects => reshare_aspects, :post => post
-      = link_to image_tag('deletelabel.png'), status_message_path(post), :confirm => t('are_you_sure'), :method => :delete, :remote => true, :class => "delete", :title => t('delete')
+      = link_to image_tag('deletelabel.png'), status_message_path(post), :confirm => t('are_you_sure'), :method => :delete, :remote => true, :class => "delete stream_element_delete", :title => t('delete')
 
   = person_image_link(post.author, :size => :thumb_small)
 
diff --git a/config/routes.rb b/config/routes.rb
index a96ccba33..ad2ebf585 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -4,7 +4,7 @@
 
 Diaspora::Application.routes.draw do
   resources :status_messages, :only => [:new, :create, :destroy, :show]
-  resources :comments,        :only => [:create]
+  resources :comments,        :only => [:create, :destroy]
   resources :requests,        :only => [:destroy, :create]
   resource :likes,            :only => [:create]
 
diff --git a/features/comments.feature b/features/comments.feature
index 5f4ca52fc..d28243e90 100644
--- a/features/comments.feature
+++ b/features/comments.feature
@@ -47,3 +47,17 @@ Feature: commenting
     And I press "comment"
     Then I should see "hahaha" within "li.comment div.content"
     And I should see "less than a minute ago" within "li.comment time"
+
+  Scenario: delete a post
+    When I sign in as "bob@bob.bob"
+    And I am on "alice@alice.alice"'s page
+    Then I should see "Look at this dog"
+    When I focus the comment field
+    And I fill in "comment" with "is that a poodle?"
+    And I press "comment"
+    And I wait for the ajax to finish
+    When I hover over the comment
+    And I preemptively confirm the alert
+    And I click to delete the first comment
+    And I wait for the ajax to finish
+    Then I should not see "is that a poodle?"
diff --git a/features/posts.feature b/features/posts.feature
index e24713a0c..5fd5484e7 100644
--- a/features/posts.feature
+++ b/features/posts.feature
@@ -28,10 +28,12 @@ Feature: posting
       Given I expand the publisher
       When I fill in "status_message_fake_text" with "I am eating a yogurt"
         And I press "Share"
+        And I wait for the ajax to finish
         And I follow "All aspects"
         And I hover over the post
         And I preemptively confirm the alert
         And I click to delete the first post
+        And I wait for the ajax to finish
         And I follow "All aspects"
         Then I should not see "I am eating a yogurt"
 
diff --git a/features/step_definitions/custom_web_steps.rb b/features/step_definitions/custom_web_steps.rb
index 9ff8a9e40..40640a85e 100644
--- a/features/step_definitions/custom_web_steps.rb
+++ b/features/step_definitions/custom_web_steps.rb
@@ -36,12 +36,21 @@ When /^I append "([^"]*)" to the publisher$/ do |stuff|
   end
 end
 
-And /^I hover over the post$/ do
-  page.execute_script('$(".stream_element").first().mouseover()')
+And /^I hover over the (\w*)$/ do |element|
+  if element == 'post'
+    name = 'stream_element'
+  elsif element == 'comment'
+    name = 'comment.posted'
+  end
+  page.execute_script("$(\".#{name}\").first().mouseover()")
 end
 
 When /^I click to delete the first post$/ do
-  page.execute_script('$(".stream_element").first().find(".delete").click()')
+  page.execute_script('$(".stream_element").first().find(".stream_element_delete").click()')
+end
+
+When /^I click to delete the first comment$/ do
+  page.execute_script('$(".comment.posted").first().find(".comment_delete").click()')
 end
 
 And /^I click "([^"]*)" button$/ do |arg1|
diff --git a/public/javascripts/stream.js b/public/javascripts/stream.js
index 105a1680a..8f981ca97 100644
--- a/public/javascripts/stream.js
+++ b/public/javascripts/stream.js
@@ -58,7 +58,7 @@ var Stream = {
       json = $.parseJSON(json);
       WebSocketReceiver.processLike(json.post_id, json.html);
     });
-    
+
     $('.like_it, .dislike_it').live('ajax:failure', function(data, html, xhr) {
       Diaspora.widgets.alert.alert('Failed to like/dislike!');
       $(this).parent().fadeIn('fast');
@@ -106,8 +106,24 @@ var Stream = {
       Diaspora.widgets.alert.alert('Failed to post message!');
     });
 
-    $(".stream").find(".delete").live('ajax:success', function(data, html, xhr) {
-      $(this).parents(".stream_element").hide('blind', { direction: 'vertical' }, 300);
+    $(".stream").find(".stream_element_delete", ".stream_element").live('ajax:success', function(data, html, xhr) {
+      var target = $(this).parents(".stream_element");
+      target.hide('blind', { direction: 'vertical' }, 300, function(){ $(this).remove() });
+    });
+
+    $(".stream").find(".comment_delete", ".comment").live('ajax:success', function(data, html, xhr) {
+      var element = $(this),
+          target = element.parents(".comment"),
+          post = element.closest('.stream_element'),
+          toggler = post.find('.show_post_comments');
+
+      target.hide('blind', { direction: 'vertical' }, 300, function(){
+        $(this).remove();
+        toggler.html(
+          toggler.html().replace(/\d+/,$('.comments', post).find('li').length -1)
+        );
+      });
+
     });
   },
 
diff --git a/public/javascripts/web-socket-receiver.js b/public/javascripts/web-socket-receiver.js
index f3c7cd8da..c541c6729 100644
--- a/public/javascripts/web-socket-receiver.js
+++ b/public/javascripts/web-socket-receiver.js
@@ -98,7 +98,6 @@ var WebSocketReceiver = {
       var post = $("*[data-guid='"+postId+"']'"),
           prevComments = $('.comment.posted', post);
 
-      
       if(prevComments.length > 0) {
         prevComments.last().after(
           $(html).fadeIn("fast", function(){})
diff --git a/public/stylesheets/sass/application.sass b/public/stylesheets/sass/application.sass
index 831038c9f..706548001 100644
--- a/public/stylesheets/sass/application.sass
+++ b/public/stylesheets/sass/application.sass
@@ -249,13 +249,6 @@ header
       :bottom 1px solid #ddd
       :top 1px solid #fff
 
-    &:hover
-      .right
-        :display inline
-      .from
-        a
-          :color $blue
-
     .youtube-player, .vimeo-player
       :border none
       :height 370px
@@ -398,7 +391,6 @@ header
         :margin
           :top -1px
 
-
   .delete
     @include opacity(0.6)
     :padding 5px
@@ -406,6 +398,27 @@ header
     &:hover
       @include opacity(1)
 
+  .right
+    :position absolute
+    :right 12px
+
+  .controls
+    :display none
+    :z-index 6
+    :color #999
+    a
+      :color #999
+      :font
+        :weight normal
+
+  &:hover, .comment:hover
+    > .controls
+      :display inline
+
+    .from
+      a
+        :color $blue
+
 .time,
 .timeago
   :color #999
@@ -595,6 +608,9 @@ div.dislikes
         a
           :color #444
 
+    .right
+      :right 4px
+
     form
       :margin
         :top -5px
@@ -635,9 +651,6 @@ div.dislikes
         :min-height 2.4em
 
 .comments 
-  time
-    :margin
-      :right -15px
   .timeago
     :color #999
 
@@ -691,32 +704,6 @@ a.paginate, #infscr-loading
 #main_stream .pagination
   :display none
 
-.stream_element
-  .right
-    :position absolute
-    :right 12px
-
-    &.controls
-      :z-index 6
-      :background
-        :color $background
-      :font
-        :size 12px
-      :color #999
-      :padding
-        :left 100px
-      a
-        :color #999
-        :font
-          :weight normal
-  &:hover
-    .controls
-      :display inline
-
-.stream_element:hover
-  .right
-    :display inline
-
 .request_buttons
   :position absolute
   :right 0
diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb
index 9682b40e5..308b4866c 100644
--- a/spec/controllers/comments_controller_spec.rb
+++ b/spec/controllers/comments_controller_spec.rb
@@ -8,13 +8,11 @@ describe CommentsController do
   render_views
 
   before do
-    @user1 = alice
-    @user2 = bob
+    @aspect1 = alice.aspects.first
+    @aspect2 = bob.aspects.first
 
-    @aspect1 = @user1.aspects.first
-    @aspect2 = @user2.aspects.first
-
-    sign_in :user, @user1
+    @controller.stub(:current_user).and_return(alice)
+    sign_in :user, alice
   end
 
   describe '#create' do
@@ -24,7 +22,7 @@ describe CommentsController do
     }
     context "on my own post" do
       before do
-        @post = @user1.post :status_message, :text => 'GIANTS', :to => @aspect1.id
+        @post = alice.post :status_message, :text => 'GIANTS', :to => @aspect1.id
       end
       it 'responds to format js' do
         post :create, comment_hash.merge(:format => 'js')
@@ -35,7 +33,7 @@ describe CommentsController do
 
     context "on a post from a contact" do
       before do
-        @post = @user2.post :status_message, :text => 'GIANTS', :to => @aspect2.id
+        @post = bob.post :status_message, :text => 'GIANTS', :to => @aspect2.id
       end
       it 'comments' do
         post :create, comment_hash
@@ -45,10 +43,10 @@ describe CommentsController do
         new_user = Factory.create(:user)
         comment_hash[:author_id] = new_user.person.id.to_s
         post :create, comment_hash
-        Comment.find_by_text(comment_hash[:text]).author_id.should == @user1.person.id
+        Comment.find_by_text(comment_hash[:text]).author_id.should == alice.person.id
       end
       it "doesn't overwrite id" do
-        old_comment = @user1.comment("hello", :on => @post)
+        old_comment = alice.comment("hello", :on => @post)
         comment_hash[:id] = old_comment.id
         post :create, comment_hash
         old_comment.reload.text.should == 'hello'
@@ -59,10 +57,53 @@ describe CommentsController do
         @post = eve.post :status_message, :text => 'GIANTS', :to => eve.aspects.first.id
       end
       it 'posts no comment' do
-        @user1.should_not_receive(:comment)
+        alice.should_not_receive(:comment)
         post :create, comment_hash
         response.code.should == '422'
       end
     end
   end
+
+  describe '#destroy' do
+    context 'your post' do
+      before do
+        @message = alice.post(:status_message, :text => "hey", :to => @aspect1.id)
+        @comment = alice.comment("hey", :on => @message)
+        @comment2 = bob.comment("hey", :on => @message)
+        @comment3 = eve.comment("hey", :on => @message)
+      end
+      it 'lets the user delete his comment' do
+        alice.should_receive(:retract).with(@comment)
+        delete :destroy, :format => "js",  :id => @comment.id
+        response.status.should == 204
+      end
+
+      it "lets the user destroy other people's comments" do
+        alice.should_receive(:retract).with(@comment2)
+        delete :destroy, :format => "js",  :id => @comment2.id
+        response.status.should == 204
+      end
+    end
+
+    context "another user's post" do
+      before do
+        @message = bob.post(:status_message, :text => "hey", :to => bob.aspects.first.id)
+        @comment = alice.comment("hey", :on => @message)
+        @comment2 = bob.comment("hey", :on => @message)
+        @comment3 = eve.comment("hey", :on => @message)
+      end
+
+      it 'let the user delete his comment' do
+        alice.should_receive(:retract).with(@comment)
+        delete :destroy, :format => "js",  :id => @comment.id
+        response.status.should == 204
+      end
+
+      it 'does not let the user destroy comments he does not own' do
+        alice.should_not_receive(:retract).with(@comment2)
+        delete :destroy, :format => "js",  :id => @comment3.id
+        response.status.should == 401
+      end
+    end
+  end
 end
diff --git a/spec/models/retraction_spec.rb b/spec/models/retraction_spec.rb
index 7db1f4207..6f0a648f3 100644
--- a/spec/models/retraction_spec.rb
+++ b/spec/models/retraction_spec.rb
@@ -44,14 +44,4 @@ describe Retraction do
       end
     end
   end
-
-  describe 'dispatching' do
-    it 'should dispatch a retraction on delete' do
-      Factory.create(:person)
-      m = mock()
-      m.should_receive(:post)
-      Postzord::Dispatch.should_receive(:new).with(instance_of(User), instance_of(Retraction)).and_return(m)
-      post.destroy
-    end
-  end
 end