Skip to content

AO3-5701 AO3-6331 Reindex works and bookmarks when a collection's parent changes #5816

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.

Sign up for GitHub

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

Open
Smuzzy-waiii wants to merge 2 commits into
base: master
Choose a base branch
from
Open

AO3-5701 AO3-6331 Reindex works and bookmarks when a collection's parent changes #5816

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3a9bb3
AO3-5701 AO3-6331 Reindex works and bookmarks when a collection's par…
Smuzzy-waiii May 13, 2026
6190c7c
AO3-5701 AO3-6331 Remove empty line
Smuzzy-waiii May 13, 2026
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
24 changes: 24 additions & 0 deletions app/models/collection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,4 +464,28 @@ def approved_works_count
def approved_bookmarked_items_count
SearchCounts.bookmarkable_count_for_collection(self)
end

# Reindex associated works and bookmarks when a collection's parent changes
# because works and bookmarks index parent collection ids in Elasticsearch.
after_commit :reindex_associated_works_and_bookmarks,
if: :saved_change_to_parent_id?

def reindex_associated_works_and_bookmarks
Copy link
Copy Markdown
Contributor

@pmonfort pmonfort May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add specs that verify works and bookmarks are enqueued for reindexing when parent_id changes?
Maybe something like:

  let(:parent_collection) { create(:collection) }
  let(:child) { create(:collection, parent: parent_collection) }

  it "enqueues reindexing of works when parent_id changes" do
    work = create(:work)
    child.collection_items.create(item: work)

    expect(IndexQueue).to receive(:enqueue_ids).with(Work, [work.id], :background)
    allow(IndexQueue).to receive(:enqueue_ids).with(Bookmark, anything, :background)

    child.update!(parent: nil)
  end

items = all_items.pluck(:item_type, :item_id)

work_ids = []
bookmark_ids = []

items.each do |item_type, item_id|
case item_type
when "Work"
work_ids << item_id
when "Bookmark"
bookmark_ids << item_id
end
end
Comment on lines +474 to +486
Copy link
Copy Markdown
Contributor

@pmonfort pmonfort May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works great, maybe nitpick improvement could be

Suggested change
items = all_items.pluck(:item_type, :item_id)
work_ids = []
bookmark_ids = []
items.each do |item_type, item_id|
case item_type
when "Work"
work_ids << item_id
when "Bookmark"
bookmark_ids << item_id
end
end
work_ids = all_items.where(item_type: "Work").pluck(:item_id).uniq
bookmark_ids = all_items.where(item_type: "Bookmark").pluck(:item_id).uniq


IndexQueue.enqueue_ids(Work, work_ids.uniq, :background) if work_ids.present?
IndexQueue.enqueue_ids(Bookmark, bookmark_ids.uniq, :background) if bookmark_ids.present?
end
end
Loading