Convert remaining registration policy bucket key dependencies to id - #11893
Merged
Conversation
…11892) Converts SignupBucketFinder, RegistrationPolicyBucket's occupancy checks, and EventChangeRegistrationPolicyService's simulation/removed-bucket logic away from RegistrationPolicyBucket#key as an identity mechanism. Buckets are now compared by id when persisted, or by object identity for not-yet-persisted candidate buckets during registration policy simulation. Removes RegistrationPolicy#bucket_with_key and the bucket_key/ requested_bucket_key readers on Signup/SignupRequest/SignupRankedChoice, inlining the handful of legitimate remaining key-based lookups (deprecated GraphQL arguments, import data, and the two structurally-necessary candidate<->persisted bridges in EventChangeRegistrationPolicyService). No GraphQL/frontend changes: the bucket_key_mappings mutation argument must stay key-based on its "to" side, since it can name a bucket created in the same edit that has no id yet. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Code Coverage Report: Only Changed Files listed
Minimum allowed coverage is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11892
Summary
Follow-up to #11870/#11891, which handled bucket key/id correlation only for editing an existing registration policy. This converts the remaining internal identity uses of
RegistrationPolicyBucket#key— inSignupBucketFinder,RegistrationPolicyBucket's occupancy checks, andEventChangeRegistrationPolicyService's simulation/removed-bucket logic — to useidfor persisted buckets, or object identity for not-yet-persisted candidate buckets during registration policy simulation (a bucket being added in the same edit has no real id until the edit is persisted, so key is the only identity that predates the object entirely disappearing — object identity replaces it everywhere it's actually needed).RegistrationPolicyBucket#occupies_bucket_as_signup?/#signup_definitely_occupies_slot_in_bucket?: single id-or-object-identity comparison, key-based fallback removed.SignupBucketFinder/FakeSignup: track bucket objects (bucket/requested_bucket) instead of key strings.EventChangeRegistrationPolicyService:bucket_key_mappingsresolves to id (bucket_id_mappings) up front instead of round-tripping through a removed bucket's key at every use site; the simulation bridges a real signup's persisted bucket to its detached candidate counterpart via one clearly-commented key lookup (structurally required — see comments), instead of using key everywhere.RegistrationPolicy#bucket_with_keyand thebucket_key/requested_bucket_keyreaders onSignup/SignupRequest/SignupRankedChoice, sinceSignupBucketFinderwas meant to be their only caller. Turned out there were a handful of other legitimate callers (deprecated GraphQLbucketKey/requestedBucketKeymutation arguments, andimport_convention_data_service.rb) — those got an inlined equivalent lookup rather than resurrecting the shared method.sync_buckets_from_hash!'s key-based fallback (from Correlate registration policy buckets by id instead of key when editing/simulating policy changes #11870) is untouched, as planned — dropping thekeycolumn is tracked separately.No GraphQL/frontend changes
update_event'sbucket_key_mappingsargument,BucketKeyMappingInputType, and the frontend bucket remapping UI are unchanged. The registration policy editor never has real bucket ids for candidate buckets (confirmed viaRegistrationPolicy#as_json/RegistrationPolicyBucket#as_json, which deliberately omitid), soto_keymust stay key-based — it can name a bucket being created in the very same edit.Test plan
bin/rubocopclean on all changed filesRegistrationPolicyBucketTest#signup_definitely_occupies_slot_in_bucket?)🤖 Generated with Claude Code