posts/index: fix several "This tag is under discussion" issues.

Several fixes for the "This tag is under discussion" notice on the post
index page:

* Fix the notice appearing for BURs that aren't pending.
* Fix the notice never going away because of the cache never expiring.
* List all topics when a tag is involved in multiple BURs.
* Link to the forum post instead of the forum topic (fix #4421).
* Optimization: don't check for BURs when the search isn't a simple
  single tag search.
* Add a `tags` field to the bulk update requests table for tracking all
  tags involved in the request (excluding tags in mass updates that are
  negated/optional/wildcards). Known issue: doesn't handle tag type
  prefixes in mass updates correctly (e.g. `mass update foo -> artist:bar`
  doesn't detect the tag `bar`).
* Allow searching the /bulk_update_requests page by tags.

We don't really need to cache the notice here, but we do it anyway to
reduce queries on the post index page.
This commit is contained in:
evazion
2020-04-27 17:02:40 -05:00
parent 911ed34b76
commit d5a7fafca1
13 changed files with 80 additions and 81 deletions

View File

@@ -62,10 +62,6 @@ module PostsHelper
return params[:pool_id].to_i == pool.id
end
def show_tag_change_notice?
CurrentUser.user.is_member? && PostQueryBuilder.new(params[:tags]).split_query.size == 1 && TagChangeNoticeService.get_forum_topic_id(params[:tags])
end
private
def nav_params_for(page)

View File

@@ -79,27 +79,17 @@ class AliasAndImplicationImporter
end
def affected_tags
tokens = self.class.tokenize(text)
tokens.inject([]) do |all, token|
case token[0]
AliasAndImplicationImporter.tokenize(text).flat_map do |type, *args|
case type
when :create_alias, :remove_alias, :create_implication, :remove_implication
all << token[1]
all << token[2]
all
[args[0], args[1]]
when :mass_update
all += PostQueryBuilder.new(token[1]).split_query
all += PostQueryBuilder.new(token[2]).parse_tag_edit
all
tags = PostQueryBuilder.new(args[0]).tags + PostQueryBuilder.new(args[1]).tags
tags.reject(&:negated).reject(&:optional).reject(&:wildcard).map(&:name)
when :change_category
all << token[1]
all
else
all
args[0]
end
end
end.sort.uniq
end
private

View File

@@ -131,5 +131,10 @@ module PostSets
# be smarter about this in the future
posts.reject(&:is_deleted).select(&:visible?).max_by(&:fav_count)
end
def pending_bulk_update_requests
return BulkUpdateRequest.none unless query.is_simple_tag?
@pending_bulk_update_requests ||= BulkUpdateRequest.pending.where_array_includes_any(:tags, query.tags.first.name)
end
end
end

View File

@@ -1,13 +0,0 @@
module TagChangeNoticeService
module_function
def get_forum_topic_id(tag)
Cache.get("tcn:#{tag}")
end
def update_cache(affected_tags, forum_topic_id)
affected_tags.each do |tag|
Cache.put("tcn:#{tag}", forum_topic_id, 1.week)
end
end
end

View File

@@ -8,15 +8,16 @@ class BulkUpdateRequest < ApplicationRecord
belongs_to :forum_post, optional: true
belongs_to :approver, optional: true, class_name: "User"
before_validation :normalize_text
before_validation :update_tags
validates_presence_of :script
validates_presence_of :title, if: ->(rec) {rec.forum_topic_id.blank?}
validates_presence_of :forum_topic, if: ->(rec) {rec.forum_topic_id.present?}
validates_inclusion_of :status, :in => %w(pending approved rejected)
validate :script_formatted_correctly
validate :validate_script, :on => :create
before_validation :normalize_text
after_create :create_forum_topic
after_save :update_notice
scope :pending_first, -> { order(Arel.sql("(case status when 'pending' then 0 when 'approved' then 1 else 2 end)")) }
scope :pending, -> {where(status: "pending")}
@@ -34,7 +35,7 @@ class BulkUpdateRequest < ApplicationRecord
def search(params = {})
q = super
q = q.search_attributes(params, :user, :approver, :forum_topic_id, :forum_post_id, :script)
q = q.search_attributes(params, :user, :approver, :forum_topic_id, :forum_post_id, :script, :tags)
q = q.text_attribute_matches(:script, params[:script_matches])
if params[:status].present?
@@ -152,6 +153,10 @@ class BulkUpdateRequest < ApplicationRecord
self.script = script.downcase
end
def update_tags
self.tags = AliasAndImplicationImporter.new(script, nil).affected_tags
end
def skip_secondary_validations=(v)
@skip_secondary_validations = v.to_s.truthy?
end
@@ -168,13 +173,6 @@ class BulkUpdateRequest < ApplicationRecord
status == "rejected"
end
def update_notice
TagChangeNoticeService.update_cache(
AliasAndImplicationImporter.new(script, nil).affected_tags,
forum_topic_id
)
end
def self.available_includes
[:user, :forum_topic, :forum_post, :approver]
end

View File

@@ -29,7 +29,6 @@ class TagRelationship < ApplicationRecord
validates :approver, presence: { message: "must exist" }, if: -> { approver_id.present? }
validates :forum_topic, presence: { message: "must exist" }, if: -> { forum_topic_id.present? }
validate :antecedent_and_consequent_are_different
after_save :update_notice
def normalize_names
self.antecedent_name = antecedent_name.mb_chars.downcase.tr(" ", "_")
@@ -158,13 +157,6 @@ class TagRelationship < ApplicationRecord
end
end
def update_notice
TagChangeNoticeService.update_cache(
[antecedent_name, consequent_name],
forum_topic_id
)
end
def self.available_includes
[:creator, :approver, :forum_post, :forum_topic, :antecedent_tag, :consequent_tag, :antecedent_wiki, :consequent_wiki]
end

View File

@@ -1,7 +1,7 @@
<%= search_form_for(bulk_update_requests_path) do |f| %>
<%= f.input :user_name, label: "Creator", input_html: { value: params[:search][:user_name], data: { autocomplete: "user" } } %>
<%= f.input :approver_name, label: "Approver", input_html: { value: params[:search][:approver_name], data: { autocomplete: "user" } } %>
<%= f.input :script_matches, label: "Script", input_html: { value: params[:search][:script_matches] } %>
<%= f.input :tags_include_any, label: "Tags", input_html: { value: params[:search][:tags_include_any], data: { autocomplete: "tag-query" } } %>
<%= f.input :status, label: "Status", collection: %w[pending approved rejected], include_blank: true, selected: params[:search][:status] %>
<%= f.input :order, collection: [%w[Status status_desc], %w[Last\ updated updated_at_desc], %w[Newest id_desc], %w[Oldest id_asc]], include_blank: false, selected: params[:search][:order] %>
<%= f.submit "Search" %>

View File

@@ -19,10 +19,17 @@
</div>
<% end %>
<% if show_tag_change_notice? %>
<div class="fineprint tag-change-notice">
<p>This tag is the subject of an ongoing discussion. If you have any relevant information, please join the <%= link_to "discussion", forum_topic_path(TagChangeNoticeService.get_forum_topic_id(params[:tags]) || 1) %>.</p>
</div>
<% if CurrentUser.user.is_member? && post_set.query.is_simple_tag? %>
<% cache("tag-change-notice:#{post_set.query.normalize_query}", expires_in: 4.hours) do %>
<% if post_set.pending_bulk_update_requests.present? %>
<div class="fineprint tag-change-notice">
<p>
This tag is being discussed in
<%= to_sentence post_set.pending_bulk_update_requests.map { |bur| link_to "Topic ##{bur.forum_topic_id}: #{bur.forum_topic.title}", bur.forum_post } %>.
</p>
</div>
<% end %>
<% end %>
<% end %>
<% unless post_set.is_random? %>