From 2e134742d4796a593aff4b9a70194653cea48df3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 4 Aug 2014 17:29:01 +0200 Subject: [PATCH] FIX: only show 'defer flags' when there are active flags on the post --- app/models/post_action.rb | 17 +++++++++++++++++ app/serializers/post_serializer.rb | 26 ++++++++++++++++++-------- lib/topic_view.rb | 20 ++++++++++++++------ spec/components/topic_view_spec.rb | 23 +++++++++++++++++++++++ 4 files changed, 72 insertions(+), 14 deletions(-) diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 2e8b75f8ceb..7d787b7a69f 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -73,6 +73,23 @@ class PostAction < ActiveRecord::Base user_actions end + def self.active_flags_counts_for(collection) + return {} if collection.blank? + + collection_ids = collection.map(&:id) + + post_actions = PostAction.active.flags.where(post_id: collection_ids) + + user_actions = {} + post_actions.each do |post_action| + user_actions[post_action.post_id] ||= {} + user_actions[post_action.post_id][post_action.post_action_type_id] ||= [] + user_actions[post_action.post_id][post_action.post_action_type_id] << post_action + end + + user_actions + end + def self.count_per_day_for_type(post_action_type, since_days_ago=30) unscoped.where(post_action_type_id: post_action_type) .where('created_at > ?', since_days_ago.days.ago) diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 08001b7c1ed..3268602d957 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -154,10 +154,12 @@ class PostSerializer < BasicPostSerializer count = object.send(count_col) if object.respond_to?(count_col) count ||= 0 - action_summary = {id: id, - count: count, - hidden: (sym == :vote), - can_act: scope.post_can_act?(object, sym, taken_actions: post_actions)} + action_summary = { + id: id, + count: count, + hidden: (sym == :vote), + can_act: scope.post_can_act?(object, sym, taken_actions: post_actions) + } if sym == :notify_user && scope.current_user.present? && scope.current_user == object.user action_summary[:can_act] = false # Don't send a pm to yourself about your own post, silly @@ -165,7 +167,10 @@ class PostSerializer < BasicPostSerializer # The following only applies if you're logged in if action_summary[:can_act] && scope.current_user.present? - action_summary[:can_defer_flags] = scope.is_staff? && PostActionType.flag_types.values.include?(id) + action_summary[:can_defer_flags] = scope.is_staff? && + PostActionType.flag_types.values.include?(id) && + active_flags.present? && + active_flags.count > 0 end if post_actions.present? && post_actions.has_key?(id) @@ -242,7 +247,12 @@ class PostSerializer < BasicPostSerializer private - def post_actions - @post_actions ||= (@topic_view.present? && @topic_view.all_post_actions.present?) ? @topic_view.all_post_actions[object.id] : nil - end + def post_actions + @post_actions ||= (@topic_view.present? && @topic_view.all_post_actions.present?) ? @topic_view.all_post_actions[object.id] : nil + end + + def active_flags + @active_flags ||= (@topic_view.present? && @topic_view.all_active_flags.present?) ? @topic_view.all_active_flags[object.id] : nil + end + end diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 8b826c94f2b..f9004cdbf60 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -188,7 +188,9 @@ class TopicView end def has_deleted? - @predelete_filtered_posts.with_deleted.where("deleted_at IS NOT NULL").exists? + @predelete_filtered_posts.with_deleted + .where("posts.deleted_at IS NOT NULL") + .exists? end def topic_user @@ -199,7 +201,11 @@ class TopicView end def post_counts_by_user - @post_counts_by_user ||= Post.where(topic_id: @topic.id).group(:user_id).order('count_all desc').limit(24).count + @post_counts_by_user ||= Post.where(topic_id: @topic.id) + .group(:user_id) + .order("count_all DESC") + .limit(24) + .count end def participants @@ -214,6 +220,10 @@ class TopicView @all_post_actions ||= PostAction.counts_for(@posts, @user) end + def all_active_flags + @all_active_flags ||= PostAction.active_flags_counts_for(@posts) + end + def links @links ||= TopicLink.topic_map(guardian, @topic.id) end @@ -282,8 +292,7 @@ class TopicView def filter_posts_by_ids(post_ids) # TODO: Sort might be off @posts = Post.where(id: post_ids, topic_id: @topic.id) - .includes(:user) - .includes(:reply_to_user) + .includes(:user, :reply_to_user) .order('sort_order') @posts = @posts.with_deleted if @guardian.can_see_deleted_posts? @posts @@ -316,7 +325,6 @@ class TopicView end def setup_filtered_posts - # Certain filters might leave gaps between posts. If that's true, we can return a gap structure @contains_gaps = false @filtered_posts = unfiltered_posts @@ -335,7 +343,7 @@ class TopicView # Username filters if @username_filters.present? usernames = @username_filters.map{|u| u.downcase} - @filtered_posts = @filtered_posts.where('post_number = 1 or user_id in (select u.id from users u where username_lower in (?))', usernames) + @filtered_posts = @filtered_posts.where('post_number = 1 OR posts.user_id IN (SELECT u.id FROM users u WHERE username_lower IN (?))', usernames) @contains_gaps = true end diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb index 2736c6f02c7..d948a415f89 100644 --- a/spec/components/topic_view_spec.rb +++ b/spec/components/topic_view_spec.rb @@ -167,6 +167,29 @@ describe TopicView do end end + context '.all_active_flags' do + it 'is blank at first' do + topic_view.all_active_flags.should be_blank + end + + it 'returns the active flags' do + PostAction.act(moderator, p1, PostActionType.types[:off_topic]) + PostAction.act(coding_horror, p1, PostActionType.types[:off_topic]) + + topic_view.all_active_flags[p1.id][PostActionType.types[:off_topic]].count.should == 2 + end + + it 'returns only the active flags' do + PostAction.act(moderator, p1, PostActionType.types[:off_topic]) + PostAction.act(coding_horror, p1, PostActionType.types[:off_topic]) + + PostAction.defer_flags!(p1, moderator) + + topic_view.all_active_flags[p1.id].should == nil + end + end + + context '.read?' do it 'tracks correctly' do # anon is assumed to have read everything