Deprecate and remove remaining bucket key-based GraphQL/Liquid surface - #11902
Merged
Conversation
#11896) Most of the fields this issue targets were already deprecation_reason-marked (the four Signup-family types' bucketKey/requestedBucketKey, and all six mutations' bucket_key/requested_bucket_key arguments) -- so the actual work here is narrower than the issue's own background section: - Remove BucketKeyMappingInputType#from_key entirely (not just deprecate -- a removed bucket is always a persisted row with a real id already, unlike to_key's bucket, which may be newly created in the same edit and have no id yet). This required converting BucketKeyRemappingModal.tsx/ useBucketKeyRemapping.tsx to submit from_bucket_id instead, since they previously always sent from_key. - Deprecate RegistrationPolicyBucketType#key (the one field the issue flagged as not yet deprecated). This surfaces new @graphql-eslint/no-deprecated warnings on the many places that still legitimately read key for editing/remapping -- expected, not something this issue addresses. - Remove RegistrationPolicy::BucketDrop#key's delegate (confirmed zero real-world Liquid usage). - Delete the dead RegistrationPolicyInputType/RegistrationPolicyBucketInputType (referenced by nothing, not even present in the generated schema). Deliberately not removing the already-deprecated bucketKey/requestedBucketKey fields or mutation arguments yet -- the issue calls for that to wait on a deprecation window passing, which is a timing/process decision rather than something to execute in this PR. 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 #11896
Summary
Most of the fields this issue lists were already
deprecation_reason-marked (the four Signup-family types'bucketKey/requestedBucketKey, and all six mutations'bucket_key/requested_bucket_keyarguments) — confirmed by reading the code before starting. So the actual work here is narrower than the issue's background section:BucketKeyMappingInputType#from_keyentirely (not just deprecated it) — a removed bucket is always a persisted row with a real id already, unliketo_key's bucket, which may be newly created in the same edit and have no id yet. This required convertingBucketKeyRemappingModal.tsx/useBucketKeyRemapping.tsx, which previously always sentfrom_key, to submitfrom_bucket_idinstead — the removed bucket's real id was already available viaRegistrationPolicyBucket#as_json(from Give the registration policy editor real bucket ids (phase 1 of dropping RegistrationPolicyBucket#key) #11895), just not yet plumbed through these components' TypeScript types.RegistrationPolicyBucketType#key— the one field the issue flagged as not yet deprecated. This surfaces new@graphql-eslint/no-deprecatedwarnings on the many places that still legitimately readkeyfor editing/remapping (confirmed non-blocking — 0 new errors from a fullyarn eslint app/javascriptsweep). Addressing those reads is out of scope for this issue; that's Retire RegistrationPolicyBucket#key as an identity mechanism in the app layer (phase 3 of dropping RegistrationPolicyBucket#key) #11897/Drop the registration_policy_buckets.key column (phase 4 of dropping RegistrationPolicyBucket#key) #11898.RegistrationPolicy::BucketDrop#key's delegate (confirmed zero real-world Liquid usage per the issue's own research).RegistrationPolicyInputType/RegistrationPolicyBucketInputType— confirmed referenced by nothing, not even present in the generated schema before this change.Deliberately not doing: removing the already-deprecated
bucketKey/requestedBucketKeyfields or mutation arguments. The issue calls for that to wait on a deprecation window passing — a timing/process decision, not something to execute automatically here.Test plan
bin/rails graphql:update(confirmed the schema diff is exactly the two expected changes:from_keyremoved,keydeprecated — no stray diffs)yarn run tsc --noEmityarn vitest run(187 passed, 1 pre-existing skip)bin/rails test(1169 passed, 1 pre-existing skip)bundle exec rubocop/yarn eslinton all changed files (0 errors)yarn eslint app/javascriptsweep to confirm the newkeydeprecation produces only warnings, no errors🤖 Generated with Claude Code