Skip to content

Commit fc09d8e

Browse files
nbudinclaude
andcommitted
Migrate registration policies from JSONB to ActiveRecord (#11238)
Registration policies and their buckets were stored as JSONB blobs on events/event_proposals, making bucket_key references in signups, signup_requests, and signup_ranked_choices plain strings with no referential integrity (the cause of #11229). This moves them into real registration_policies/registration_policy_buckets tables. Key design points: - Buckets are updated in place on edit (matched by key, never renamed) rather than replaced wholesale, so row identity is stable for anything that didn't change -- deliberately compatible with a future bucket_key -> FK conversion on signups (still deferred). - External API (GraphQL field names, Liquid) is unchanged: anything/ not_counted are preserved as aliases over the new flex/counted columns, since nothing in the frontend actually shows a bucket's key to users, only its name. - No accepts_nested_attributes_for, attributes= override, or == override -- this app doesn't use Rails form helpers, and explicit named methods (build_from_hash, equivalent_to?) are easier to trace than hooking into Rails' implicit machinery. - EventProposal is migrated in this same pass; a shared HasRegistrationPolicy concern unifies what was previously duplicated apply-the-change logic between Event and EventProposal's write paths. Verified against a full copy of production data end to end, which caught a few real bugs before they went anywhere near a shared database: an id leak in RegistrationPolicy#as_json that corrupted detached-policy comparisons, a stale association cache in bucket sync that let a destroyed bucket reappear in an audit log, a missing registration_policy default in AcceptEventProposalService now that the column is NOT NULL, and a dependent: option conflict that blocked destroying an event that owned its own policy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent bf929b6 commit fc09d8e

40 files changed

Lines changed: 1260 additions & 803 deletions

.rubocop_todo.yml

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ GraphQL/UnnecessaryArgumentCamelize:
3636
Lint/MissingCopEnableDirective:
3737
Exclude:
3838
- 'app/graphql/types/mutation_type.rb'
39+
- 'test/graphql/mutations/update_event_proposal_test.rb'
40+
- 'test/graphql/mutations/update_event_test.rb'
3941

4042
# Offense count: 1
4143
Lint/NoReturnInBeginEndBlocks:
@@ -49,7 +51,19 @@ Metrics/BlockLength:
4951
Exclude:
5052
- 'config/environments/production.rb'
5153
- 'config/initializers/doorkeeper.rb'
54+
- 'test/liquid_drops/convention_drop_test.rb'
55+
- 'test/models/event_test.rb'
56+
- 'test/models/registration_policy_bucket_test.rb'
57+
- 'test/models/registration_policy_test.rb'
5258
- 'test/presenters/signup_count_presenter_test.rb'
59+
- 'test/services/create_signup_request_service_test.rb'
60+
- 'test/services/create_team_member_service_test.rb'
61+
- 'test/services/event_change_registration_policy_service_test.rb'
62+
- 'test/services/event_signup_service_test.rb'
63+
- 'test/services/event_vacancy_fill_service_test.rb'
64+
- 'test/services/event_withdraw_service_test.rb'
65+
- 'test/services/execute_ranked_choice_signup_round_service_test.rb'
66+
- 'test/services/execute_ranked_choice_signup_service_test.rb'
5367

5468
# Offense count: 7
5569
# Configuration parameters: CountComments, Max, CountAsOne.
@@ -60,8 +74,20 @@ Metrics/ClassLength:
6074
- 'app/graphql/types/ability_type.rb'
6175
- 'app/graphql/types/mutation_type.rb'
6276
- 'app/graphql/types/query_type.rb'
77+
- 'app/models/convention.rb'
78+
- 'app/models/event.rb'
79+
- 'app/models/registration_policy.rb'
6380
- 'app/presenters/signup_count_presenter.rb'
81+
- 'app/services/event_change_registration_policy_service.rb'
82+
- 'app/services/execute_ranked_choice_signup_round_service.rb'
83+
- 'test/models/registration_policy_bucket_test.rb'
84+
- 'test/models/registration_policy_test.rb'
6485
- 'test/presenters/signup_count_presenter_test.rb'
86+
- 'test/services/create_signup_request_service_test.rb'
87+
- 'test/services/create_team_member_service_test.rb'
88+
- 'test/services/event_change_registration_policy_service_test.rb'
89+
- 'test/services/event_vacancy_fill_service_test.rb'
90+
- 'test/services/event_withdraw_service_test.rb'
6591

6692
# Offense count: 1
6793
# Configuration parameters: Max.
@@ -104,6 +130,7 @@ Rails/HasAndBelongsToMany:
104130

105131
Rails/HasManyOrHasOneDependent:
106132
Exclude:
133+
- 'app/models/event.rb'
107134
- 'app/models/page.rb'
108135

109136
Rails/HelperInstanceVariable:
@@ -114,6 +141,9 @@ Rails/SkipsModelValidations:
114141
Exclude:
115142
- 'app/controllers/application_controller.rb'
116143
- 'app/models/page.rb'
144+
- 'app/services/event_change_registration_policy_service.rb'
145+
- 'app/services/event_freeze_bucket_assignments_service.rb'
146+
- 'app/services/execute_ranked_choice_signup_round_service.rb'
117147
- 'db/migrate/20251229184620_allow_null_redirect_uri_on_oauth_appliations.rb'
118148

119149
# Offense count: 7
@@ -123,9 +153,52 @@ Rails/SkipsModelValidations:
123153
Style/FrozenStringLiteralComment:
124154
Exclude:
125155
- 'app/models/oauth_application.rb'
156+
- 'app/services/event_freeze_bucket_assignments_service.rb'
126157
- 'config/initializers/cors.rb'
127158
- 'config/initializers/doorkeeper.rb'
128159
- 'config/initializers/doorkeeper_openid_connect.rb'
129160
- 'lib/intercode/virtual_host_constraint.rb'
161+
- 'test/factories/event_proposals.rb'
130162
- 'test/factories/oauth_applications.rb'
163+
- 'test/liquid_drops/convention_drop_test.rb'
164+
- 'test/models/event_proposal_test.rb'
165+
- 'test/models/event_test.rb'
166+
- 'test/models/registration_policy/unlimited_test.rb'
167+
- 'test/models/registration_policy_bucket_test.rb'
168+
- 'test/models/registration_policy_test.rb'
131169
- 'test/policies/oauth_application_policy_test.rb'
170+
- 'test/services/create_signup_request_service_test.rb'
171+
- 'test/services/create_team_member_service_test.rb'
172+
- 'test/services/event_change_registration_policy_service_test.rb'
173+
- 'test/services/event_freeze_bucket_assignments_service_test.rb'
174+
- 'test/services/event_signup_service_test.rb'
175+
- 'test/services/event_vacancy_fill_service_test.rb'
176+
- 'test/services/event_withdraw_service_test.rb'
177+
- 'test/services/execute_ranked_choice_signup_round_service_test.rb'
178+
- 'test/services/execute_ranked_choice_signup_service_test.rb'
179+
180+
Layout/LineLength:
181+
Exclude:
182+
- 'test/models/registration_policy_bucket_test.rb'
183+
184+
Lint/MissingSuper:
185+
Exclude:
186+
- 'app/services/accept_event_proposal_service.rb'
187+
- 'app/services/event_change_registration_policy_service.rb'
188+
- 'app/services/event_freeze_bucket_assignments_service.rb'
189+
- 'app/services/execute_ranked_choice_signup_round_service.rb'
190+
191+
Performance/StringInclude:
192+
Exclude:
193+
- 'test/models/registration_policy_test.rb'
194+
195+
Rails/SquishedSQLHeredocs:
196+
Exclude:
197+
- 'app/services/execute_ranked_choice_signup_round_service.rb'
198+
199+
Rails/TimeZone:
200+
Exclude:
201+
- 'app/models/convention.rb'
202+
- 'test/services/event_vacancy_fill_service_test.rb'
203+
- 'test/services/execute_ranked_choice_signup_round_service_test.rb'
204+

app/graphql/mutations/freeze_bucket_assignments.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,14 @@ class Mutations::FreezeBucketAssignments < Mutations::BaseMutation
1313
load_and_authorize_convention_associated_model :events, :id, :update
1414

1515
def resolve(**)
16-
old_registration_policy = event.registration_policy.dup
16+
old_registration_policy_json = event.registration_policy.as_json
1717

1818
EventFreezeBucketAssignmentsService.new(event:, whodunit: current_user).call!
1919
event.reload
2020

2121
log_form_response_changes(
2222
event,
23-
{ "registration_policy" => [old_registration_policy.as_json, event.registration_policy.as_json] }
23+
{ "registration_policy" => [old_registration_policy_json, event.registration_policy.as_json] }
2424
)
2525

2626
{ event: }

app/graphql/mutations/update_event.rb

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,10 @@ def resolve(**args)
2828
private
2929

3030
def apply_registration_policy(event, registration_policy_attributes, bucket_key_mappings)
31-
new_registration_policy = RegistrationPolicy.new(registration_policy_attributes)
32-
return {} if event.registration_policy == new_registration_policy
33-
34-
old_registration_policy = event.registration_policy
35-
EventChangeRegistrationPolicyService.new(event, new_registration_policy, current_user, bucket_key_mappings).call!
36-
37-
event.reload
38-
39-
{ "registration_policy" => [old_registration_policy.as_json, new_registration_policy.as_json] }
31+
event.apply_registration_policy_change(registration_policy_attributes) do |new_registration_policy|
32+
EventChangeRegistrationPolicyService.new(event, new_registration_policy, current_user, bucket_key_mappings).call!
33+
event.reload
34+
end
4035
end
4136

4237
def apply_form_response_attrs(event, form_response_attrs)
Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,68 @@
11
# frozen_string_literal: true
22
class Mutations::UpdateEventProposal < Mutations::BaseMutation
3-
field :event_proposal, Types::EventProposalType, null: false
3+
description "Update an event proposal"
44

5-
argument :event_proposal, Types::EventProposalInputType, required: true, camelize: false
6-
argument :id, ID, required: false
5+
field :event_proposal, Types::EventProposalType, null: false, description: "The updated event proposal"
6+
7+
argument :event_proposal,
8+
Types::EventProposalInputType,
9+
required: true,
10+
camelize: false,
11+
description: "The event proposal attributes to update"
12+
argument :id, ID, required: false, description: "The ID of the event proposal to update"
713

814
load_and_authorize_convention_associated_model :event_proposals, :id, :update
915

10-
# rubocop:disable Metrics/MethodLength
1116
def resolve(**args)
1217
event_proposal_attrs = args[:event_proposal].to_h.stringify_keys
18+
form_response_attrs = JSON.parse(event_proposal_attrs.delete("form_response_attrs_json"))
19+
registration_policy_attributes = form_response_attrs.delete("registration_policy")
20+
21+
changes = apply_registration_policy(event_proposal, registration_policy_attributes)
22+
changes.update(apply_form_response_attrs(event_proposal, form_response_attrs))
23+
event_proposal.assign_attributes(event_proposal_attrs)
24+
event_proposal.save!
25+
26+
log_form_response_changes(event_proposal, changes) if event_proposal.status != "draft"
27+
28+
{ event_proposal: }
29+
end
30+
31+
private
32+
33+
def apply_registration_policy(event_proposal, registration_policy_attributes)
34+
event_proposal.apply_registration_policy_change(registration_policy_attributes) do |new_registration_policy|
35+
ActiveRecord::Base.transaction do
36+
if event_proposal.registration_policy
37+
event_proposal.registration_policy.update_from!(new_registration_policy)
38+
else
39+
event_proposal.registration_policy = new_registration_policy
40+
event_proposal.save!
41+
end
42+
end
43+
end
44+
end
45+
46+
def apply_form_response_attrs(event_proposal, form_response_attrs)
1347
event_proposal.assign_form_response_attributes(
1448
event_proposal.filter_form_response_attributes_for_assignment(
15-
JSON.parse(event_proposal_attrs.delete("form_response_attrs_json")),
49+
form_response_attrs,
1650
event_proposal.event_category.event_proposal_form.form_items,
1751
context[:pundit_user]
1852
)
1953
)
20-
event_proposal.assign_attributes(event_proposal_attrs)
21-
event_proposal.save!
54+
event_proposal.form_response_attribute_changes
55+
end
2256

23-
if event_proposal.status != "draft"
24-
event_proposal.form_response_attribute_changes.each do |(key, (previous_value, new_value))|
25-
FormResponseChange.create!(
26-
response: event_proposal,
27-
user_con_profile:,
28-
field_identifier: key,
29-
previous_value:,
30-
new_value:
31-
)
32-
end
57+
def log_form_response_changes(event_proposal, changes)
58+
changes.each do |(key, (previous_value, new_value))|
59+
FormResponseChange.create!(
60+
response: event_proposal,
61+
user_con_profile:,
62+
field_identifier: key,
63+
previous_value:,
64+
new_value:
65+
)
3366
end
34-
35-
{ event_proposal: }
3667
end
37-
# rubocop:enable Metrics/MethodLength
3868
end
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# frozen_string_literal: true
2+
module HasRegistrationPolicy
3+
extend ActiveSupport::Concern
4+
5+
# Builds a detached proposed RegistrationPolicy from `hash`, and -- if it actually differs
6+
# from the current one -- captures an eager "before" JSON snapshot (the caller's block may
7+
# update the current policy's rows in place rather than replacing them, so capturing
8+
# afterward would show the already-updated state), yields the proposal for the caller to
9+
# apply however is appropriate, and returns a {"registration_policy" => [old, new]} change
10+
# hash for FormResponseChange logging. Returns {} if there's nothing to apply.
11+
def apply_registration_policy_change(hash)
12+
return {} unless hash
13+
14+
new_registration_policy = RegistrationPolicy.build_from_hash(hash)
15+
return {} if registration_policy&.equivalent_to?(new_registration_policy)
16+
17+
old_registration_policy_json = registration_policy&.as_json
18+
yield new_registration_policy
19+
{ "registration_policy" => [old_registration_policy_json, registration_policy.as_json] }
20+
end
21+
end

app/models/convention.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,11 @@ def timezone
199199
end
200200

201201
def bucket_metadata_from_events
202-
events.pluck(:registration_policy).flat_map { |p| p.buckets.flat_map(&:metadata) }.uniq
202+
RegistrationPolicyBucket
203+
.where(registration_policy_id: events.select(:registration_policy_id))
204+
.pluck(:key, :name, :description)
205+
.map { |key, name, description| { key:, name:, description: } }
206+
.uniq
203207
end
204208

205209
def to_liquid

app/models/event.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ class Event < ApplicationRecord
5656
include AgeRestrictions
5757
include EventEmail
5858
include FormResponse
59+
include HasRegistrationPolicy
5960
include MarkdownIndexing
6061
include OrderByTitle
6162
include PgSearch::Model
@@ -116,6 +117,7 @@ class Event < ApplicationRecord
116117

117118
belongs_to :convention
118119
belongs_to :event_category
120+
belongs_to :registration_policy, inverse_of: :events, dependent: :destroy
119121

120122
has_many :maximum_event_provided_tickets_overrides, dependent: :destroy
121123
has_many :provided_tickets, class_name: "Ticket", inverse_of: "provided_by_event", foreign_key: "provided_by_event_id"
@@ -199,8 +201,6 @@ class Event < ApplicationRecord
199201

200202
scope :active, -> { where(status: "active") }
201203

202-
serialize :registration_policy, coder: ActiveModelCoder.new("RegistrationPolicy")
203-
204204
attr_accessor :bypass_single_event_run_check, :allow_registration_policy_change
205205

206206
def to_param
@@ -266,12 +266,12 @@ def single_run_events_must_have_no_more_than_one_run
266266

267267
def registration_policy_cannot_change
268268
return if new_record?
269-
return unless registration_policy_changed?
269+
return unless registration_policy_id_changed?
270270

271-
before, after = changes["registration_policy"]
271+
before, after = changes["registration_policy_id"]
272272
return if before == after # ActiveRecord is being overzealous about change detection
273273

274-
errors.add :registration_policy,
274+
errors.add :registration_policy_id,
275275
"cannot be changed via ActiveRecord on an existing event. \
276276
Use EventChangeRegistrationPolicyService instead."
277277
end

app/models/event_proposal.rb

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
# description :text
1212
# email :text
1313
# length_seconds :integer
14-
# registration_policy :jsonb
1514
# reminded_at :datetime
1615
# short_blurb :text
1716
# status :string
@@ -25,27 +24,31 @@
2524
# event_category_id :bigint not null
2625
# event_id :bigint
2726
# owner_id :bigint
27+
# registration_policy_id :bigint
2828
#
2929
# Indexes
3030
#
31-
# index_event_proposals_on_convention_id (convention_id)
32-
# index_event_proposals_on_event_category_id (event_category_id)
33-
# index_event_proposals_on_event_id (event_id)
34-
# index_event_proposals_on_owner_id (owner_id)
31+
# index_event_proposals_on_convention_id (convention_id)
32+
# index_event_proposals_on_event_category_id (event_category_id)
33+
# index_event_proposals_on_event_id (event_id)
34+
# index_event_proposals_on_owner_id (owner_id)
35+
# index_event_proposals_on_registration_policy_id (registration_policy_id)
3536
#
3637
# Foreign Keys
3738
#
3839
# fk_rails_... (convention_id => conventions.id)
3940
# fk_rails_... (event_category_id => event_categories.id)
4041
# fk_rails_... (event_id => events.id)
4142
# fk_rails_... (owner_id => user_con_profiles.id)
43+
# fk_rails_... (registration_policy_id => registration_policies.id)
4244
#
4345
# rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective
4446

4547
class EventProposal < ApplicationRecord
4648
include AgeRestrictions
4749
include EventEmail
4850
include FormResponse
51+
include HasRegistrationPolicy
4952
include MarkdownIndexing
5053
include OrderByTitle
5154
include PgSearch::Model
@@ -68,6 +71,7 @@ class EventProposal < ApplicationRecord
6871
belongs_to :owner, class_name: "UserConProfile", optional: true
6972
belongs_to :event, optional: true
7073
belongs_to :event_category
74+
belongs_to :registration_policy, inverse_of: :event_proposals, optional: true, dependent: :destroy
7175

7276
has_many_attached :images
7377

@@ -77,7 +81,6 @@ class EventProposal < ApplicationRecord
7781
scope :reminded, -> { where.not(reminded_at: nil) }
7882
scope :not_reminded, -> { where(reminded_at: nil) }
7983

80-
serialize :registration_policy, coder: ActiveModelCoder.new("RegistrationPolicy")
8184
serialize :timeblock_preferences,
8285
coder: JSONArrayCoderWrapper.new(ActiveModelCoder.new("EventProposal::TimeblockPreference"))
8386

0 commit comments

Comments
 (0)