AO3-6210 Load the tags in the mass wrangling bins from Elasticsearch (#…

…4646) * Move copy-pasted count to a single function * Load unwrangled counts from ES * Now count the right thing * Hound * Fix (some?) tests * Load tags on Index from ES * Restore ordering by name ASC second on uses * Use _cache everywhere * Hound * Linter * Update tests * Hound * Some sort of solution for seemingly noop * Linter * Now unused? * Syntax/Typo * Only store unwrangeable value * Hound * Fix Grandparent bug? * More subtle cache bursting * Review * More explicit uncovered case * Syntax confusion * Hopefully helpful comment * Review * Obsolete See #4658 * Review * Linter * Split into (almost) unitary tags in_use still doing two things As it contains a OR * Cop * Review
otwcode · Jan 26, 2024 · 9955683 · 9955683
1 parent 9ed7136
commit 9955683
Show file tree

Hide file tree

Showing 16 changed files with 204 additions and 73 deletions.
diff --git a/app/controllers/tag_wranglers_controller.rb b/app/controllers/tag_wranglers_controller.rb
@@ -1,5 +1,6 @@
 class TagWranglersController < ApplicationController
   include ExportsHelper
+  include WranglingHelper
 
   before_action :check_user_status
   before_action :check_permission_to_wrangle, except: [:report_csv]
@@ -41,15 +42,7 @@ def show
     @wrangler = User.find_by!(login: params[:id])
     @page_subtitle = @wrangler.login
     @fandoms = @wrangler.fandoms.by_name
-    @counts = {}
-    [Fandom, Character, Relationship, Freeform].each do |klass|
-      @counts[klass.to_s.downcase.pluralize.to_sym] = Rails.cache.fetch("/wrangler/counts/sidebar/#{klass}", race_condition_ttl: 10, expires_in: 1.hour) do
-        klass.unwrangled.in_use.count
-      end
-    end
-    @counts[:UnsortedTag] = Rails.cache.fetch("/wrangler/counts/sidebar/UnsortedTag", race_condition_ttl: 10, expires_in: 1.hour) do
-      UnsortedTag.count
-    end
+    @counts = tag_counts_per_category
   end
 
   def report_csv

diff --git a/app/controllers/tag_wranglings_controller.rb b/app/controllers/tag_wranglings_controller.rb
@@ -1,34 +1,35 @@
 class TagWranglingsController < ApplicationController
   include TagWrangling
+  include WranglingHelper
 
   before_action :check_user_status
   before_action :check_permission_to_wrangle
   around_action :record_wrangling_activity, only: [:wrangle]
 
   def index
-    @counts = {}
-    [Fandom, Character, Relationship, Freeform].each do |klass|
-      @counts[klass.to_s.downcase.pluralize.to_sym] = Rails.cache.fetch("/wrangler/counts/sidebar/#{klass}", race_condition_ttl: 10, expires_in: 1.hour) do
-        klass.unwrangled.in_use.count
-      end
-    end
-    @counts[:UnsortedTag] = Rails.cache.fetch("/wrangler/counts/sidebar/UnsortedTag", race_condition_ttl: 10, expires_in: 1.hour) do
-      UnsortedTag.count
-    end
+    @counts = tag_counts_per_category
     unless params[:show].blank?
+      raise "Redshirt: Attempted to constantize invalid class initialize tag_wranglings_controller_index #{params[:show].classify}" unless Tag::USER_DEFINED.include?(params[:show].classify)
+
       params[:sort_column] = 'created_at' if !valid_sort_column(params[:sort_column], 'tag')
       params[:sort_direction] = 'ASC' if !valid_sort_direction(params[:sort_direction])
-      sort = params[:sort_column] + " " + params[:sort_direction]
-      sort = sort + ", name ASC" if sort.include?('taggings_count_cache')
+
       if params[:show] == "fandoms"
         @media_names = Media.by_name.pluck(:name)
         @page_subtitle = ts("fandoms")
-        @tags = Fandom.unwrangled.in_use.order(sort).paginate(page: params[:page], per_page: ArchiveConfig.ITEMS_PER_PAGE)
-      else # by fandom
-        raise "Redshirt: Attempted to constantize invalid class initialize tag_wranglings_controller_index #{params[:show].classify}" unless Tag::USER_DEFINED.include?(params[:show].classify)
-        klass = params[:show].classify.constantize
-        @tags = klass.unwrangled.in_use.order(sort).paginate(page: params[:page], per_page: ArchiveConfig.ITEMS_PER_PAGE)
       end
+
+      type = params[:show].singularize.capitalize
+      @tags = TagQuery.new({
+                             type: type,
+                             in_use: true,
+                             unwrangleable: false,
+                             unwrangled: true,
+                             sort_column: params[:sort_column],
+                             sort_direction: params[:sort_direction],
+                             page: params[:page],
+                             per_page: ArchiveConfig.ITEMS_PER_PAGE
+                           }).search_results
     end
   end
 

diff --git a/app/controllers/unsorted_tags_controller.rb b/app/controllers/unsorted_tags_controller.rb
@@ -1,19 +1,12 @@
 class UnsortedTagsController < ApplicationController
+  include WranglingHelper
 
   before_action :check_user_status
   before_action :check_permission_to_wrangle
 
   def index
     @tags = UnsortedTag.page(params[:page])
-    @counts = {}
-    [Fandom, Character, Relationship, Freeform].each do |klass|
-      @counts[klass.to_s.downcase.pluralize.to_sym] = Rails.cache.fetch("/wrangler/counts/sidebar/#{klass}", race_condition_ttl: 10, expires_in: 1.hour) do
-        klass.unwrangled.in_use.count
-      end
-    end
-    @counts[:UnsortedTag] = Rails.cache.fetch("/wrangler/counts/sidebar/UnsortedTag", race_condition_ttl: 10, expires_in: 1.hour) do 
-      UnsortedTag.count 
-    end
+    @counts = tag_counts_per_category
   end
 
   def mass_update

diff --git a/app/helpers/wrangling_helper.rb b/app/helpers/wrangling_helper.rb
@@ -0,0 +1,14 @@
+module WranglingHelper
+  def tag_counts_per_category
+    counts = {}
+    [Fandom, Character, Relationship, Freeform].each do |klass|
+      counts[klass.to_s.downcase.pluralize.to_sym] = Rails.cache.fetch("/wrangler/counts/sidebar/#{klass}", race_condition_ttl: 10, expires_in: 1.hour) do
+        TagQuery.new(type: klass.to_s, in_use: true, unwrangleable: false, unwrangled: true).count
+      end
+    end
+    counts[:UnsortedTag] = Rails.cache.fetch("/wrangler/counts/sidebar/UnsortedTag", race_condition_ttl: 10, expires_in: 1.hour) do
+      UnsortedTag.count
+    end
+    counts
+  end
+end
diff --git a/app/models/fandom.rb b/app/models/fandom.rb
@@ -14,9 +14,8 @@ class Fandom < Tag
 
   scope :by_media, lambda {|media| where(media_id: media.id)}
 
-  def self.unwrangled
-    joins(:common_taggings).
-    where("unwrangleable = 0 AND common_taggings.filterable_id = ? AND common_taggings.filterable_type = 'Tag'", Media.uncategorized.try(:id))
+  def unwrangled?
+    Media.uncategorized.fandoms.include?(self)
   end
 
   # An association callback to add the default media if all others have been removed

diff --git a/app/models/search/tag_indexer.rb b/app/models/search/tag_indexer.rb
@@ -23,7 +23,8 @@ def self.mapping
         },
         tag_type: { type: "keyword" },
         sortable_name: { type: "keyword" },
-        uses: { type: "integer" }
+        uses: { type: "integer" },
+        unwrangled: { type: "boolean" }
       }
     }
   end
@@ -67,7 +68,8 @@ def document(object)
     ).merge(
       has_posted_works: object.has_posted_works?,
       tag_type: object.type,
-      uses: object.taggings_count_cache
+      uses: object.taggings_count_cache,
+      unwrangled: object.unwrangled?
     ).merge(parent_data(object))
   end
 

diff --git a/app/models/search/tag_query.rb b/app/models/search/tag_query.rb
@@ -22,7 +22,9 @@ def filters
       fandom_filter,
       character_filter,
       suggested_fandom_filter,
-      suggested_character_filter
+      suggested_character_filter,
+      in_use_filter,
+      unwrangled_filter
     ].flatten.compact
   end
 
@@ -60,7 +62,11 @@ def sort
       sort_hash[column][:unmapped_type] = "date"
     end
 
-    [sort_hash, { id: { order: direction } }]
+    sort_by_id = { id: { order: direction } }
+
+    return [sort_hash, { "name.keyword" => { order: "asc" } }, sort_by_id] if column == "uses"
+
+    [sort_hash, sort_by_id]
   end
 
   ################
@@ -103,6 +109,23 @@ def suggested_character_filter
     terms_filter(:pre_character_ids, options[:pre_character_ids]) if options[:pre_character_ids]
   end
 
+  # Canonical tags are treated as used even if they technically aren't
+  def in_use_filter
+    return if options[:in_use].nil?
+
+    unless options[:in_use]
+      # Check if not used AND not canonical
+      return [term_filter(:uses, 0), term_filter(:canonical, false)]
+    end
+
+    # Check if used OR canonical
+    { bool: { should: [{ range: { uses: { gt: 0 } } }, term_filter(:canonical, true)] } }
+  end
+
+  def unwrangled_filter
+    term_filter(:unwrangled, bool_value(options[:unwrangled])) unless options[:unwrangled].nil?
+  end
+
   # Filter to only include tags that have no assigned fandom_ids. Checks that
   # the fandom exists, because this particular filter is included in the
   # exclusion_filters section.

diff --git a/app/models/tag.rb b/app/models/tag.rb
@@ -272,9 +272,6 @@ def destroy_common_tagging(parent)
   scope :unfilterable, -> { nonsynonymous.where(unwrangleable: false) }
   scope :unwrangleable, -> { where(unwrangleable: true) }
 
-  # we need to manually specify a LEFT JOIN instead of just joins(:common_taggings or :meta_taggings) here because
-  # what we actually need are the empty rows in the results
-  scope :unwrangled, -> { joins("LEFT JOIN `common_taggings` ON common_taggings.common_tag_id = tags.id").where("unwrangleable = 0 AND common_taggings.id IS NULL") }
   scope :in_use, -> { where("canonical = 1 OR taggings_count_cache > 0") }
   scope :first_class, -> { joins("LEFT JOIN `meta_taggings` ON meta_taggings.sub_tag_id = tags.id").where("meta_taggings.id IS NULL") }
 
@@ -1034,6 +1031,20 @@ def merger_string=(tag_string)
     end
   end
 
+  # unwrangleable:
+  #   - A boolean stored in the tags table
+  #   - Default false
+  #   - Set to true by wranglers on tags that should be excluded from the wrangling process altogether. Example: freeform tags like "idk how to explain it but trust me"
+  #
+  # unwrangled:
+  #   - A computed value
+  #   - True for "orphan" tags yet to be tied to something (fandom, character, etc.) by wranglers
+  #   - Exact meaning may change depending on the nature of the tag (search for definitions of unwrangled? overriding this one)
+  #
+  def unwrangled?
+    common_taggings.empty?
+  end
+
   #################################
   ## SEARCH #######################
   #################################

diff --git a/app/views/tag_wranglings/index.html.erb b/app/views/tag_wranglings/index.html.erb
@@ -104,32 +104,34 @@
 
             <tbody>
               <% @tags.each do |tag| %>
-                <tr>
-                  <th scope="row" title="<%= ts('tag') %>">
-                    <%= check_box_tag 'selected_tags[]', tag.id, nil, id: "selected_tags_#{tag.id}" %>
-                    <%= label_tag "selected_tags_#{tag.id}", "#{tag.name}" %>
-                  </th>
-
-                  <td title="<%= ts('created') %>"><%= l(tag.created_at.to_date) %></td>
-
-                  <td title="<%= ts('canonical?') %>">
-                    <% if tag.canonical? %>
-                      <%= ts('Yes') %>
-                    <% else %>
-                      <%= check_box_tag 'canonicals[]', tag.id, tag.canonical?, id: "canonicals_#{tag.id}" %>
-                    <% end %>
-                  </td>
-
-                  <td title="<%= ts('taggings') %>"><%= tag.taggings_count %></td>
-
-                  <td>
-                    <ul class="actions" role="navigation">
-                      <li><%= link_to ts('Edit'), {controller: :tags, action: :edit, id: tag} %></li>
-                      <li><%= link_to ts('Wrangle'), {controller: :tags, action: :wrangle, id: tag} %></li>
-                      <li><%= link_to ts('Works'), {controller: :works, action: :index, tag_id: tag} %></li>
-                    </ul>
-                  </td>
-                </tr>
+                <% if tag.unwrangled? %>
+                  <tr>
+                    <th scope="row" title="<%= ts("tag") %>">
+                      <%= check_box_tag "selected_tags[]", tag.id, nil, id: "selected_tags_#{tag.id}" %>
+                      <%= label_tag "selected_tags_#{tag.id}", tag.name.to_s %>
+                    </th>
+
+                    <td title="<%= ts("created") %>"><%= l(tag.created_at.to_date) %></td>
+
+                    <td title="<%= ts("canonical?") %>">
+                      <% if tag.canonical? %>
+                        <%= ts("Yes") %>
+                      <% else %>
+                        <%= check_box_tag "canonicals[]", tag.id, tag.canonical?, id: "canonicals_#{tag.id}" %>
+                      <% end %>
+                    </td>
+
+                    <td title="<%= ts("taggings") %>"><%= tag.taggings_count_cache %></td>
+
+                    <td>
+                      <ul class="actions" role="navigation">
+                        <li><%= link_to ts("Edit"), { controller: :tags, action: :edit, id: tag } %></li>
+                        <li><%= link_to ts("Wrangle"), { controller: :tags, action: :wrangle, id: tag } %></li>
+                        <li><%= link_to ts("Works"), { controller: :works, action: :index, tag_id: tag } %></li>
+                      </ul>
+                    </td>
+                  </tr>
+                <% end %>
               <% end %>
             </tbody>
           </table>

diff --git a/app/views/tags/wrangle.html.erb b/app/views/tags/wrangle.html.erb
@@ -133,7 +133,7 @@
 
             <td title="<%= ts('created') %>"><%= tag.created_at.to_date %></td>
 
-            <td title="<%= ts('taggings') %>"><%= tag.taggings_count %></td>
+            <td title="<%= ts("taggings") %>"><%= tag.taggings_count_cache %></td>
 
             <td>
               <ul class="actions" role="menu">

diff --git a/features/step_definitions/tag_steps.rb b/features/step_definitions/tag_steps.rb
@@ -192,6 +192,7 @@
   step %{I am logged in as a random user}
   step %{I post the work "Revenge of the Sith 2" with fandom "Star Wars, Stargate SG-1" with character "Daniel Jackson" with second character "Jack O'Neil" with rating "Not Rated" with relationship "JackDaniel"}
   step %{The periodic tag count task is run}
+  step %{all indexing jobs have been run}
   step %{I flush the wrangling sidebar caches}
 end
 
@@ -443,3 +444,7 @@
   tag = Tag.find_by(name: tagname)
   puts tag.inspect
 end
+
+Then "no tag is scheduled for count update from now on" do
+  expect_any_instance_of(Tag).not_to receive(:update_filters_for_filterables)
+end
diff --git a/features/tags_and_wrangling/brand_new_fandoms.feature b/features/tags_and_wrangling/brand_new_fandoms.feature
@@ -12,6 +12,7 @@ Feature: Brand new fandoms
     Given I am logged in as a random user
       And I post a work "My New Work" with fandom "My Brand New Fandom"
       And the periodic tag count task is run
+      And all indexing jobs have been run
     When I follow "Uncategorized Fandoms" within "#header"
     Then I should see "My Brand New Fandom"
 
@@ -21,13 +22,15 @@ Feature: Brand new fandoms
       And I fill in "Fandoms" with "My Brand New Fandom"
       And I submit
       And the periodic tag count task is run
+      And all indexing jobs have been run
     When I follow "Uncategorized Fandoms" within "#header"
     Then I should see "My Brand New Fandom"
 
   Scenario: When the only work with a brand new fandom is destroyed, the fandom should not be visible on the Uncategorized Fandoms page.
     Given I am logged in as a random user
       And I post a work "My New Work" with fandom "My Brand New Fandom"
       And the periodic tag count task is run
+      And all indexing jobs have been run
     When I follow "Edit"
       And I follow "Delete Work"
       And I press "Yes"
@@ -43,6 +46,7 @@ Feature: Brand new fandoms
       And I fill in "Fandoms" with "My Brand New Fandom"
       And I submit
       And the periodic tag count task is run
+      And all indexing jobs have been run
     When I am logged in as a "policy_and_abuse" admin
       And I view the external work "External Work To Be Deleted"
       And I follow "Delete External Work"
@@ -55,6 +59,7 @@ Feature: Brand new fandoms
     Given I am logged in as a tag wrangler
       And I post a work "My New Work" with fandom "My Brand New Fandom"
       And the periodic tag count task is run
+      And all indexing jobs have been run
     When I follow "Tag Wrangling" within "#header"
       And I follow "Fandoms by media"
     Then I should see "My Brand New Fandom"
@@ -65,6 +70,7 @@ Feature: Brand new fandoms
       And I fill in "Fandoms" with "My Brand New Fandom"
       And I submit
       And the periodic tag count task is run
+      And all indexing jobs have been run
     When I follow "Tag Wrangling" within "#header"
       And I follow "Fandoms by media"
     Then I should see "My Brand New Fandom"
@@ -73,11 +79,13 @@ Feature: Brand new fandoms
     Given I am logged in as a tag wrangler
       And I post a work "My New Work" with fandom "My Brand New Fandom"
       And the periodic tag count task is run
+      And all indexing jobs have been run
     When I follow "Edit"
       And I follow "Delete Work"
       And I press "Yes"
     Then I should see "Your work My New Work was deleted."
     When the periodic tag count task is run
+      And all indexing jobs have been run
       And I follow "Tag Wrangling" within "#header"
       And I follow "Fandoms by media"
     Then I should not see "My Brand New Fandom"
@@ -89,11 +97,13 @@ Feature: Brand new fandoms
       And I fill in "Fandoms" with "My Brand New Fandom"
       And I submit
       And the periodic tag count task is run
+      And all indexing jobs have been run
     When I am logged in as a "policy_and_abuse" admin
       And I view the external work "External Work To Be Deleted"
       And I follow "Delete External Work"
     Then I should see "Item was successfully deleted."
     When the periodic tag count task is run
+      And all indexing jobs have been run
       And I am logged in as a tag wrangler
       And I follow "Tag Wrangling" within "#header"
       And I follow "Fandoms by media"

diff --git a/features/tags_and_wrangling/tag_wrangling.feature b/features/tags_and_wrangling/tag_wrangling.feature
@@ -355,3 +355,13 @@ Feature: Tag wrangling
     Then I should see "Youngest"
       But I should not see "Oldest"
       And I should not see "Middle"
+
+  Scenario: No call to Redis when no action is taken
+    Given the tag wrangling setup
+      And I am logged in as a tag wrangler
+    Then no tag is scheduled for count update from now on
+    When I go to my wrangling page
+    Then I should see "Wrangling Home"
+      And I should see "Characters by fandom (2)"
+    When I follow "Characters by fandom (2)"
+    Then I should see "Mass Wrangle New/Unwrangled Tags"