diff --git a/app/controllers/likes_controller.rb b/app/controllers/likes_controller.rb index 41edf780b..83be0a7cc 100644 --- a/app/controllers/likes_controller.rb +++ b/app/controllers/likes_controller.rb @@ -9,12 +9,6 @@ class LikesController < ApplicationController respond_to :html, :mobile, :json def create - if params[:post_id] - target = current_user.find_visible_post_by_id params[:post_id] - else - target = Comment.find(params[:comment_id]) - end - positive = (params[:positive] == 'true') ? true : false if target @like = current_user.build_like(:positive => positive, :target => target) @@ -39,7 +33,6 @@ class LikesController < ApplicationController def destroy if @like = Like.where(:id => params[:id], :author_id => current_user.person.id).first current_user.retract(@like) - pp @like respond_to do |format| format.all{} format.js{ render 'likes/update' } @@ -53,11 +46,21 @@ class LikesController < ApplicationController end def index - if target = current_user.find_visible_post_by_id(params[:post_id]) + if target @likes = target.likes.includes(:author => :profile) render :layout => false else render :nothing => true, :status => 404 end end + + def target + @target ||= if params[:post_id] + current_user.find_visible_post_by_id(params[:post_id]) + else + comment = Comment.find(params[:comment_id]) + comment = nil unless current_user.find_visible_post_by_id(comment.post_id) + comment + end + end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 81c4b9d0e..4014e9990 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -62,4 +62,10 @@ class Comment < ActiveRecord::Base def parent= parent self.post = parent end + + # @return [Integer] + def update_likes_counter + self.likes_count = self.likes.count + self.save + end end diff --git a/app/models/like.rb b/app/models/like.rb index f86c28dc6..a645aca71 100644 --- a/app/models/like.rb +++ b/app/models/like.rb @@ -17,12 +17,20 @@ class Like < ActiveRecord::Base xml_attr :positive xml_attr :diaspora_handle - belongs_to :target, :polymorphic => true #, :counter_cache => true + belongs_to :target, :polymorphic => true belongs_to :author, :class_name => 'Person' validates_uniqueness_of :target_id, :scope => [:target_type, :author_id] validates_presence_of :author, :target + after_create do + self.target.update_likes_counter + end + + after_destroy do + self.target.update_likes_counter + end + def diaspora_handle self.author.diaspora_handle end @@ -44,6 +52,8 @@ class Like < ActiveRecord::Base end def notification_type(user, person) + #TODO(dan) need to have a notification for likes on comments, until then, return nil + return nil if self.target_type == "Comment" Notifications::Liked if self.target.author == user.person && user.person != person end end diff --git a/app/models/post.rb b/app/models/post.rb index 91f400ec2..f3c9ab9cc 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -52,6 +52,12 @@ class Post < ActiveRecord::Base new_post end + # @return [Integer] + def update_likes_counter + self.likes_count = self.likes.count + self.save + end + # @return Returns true if this Post will accept updates (i.e. updates to the caption of a photo). def mutable? false diff --git a/app/models/user.rb b/app/models/user.rb index 4b24f4e26..f8c2ecf47 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -217,17 +217,8 @@ class User < ActiveRecord::Base ######### Posts and Such ############### def retract(target) if target.respond_to?(:relayable?) && target.relayable? - - parent = if target.parent.instance_of?(Comment) - target.parent.parent - else - target.parent - end - - aspects = parent.aspects retraction = RelayableRetraction.build(self, target) else - aspects = target.aspects retraction = Retraction.for(target) end diff --git a/app/views/comments/_comment.html.haml b/app/views/comments/_comment.html.haml index 923929ac2..516266519 100644 --- a/app/views/comments/_comment.html.haml +++ b/app/views/comments/_comment.html.haml @@ -22,9 +22,9 @@ · .likes - = render "likes/likes_container", :target_id => comment.id, :likes_count => comment.likes.count + = render "likes/likes_container", :target_id => comment.id, :likes_count => comment.likes_count - - if comment.likes.count > 0 + - if comment.likes_count > 0 · - unless (defined?(@commenting_disabled) && @commenting_disabled) diff --git a/app/views/likes/update.js.erb b/app/views/likes/update.js.erb index 7a447796e..c93faafa3 100644 --- a/app/views/likes/update.js.erb +++ b/app/views/likes/update.js.erb @@ -1,2 +1,2 @@ $(".like_action", "#<%=@like.target.guid%>").first().html("<%= escape_javascript(like_action(@like.target))%>"); -WebSocketReceiver.processLike("<%=@like.target.guid%>", "<%= escape_javascript(render("likes/likes_container", :target_id => @like.target_id, :likes_count => @like.target.likes.count)) %>"); +WebSocketReceiver.processLike("<%=@like.target.guid%>", "<%= escape_javascript(render("likes/likes_container", :target_id => @like.target_id, :likes_count => @like.target.likes_count)) %>"); diff --git a/app/views/photos/show.html.haml b/app/views/photos/show.html.haml index 9b9f475ef..d71b1f6bf 100644 --- a/app/views/photos/show.html.haml +++ b/app/views/photos/show.html.haml @@ -65,5 +65,5 @@ #photo_stream.stream.show %div{:data=>{:guid=>parent.id}} .likes_container - = render "likes/likes_container", :post_id => parent.id, :likes_count => parent.likes.count + = render "likes/likes_container", :post_id => parent.id, :likes_count => parent.likes_count = render "comments/comments", :post => parent, :comments => parent.comments, :always_expanded => true diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml index d8fb9b2f1..415a96811 100644 --- a/app/views/shared/_stream_element.html.haml +++ b/app/views/shared/_stream_element.html.haml @@ -56,6 +56,6 @@ = link_to t('comments.new_comment.comment'), '#', :class => 'focus_comment_textarea' .likes - = render "likes/likes_container", :target_id => post.id, :likes_count => post.likes.count, :current_user => current_user + = render "likes/likes_container", :target_id => post.id, :likes_count => post.likes_count, :current_user => current_user = render "comments/comments", :post => post, :current_user => current_user, :commenting_disabled => (defined?(@commenting_disabled) && @commenting_disabled) diff --git a/db/migrate/20110707234802_likes_on_comments.rb b/db/migrate/20110707234802_likes_on_comments.rb index 2df5e02c5..3438c4a9b 100644 --- a/db/migrate/20110707234802_likes_on_comments.rb +++ b/db/migrate/20110707234802_likes_on_comments.rb @@ -1,19 +1,26 @@ class LikesOnComments < ActiveRecord::Migration def self.up + remove_foreign_key :likes, :posts + add_column :likes, :target_type, :string, :null => false - add_column :likes, :target_id, :integer, :null => false - remove_column :posts, :likes_count + rename_column :likes, :post_id, :target_id + + add_column :comments, :likes_count, :integer, :default => 0, :null => false execute < true end def self.down - add_column :posts, :likes_count, :integer + remove_column :comments, :likes_count + remove_column :likes, :target_type rename_column :likes, :target_id, :post_id add_index :likes, :post_id + remove_index :likes, :target_id end end diff --git a/db/schema.rb b/db/schema.rb index e05d2feb8..6ca59081c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -46,15 +46,16 @@ ActiveRecord::Schema.define(:version => 20110707234802) do add_index "aspects", ["user_id"], :name => "index_aspects_on_user_id" create_table "comments", :force => true do |t| - t.text "text", :null => false - t.integer "post_id", :null => false - t.integer "author_id", :null => false - t.string "guid", :null => false + t.text "text", :null => false + t.integer "post_id", :null => false + t.integer "author_id", :null => false + t.string "guid", :null => false t.text "author_signature" t.text "parent_author_signature" t.text "youtube_titles" t.datetime "created_at" t.datetime "updated_at" + t.integer "likes_count", :default => 0, :null => false end add_index "comments", ["author_id"], :name => "index_comments_on_person_id" @@ -110,7 +111,7 @@ ActiveRecord::Schema.define(:version => 20110707234802) do create_table "likes", :force => true do |t| t.boolean "positive", :default => true - t.integer "post_id" + t.integer "target_id" t.integer "author_id" t.string "guid" t.text "author_signature" @@ -118,12 +119,11 @@ ActiveRecord::Schema.define(:version => 20110707234802) do t.datetime "created_at" t.datetime "updated_at" t.string "target_type", :null => false - t.integer "target_id", :null => false end add_index "likes", ["author_id"], :name => "likes_author_id_fk" add_index "likes", ["guid"], :name => "index_likes_on_guid", :unique => true - add_index "likes", ["post_id"], :name => "index_likes_on_post_id" + add_index "likes", ["target_id"], :name => "index_likes_on_post_id" create_table "mentions", :force => true do |t| t.integer "post_id", :null => false @@ -268,6 +268,7 @@ ActiveRecord::Schema.define(:version => 20110707234802) do t.string "actor_url" t.integer "objectId" t.string "status_message_guid" + t.integer "likes_count", :default => 0 end add_index "posts", ["author_id"], :name => "index_posts_on_person_id" @@ -419,7 +420,6 @@ ActiveRecord::Schema.define(:version => 20110707234802) do add_foreign_key "invitations", "users", :name => "invitations_sender_id_fk", :column => "sender_id", :dependent => :delete add_foreign_key "likes", "people", :name => "likes_author_id_fk", :column => "author_id", :dependent => :delete - add_foreign_key "likes", "posts", :name => "likes_post_id_fk", :dependent => :delete add_foreign_key "messages", "conversations", :name => "messages_conversation_id_fk", :dependent => :delete add_foreign_key "messages", "people", :name => "messages_author_id_fk", :column => "author_id", :dependent => :delete diff --git a/lib/diaspora/relayable.rb b/lib/diaspora/relayable.rb index 4ebbae7ce..e45bfb623 100644 --- a/lib/diaspora/relayable.rb +++ b/lib/diaspora/relayable.rb @@ -55,7 +55,7 @@ module Diaspora Postzord::Dispatch.new(user, object).post end - object.socket_to_user(user, :aspect_ids => object.parent.aspect_ids) if object.respond_to? :socket_to_user + object.socket_to_user(user) if object.respond_to? :socket_to_user if object.after_receive(user, person) object end diff --git a/lib/diaspora/user/querying.rb b/lib/diaspora/user/querying.rb index 4a6fa6680..3e7cf3478 100644 --- a/lib/diaspora/user/querying.rb +++ b/lib/diaspora/user/querying.rb @@ -7,7 +7,7 @@ module Diaspora module Querying def find_visible_post_by_id( id, opts={} ) - post = Post.where(:id => id).joins(:contacts).where(:contacts => {:user_id => self.id}).where(opts).first + post = Post.where(:id => id).joins(:contacts).where(:contacts => {:user_id => self.id}).where(opts).select("posts.*").first post ||= Post.where(:id => id, :author_id => self.person.id).where(opts).first post ||= Post.where(:id => id, :public => true).where(opts).first end diff --git a/public/javascripts/content-updater.js b/public/javascripts/content-updater.js index c9b074cc8..028670c65 100644 --- a/public/javascripts/content-updater.js +++ b/public/javascripts/content-updater.js @@ -6,9 +6,9 @@ var ContentUpdater = { addPostToStream: function(html) { var streamElement = $(html); - var postId = streamElement.id; + var postGUID = streamElement.id; - if($(".stream_element#" + postId).length === 0) { + if($("#"+postGUID).length === 0) { if($("#no_posts").length) { $("#no_posts").detach(); } @@ -17,14 +17,14 @@ var ContentUpdater = { streamElement.find("label").inFieldLabels(); }); - Diaspora.widgets.publish("stream/postAdded", [postId]); + Diaspora.widgets.publish("stream/postAdded", [postGUID]); Diaspora.widgets.timeago.updateTimeAgo(); Diaspora.widgets.directionDetector.updateBinds(); } }, - addLikesToPost: function(postId, html) { - var post = $("div#" + postId); + addLikesToPost: function(postGUID, html) { + var post = $("#" + postGUID); $(".likes_container", post) .fadeOut("fast") diff --git a/public/javascripts/web-socket-receiver.js b/public/javascripts/web-socket-receiver.js index e7f32eeb2..1ae0db45a 100644 --- a/public/javascripts/web-socket-receiver.js +++ b/public/javascripts/web-socket-receiver.js @@ -33,7 +33,12 @@ var WebSocketReceiver = { WebSocketReceiver.processPerson(obj); } else { - WSR.debug("got a " + obj['class'] + " for aspects " + obj.aspect_ids); + debug_string = "got a " + obj['class']; + if(obj.aspect_ids !== undefined){ + debug_string += " for aspects " + obj.aspect_ids; + } + + WSR.debug(debug_string); if (obj['class']=="retractions") { WebSocketReceiver.processRetraction(obj.post_id); diff --git a/spec/controllers/likes_controller_spec.rb b/spec/controllers/likes_controller_spec.rb index b017e37dd..71a2e097d 100644 --- a/spec/controllers/likes_controller_spec.rb +++ b/spec/controllers/likes_controller_spec.rb @@ -15,108 +15,122 @@ describe LikesController do sign_in :user, @user1 end - describe '#create' do - let(:like_hash) { - {:positive => 1, - :post_id => "#{@post.id}"} - } - let(:dislike_hash) { - {:positive => 0, - :post_id => "#{@post.id}"} - } + [Comment, Post].each do |class_const| + context class_const.to_s do + let(:id_field){ + "#{class_const.to_s.underscore}_id" + } - context "on my own post" do - before do - @post = @user1.post :status_message, :text => "AWESOME", :to => @aspect1.id + describe '#create' do + let(:like_hash) { + {:positive => 1, + id_field => "#{@target.id}"} + } + let(:dislike_hash) { + {:positive => 0, + id_field => "#{@target.id}"} + } + + context "on my own post" do + before do + @target = @user1.post :status_message, :text => "AWESOME", :to => @aspect1.id + + @target = @user1.comment "hey", :post => @target if class_const == Comment + end + + it 'responds to format js' do + post :create, like_hash.merge(:format => 'js') + response.code.should == '201' + end + end + + context "on a post from a contact" do + before do + @target = @user2.post :status_message, :text => "AWESOME", :to => @aspect2.id + @target = @user2.comment "hey", :post => @target if class_const == Comment + end + + it 'likes' do + post :create, like_hash + response.code.should == '201' + end + + it 'dislikes' do + post :create, dislike_hash + response.code.should == '201' + end + + it "doesn't post multiple times" do + @user1.like(1, :target => @target) + post :create, dislike_hash + response.code.should == '422' + end + end + + context "on a post from a stranger" do + before do + @target = eve.post :status_message, :text => "AWESOME", :to => eve.aspects.first.id + @target = eve.comment "hey", :post => @target if class_const == Comment + end + + it "doesn't post" do + @user1.should_not_receive(:like) + post :create, like_hash + response.code.should == '422' + end + end end - it 'responds to format js' do - post :create, like_hash.merge(:format => 'js') - response.code.should == '201' - end - end + describe '#index' do + before do + @message = alice.post(:status_message, :text => "hey", :to => @aspect1.id) + @message = alice.comment( "hey", :post => @message) if class_const == Comment + end + it 'returns a 404 for a post not visible to the user' do + sign_in eve + get :index, id_field => @message.id + end - context "on a post from a contact" do - before do - @post = @user2.post :status_message, :text => "AWESOME", :to => @aspect2.id + it 'returns an array of likes for a post' do + like = bob.build_like(:positive => true, :target => @message) + like.save! + + get :index, id_field => @message.id + assigns[:likes].map(&:id).should == @message.likes.map(&:id) + end + + it 'returns an empty array for a post with no likes' do + get :index, id_field => @message.id + assigns[:likes].should == [] + end end - it 'likes' do - post :create, like_hash - response.code.should == '201' + describe '#destroy' do + before do + @message = bob.post(:status_message, :text => "hey", :to => @aspect1.id) + @message = bob.comment( "hey", :post => @message) if class_const == Comment + @like = alice.build_like(:positive => true, :target => @message) + @like.save + end + + it 'lets a user destroy their like' do + expect { + delete :destroy, :format => "js", id_field => @like.target_id, :id => @like.id + }.should change(Like, :count).by(-1) + response.status.should == 200 + end + + it 'does not let a user destroy other likes' do + like2 = eve.build_like(:positive => true, :target => @message) + like2.save + + expect { + delete :destroy, :format => "js", id_field => like2.target_id, :id => like2.id + }.should_not change(Like, :count) + + response.status.should == 403 + end end - - it 'dislikes' do - post :create, dislike_hash - response.code.should == '201' - end - - it "doesn't post multiple times" do - @user1.like(1, :target => @post) - post :create, dislike_hash - response.code.should == '422' - end - end - - context "on a post from a stranger" do - before do - @post = eve.post :status_message, :text => "AWESOME", :to => eve.aspects.first.id - end - - it "doesn't post" do - @user1.should_not_receive(:like) - post :create, like_hash - response.code.should == '422' - end - end - end - - describe '#index' do - before do - @message = alice.post(:status_message, :text => "hey", :to => @aspect1.id) - end - it 'returns a 404 for a post not visible to the user' do - sign_in eve - get :index, :post_id => @message.id - end - - it 'returns an array of likes for a post' do - like = bob.build_like(:positive => true, :target => @message) - like.save! - - get :index, :post_id => @message.id - assigns[:likes].map(&:id).should == @message.likes.map(&:id) - end - - it 'returns an empty array for a post with no likes' do - get :index, :post_id => @message.id - assigns[:likes].should == [] - end - end - - describe '#destroy' do - before do - @message = bob.post(:status_message, :text => "hey", :to => @aspect1.id) - @like = alice.build_like(:positive => true, :target => @message) - @like.save - end - - it 'lets a user destroy their like' do - expect { - delete :destroy, :format => "js", :post_id => @like.target_id, :id => @like.id - }.should change(Like, :count).by(-1) - response.status.should == 200 - end - - it 'does not let a user destroy other likes' do - like2 = eve.build_like(:positive => true, :target => @message) - like2.save - - expect { - delete :destroy, :format => "js", :post_id => like2.target_id, :id => like2.id - }.should_not change(Like, :count) - - response.status.should == 403 end end end diff --git a/spec/models/like_spec.rb b/spec/models/like_spec.rb index c3b02b0d7..018b2fc79 100644 --- a/spec/models/like_spec.rb +++ b/spec/models/like_spec.rb @@ -58,11 +58,17 @@ describe Like do describe 'counter cache' do it 'increments the counter cache on its post' do - pending lambda { @alice.like(1, :target => @status) }.should change{ @status.reload.likes_count }.by(1) end + + it 'increments the counter cache on its comment' do + comment = Factory(:comment, :post => @status) + lambda { + @alice.like(1, :target => comment) + }.should change{ comment.reload.likes_count }.by(1) + end end describe 'xml' do