Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 33 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
38d93a4
Move copy-pasted count to a single function
ceithir Oct 23, 2023
0c54c40
Load unwrangled counts from ES
ceithir Oct 23, 2023
328ff60
Now count the right thing
ceithir Oct 23, 2023
801c0ac
Hound
ceithir Oct 23, 2023
da58de2
Fix (some?) tests
ceithir Oct 23, 2023
5bdc9e3
Load tags on Index from ES
ceithir Oct 23, 2023
aa84894
Restore ordering by name ASC second on uses
ceithir Oct 23, 2023
058299d
Use _cache everywhere
ceithir Oct 23, 2023
f5f7110
Hound
ceithir Oct 23, 2023
70cf0c5
Linter
ceithir Oct 23, 2023
b6009de
Update tests
ceithir Oct 23, 2023
cddde94
Hound
ceithir Oct 23, 2023
3a56132
Some sort of solution for seemingly noop
ceithir Oct 24, 2023
7cb1b04
Linter
ceithir Oct 24, 2023
3b91e00
Now unused?
ceithir Oct 24, 2023
2a7434b
Syntax/Typo
ceithir Oct 30, 2023
96d0c3c
Only store unwrangeable value
ceithir Oct 30, 2023
8d11c1b
Hound
ceithir Oct 30, 2023
9ba58b1
Fix Grandparent bug?
ceithir Oct 30, 2023
0d5c8a7
More subtle cache bursting
ceithir Oct 31, 2023
a8d2f63
Merge remote-tracking branch 'upstream/master' into AO3-6210-reboot
ceithir Dec 17, 2023
37ccd52
Review
ceithir Dec 17, 2023
7811358
More explicit uncovered case
ceithir Dec 18, 2023
5f2fd5b
Syntax confusion
ceithir Dec 18, 2023
8039634
Merge remote-tracking branch 'upstream/master' into AO3-6210-reboot
ceithir Dec 26, 2023
b4878b7
Hopefully helpful comment
ceithir Dec 26, 2023
7e0e7fc
Review
ceithir Dec 26, 2023
21016b7
Obsolete
ceithir Dec 26, 2023
028a96f
Review
ceithir Dec 26, 2023
7f6ab92
Linter
ceithir Dec 26, 2023
ae11255
Split into (almost) unitary tags
ceithir Dec 26, 2023
56756de
Cop
ceithir Dec 26, 2023
e7d54d6
Review
ceithir Dec 28, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions app/controllers/tag_wranglers_controller.rb
Original file line number Diff line number Diff line change
@@ -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]
Expand Down Expand Up @@ -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
Expand Down
31 changes: 15 additions & 16 deletions app/controllers/tag_wranglings_controller.rb
Original file line number Diff line number Diff line change
@@ -1,34 +1,33 @@
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,
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

Expand Down
11 changes: 2 additions & 9 deletions app/controllers/unsorted_tags_controller.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
14 changes: 14 additions & 0 deletions app/helpers/wrangling_helper.rb
Original file line number Diff line number Diff line change
@@ -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, to_wrangle: 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
5 changes: 2 additions & 3 deletions app/models/fandom.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
ceithir marked this conversation as resolved.
Show resolved Hide resolved
end

# An association callback to add the default media if all others have been removed
Expand Down
6 changes: 4 additions & 2 deletions app/models/search/tag_indexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
19 changes: 17 additions & 2 deletions app/models/search/tag_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ def filters
fandom_filter,
character_filter,
suggested_fandom_filter,
suggested_character_filter
suggested_character_filter,
*to_wrangle_filter
ceithir marked this conversation as resolved.
Show resolved Hide resolved
].flatten.compact
end

Expand Down Expand Up @@ -60,7 +61,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

################
Expand Down Expand Up @@ -103,6 +108,16 @@ def suggested_character_filter
terms_filter(:pre_character_ids, options[:pre_character_ids]) if options[:pre_character_ids]
end

def to_wrangle_filter
ceithir marked this conversation as resolved.
Show resolved Hide resolved
return [] unless options[:to_wrangle]

[
{ bool: { should: [{ range: { uses: { gt: 0 } } }, term_filter(:canonical, true)] } }, # Check if used OR canonical
term_filter(:unwrangleable, false),
term_filter(:unwrangled, true)
]
ceithir marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down
7 changes: 4 additions & 3 deletions app/models/tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,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") }

Expand Down Expand Up @@ -1033,6 +1030,10 @@ def merger_string=(tag_string)
end
end

def unwrangled?
ceithir marked this conversation as resolved.
Show resolved Hide resolved
common_taggings.empty?
end

#################################
## SEARCH #######################
#################################
Expand Down
54 changes: 28 additions & 26 deletions app/views/tag_wranglings/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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? %>
ceithir marked this conversation as resolved.
Show resolved Hide resolved
<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>
Expand Down
4 changes: 4 additions & 0 deletions features/step_definitions/generic_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,7 @@ def assure_xpath_not_present(tag, attribute, value, selector)
Time.zone = zone
page.body.should =~ /#{Regexp.escape(Time.zone.now.zone)}/
end

When "I clear Rails cache" do
Rails.cache.clear
end
5 changes: 5 additions & 0 deletions features/step_definitions/tag_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Tag.any_instance.should_not_receive(:write_taggings_to_redis)
ceithir marked this conversation as resolved.
Show resolved Hide resolved
end
10 changes: 10 additions & 0 deletions features/tags_and_wrangling/brand_new_fandoms.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand Down
13 changes: 12 additions & 1 deletion features/tags_and_wrangling/tag_wrangling.feature
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,8 @@ Feature: Tag wrangling
And I should see "Grandparent" within "#parent_MetaTag_associations_to_remove_checkboxes"
But I should not see "Parent" within "#parent_MetaTag_associations_to_remove_checkboxes"

When I view the tag "Child"
When I clear Rails cache
ceithir marked this conversation as resolved.
Show resolved Hide resolved
And I view the tag "Child"
Then I should see "Grandparent" within ".meta"
But I should not see "Parent" within ".meta"

Expand All @@ -353,3 +354,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"
Loading
Loading