AO3-6058 Merge branch 'master' into AO3-6058

app/controllers/admin/admin_users_controller.rb

@@ -68,34 +68,30 @@ def update
 
   def update_next_of_kin
     @user = authorize User.find_by!(login: params[:user_login])
-    fnok = @user.fannish_next_of_kin
     kin = User.find_by(login: params[:next_of_kin_name])
     kin_email = params[:next_of_kin_email]
 
-    if kin.blank? && kin_email.blank?
-      if fnok.present?
-        fnok.destroy
-        @user.create_log_item({
-                                action: ArchiveConfig.ACTION_REMOVE_FNOK,
-                                fnok_user_id: fnok.kin.id,
-                                admin_id: current_admin.id,
-                                note: "Change made by #{current_admin.login}"
-                              })
-        flash[:notice] = ts("Fannish next of kin was removed.")
-      end
-      redirect_to admin_user_path(@user)
-      return
+    fnok = @user.fannish_next_of_kin
+    previous_kin = fnok&.kin
+    fnok ||= @user.build_fannish_next_of_kin
+    fnok.assign_attributes(kin: kin, kin_email: kin_email)
+
+    unless fnok.changed?
+      flash[:notice] = ts("No change to fannish next of kin.")
+      redirect_to admin_user_path(@user) and return
+    end
+
+    # Remove FNOK that already exists.
+    if fnok.persisted? && kin.blank? && kin_email.blank?
+      fnok.destroy
+      @user.log_removal_of_next_of_kin(previous_kin, admin: current_admin)
+      flash[:notice] = ts("Fannish next of kin was removed.")
+      redirect_to admin_user_path(@user) and return
     end
 
-    fnok = @user.build_fannish_next_of_kin if fnok.blank?
-    fnok.assign_attributes(kin: kin, kin_email: kin_email)
     if fnok.save
-      @user.create_log_item({
-                              action: ArchiveConfig.ACTION_ADD_FNOK,
-                              fnok_user_id: fnok.kin.id,
-                              admin_id: current_admin.id,
-                              note: "Change made by #{current_admin.login}"
-                            })
+      @user.log_removal_of_next_of_kin(previous_kin, admin: current_admin)
+      @user.log_assignment_of_next_of_kin(kin, admin: current_admin)
       flash[:notice] = ts("Fannish next of kin was updated.")
       redirect_to admin_user_path(@user)
     else
@@ -182,6 +178,6 @@ def activate
   end
 
   def log_items
-    @log_items ||= (@user.log_items + LogItem.where(fnok_user_id: @user.id)).sort_by(&:created_at).reverse
+    @log_items ||= @user.log_items.sort_by(&:created_at).reverse
   end
 end

app/controllers/application_controller.rb

@@ -485,7 +485,7 @@ def valid_sort_column(param, model='work')
     if model.to_s.downcase == 'work'
       allowed = %w(author title date created_at word_count hit_count)
     elsif model.to_s.downcase == 'tag'
-      allowed = %w(name created_at taggings_count_cache)
+      allowed = %w[name created_at taggings_count_cache uses]
     elsif model.to_s.downcase == 'collection'
       allowed = %w(collections.title collections.created_at)
     elsif model.to_s.downcase == 'prompt'

app/controllers/tags_controller.rb

@@ -409,6 +409,7 @@ def tag_search_params
       :type,
       :canonical,
       :created_at,
+      :uses,
       :sort_column,
       :sort_direction
     )

app/helpers/users_helper.rb

@@ -119,10 +119,10 @@ def authored_items(pseud, work_counts = {}, rec_counts = {})
     items.html_safe
   end
 
-  def log_item_action_name(item, user)
+  def log_item_action_name(item)
     action = item.action
 
-    return fnok_action_name(item, user) if [ArchiveConfig.ACTION_ADD_FNOK, ArchiveConfig.ACTION_REMOVE_FNOK].include?(action)
+    return fnok_action_name(item) if fnok_action?(action)
 
     case action
     when ArchiveConfig.ACTION_ACTIVATE
@@ -152,23 +152,6 @@ def log_item_action_name(item, user)
     end
   end
 
-  def fnok_action_name(item, user)
-    action = item.action == ArchiveConfig.ACTION_REMOVE_FNOK ? "removed" : "added"
-
-    if item.fnok_user_id == user.id
-      user_id = item.user_id
-      action_leaf = "was_#{action}"
-    else
-      user_id = item.fnok_user_id
-      action_leaf = "has_#{action}"
-    end
-
-    t(
-      "users_helper.log.fnok.#{action_leaf}",
-      user_id: user_id
-    )
-  end
-
   # Give the TOS field in the new user form a different name in non-production environments
   # so that it can be filtered out of the log, for ease of debugging
   def tos_field_name
@@ -178,4 +161,34 @@ def tos_field_name
       'terms_of_service_non_production'
     end
   end
+
+  private
+
+  def fnok_action?(action)
+    [
+      ArchiveConfig.ACTION_ADD_FNOK,
+      ArchiveConfig.ACTION_REMOVE_FNOK,
+      ArchiveConfig.ACTION_ADDED_AS_FNOK,
+      ArchiveConfig.ACTION_REMOVED_AS_FNOK
+    ].include?(action)
+  end
+
+  def fnok_action_name(item)
+    action_leaf =
+      case item.action
+      when ArchiveConfig.ACTION_ADD_FNOK
+        "has_added"
+      when ArchiveConfig.ACTION_REMOVE_FNOK
+        "has_removed"
+      when ArchiveConfig.ACTION_ADDED_AS_FNOK
+        "was_added"
+      when ArchiveConfig.ACTION_REMOVED_AS_FNOK
+        "was_removed"
+      end
+
+    t(
+      "users_helper.log.fnok.#{action_leaf}",
+      user_id: item.fnok_user_id
+    )
+  end
 end

app/jobs/tag_count_update_job.rb

@@ -17,7 +17,7 @@ def perform_on_batch(tag_ids)
     Tag.transaction do
       tag_ids.each do |id|
         value = REDIS_GENERAL.get("tag_update_#{id}_value")
-        Tag.where(id: id).update_all(taggings_count_cache: value) if value.present?
+        Tag.where(id: id).update(taggings_count_cache: value) if value.present?
       end
     end
   end

app/models/common_tagging.rb

@@ -19,7 +19,9 @@ class CommonTagging < ApplicationRecord
   after_create :update_wrangler
   after_create :inherit_parents
   after_create :remove_uncategorized_media
-  after_create :update_child_autocomplete
+  after_create :add_to_autocomplete
+
+  before_destroy :remove_from_autocomplete
 
   after_commit :update_search
 
@@ -29,8 +31,16 @@ def update_wrangler
     end
   end
 
-  def update_child_autocomplete
-    common_tag.refresh_autocomplete
+  def add_to_autocomplete
+    return unless filterable.is_a?(Fandom) && common_tag.eligible_for_fandom_autocomplete?
+
+    common_tag.add_to_fandom_autocomplete(filterable)
+  end
+
+  def remove_from_autocomplete
+    return unless filterable.is_a?(Fandom) && common_tag&.was_eligible_for_fandom_autocomplete?
+
+    common_tag.remove_from_fandom_autocomplete(filterable)
   end
 
   # A relationship should inherit its characters' fandoms

app/models/concerns/user_loggable.rb

@@ -0,0 +1,61 @@
+module UserLoggable
+  extend ActiveSupport::Concern
+
+  included do
+    before_destroy :log_removal_of_self_from_fnok_relationships
+  end
+
+  def log_removal_of_self_from_fnok_relationships
+    fannish_next_of_kins.each do |fnok|
+      fnok.user.log_removal_of_next_of_kin(self)
+    end
+
+    successor = fannish_next_of_kin&.kin
+    log_removal_of_next_of_kin(successor)
+  end
+
+  def log_assignment_of_next_of_kin(kin, admin:)
+    log_user_history(
+      ArchiveConfig.ACTION_ADD_FNOK,
+      options: { fnok_user_id: kin.id },
+      admin: admin
+    )
+
+    kin.log_user_history(
+      ArchiveConfig.ACTION_ADDED_AS_FNOK,
+      options: { fnok_user_id: self.id },
+      admin: admin
+    )
+  end
+
+  def log_removal_of_next_of_kin(kin, admin: nil)
+    return if kin.blank?
+
+    log_user_history(
+      ArchiveConfig.ACTION_REMOVE_FNOK,
+      options: { fnok_user_id: kin.id },
+      admin: admin
+    )
+
+    kin.log_user_history(
+      ArchiveConfig.ACTION_REMOVED_AS_FNOK,
+      options: { fnok_user_id: self.id },
+      admin: admin
+    )
+  end
+
+  def log_user_history(action, options: {}, admin: nil)
+    if admin.present?
+      options = {
+        admin_id: admin.id,
+        note: "Change made by #{admin.login}",
+        **options
+      }
+    end
+
+    create_log_item({
+                      action: action,
+                      **options
+                    })
+  end
+end

app/models/indexing/index_queue.rb

@@ -12,6 +12,7 @@ def self.all
   end
 
   def self.get_key(klass, label)
+    klass = klass.is_a?(Class) ? klass.base_class : klass
     "index:#{klass.to_s.underscore}:#{label}"
   end
 

app/models/search/tag_query.rb

@@ -44,7 +44,7 @@ def per_page
   def sort
     direction = options[:sort_direction]&.downcase
     case options[:sort_column]
-    when "taggings_count_cache"
+    when "taggings_count_cache", "uses"
       column = "uses"
       direction ||= "desc"
     when "created_at"

app/models/search/tag_search_form.rb

@@ -11,6 +11,7 @@ class TagSearchForm
     :fandoms,
     :type,
     :created_at,
+    :uses,
     :sort_column,
     :sort_direction
   ]
@@ -53,11 +54,12 @@ def sort_direction
   def sort_options
     [
       %w[Name name],
-      ["Date Created", "created_at"]
+      ["Date Created", "created_at"],
+      %w[Uses uses]
     ]
   end
 
   def default_sort_direction
-    %w[created_at].include?(sort_column) ? "desc" : "asc"
+    %w[created_at uses].include?(sort_column) ? "desc" : "asc"
   end
 end

app/models/tag.rb

@@ -50,8 +50,17 @@ def taggings_count_cache_key
   end
 
   def write_taggings_to_redis(value)
+    # Atomically set the value while extracting the old value.
+    old_redis_value = REDIS_GENERAL.getset("tag_update_#{id}_value", value).to_i
+
+    # If the value hasn't changed from the saved version or the REDIS version,
+    # there's no need to write an update to the database, so let's just bail
+    # out.
+    return value if value == old_redis_value && value == taggings_count_cache
+
+    # If we've reached here, then the value has changed, and we need to make
+    # sure that the new value is written to the database.
     REDIS_GENERAL.sadd("tag_update", id)
-    REDIS_GENERAL.set("tag_update_#{id}_value", value)
     value
   end
 
@@ -471,30 +480,48 @@ def autocomplete_prefixes
   end
 
   def add_to_autocomplete(score = nil)
-    score ||= autocomplete_score
-    if self.is_a?(Character) || self.is_a?(Relationship)
+    if eligible_for_fandom_autocomplete?
       parents.each do |parent|
-        REDIS_AUTOCOMPLETE.zadd(self.transliterate("autocomplete_fandom_#{parent.name.downcase}_#{type.downcase}"), score, autocomplete_value) if parent.is_a?(Fandom)
+        add_to_fandom_autocomplete(parent, score) if parent.is_a?(Fandom)
       end
     end
     super
   end
 
+  def add_to_fandom_autocomplete(fandom, score = nil)
+    score ||= autocomplete_score
+    REDIS_AUTOCOMPLETE.zadd(self.transliterate("autocomplete_fandom_#{fandom.name.downcase}_#{type.downcase}"), score, autocomplete_value)
+  end
+
   def remove_from_autocomplete
     super
-    if self.is_a?(Character) || self.is_a?(Relationship)
-      parents.each do |parent|
-        REDIS_AUTOCOMPLETE.zrem(self.transliterate("autocomplete_fandom_#{parent.name.downcase}_#{type.downcase}"), autocomplete_value) if parent.is_a?(Fandom)
-      end
+
+    return unless was_eligible_for_fandom_autocomplete?
+
+    parents.each do |parent|
+      remove_from_fandom_autocomplete(parent) if parent.is_a?(Fandom)
     end
   end
 
+  def remove_from_fandom_autocomplete(fandom)
+    REDIS_AUTOCOMPLETE.zrem(self.transliterate("autocomplete_fandom_#{fandom.name.downcase}_#{type.downcase}"), autocomplete_value)
+  end
+
+  def eligible_for_fandom_autocomplete?
+    (self.is_a?(Character) || self.is_a?(Relationship)) && canonical
+  end
+
+  def was_eligible_for_fandom_autocomplete?
+    (self.is_a?(Character) || self.is_a?(Relationship)) && (canonical || canonical_before_last_save)
+  end
+
   def remove_stale_from_autocomplete
     super
-    if self.is_a?(Character) || self.is_a?(Relationship)
-      parents.each do |parent|
-        REDIS_AUTOCOMPLETE.zrem(self.transliterate("autocomplete_fandom_#{parent.name.downcase}_#{type.downcase}"), autocomplete_value_before_last_save) if parent.is_a?(Fandom)
-      end
+
+    return unless was_eligible_for_fandom_autocomplete?
+
+    parents.each do |parent|
+      REDIS_AUTOCOMPLETE.zrem(self.transliterate("autocomplete_fandom_#{parent.name.downcase}_#{type.downcase}"), autocomplete_value_before_last_save) if parent.is_a?(Fandom)
     end
   end