From 38d93a4af1174917535402345b8a5f66b154d2e0 Mon Sep 17 00:00:00 2001 From: Ceithir Date: Mon, 23 Oct 2023 08:01:36 +0200 Subject: [PATCH 01/31] Move copy-pasted count to a single function --- app/controllers/tag_wranglers_controller.rb | 11 ++--------- app/controllers/tag_wranglings_controller.rb | 11 ++--------- app/controllers/unsorted_tags_controller.rb | 11 ++--------- app/helpers/wrangling_helper.rb | 14 ++++++++++++++ 4 files changed, 20 insertions(+), 27 deletions(-) create mode 100644 app/helpers/wrangling_helper.rb diff --git a/app/controllers/tag_wranglers_controller.rb b/app/controllers/tag_wranglers_controller.rb index 9842dcbb163..8419905245d 100644 --- 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 index cc3d195294b..45c7d965bf4 100644 --- a/app/controllers/tag_wranglings_controller.rb +++ b/app/controllers/tag_wranglings_controller.rb @@ -1,20 +1,13 @@ 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? params[:sort_column] = 'created_at' if !valid_sort_column(params[:sort_column], 'tag') params[:sort_direction] = 'ASC' if !valid_sort_direction(params[:sort_direction]) diff --git a/app/controllers/unsorted_tags_controller.rb b/app/controllers/unsorted_tags_controller.rb index 4def324d62a..fc6c8fd6c68 100644 --- 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 new file mode 100644 index 00000000000..7dbccdce6bc --- /dev/null +++ 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 + 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 + end +end From 0c54c40438aab1d5d5c7220cda20dfa78bff34c8 Mon Sep 17 00:00:00 2001 From: Ceithir Date: Mon, 23 Oct 2023 09:04:27 +0200 Subject: [PATCH 02/31] Load unwrangled counts from ES --- app/helpers/wrangling_helper.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/helpers/wrangling_helper.rb b/app/helpers/wrangling_helper.rb index 7dbccdce6bc..b6f22a9ffc5 100644 --- a/app/helpers/wrangling_helper.rb +++ b/app/helpers/wrangling_helper.rb @@ -3,7 +3,11 @@ 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 - klass.unwrangled.in_use.count + TagQuery.new({ + type: klass.to_s, + unwrangleable: false, + wrangled: false + }).count end end counts[:UnsortedTag] = Rails.cache.fetch("/wrangler/counts/sidebar/UnsortedTag", race_condition_ttl: 10, expires_in: 1.hour) do From 328ff607261132c6188c80ffbc3ab64bd1b6ca86 Mon Sep 17 00:00:00 2001 From: Ceithir Date: Mon, 23 Oct 2023 12:14:57 +0200 Subject: [PATCH 03/31] Now count the right thing --- app/helpers/wrangling_helper.rb | 3 +-- app/models/fandom.rb | 4 ++++ app/models/search/tag_indexer.rb | 6 ++++-- app/models/search/tag_query.rb | 7 ++++++- app/models/tag.rb | 12 ++++++++++++ 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/app/helpers/wrangling_helper.rb b/app/helpers/wrangling_helper.rb index b6f22a9ffc5..4df9fad024e 100644 --- a/app/helpers/wrangling_helper.rb +++ b/app/helpers/wrangling_helper.rb @@ -5,8 +5,7 @@ def tag_counts_per_category 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, - unwrangleable: false, - wrangled: false + to_wrangle: true }).count end end diff --git a/app/models/fandom.rb b/app/models/fandom.rb index 5ccf831e1f8..14a6d77f021 100644 --- a/app/models/fandom.rb +++ b/app/models/fandom.rb @@ -19,6 +19,10 @@ def self.unwrangled where("unwrangleable = 0 AND common_taggings.filterable_id = ? AND common_taggings.filterable_type = 'Tag'", Media.uncategorized.try(:id)) end + def unwrangled? + Media.uncategorized.fandoms.include?(self) + end + # An association callback to add the default media if all others have been removed def check_media(media) self.add_media_for_uncategorized diff --git a/app/models/search/tag_indexer.rb b/app/models/search/tag_indexer.rb index 137ca8bf0bb..ddc93dd9328 100644 --- 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" }, + to_wrangle: { 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, + to_wrangle: object.wrangleable? && object.used? && object.unwrangled? ).merge(parent_data(object)) end diff --git a/app/models/search/tag_query.rb b/app/models/search/tag_query.rb index 1e83dab1f7c..b25686fa876 100644 --- a/app/models/search/tag_query.rb +++ b/app/models/search/tag_query.rb @@ -22,7 +22,8 @@ def filters fandom_filter, character_filter, suggested_fandom_filter, - suggested_character_filter + suggested_character_filter, + to_wrangle_filter ].flatten.compact end @@ -103,6 +104,10 @@ def suggested_character_filter terms_filter(:pre_character_ids, options[:pre_character_ids]) if options[:pre_character_ids] end + def to_wrangle_filter + term_filter(:to_wrangle, bool_value(options[:to_wrangle])) unless options[:to_wrangle].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 index 519a36de453..2e2093fb6f9 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -1033,6 +1033,18 @@ def merger_string=(tag_string) end end + def wrangleable? + !unwrangleable? + end + + def used? + canonical? || taggings_count_cache > 0 + end + + def unwrangled? + common_taggings.empty? + end + ################################# ## SEARCH ####################### ################################# From 801c0ac2e840322fa6c538ed21c54173d6469a21 Mon Sep 17 00:00:00 2001 From: Ceithir Date: Mon, 23 Oct 2023 12:25:39 +0200 Subject: [PATCH 04/31] Hound --- app/helpers/wrangling_helper.rb | 5 +---- app/models/tag.rb | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/helpers/wrangling_helper.rb b/app/helpers/wrangling_helper.rb index 4df9fad024e..00d118d366e 100644 --- a/app/helpers/wrangling_helper.rb +++ b/app/helpers/wrangling_helper.rb @@ -3,10 +3,7 @@ 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, - to_wrangle: true - }).count + TagQuery.new({ type: klass.to_s, to_wrangle: true }).count end end counts[:UnsortedTag] = Rails.cache.fetch("/wrangler/counts/sidebar/UnsortedTag", race_condition_ttl: 10, expires_in: 1.hour) do diff --git a/app/models/tag.rb b/app/models/tag.rb index 2e2093fb6f9..2f4003ee4d5 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -1038,7 +1038,7 @@ def wrangleable? end def used? - canonical? || taggings_count_cache > 0 + canonical? || taggings_count_cache.positive? end def unwrangled? From da58de2a3bfbc3d6a1eba28f7074dfb49c87d730 Mon Sep 17 00:00:00 2001 From: Ceithir Date: Mon, 23 Oct 2023 13:20:01 +0200 Subject: [PATCH 05/31] Fix (some?) tests --- features/step_definitions/tag_steps.rb | 1 + features/tags_and_wrangling/tag_wrangling_more.feature | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/features/step_definitions/tag_steps.rb b/features/step_definitions/tag_steps.rb index 99704c48311..aa6f34a403b 100644 --- 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 diff --git a/features/tags_and_wrangling/tag_wrangling_more.feature b/features/tags_and_wrangling/tag_wrangling_more.feature index 44463d177f9..4ebfef407b2 100644 --- a/features/tags_and_wrangling/tag_wrangling_more.feature +++ b/features/tags_and_wrangling/tag_wrangling_more.feature @@ -151,6 +151,7 @@ Feature: Tag wrangling: assigning wranglers, using the filters on the Wranglers And the following typed tags exists | name | type | canonical | | Cowboy Bebop | Fandom | true | + And all indexing jobs have been run When I go to the wrangling tools page And I follow "Fandoms by media (1)" And I check the wrangling option for "Cowboy Bebop" @@ -165,6 +166,7 @@ Feature: Tag wrangling: assigning wranglers, using the filters on the Wranglers | name | type | canonical | | Toby Daye/Tybalt | Relationship | true | | October Daye Series - Seanan McGuire | Fandom | false | + And all indexing jobs have been run When I go to the wrangling tools page And I follow "Relationships by fandom (1)" And I check the wrangling option for "Toby Daye/Tybalt" @@ -179,6 +181,7 @@ Feature: Tag wrangling: assigning wranglers, using the filters on the Wranglers | name | type | canonical | | Toby Daye/Tybalt | Relationship | true | | October Daye Series - Seanan McGuire | Fandom | true | + And all indexing jobs have been run When I go to the wrangling tools page And I follow "Relationships by fandom (1)" And I check the wrangling option for "Toby Daye/Tybalt" @@ -194,6 +197,7 @@ Feature: Tag wrangling: assigning wranglers, using the filters on the Wranglers | Faye Valentine | Character | false | | Ed | Character | false | And I post the work "Honky Tonk Women" with fandom "Cowboy Bebop" with character "Faye Valentine" with second character "Ed" + And all indexing jobs have been run When I go to the wrangling tools page And I follow "Characters by fandom (2)" And I fill in "Wrangle to Fandom(s)" with "Cowboy Bebop" From 5bdc9e3fd90b96192d2e7b432323941aedbaea49 Mon Sep 17 00:00:00 2001 From: Ceithir Date: Mon, 23 Oct 2023 20:02:00 +0200 Subject: [PATCH 06/31] Load tags on Index from ES --- app/controllers/tag_wranglings_controller.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/app/controllers/tag_wranglings_controller.rb b/app/controllers/tag_wranglings_controller.rb index 45c7d965bf4..88a8d0e8e8e 100644 --- a/app/controllers/tag_wranglings_controller.rb +++ b/app/controllers/tag_wranglings_controller.rb @@ -9,19 +9,25 @@ class TagWranglingsController < ApplicationController def index @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, + to_wrangle: true, + sort_column: params[:sort_column], + sort_direction: params[:sort_direction], + page: params[:page], + per_page: ArchiveConfig.ITEMS_PER_PAGE + }).search_results end end From aa84894984c7cd55a280a61358ae70611e849cef Mon Sep 17 00:00:00 2001 From: Ceithir Date: Mon, 23 Oct 2023 20:12:56 +0200 Subject: [PATCH 07/31] Restore ordering by name ASC second on uses --- app/models/search/tag_query.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/models/search/tag_query.rb b/app/models/search/tag_query.rb index b25686fa876..be1a654ab71 100644 --- a/app/models/search/tag_query.rb +++ b/app/models/search/tag_query.rb @@ -61,7 +61,13 @@ def sort sort_hash[column][:unmapped_type] = "date" end - [sort_hash, { id: { order: direction } }] + sort_by_id = { id: { order: direction } } + + if column == "uses" + return [sort_hash, { "name.keyword" => { order: "asc" } }, sort_by_id] + end + + [sort_hash, sort_by_id] end ################ From 058299d807f74ac374b451947b5fbab5d2223843 Mon Sep 17 00:00:00 2001 From: Ceithir Date: Mon, 23 Oct 2023 20:14:59 +0200 Subject: [PATCH 08/31] Use _cache everywhere --- app/views/tag_wranglings/index.html.erb | 2 +- features/step_definitions/tag_steps.rb | 4 ++++ features/tags_and_wrangling/tag_wrangling.feature | 10 ++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/views/tag_wranglings/index.html.erb b/app/views/tag_wranglings/index.html.erb index ee738e57e16..b226a334de0 100755 --- a/app/views/tag_wranglings/index.html.erb +++ b/app/views/tag_wranglings/index.html.erb @@ -120,7 +120,7 @@ <% end %> - <%= tag.taggings_count %> + <%= tag.taggings_count_cache %>