From 9b40016895c94fdec6b00f20088116e45de699d0 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 24 Feb 2017 19:11:48 -0600 Subject: [PATCH 1/3] /posts/NNN, modqueue: allow dtext in disapproval reasons. --- app/views/post_disapprovals/_compact_counts.html.erb | 4 ++-- app/views/post_disapprovals/_counts.html.erb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/post_disapprovals/_compact_counts.html.erb b/app/views/post_disapprovals/_compact_counts.html.erb index 5bb3a6b12..9e08a9745 100644 --- a/app/views/post_disapprovals/_compact_counts.html.erb +++ b/app/views/post_disapprovals/_compact_counts.html.erb @@ -12,6 +12,6 @@ <% end %> <% if disapprovals.with_message.any? %> - (messages: <%= disapprovals.disinterest.with_message.map(&:message).to_sentence %>) + (messages: <%= disapprovals.map(&:message).select(&:present?).map { |msg| format_text(msg, ragel: true, inline: true) }.to_sentence.html_safe %>) <% end %> -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/post_disapprovals/_counts.html.erb b/app/views/post_disapprovals/_counts.html.erb index e6ec12ead..612e4f7e7 100644 --- a/app/views/post_disapprovals/_counts.html.erb +++ b/app/views/post_disapprovals/_counts.html.erb @@ -15,7 +15,7 @@ <% end %> <% if disapprovals.with_message.any? %> - Messages: <%= disapprovals.with_message.map(&:message).to_sentence %> + Messages: <%= disapprovals.map(&:message).select(&:present?).map { |msg| format_text(msg, ragel: true, inline: true) }.to_sentence.html_safe %>. <% end %>

-<% end %> \ No newline at end of file +<% end %> From 83915a5d468a53889f4bfd41a0f88aa56340b169 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 24 Feb 2017 19:13:02 -0600 Subject: [PATCH 2/3] modqueue: avoid N+1 queries for disapprovals, uploaders. Fixes an N+1 queries problem in the /moderator/post/queue view by prefetching disapprovals and uploaders. Also the way disapproval messages were previously rendered triggered a bunch of sql queries for each post: SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 [["post_id", 52]] SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND "post_disapprovals"."reason" = $2 [["post_id", 52], ["reason", "breaks_rules"]] SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND "post_disapprovals"."reason" = $2 [["post_id", 52], ["reason", "poor_quality"]] SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND "post_disapprovals"."reason" IN ('disinterest', 'legacy') [["post_id", 52]] SELECT COUNT(*) FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND (message is not null and message <> '') [["post_id", 52]] SELECT "post_disapprovals".* FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 AND (message is not null and message <> '') [["post_id", 52]] This refactors to bring it down to one: SELECT "post_disapprovals".* FROM "post_disapprovals" WHERE "post_disapprovals"."post_id" = $1 [["post_id", 52]] --- .../moderator/post/queues_controller.rb | 4 ++-- .../post_disapprovals/_compact_counts.html.erb | 16 ++++++++-------- app/views/post_disapprovals/_counts.html.erb | 18 +++++++++--------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/controllers/moderator/post/queues_controller.rb b/app/controllers/moderator/post/queues_controller.rb index 06b80dd18..e274c19fb 100644 --- a/app/controllers/moderator/post/queues_controller.rb +++ b/app/controllers/moderator/post/queues_controller.rb @@ -15,7 +15,7 @@ module Moderator end ::Post.without_timeout do - @posts = ::Post.order("posts.id asc").pending_or_flagged.available_for_moderation(params[:hidden]).tag_match(params[:query]).paginate(params[:page], :limit => per_page) + @posts = ::Post.includes(:disapprovals, :uploader).order("posts.id asc").pending_or_flagged.available_for_moderation(params[:hidden]).tag_match(params[:query]).paginate(params[:page], :limit => per_page) @posts.each # hack to force rails to eager load end respond_with(@posts) @@ -25,7 +25,7 @@ module Moderator cookies.permanent[:moderated] = Time.now.to_i ::Post.without_timeout do - @posts = ::Post.order("posts.id asc").pending_or_flagged.available_for_moderation(false).reorder("random()").limit(RANDOM_COUNT) + @posts = ::Post.includes(:disapprovals, :uploader).order("posts.id asc").pending_or_flagged.available_for_moderation(false).reorder("random()").limit(RANDOM_COUNT) @posts.each # hack to force rails to eager load if @posts.empty? diff --git a/app/views/post_disapprovals/_compact_counts.html.erb b/app/views/post_disapprovals/_compact_counts.html.erb index 9e08a9745..78ced11bc 100644 --- a/app/views/post_disapprovals/_compact_counts.html.erb +++ b/app/views/post_disapprovals/_compact_counts.html.erb @@ -1,17 +1,17 @@ -<% if disapprovals.count > 0 && (CurrentUser.can_approve_posts? || post.created_at < 3.days.ago) %> - <% if disapprovals.breaks_rules.count > 0 %> - (breaks rules: <%= disapprovals.breaks_rules.count %>) +<% if CurrentUser.can_approve_posts? || post.created_at < 3.days.ago %> + <% if disapprovals.map(&:reason).grep("breaks_rules").count > 0 %> + (breaks rules: <%= disapprovals.map(&:reason).grep("breaks_rules").count %>) <% end %> - <% if disapprovals.poor_quality.count > 0 %> - (poor quality: <%= disapprovals.poor_quality.count %>) + <% if disapprovals.map(&:reason).grep("poor_quality").count > 0 %> + (poor quality: <%= disapprovals.map(&:reason).grep("poor_quality").count %>) <% end %> - <% if disapprovals.disinterest.count > 0 %> - (no interest: <%= disapprovals.disinterest.count %>) + <% if disapprovals.map(&:reason).grep(/disinterest|legacy/).count > 0 %> + (no interest: <%= disapprovals.map(&:reason).grep(/disinterest|legacy/).count %>) <% end %> - <% if disapprovals.with_message.any? %> + <% if disapprovals.map(&:message).any?(&:present?) %> (messages: <%= disapprovals.map(&:message).select(&:present?).map { |msg| format_text(msg, ragel: true, inline: true) }.to_sentence.html_safe %>) <% end %> <% end %> diff --git a/app/views/post_disapprovals/_counts.html.erb b/app/views/post_disapprovals/_counts.html.erb index 612e4f7e7..f858c96af 100644 --- a/app/views/post_disapprovals/_counts.html.erb +++ b/app/views/post_disapprovals/_counts.html.erb @@ -1,20 +1,20 @@ -<% if disapprovals.count > 0 && (CurrentUser.can_approve_posts? || post.created_at < 3.days.ago) %> +<% if CurrentUser.can_approve_posts? || post.created_at < 3.days.ago %>

- It has been reviewed by <%= pluralize disapprovals.count, "moderator" %>. + It has been reviewed by <%= pluralize disapprovals.length, "moderator" %>. - <% if disapprovals.breaks_rules.count > 0 %> - <%= disapprovals.breaks_rules.count %> believe it breaks the rules. + <% if disapprovals.map(&:reason).grep("breaks_rules").count > 0 %> + <%= disapprovals.map(&:reason).grep("breaks_rules").count %> believe it breaks the rules. <% end %> - <% if disapprovals.poor_quality.count > 0 %> - <%= disapprovals.poor_quality.count %> believe it has poor quality. + <% if disapprovals.map(&:reason).grep("poor_quality").count > 0 %> + <%= disapprovals.map(&:reason).grep("poor_quality").count %> believe it has poor quality. <% end %> - <% if disapprovals.disinterest.count > 0 %> - <%= disapprovals.disinterest.count %> did not like the post enough to approve it. + <% if disapprovals.map(&:reason).grep(/disinterest|legacy/).count > 0 %> + <%= disapprovals.map(&:reason).grep(/disinterest|legacy/).count %> did not like the post enough to approve it. <% end %> - <% if disapprovals.with_message.any? %> + <% if disapprovals.map(&:message).any?(&:present?) %> Messages: <%= disapprovals.map(&:message).select(&:present?).map { |msg| format_text(msg, ragel: true, inline: true) }.to_sentence.html_safe %>. <% end %>

From b80976bce716f05b616ce5d10218bc1a98ff0b31 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 24 Feb 2017 20:59:21 -0600 Subject: [PATCH 3/3] modqueue: link tags, like they are in /comments. --- app/presenters/post_presenter.rb | 14 ++++++++++---- app/presenters/tag_set_presenter.rb | 11 +++++++++++ app/views/comments/partials/index/_header.html.erb | 6 +----- app/views/moderator/post/queues/show.html.erb | 2 +- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/app/presenters/post_presenter.rb b/app/presenters/post_presenter.rb index adf3a1b9a..9cfa5e712 100644 --- a/app/presenters/post_presenter.rb +++ b/app/presenters/post_presenter.rb @@ -95,6 +95,10 @@ class PostPresenter < Presenter @post = post end + def tag_set_presenter + @tag_set_presenter ||= TagSetPresenter.new(@post.tag_array) + end + def preview_html PostPresenter.preview(@post) end @@ -170,13 +174,15 @@ class PostPresenter < Presenter end def tag_list_html(template, options = {}) - @tag_set_presenter ||= TagSetPresenter.new(@post.tag_array) - @tag_set_presenter.tag_list_html(template, options.merge(:show_extra_links => CurrentUser.user.is_gold?)) + tag_set_presenter.tag_list_html(template, options.merge(:show_extra_links => CurrentUser.user.is_gold?)) end def split_tag_list_html(template, options = {}) - @tag_set_presenter ||= TagSetPresenter.new(@post.tag_array) - @tag_set_presenter.split_tag_list_html(template, options.merge(:show_extra_links => CurrentUser.user.is_gold?)) + tag_set_presenter.split_tag_list_html(template, options.merge(:show_extra_links => CurrentUser.user.is_gold?)) + end + + def inline_tag_list_html(template) + tag_set_presenter.inline_tag_list(template) end def has_nav_links?(template) diff --git a/app/presenters/tag_set_presenter.rb b/app/presenters/tag_set_presenter.rb index 2321bc03f..a09d3e586 100644 --- a/app/presenters/tag_set_presenter.rb +++ b/app/presenters/tag_set_presenter.rb @@ -64,6 +64,17 @@ class TagSetPresenter < Presenter html.html_safe end + # compact (horizontal) list, as seen in the /comments index. + def inline_tag_list(template) + @tags.map do |tag_name| + <<-EOS + + #{template.link_to(tag_name.tr("_", " "), template.posts_path(tags: tag_name))} + + EOS + end.join.html_safe + end + private def general_tags @general_tags ||= categories.select {|k, v| v == Tag.categories.general} diff --git a/app/views/comments/partials/index/_header.html.erb b/app/views/comments/partials/index/_header.html.erb index 0e0e1ce02..1815dc23b 100644 --- a/app/views/comments/partials/index/_header.html.erb +++ b/app/views/comments/partials/index/_header.html.erb @@ -24,11 +24,7 @@
Tags - <% post.tag_array.each do |tag_name| %> - - <%= link_to(tag_name.tr("_", " "), posts_path(:tags => tag_name)) %> - - <% end %> + <%= post.presenter.inline_tag_list_html(self) %>
diff --git a/app/views/moderator/post/queues/show.html.erb b/app/views/moderator/post/queues/show.html.erb index 1b9fe1aba..f23b1714b 100644 --- a/app/views/moderator/post/queues/show.html.erb +++ b/app/views/moderator/post/queues/show.html.erb @@ -60,7 +60,7 @@ Hidden: <%= render "post_disapprovals/compact_counts", :disapprovals => post.disapprovals, :post => post %>
  • Source: <%= post.source %>
    • -
  • Tags: <%= post.tag_string %>
    • +
  • Tags: <%= post.presenter.inline_tag_list_html(self) %>