Skip to content

Scope clear_search_records to the account being destroyed#2847

Merged
flavorjones merged 1 commit intomainfrom
diagnose-search-problems
Apr 16, 2026
Merged

Scope clear_search_records to the account being destroyed#2847
flavorjones merged 1 commit intomainfrom
diagnose-search-problems

Conversation

@flavorjones
Copy link
Copy Markdown
Member

@flavorjones flavorjones commented Apr 16, 2026

Account::Searchable#clear_search_records called
Search::Record.for(id).destroy_all which deleted every row in the shard's table, wiping search records belonging to every other account that happened to hash to the same shard.

ref: #2804
ref: https://3.basecamp.com/2914079/buckets/27/card_tables/cards/9782824728

Account::Searchable#clear_search_records called
Search::Record.for(id).destroy_all which deleted every row in the
shard's table, wiping search records belonging to every other account
that happened to hash to the same shard.

ref: https://3.basecamp.com/2914079/buckets/27/card_tables/cards/9782824728
Copilot AI review requested due to automatic review settings April 16, 2026 19:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes account incineration so clearing sharded Search::Record entries only deletes records belonging to the account being destroyed, preventing cross-account data loss when multiple accounts hash to the same shard.

Changes:

  • Scope Account::Searchable#clear_search_records to where(account_id: id) before destroy_all.
  • Add a regression test that simulates a shard collision and asserts only the doomed account’s search records are removed.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/models/account/incineratable_test.rb Adds coverage ensuring incineration only clears the account’s own sharded search records while preserving others on the same shard.
app/models/account/searchable.rb Fixes the deletion query to be account-scoped within the shard to avoid wiping other accounts’ search records.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@flavorjones flavorjones merged commit 9a27ca8 into main Apr 16, 2026
17 checks passed
@flavorjones flavorjones deleted the diagnose-search-problems branch April 16, 2026 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants