From bf929b67e983b04257357e835a85e47ede7349cb Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Tue, 16 Jun 2026 08:41:34 -0700 Subject: [PATCH 1/8] wip --- app/models/event.rb | 14 +- app/models/registration_policy_bucket.rb | 29 ++++ ...convert_registration_policies_to_tables.rb | 124 ++++++++++++++++ db/structure.sql | 138 +++++++++++++++++- test/factories/events.rb | 14 +- test/models/event_test.rb | 14 +- .../models/registration_policy_bucket_test.rb | 34 +++++ 7 files changed, 347 insertions(+), 20 deletions(-) create mode 100644 app/models/registration_policy_bucket.rb create mode 100644 db/migrate/20260615192952_convert_registration_policies_to_tables.rb create mode 100644 test/models/registration_policy_bucket_test.rb diff --git a/app/models/event.rb b/app/models/event.rb index ebe2df48656..41513df12e1 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -19,7 +19,6 @@ # organization :string # participant_communications :text # private_signup_list :boolean default(FALSE), not null -# registration_policy :jsonb # short_blurb :text # status :string default("active"), not null # team_mailing_list_name :text @@ -31,21 +30,24 @@ # convention_id :bigint not null # event_category_id :bigint not null # owner_id :bigint +# registration_policy_id :bigint not null # updated_by_id :bigint # # Indexes # -# index_events_on_convention_id (convention_id) -# index_events_on_event_category_id (event_category_id) -# index_events_on_owner_id (owner_id) -# index_events_on_title_vector (title_vector) USING gin -# index_events_on_updated_by_id (updated_by_id) +# index_events_on_convention_id (convention_id) +# index_events_on_event_category_id (event_category_id) +# index_events_on_owner_id (owner_id) +# index_events_on_registration_policy_id (registration_policy_id) +# index_events_on_title_vector (title_vector) USING gin +# index_events_on_updated_by_id (updated_by_id) # # Foreign Keys # # fk_rails_... (convention_id => conventions.id) # fk_rails_... (event_category_id => event_categories.id) # fk_rails_... (owner_id => users.id) +# fk_rails_... (registration_policy_id => registration_policies.id) # fk_rails_... (updated_by_id => users.id) # # rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective diff --git a/app/models/registration_policy_bucket.rb b/app/models/registration_policy_bucket.rb new file mode 100644 index 00000000000..c6280582fd8 --- /dev/null +++ b/app/models/registration_policy_bucket.rb @@ -0,0 +1,29 @@ +# rubocop:disable Layout/LineLength, Lint/RedundantCopDisableDirective +# == Schema Information +# +# Table name: registration_policy_buckets +# +# id :bigint not null, primary key +# counted :boolean default(TRUE), not null +# description :text +# expose_attendees :boolean default(FALSE), not null +# flex :boolean default(FALSE), not null +# key :string not null +# minimum_slots :integer +# name :text not null +# position :integer not null +# preferred_slots :integer +# slots_limited :boolean default(FALSE), not null +# total_slots :integer +# created_at :datetime not null +# updated_at :datetime not null +# registration_policy_id :bigint not null +# +# Indexes +# +# idx_on_registration_policy_id_key_b71cb40026 (registration_policy_id,key) UNIQUE +# index_registration_policy_buckets_on_registration_policy_id (registration_policy_id) +# +# rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective +class RegistrationPolicyBucket < ApplicationRecord +end diff --git a/db/migrate/20260615192952_convert_registration_policies_to_tables.rb b/db/migrate/20260615192952_convert_registration_policies_to_tables.rb new file mode 100644 index 00000000000..125533ae7b3 --- /dev/null +++ b/db/migrate/20260615192952_convert_registration_policies_to_tables.rb @@ -0,0 +1,124 @@ +class ConvertRegistrationPoliciesToTables < ActiveRecord::Migration[8.1] + def change + create_table :registration_policies do |t| + t.boolean :prevent_no_preference_signups, null: false, default: false + t.boolean :freeze_no_preference_buckets, null: false, default: false + + t.timestamps + end + + create_table :registration_policy_buckets do |t| + t.references :registration_policy, null: false + t.integer :position, null: false + t.string :key, null: false + t.text :name, null: false + t.text :description + t.integer :minimum_slots + t.integer :preferred_slots + t.integer :total_slots + t.boolean :slots_limited, default: false, null: false + t.boolean :flex, default: false, null: false + t.boolean :counted, default: true, null: false + t.boolean :expose_attendees, default: false, null: false + + t.timestamps + + t.index [:registration_policy_id, :key], unique: true + end + + add_reference :events, :registration_policy, foreign_key: true + + reversible do |dir| + dir.up do + data = select_rows("SELECT id, registration_policy FROM events") + data.each do |(event_id, policy_json)| + policy = JSON.parse(policy_json || "{}") + now = Time.zone.now + + registration_policy_id = exec_insert( + "INSERT INTO registration_policies (prevent_no_preference_signups, freeze_no_preference_buckets, created_at, updated_at) VALUES ($1, $2, $3, $4)", + "RegistrationPolicy insert", + [ + !!policy["prevent_no_preference_signups"], + !!policy["freeze_no_preference_buckets"], + now, + now + ], + returning: [:id] + ).rows.first.first + + now = Time.zone.now + bucket_values = (policy["buckets"] || []).each_with_index.map do |bucket, index| + { + registration_policy_id:, + position: index + 1, + key: bucket["key"], + name: bucket["name"] || bucket["key"], + description: bucket["description"], + minimum_slots: bucket["minimum_slots"], + preferred_slots: bucket["preferred_slots"], + total_slots: bucket["total_slots"], + slots_limited: !!bucket["slots_limited"], + flex: !!bucket["anything"], + counted: !bucket["not_counted"], # we're inverting the meaning on this one + expose_attendees: !!bucket["expose_attendees"], + created_at: now, + updated_at: now + } + end + + exec_insert( + <<~SQL.squish, + INSERT INTO registration_policy_buckets + ( + registration_policy_id, + position, + key, + name, + description, + minimum_slots, + preferred_slots, + total_slots, + slots_limited, + flex, + counted, + expose_attendees, + created_at, + updated_at + ) + SELECT + registration_policy_id, + position, + key, + name, + description, + minimum_slots, + preferred_slots, + total_slots, + slots_limited, + flex, + counted, + expose_attendees, + created_at, + updated_at + FROM jsonb_populate_recordset(NULL::registration_policy_buckets, $1::jsonb) + SQL + "RegistrationPolicyBucket bulk insert", + [bucket_values.to_json] + ) + + exec_update( + "UPDATE events SET registration_policy_id = $1 WHERE id = $2", + "Event update", + [registration_policy_id, event_id] + ) + end + end + end + + change_table :events, bulk: true do |t| + t.remove :registration_policy, type: :jsonb + t.change_null :registration_policy_id, false + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 93553a7c008..b6328ec57eb 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1544,7 +1544,6 @@ CREATE TABLE public.events ( convention_id bigint NOT NULL, owner_id bigint, status character varying DEFAULT 'active'::character varying NOT NULL, - registration_policy jsonb, participant_communications text, age_restrictions_description text, content_warnings text, @@ -1554,7 +1553,8 @@ CREATE TABLE public.events ( private_signup_list boolean DEFAULT false NOT NULL, event_category_id bigint NOT NULL, minimum_age integer, - title_vector tsvector + title_vector tsvector, + registration_policy_id bigint NOT NULL ); @@ -2382,6 +2382,80 @@ CREATE SEQUENCE public.ranked_choice_user_constraints_id_seq ALTER SEQUENCE public.ranked_choice_user_constraints_id_seq OWNED BY public.ranked_choice_user_constraints.id; +-- +-- Name: registration_policies; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.registration_policies ( + id bigint NOT NULL, + prevent_no_preference_signups boolean DEFAULT false NOT NULL, + freeze_no_preference_buckets boolean DEFAULT false NOT NULL, + created_at timestamp(6) without time zone NOT NULL, + updated_at timestamp(6) without time zone NOT NULL +); + + +-- +-- Name: registration_policies_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.registration_policies_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: registration_policies_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.registration_policies_id_seq OWNED BY public.registration_policies.id; + + +-- +-- Name: registration_policy_buckets; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.registration_policy_buckets ( + id bigint NOT NULL, + registration_policy_id bigint NOT NULL, + "position" integer NOT NULL, + key character varying NOT NULL, + name text NOT NULL, + description text, + minimum_slots integer, + preferred_slots integer, + total_slots integer, + slots_limited boolean DEFAULT false NOT NULL, + flex boolean DEFAULT false NOT NULL, + counted boolean DEFAULT true NOT NULL, + expose_attendees boolean DEFAULT false NOT NULL, + created_at timestamp(6) without time zone NOT NULL, + updated_at timestamp(6) without time zone NOT NULL +); + + +-- +-- Name: registration_policy_buckets_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.registration_policy_buckets_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: registration_policy_buckets_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.registration_policy_buckets_id_seq OWNED BY public.registration_policy_buckets.id; + + -- -- Name: rooms; Type: TABLE; Schema: public; Owner: - -- @@ -3356,6 +3430,20 @@ ALTER TABLE ONLY public.ranked_choice_decisions ALTER COLUMN id SET DEFAULT next ALTER TABLE ONLY public.ranked_choice_user_constraints ALTER COLUMN id SET DEFAULT nextval('public.ranked_choice_user_constraints_id_seq'::regclass); +-- +-- Name: registration_policies id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.registration_policies ALTER COLUMN id SET DEFAULT nextval('public.registration_policies_id_seq'::regclass); + + +-- +-- Name: registration_policy_buckets id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.registration_policy_buckets ALTER COLUMN id SET DEFAULT nextval('public.registration_policy_buckets_id_seq'::regclass); + + -- -- Name: rooms id; Type: DEFAULT; Schema: public; Owner: - -- @@ -3900,6 +3988,22 @@ ALTER TABLE ONLY public.ranked_choice_user_constraints ADD CONSTRAINT ranked_choice_user_constraints_pkey PRIMARY KEY (id); +-- +-- Name: registration_policies registration_policies_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.registration_policies + ADD CONSTRAINT registration_policies_pkey PRIMARY KEY (id); + + +-- +-- Name: registration_policy_buckets registration_policy_buckets_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.registration_policy_buckets + ADD CONSTRAINT registration_policy_buckets_pkey PRIMARY KEY (id); + + -- -- Name: rooms rooms_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -4080,6 +4184,13 @@ CREATE INDEX idx_max_event_provided_tickets_on_event_id ON public.maximum_event_ CREATE INDEX idx_max_event_provided_tickets_on_ticket_type_id ON public.maximum_event_provided_tickets_overrides USING btree (ticket_type_id); +-- +-- Name: idx_on_registration_policy_id_key_b71cb40026; Type: INDEX; Schema: public; Owner: - +-- + +CREATE UNIQUE INDEX idx_on_registration_policy_id_key_b71cb40026 ON public.registration_policy_buckets USING btree (registration_policy_id, key); + + -- -- Name: idx_on_user_con_profile_id_state_priority_7c693e2c51; Type: INDEX; Schema: public; Owner: - -- @@ -4479,6 +4590,13 @@ CREATE INDEX index_events_on_event_category_id ON public.events USING btree (eve CREATE INDEX index_events_on_owner_id ON public.events USING btree (owner_id); +-- +-- Name: index_events_on_registration_policy_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_events_on_registration_policy_id ON public.events USING btree (registration_policy_id); + + -- -- Name: index_events_on_title_vector; Type: INDEX; Schema: public; Owner: - -- @@ -4822,6 +4940,13 @@ CREATE INDEX index_ranked_choice_decisions_on_user_con_profile_id ON public.rank CREATE INDEX index_ranked_choice_user_constraints_on_user_con_profile_id ON public.ranked_choice_user_constraints USING btree (user_con_profile_id); +-- +-- Name: index_registration_policy_buckets_on_registration_policy_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_registration_policy_buckets_on_registration_policy_id ON public.registration_policy_buckets USING btree (registration_policy_id); + + -- -- Name: index_rooms_on_convention_id; Type: INDEX; Schema: public; Owner: - -- @@ -5218,6 +5343,14 @@ ALTER TABLE ONLY public.runs ADD CONSTRAINT fk_rails_058061fb50 FOREIGN KEY (updated_by_id) REFERENCES public.users(id); +-- +-- Name: events fk_rails_0682145037; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events + ADD CONSTRAINT fk_rails_0682145037 FOREIGN KEY (registration_policy_id) REFERENCES public.registration_policies(id); + + -- -- Name: assumed_identity_request_logs fk_rails_072c03953e; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -6161,6 +6294,7 @@ ALTER TABLE ONLY public.cms_files_pages SET search_path TO "$user", public; INSERT INTO "schema_migrations" (version) VALUES +('20260615192952'), ('20260601000000'), ('20260524175914'), ('20260519000000'), diff --git a/test/factories/events.rb b/test/factories/events.rb index d75dca46049..012e5788830 100644 --- a/test/factories/events.rb +++ b/test/factories/events.rb @@ -18,7 +18,6 @@ # organization :string # participant_communications :text # private_signup_list :boolean default(FALSE), not null -# registration_policy :jsonb # short_blurb :text # status :string default("active"), not null # team_mailing_list_name :text @@ -30,21 +29,24 @@ # convention_id :bigint not null # event_category_id :bigint not null # owner_id :bigint +# registration_policy_id :bigint not null # updated_by_id :bigint # # Indexes # -# index_events_on_convention_id (convention_id) -# index_events_on_event_category_id (event_category_id) -# index_events_on_owner_id (owner_id) -# index_events_on_title_vector (title_vector) USING gin -# index_events_on_updated_by_id (updated_by_id) +# index_events_on_convention_id (convention_id) +# index_events_on_event_category_id (event_category_id) +# index_events_on_owner_id (owner_id) +# index_events_on_registration_policy_id (registration_policy_id) +# index_events_on_title_vector (title_vector) USING gin +# index_events_on_updated_by_id (updated_by_id) # # Foreign Keys # # fk_rails_... (convention_id => conventions.id) # fk_rails_... (event_category_id => event_categories.id) # fk_rails_... (owner_id => users.id) +# fk_rails_... (registration_policy_id => registration_policies.id) # fk_rails_... (updated_by_id => users.id) # # rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective diff --git a/test/models/event_test.rb b/test/models/event_test.rb index 452572672ef..174cfec0c50 100644 --- a/test/models/event_test.rb +++ b/test/models/event_test.rb @@ -18,7 +18,6 @@ # organization :string # participant_communications :text # private_signup_list :boolean default(FALSE), not null -# registration_policy :jsonb # short_blurb :text # status :string default("active"), not null # team_mailing_list_name :text @@ -30,21 +29,24 @@ # convention_id :bigint not null # event_category_id :bigint not null # owner_id :bigint +# registration_policy_id :bigint not null # updated_by_id :bigint # # Indexes # -# index_events_on_convention_id (convention_id) -# index_events_on_event_category_id (event_category_id) -# index_events_on_owner_id (owner_id) -# index_events_on_title_vector (title_vector) USING gin -# index_events_on_updated_by_id (updated_by_id) +# index_events_on_convention_id (convention_id) +# index_events_on_event_category_id (event_category_id) +# index_events_on_owner_id (owner_id) +# index_events_on_registration_policy_id (registration_policy_id) +# index_events_on_title_vector (title_vector) USING gin +# index_events_on_updated_by_id (updated_by_id) # # Foreign Keys # # fk_rails_... (convention_id => conventions.id) # fk_rails_... (event_category_id => event_categories.id) # fk_rails_... (owner_id => users.id) +# fk_rails_... (registration_policy_id => registration_policies.id) # fk_rails_... (updated_by_id => users.id) # # rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective diff --git a/test/models/registration_policy_bucket_test.rb b/test/models/registration_policy_bucket_test.rb new file mode 100644 index 00000000000..f93cd4111a0 --- /dev/null +++ b/test/models/registration_policy_bucket_test.rb @@ -0,0 +1,34 @@ +# rubocop:disable Layout/LineLength, Lint/RedundantCopDisableDirective +# == Schema Information +# +# Table name: registration_policy_buckets +# +# id :bigint not null, primary key +# counted :boolean default(TRUE), not null +# description :text +# expose_attendees :boolean default(FALSE), not null +# flex :boolean default(FALSE), not null +# key :string not null +# minimum_slots :integer +# name :text not null +# position :integer not null +# preferred_slots :integer +# slots_limited :boolean default(FALSE), not null +# total_slots :integer +# created_at :datetime not null +# updated_at :datetime not null +# registration_policy_id :bigint not null +# +# Indexes +# +# idx_on_registration_policy_id_key_b71cb40026 (registration_policy_id,key) UNIQUE +# index_registration_policy_buckets_on_registration_policy_id (registration_policy_id) +# +# rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective +require "test_helper" + +class RegistrationPolicyBucketTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end From fc09d8ee9cbc3846b977b44678a93bb7bb6bfe1b Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Wed, 15 Jul 2026 11:09:20 -0700 Subject: [PATCH 2/8] 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 --- .rubocop_todo.yml | 73 +++++ .../mutations/freeze_bucket_assignments.rb | 4 +- app/graphql/mutations/update_event.rb | 13 +- .../mutations/update_event_proposal.rb | 70 +++-- .../concerns/has_registration_policy.rb | 21 ++ app/models/convention.rb | 6 +- app/models/event.rb | 10 +- app/models/event_proposal.rb | 15 +- app/models/registration_policy.rb | 238 +++++++++------- app/models/registration_policy/bucket.rb | 116 -------- app/models/registration_policy/error.rb | 8 - app/models/registration_policy_bucket.rb | 128 ++++++++- app/services/accept_event_proposal_service.rb | 20 +- ...vent_change_registration_policy_service.rb | 3 +- ...event_freeze_bucket_assignments_service.rb | 18 +- ...cute_ranked_choice_signup_round_service.rb | 14 +- .../import_convention_data_service.rb | 5 +- ...convert_registration_policies_to_tables.rb | 191 +++++++------ db/structure.sql | 256 ++---------------- test/factories/event_proposals.rb | 12 +- test/factories/registration_policies.rb | 15 + test/factories/registration_policy_buckets.rb | 37 +++ .../mutations/update_event_proposal_test.rb | 6 +- test/graphql/mutations/update_event_test.rb | 6 +- test/liquid_drops/convention_drop_test.rb | 30 +- test/models/event_proposal_test.rb | 12 +- test/models/event_test.rb | 26 +- .../registration_policy/unlimited_test.rb | 8 +- .../models/registration_policy_bucket_test.rb | 136 +++++++++- test/models/registration_policy_test.rb | 184 +++++++++++-- .../presenters/signup_count_presenter_test.rb | 6 +- .../create_signup_request_service_test.rb | 18 +- .../create_team_member_service_test.rb | 69 ++--- ...change_registration_policy_service_test.rb | 64 +++-- ..._freeze_bucket_assignments_service_test.rb | 24 +- test/services/event_signup_service_test.rb | 61 +++-- .../event_vacancy_fill_service_test.rb | 53 ++-- test/services/event_withdraw_service_test.rb | 17 +- ...ranked_choice_signup_round_service_test.rb | 28 +- ...ecute_ranked_choice_signup_service_test.rb | 42 ++- 40 files changed, 1260 insertions(+), 803 deletions(-) create mode 100644 app/models/concerns/has_registration_policy.rb delete mode 100644 app/models/registration_policy/bucket.rb delete mode 100644 app/models/registration_policy/error.rb create mode 100644 test/factories/registration_policies.rb create mode 100644 test/factories/registration_policy_buckets.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 4de021227b5..72083d7564f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -36,6 +36,8 @@ GraphQL/UnnecessaryArgumentCamelize: Lint/MissingCopEnableDirective: Exclude: - 'app/graphql/types/mutation_type.rb' + - 'test/graphql/mutations/update_event_proposal_test.rb' + - 'test/graphql/mutations/update_event_test.rb' # Offense count: 1 Lint/NoReturnInBeginEndBlocks: @@ -49,7 +51,19 @@ Metrics/BlockLength: Exclude: - 'config/environments/production.rb' - 'config/initializers/doorkeeper.rb' + - 'test/liquid_drops/convention_drop_test.rb' + - 'test/models/event_test.rb' + - 'test/models/registration_policy_bucket_test.rb' + - 'test/models/registration_policy_test.rb' - 'test/presenters/signup_count_presenter_test.rb' + - 'test/services/create_signup_request_service_test.rb' + - 'test/services/create_team_member_service_test.rb' + - 'test/services/event_change_registration_policy_service_test.rb' + - 'test/services/event_signup_service_test.rb' + - 'test/services/event_vacancy_fill_service_test.rb' + - 'test/services/event_withdraw_service_test.rb' + - 'test/services/execute_ranked_choice_signup_round_service_test.rb' + - 'test/services/execute_ranked_choice_signup_service_test.rb' # Offense count: 7 # Configuration parameters: CountComments, Max, CountAsOne. @@ -60,8 +74,20 @@ Metrics/ClassLength: - 'app/graphql/types/ability_type.rb' - 'app/graphql/types/mutation_type.rb' - 'app/graphql/types/query_type.rb' + - 'app/models/convention.rb' + - 'app/models/event.rb' + - 'app/models/registration_policy.rb' - 'app/presenters/signup_count_presenter.rb' + - 'app/services/event_change_registration_policy_service.rb' + - 'app/services/execute_ranked_choice_signup_round_service.rb' + - 'test/models/registration_policy_bucket_test.rb' + - 'test/models/registration_policy_test.rb' - 'test/presenters/signup_count_presenter_test.rb' + - 'test/services/create_signup_request_service_test.rb' + - 'test/services/create_team_member_service_test.rb' + - 'test/services/event_change_registration_policy_service_test.rb' + - 'test/services/event_vacancy_fill_service_test.rb' + - 'test/services/event_withdraw_service_test.rb' # Offense count: 1 # Configuration parameters: Max. @@ -104,6 +130,7 @@ Rails/HasAndBelongsToMany: Rails/HasManyOrHasOneDependent: Exclude: + - 'app/models/event.rb' - 'app/models/page.rb' Rails/HelperInstanceVariable: @@ -114,6 +141,9 @@ Rails/SkipsModelValidations: Exclude: - 'app/controllers/application_controller.rb' - 'app/models/page.rb' + - 'app/services/event_change_registration_policy_service.rb' + - 'app/services/event_freeze_bucket_assignments_service.rb' + - 'app/services/execute_ranked_choice_signup_round_service.rb' - 'db/migrate/20251229184620_allow_null_redirect_uri_on_oauth_appliations.rb' # Offense count: 7 @@ -123,9 +153,52 @@ Rails/SkipsModelValidations: Style/FrozenStringLiteralComment: Exclude: - 'app/models/oauth_application.rb' + - 'app/services/event_freeze_bucket_assignments_service.rb' - 'config/initializers/cors.rb' - 'config/initializers/doorkeeper.rb' - 'config/initializers/doorkeeper_openid_connect.rb' - 'lib/intercode/virtual_host_constraint.rb' + - 'test/factories/event_proposals.rb' - 'test/factories/oauth_applications.rb' + - 'test/liquid_drops/convention_drop_test.rb' + - 'test/models/event_proposal_test.rb' + - 'test/models/event_test.rb' + - 'test/models/registration_policy/unlimited_test.rb' + - 'test/models/registration_policy_bucket_test.rb' + - 'test/models/registration_policy_test.rb' - 'test/policies/oauth_application_policy_test.rb' + - 'test/services/create_signup_request_service_test.rb' + - 'test/services/create_team_member_service_test.rb' + - 'test/services/event_change_registration_policy_service_test.rb' + - 'test/services/event_freeze_bucket_assignments_service_test.rb' + - 'test/services/event_signup_service_test.rb' + - 'test/services/event_vacancy_fill_service_test.rb' + - 'test/services/event_withdraw_service_test.rb' + - 'test/services/execute_ranked_choice_signup_round_service_test.rb' + - 'test/services/execute_ranked_choice_signup_service_test.rb' + +Layout/LineLength: + Exclude: + - 'test/models/registration_policy_bucket_test.rb' + +Lint/MissingSuper: + Exclude: + - 'app/services/accept_event_proposal_service.rb' + - 'app/services/event_change_registration_policy_service.rb' + - 'app/services/event_freeze_bucket_assignments_service.rb' + - 'app/services/execute_ranked_choice_signup_round_service.rb' + +Performance/StringInclude: + Exclude: + - 'test/models/registration_policy_test.rb' + +Rails/SquishedSQLHeredocs: + Exclude: + - 'app/services/execute_ranked_choice_signup_round_service.rb' + +Rails/TimeZone: + Exclude: + - 'app/models/convention.rb' + - 'test/services/event_vacancy_fill_service_test.rb' + - 'test/services/execute_ranked_choice_signup_round_service_test.rb' + diff --git a/app/graphql/mutations/freeze_bucket_assignments.rb b/app/graphql/mutations/freeze_bucket_assignments.rb index 681065b0d98..42834613187 100644 --- a/app/graphql/mutations/freeze_bucket_assignments.rb +++ b/app/graphql/mutations/freeze_bucket_assignments.rb @@ -13,14 +13,14 @@ class Mutations::FreezeBucketAssignments < Mutations::BaseMutation load_and_authorize_convention_associated_model :events, :id, :update def resolve(**) - old_registration_policy = event.registration_policy.dup + old_registration_policy_json = event.registration_policy.as_json EventFreezeBucketAssignmentsService.new(event:, whodunit: current_user).call! event.reload log_form_response_changes( event, - { "registration_policy" => [old_registration_policy.as_json, event.registration_policy.as_json] } + { "registration_policy" => [old_registration_policy_json, event.registration_policy.as_json] } ) { event: } diff --git a/app/graphql/mutations/update_event.rb b/app/graphql/mutations/update_event.rb index a6e63e92d2f..89c63fc36ef 100644 --- a/app/graphql/mutations/update_event.rb +++ b/app/graphql/mutations/update_event.rb @@ -28,15 +28,10 @@ def resolve(**args) private def apply_registration_policy(event, registration_policy_attributes, bucket_key_mappings) - new_registration_policy = RegistrationPolicy.new(registration_policy_attributes) - return {} if event.registration_policy == new_registration_policy - - old_registration_policy = event.registration_policy - EventChangeRegistrationPolicyService.new(event, new_registration_policy, current_user, bucket_key_mappings).call! - - event.reload - - { "registration_policy" => [old_registration_policy.as_json, new_registration_policy.as_json] } + event.apply_registration_policy_change(registration_policy_attributes) do |new_registration_policy| + EventChangeRegistrationPolicyService.new(event, new_registration_policy, current_user, bucket_key_mappings).call! + event.reload + end end def apply_form_response_attrs(event, form_response_attrs) diff --git a/app/graphql/mutations/update_event_proposal.rb b/app/graphql/mutations/update_event_proposal.rb index 4d4af2eb5f3..c8b4bcd9789 100644 --- a/app/graphql/mutations/update_event_proposal.rb +++ b/app/graphql/mutations/update_event_proposal.rb @@ -1,38 +1,68 @@ # frozen_string_literal: true class Mutations::UpdateEventProposal < Mutations::BaseMutation - field :event_proposal, Types::EventProposalType, null: false + description "Update an event proposal" - argument :event_proposal, Types::EventProposalInputType, required: true, camelize: false - argument :id, ID, required: false + field :event_proposal, Types::EventProposalType, null: false, description: "The updated event proposal" + + argument :event_proposal, + Types::EventProposalInputType, + required: true, + camelize: false, + description: "The event proposal attributes to update" + argument :id, ID, required: false, description: "The ID of the event proposal to update" load_and_authorize_convention_associated_model :event_proposals, :id, :update - # rubocop:disable Metrics/MethodLength def resolve(**args) event_proposal_attrs = args[:event_proposal].to_h.stringify_keys + form_response_attrs = JSON.parse(event_proposal_attrs.delete("form_response_attrs_json")) + registration_policy_attributes = form_response_attrs.delete("registration_policy") + + changes = apply_registration_policy(event_proposal, registration_policy_attributes) + changes.update(apply_form_response_attrs(event_proposal, form_response_attrs)) + event_proposal.assign_attributes(event_proposal_attrs) + event_proposal.save! + + log_form_response_changes(event_proposal, changes) if event_proposal.status != "draft" + + { event_proposal: } + end + + private + + def apply_registration_policy(event_proposal, registration_policy_attributes) + event_proposal.apply_registration_policy_change(registration_policy_attributes) do |new_registration_policy| + ActiveRecord::Base.transaction do + if event_proposal.registration_policy + event_proposal.registration_policy.update_from!(new_registration_policy) + else + event_proposal.registration_policy = new_registration_policy + event_proposal.save! + end + end + end + end + + def apply_form_response_attrs(event_proposal, form_response_attrs) event_proposal.assign_form_response_attributes( event_proposal.filter_form_response_attributes_for_assignment( - JSON.parse(event_proposal_attrs.delete("form_response_attrs_json")), + form_response_attrs, event_proposal.event_category.event_proposal_form.form_items, context[:pundit_user] ) ) - event_proposal.assign_attributes(event_proposal_attrs) - event_proposal.save! + event_proposal.form_response_attribute_changes + end - if event_proposal.status != "draft" - event_proposal.form_response_attribute_changes.each do |(key, (previous_value, new_value))| - FormResponseChange.create!( - response: event_proposal, - user_con_profile:, - field_identifier: key, - previous_value:, - new_value: - ) - end + def log_form_response_changes(event_proposal, changes) + changes.each do |(key, (previous_value, new_value))| + FormResponseChange.create!( + response: event_proposal, + user_con_profile:, + field_identifier: key, + previous_value:, + new_value: + ) end - - { event_proposal: } end - # rubocop:enable Metrics/MethodLength end diff --git a/app/models/concerns/has_registration_policy.rb b/app/models/concerns/has_registration_policy.rb new file mode 100644 index 00000000000..569f52480c8 --- /dev/null +++ b/app/models/concerns/has_registration_policy.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true +module HasRegistrationPolicy + extend ActiveSupport::Concern + + # Builds a detached proposed RegistrationPolicy from `hash`, and -- if it actually differs + # from the current one -- captures an eager "before" JSON snapshot (the caller's block may + # update the current policy's rows in place rather than replacing them, so capturing + # afterward would show the already-updated state), yields the proposal for the caller to + # apply however is appropriate, and returns a {"registration_policy" => [old, new]} change + # hash for FormResponseChange logging. Returns {} if there's nothing to apply. + def apply_registration_policy_change(hash) + return {} unless hash + + new_registration_policy = RegistrationPolicy.build_from_hash(hash) + return {} if registration_policy&.equivalent_to?(new_registration_policy) + + old_registration_policy_json = registration_policy&.as_json + yield new_registration_policy + { "registration_policy" => [old_registration_policy_json, registration_policy.as_json] } + end +end diff --git a/app/models/convention.rb b/app/models/convention.rb index b08800fb4dd..9e7aab46d98 100644 --- a/app/models/convention.rb +++ b/app/models/convention.rb @@ -199,7 +199,11 @@ def timezone end def bucket_metadata_from_events - events.pluck(:registration_policy).flat_map { |p| p.buckets.flat_map(&:metadata) }.uniq + RegistrationPolicyBucket + .where(registration_policy_id: events.select(:registration_policy_id)) + .pluck(:key, :name, :description) + .map { |key, name, description| { key:, name:, description: } } + .uniq end def to_liquid diff --git a/app/models/event.rb b/app/models/event.rb index 41513df12e1..2adcbec364a 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -56,6 +56,7 @@ class Event < ApplicationRecord include AgeRestrictions include EventEmail include FormResponse + include HasRegistrationPolicy include MarkdownIndexing include OrderByTitle include PgSearch::Model @@ -116,6 +117,7 @@ class Event < ApplicationRecord belongs_to :convention belongs_to :event_category + belongs_to :registration_policy, inverse_of: :events, dependent: :destroy has_many :maximum_event_provided_tickets_overrides, dependent: :destroy 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 scope :active, -> { where(status: "active") } - serialize :registration_policy, coder: ActiveModelCoder.new("RegistrationPolicy") - attr_accessor :bypass_single_event_run_check, :allow_registration_policy_change def to_param @@ -266,12 +266,12 @@ def single_run_events_must_have_no_more_than_one_run def registration_policy_cannot_change return if new_record? - return unless registration_policy_changed? + return unless registration_policy_id_changed? - before, after = changes["registration_policy"] + before, after = changes["registration_policy_id"] return if before == after # ActiveRecord is being overzealous about change detection - errors.add :registration_policy, + errors.add :registration_policy_id, "cannot be changed via ActiveRecord on an existing event. \ Use EventChangeRegistrationPolicyService instead." end diff --git a/app/models/event_proposal.rb b/app/models/event_proposal.rb index e3175739b9e..e1227afff67 100644 --- a/app/models/event_proposal.rb +++ b/app/models/event_proposal.rb @@ -11,7 +11,6 @@ # description :text # email :text # length_seconds :integer -# registration_policy :jsonb # reminded_at :datetime # short_blurb :text # status :string @@ -25,13 +24,15 @@ # event_category_id :bigint not null # event_id :bigint # owner_id :bigint +# registration_policy_id :bigint # # Indexes # -# index_event_proposals_on_convention_id (convention_id) -# index_event_proposals_on_event_category_id (event_category_id) -# index_event_proposals_on_event_id (event_id) -# index_event_proposals_on_owner_id (owner_id) +# index_event_proposals_on_convention_id (convention_id) +# index_event_proposals_on_event_category_id (event_category_id) +# index_event_proposals_on_event_id (event_id) +# index_event_proposals_on_owner_id (owner_id) +# index_event_proposals_on_registration_policy_id (registration_policy_id) # # Foreign Keys # @@ -39,6 +40,7 @@ # fk_rails_... (event_category_id => event_categories.id) # fk_rails_... (event_id => events.id) # fk_rails_... (owner_id => user_con_profiles.id) +# fk_rails_... (registration_policy_id => registration_policies.id) # # rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective @@ -46,6 +48,7 @@ class EventProposal < ApplicationRecord include AgeRestrictions include EventEmail include FormResponse + include HasRegistrationPolicy include MarkdownIndexing include OrderByTitle include PgSearch::Model @@ -68,6 +71,7 @@ class EventProposal < ApplicationRecord belongs_to :owner, class_name: "UserConProfile", optional: true belongs_to :event, optional: true belongs_to :event_category + belongs_to :registration_policy, inverse_of: :event_proposals, optional: true, dependent: :destroy has_many_attached :images @@ -77,7 +81,6 @@ class EventProposal < ApplicationRecord scope :reminded, -> { where.not(reminded_at: nil) } scope :not_reminded, -> { where(reminded_at: nil) } - serialize :registration_policy, coder: ActiveModelCoder.new("RegistrationPolicy") serialize :timeblock_preferences, coder: JSONArrayCoderWrapper.new(ActiveModelCoder.new("EventProposal::TimeblockPreference")) diff --git a/app/models/registration_policy.rb b/app/models/registration_policy.rb index d9ecddcf22f..1e2f50a03b0 100644 --- a/app/models/registration_policy.rb +++ b/app/models/registration_policy.rb @@ -1,17 +1,45 @@ # frozen_string_literal: true +# rubocop:disable Layout/LineLength, Lint/RedundantCopDisableDirective +# == Schema Information +# +# Table name: registration_policies +# +# id :bigint not null, primary key +# freeze_no_preference_buckets :boolean default(FALSE), not null +# prevent_no_preference_signups :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# +# rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective + # A RegistrationPolicy manages the specific signup requirements for a particular Event. It consists # of one or more "buckets", each of which can accept signups. Buckets can limit signups to a # particular number if they choose to. -class RegistrationPolicy - include ActiveModel::Model - include ActiveModel::Serializers::JSON - - validate :validate_anything_bucket, :validate_key_uniqueness +class RegistrationPolicy < ApplicationRecord + has_many :buckets, + -> { order(:position) }, + class_name: "RegistrationPolicyBucket", + inverse_of: :registration_policy, + dependent: :destroy + # These exist for the inverse_of relationship with Event/EventProposal's belongs_to. The + # intended cleanup direction is the other way around (Event/EventProposal's belongs_to has + # dependent: :destroy, so destroying an event/proposal destroys its own policy along with it). + # :nullify rather than :restrict_with_exception here deliberately: when an event's own destroy + # cascades into destroying its policy, Rails processes that as an after_destroy callback on the + # *already-destroyed* event -- so a :restrict_with_exception check at that point would + # (incorrectly) still see this association as needing to be checked and block the legitimate + # cascade. :nullify only matters if something destroys a policy directly while an unrelated + # event/proposal still references it, which -- since events.registration_policy_id is NOT NULL + # -- surfaces immediately as a loud NotNullViolation instead of silently orphaning the FK. + has_many :events, inverse_of: :registration_policy, dependent: :nullify + has_many :event_proposals, inverse_of: :registration_policy, dependent: :nullify + + validate :validate_flex_bucket_uniqueness, :validate_bucket_key_uniqueness def self.unlimited new( buckets: [ - RegistrationPolicy::Bucket.new( + RegistrationPolicyBucket.new( key: "unlimited", name: "Signups", description: "Signups for this event", @@ -21,25 +49,89 @@ def self.unlimited ) end - attr_reader :buckets - - def initialize(attributes = {}) - super - @buckets ||= [] + # Explicit builder for callers constructing a policy from a plain hash (GraphQL args, import + # data, etc). Deliberately NOT hooked into `new`/`assign_attributes` -- RegistrationPolicy + # behaves like an ordinary AR model everywhere else; call sites that need hash construction + # call this by name instead of relying on implicit coercion. + # Keys that must never flow into `new(...)` here: `__typename` can come in from parsed + # form_response_attrs_json built from GraphQL query results (Apollo Client echoes it onto every + # object for cache normalization). `id`/`created_at`/`updated_at` aren't in any GraphQL type + # today, but if they ever were, letting one through would be a serious bug -- `new(id: ...)` + # would make this supposedly-detached, unsaved policy carry the same id as a real persisted + # row, corrupting association/dirty-tracking behavior. Stripped here rather than tolerated by + # the models themselves, which otherwise behave like ordinary AR models with no + # unknown-attribute tolerance. + IGNORED_HASH_KEYS = %w[__typename id created_at updated_at].freeze + + def self.build_from_hash(hash) + hash = hash.to_h.stringify_keys.except(*IGNORED_HASH_KEYS) + bucket_hashes = hash.delete("buckets") || [] + new(hash).tap do |policy| + policy.buckets = + bucket_hashes.each_with_index.map do |bucket_hash, index| + RegistrationPolicyBucket.new( + bucket_hash.to_h.stringify_keys.except(*IGNORED_HASH_KEYS).merge("position" => index + 1) + ) + end + end end def bucket_with_key(key) - key = RegistrationPolicy::Bucket.normalize_key(key) - buckets_by_key[key] + normalized_key = RegistrationPolicyBucket.normalize_key(key) + buckets.find { |bucket| bucket.key == normalized_key } + end + + # Applies another (typically detached, e.g. built via .build_from_hash) policy's values onto + # this persisted one in place. Shared by EventChangeRegistrationPolicyService (after signup + # simulation succeeds) and EventProposal's update path, which otherwise duplicated this exact + # pair of calls. + def update_from!(other) + update!( + prevent_no_preference_signups: other.prevent_no_preference_signups, + freeze_no_preference_buckets: other.freeze_no_preference_buckets + ) + sync_buckets_from_hash!(other.buckets.map(&:attributes)) + end + + # Updates this policy's buckets in place to match `bucket_hashes`, matching existing rows by + # `key` (never rewriting `key` itself -- it's not user-editable anywhere, so a key + # disappearing/appearing is always a removal/creation, never a rename). Destroys removed + # buckets FIRST, before updating/creating the rest, to avoid a transient collision on the + # `key` unique index if an incoming key happens to match one being freed up in the same call -- + # this ordering is still our responsibility, since it's about `key`, not `position`. Explicit + # `position:` values are set on every create/update to match `bucket_hashes`' array order; the + # `positioned` gem (see RegistrationPolicyBucket) handles reassigning positions safely without + # collisions, so no equivalent ordering care is needed for it. Callers are responsible for + # wrapping this in a transaction alongside any other related writes. + def sync_buckets_from_hash!(bucket_hashes) + bucket_hashes = bucket_hashes.map { |hash| hash.to_h.stringify_keys } + desired_keys = bucket_hashes.map { |hash| RegistrationPolicyBucket.normalize_key(hash["key"]) } + existing_by_key = buckets.index_by(&:key) + + existing_by_key.except(*desired_keys).each_value(&:destroy!) + + bucket_hashes.each_with_index do |hash, index| + normalized_key = RegistrationPolicyBucket.normalize_key(hash["key"]) + attrs = hash.except("id", "registration_policy_id", "created_at", "updated_at").merge("position" => index + 1) + existing = existing_by_key[normalized_key] + + existing ? existing.update!(attrs.except("key")) : buckets.create!(attrs) + end + + # The destroys above happened directly on the fetched records, not through the association + # itself, so the association's cached target array doesn't know about them -- it would keep + # serving the stale (now-destroyed) records alongside anything newly created. Reset it so the + # next read re-queries fresh. + buckets.reset end %i[total_slots minimum_slots preferred_slots].each do |method| define_method method do - buckets.select(&:counted?).sum(&method) + buckets.select(&:counted?).sum { |bucket| bucket.public_send(method) } end define_method :"#{method}_including_not_counted" do - buckets.sum(&method) + buckets.sum { |bucket| bucket.public_send(method) } end end @@ -59,111 +151,61 @@ def only_uncounted? buckets.none?(&:counted?) end - def prevent_no_preference_signups # rubocop:disable Naming/PredicateMethod - !!@prevent_no_preference_signups + def allow_no_preference_signups? + !prevent_no_preference_signups? end - alias prevent_no_preference_signups? prevent_no_preference_signups - def allow_no_preference_signups? - !prevent_no_preference_signups + def anything_bucket + buckets.find(&:anything?) end - def freeze_no_preference_buckets # rubocop:disable Naming/PredicateMethod - !!@freeze_no_preference_buckets + def blank? + buckets.none? end - alias freeze_no_preference_buckets? freeze_no_preference_buckets - def attributes + # Hand-curated (not AR's default as_json) to preserve the old external shape -- AR's default + # would include id/created_at/updated_at, which would round-trip back through build_from_hash + # and cause the "detached" policy it builds to carry the SAME id as a real persisted row + # (new(hash) accepts an explicit id), corrupting association/dirty-tracking behavior on what's + # supposed to be a genuinely separate, unsaved object. + def as_json(options = {}) { - buckets:, - prevent_no_preference_signups: prevent_no_preference_signups?, - freeze_no_preference_buckets: freeze_no_preference_buckets? + "prevent_no_preference_signups" => prevent_no_preference_signups, + "freeze_no_preference_buckets" => freeze_no_preference_buckets, + "buckets" => buckets.map { |bucket| bucket.as_json(options) } } end - def buckets=(buckets) - @buckets = - buckets.map do |value| - case value - when RegistrationPolicy::Bucket - value - else - RegistrationPolicy::Bucket.new(value) - end - end - - @buckets_by_key = nil - @anything_bucket = nil - end - - def prevent_no_preference_signups=(value) - @prevent_no_preference_signups = !!value - end - - def freeze_no_preference_buckets=(value) - @freeze_no_preference_buckets = !!value - end - - def attributes=(attributes) - attributes.each do |key, value| - case key.to_sym - when :buckets - self.buckets = value - when :prevent_no_preference_signups - self.prevent_no_preference_signups = value - when :freeze_no_preference_buckets - self.freeze_no_preference_buckets = value - when :__typename - next - else - raise ActiveModel::MissingAttributeError.new("No attribute called #{key.inspect}", key) - end - end - end - alias assign_attributes attributes= - - def buckets_by_key - @buckets_by_key ||= buckets.index_by(&:key) - end - - def anything_bucket - @anything_bucket ||= buckets.find(&:anything?) - end - - def ==(other) - return self == RegistrationPolicy.new(other) if other.is_a?(Hash) - return self == other.to_unsafe_h if other.is_a?(ActionController::Parameters) + # Explicit value-equality check, deliberately NOT `==` -- every call site already has two + # actual RegistrationPolicy instances in hand (build_from_hash always runs before any + # comparison), so there's no need to accept a raw hash here, and naming this explicitly avoids + # the implicit-everywhere reach of overriding `==` (assert_equal, Array#include?, case/when, + # uniq, etc. would all silently pick up value semantics otherwise). + def equivalent_to?(other) return false unless other.is_a?(RegistrationPolicy) - return false unless prevent_no_preference_signups == other.prevent_no_preference_signups - return false unless freeze_no_preference_buckets == other.freeze_no_preference_buckets + return false unless prevent_no_preference_signups? == other.prevent_no_preference_signups? + return false unless freeze_no_preference_buckets? == other.freeze_no_preference_buckets? return false unless buckets.size == other.buckets.size - buckets.all? { |bucket| bucket == other.bucket_with_key(bucket.key) } - end - - def as_json(*) - super.merge("buckets" => buckets.as_json(*)) - end - - def blank? - buckets.none? + buckets.all? { |bucket| bucket.equivalent_to?(other.bucket_with_key(bucket.key)) } end private - def validate_anything_bucket - anything_buckets = buckets.select(&:anything?) + def validate_flex_bucket_uniqueness + flex_buckets = buckets.reject(&:marked_for_destruction?).select(&:anything?) - return unless anything_buckets.size > 1 - errors.add(:buckets, "can contain at most 1 flex bucket, but there are #{anything_buckets.size}") + return unless flex_buckets.size > 1 + errors.add(:buckets, "can contain at most 1 flex bucket, but there are #{flex_buckets.size}") end - def validate_key_uniqueness + def validate_bucket_key_uniqueness buckets + .reject(&:marked_for_destruction?) .group_by(&:key) - .each do |key, buckets| - next unless buckets.size > 1 - errors.add(:buckets, "has #{buckets.size} buckets with the key #{key.inspect}") + .each do |key, group| + next unless group.size > 1 + errors.add(:buckets, "has #{group.size} buckets with the key #{key.inspect}") end end end diff --git a/app/models/registration_policy/bucket.rb b/app/models/registration_policy/bucket.rb deleted file mode 100644 index 56731fb2831..00000000000 --- a/app/models/registration_policy/bucket.rb +++ /dev/null @@ -1,116 +0,0 @@ -# frozen_string_literal: true -class RegistrationPolicy::Bucket - include ActiveModel::Model - include ActiveModel::Serializers::JSON - - attr_reader :key - attr_accessor :name, - :description, - :minimum_slots, - :preferred_slots, - :total_slots, - :slots_limited, - :anything, - :not_counted, - :expose_attendees - alias slots_limited? slots_limited - alias anything? anything - alias not_counted? not_counted - alias expose_attendees? expose_attendees - - def self.normalize_key(key) - key.to_s.downcase.gsub(/[^0-9a-z]/, "_") - end - - %w[minimum_slots preferred_slots total_slots].each do |method| - define_method method do - instance_variable_get(:"@#{method}") || 0 - end - end - - def __typename=(value) - # These can come in from parsed form_response_values_json built from GraphQL responses, - # but shouldn't actually wind up in the database - end - - def slots_unlimited=(value) - self.slots_limited = !value - end - - def slots_unlimited? - !slots_limited? - end - - def counted? - !not_counted? - end - - def key=(key) - @key = self.class.normalize_key(key) - end - - def full?(signups) - available_slots(signups)&.zero? - end - - def has_available_slots?(signups) - slots_unlimited? || available_slots(signups).positive? - end - - def available_slots(signups) - return nil if slots_unlimited? - my_signups_count = signups.count { |signup| signup_definitely_occupies_slot_in_bucket?(signup) } - [total_slots - my_signups_count, 0].max - end - - def signup_definitely_occupies_slot_in_bucket?(signup) - case signup - when Signup, SignupBucketFinder::FakeSignup - signup.occupying_slot? && signup.bucket_key == key && - ( - not_counted? || signup.counted # don't count non-counted signups in a counted bucket - ) - when SignupRequest - signup.state == "pending" && signup.requested_bucket_key == key - else - raise ArgumentError, - "RegistrationPolicy::Bucket doesn't know how to count #{signup.class.name} objects as signups" - end - end - - def errors_for_signup(_signup, _other_signups) - [] - end - - def attributes - { - key:, - name:, - description:, - total_slots:, - minimum_slots:, - preferred_slots:, - slots_limited:, - anything:, - not_counted:, - expose_attendees: - } - end - - def metadata - { key:, name:, description: } - end - - def ==(other) - case other - when RegistrationPolicy::BucketDrop then self == other.bucket - else attributes == other&.attributes - end - end - - delegate :hash, to: :attributes - - def to_liquid - RegistrationPolicy::BucketDrop.new(self) - end -end diff --git a/app/models/registration_policy/error.rb b/app/models/registration_policy/error.rb deleted file mode 100644 index 7747ef6584c..00000000000 --- a/app/models/registration_policy/error.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true -class RegistrationPolicy::Error - attr_reader :message - - def initialize(message) - @message = message - end -end diff --git a/app/models/registration_policy_bucket.rb b/app/models/registration_policy_bucket.rb index c6280582fd8..ec95f308e6b 100644 --- a/app/models/registration_policy_bucket.rb +++ b/app/models/registration_policy_bucket.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true # rubocop:disable Layout/LineLength, Lint/RedundantCopDisableDirective # == Schema Information # @@ -9,12 +10,12 @@ # expose_attendees :boolean default(FALSE), not null # flex :boolean default(FALSE), not null # key :string not null -# minimum_slots :integer +# minimum_slots :integer default(0), not null # name :text not null # position :integer not null -# preferred_slots :integer +# preferred_slots :integer default(0), not null # slots_limited :boolean default(FALSE), not null -# total_slots :integer +# total_slots :integer default(0), not null # created_at :datetime not null # updated_at :datetime not null # registration_policy_id :bigint not null @@ -22,8 +23,129 @@ # Indexes # # idx_on_registration_policy_id_key_b71cb40026 (registration_policy_id,key) UNIQUE +# idx_on_registration_policy_id_position_c9150cdc46 (registration_policy_id,position) UNIQUE # index_registration_policy_buckets_on_registration_policy_id (registration_policy_id) # # rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective class RegistrationPolicyBucket < ApplicationRecord + belongs_to :registration_policy, inverse_of: :buckets + + # Reuses this codebase's existing convention for an ordered-list-within-a-scope column (see + # SignupRankedChoice's `positioned on: %i[user_con_profile state], column: :priority`), rather + # than hand-rolling position management. Scope is inferred from the belongs_to; column + # defaults to `position`, matching our column name. + positioned on: :registration_policy + + COMPARABLE_ATTRIBUTES = %w[ + key + name + description + total_slots + minimum_slots + preferred_slots + slots_limited + flex + counted + expose_attendees + ].freeze + + def self.normalize_key(key) + key.to_s.downcase.gsub(/[^0-9a-z]/, "_") + end + + def key=(value) + super(self.class.normalize_key(value)) + end + + def slots_unlimited? + !slots_limited? + end + + def slots_unlimited=(value) + self.slots_limited = !value + end + + # External API compatibility: GraphQL/Liquid keep calling anything?/not_counted? and + # anything=/not_counted=. Plain methods, not `alias`, because flex?/counted? are generated + # lazily by AR's attribute-methods module and a class-body `alias` can race that. + def anything? + flex? + end + + def anything=(value) + self.flex = value + end + + def not_counted? + !counted? + end + + def not_counted=(value) + self.counted = !ActiveModel::Type::Boolean.new.cast(value) + end + + def full?(signups) + available_slots(signups)&.zero? + end + + def has_available_slots?(signups) # rubocop:disable Naming/PredicatePrefix + slots_unlimited? || available_slots(signups).positive? + end + + def available_slots(signups) + return nil if slots_unlimited? + my_signups_count = signups.count { |signup| signup_definitely_occupies_slot_in_bucket?(signup) } + [total_slots - my_signups_count, 0].max + end + + # Ported verbatim from the old RegistrationPolicy::Bucket (app/models/registration_policy/bucket.rb) + def signup_definitely_occupies_slot_in_bucket?(signup) + case signup + when Signup, SignupBucketFinder::FakeSignup + signup.occupying_slot? && signup.bucket_key == key && + ( + not_counted? || signup.counted # don't count non-counted signups in a counted bucket + ) + when SignupRequest + signup.state == "pending" && signup.requested_bucket_key == key + else + raise ArgumentError, "RegistrationPolicyBucket doesn't know how to count #{signup.class.name} objects as signups" + end + end + + def metadata + { key:, name:, description: } + end + + # Hand-curated (not AR's default as_json) to preserve the old external shape -- this JSON is + # persisted verbatim into FormResponseChange audit records, so keeping anything/not_counted + # names and omitting id/registration_policy_id/timestamps avoids a shape discontinuity + # between historical and newly-created audit rows. + def as_json(_options = {}) + { + "key" => key, + "name" => name, + "description" => description, + "total_slots" => total_slots, + "minimum_slots" => minimum_slots, + "preferred_slots" => preferred_slots, + "slots_limited" => slots_limited, + "anything" => anything?, + "not_counted" => not_counted?, + "expose_attendees" => expose_attendees + } + end + + # Explicit value-equality check, deliberately NOT `==` -- see RegistrationPolicy#equivalent_to? + # for the rationale (overriding `==` reaches implicitly into assert_equal, Array#include?, + # case/when, uniq, etc). + def equivalent_to?(other) + return equivalent_to?(other.bucket) if other.is_a?(RegistrationPolicy::BucketDrop) + return false unless other.is_a?(RegistrationPolicyBucket) + COMPARABLE_ATTRIBUTES.all? { |attr| public_send(attr) == other.public_send(attr) } + end + + def to_liquid + RegistrationPolicy::BucketDrop.new(self) + end end diff --git a/app/services/accept_event_proposal_service.rb b/app/services/accept_event_proposal_service.rb index 788e3f22dfa..b3676654079 100644 --- a/app/services/accept_event_proposal_service.rb +++ b/app/services/accept_event_proposal_service.rb @@ -18,13 +18,21 @@ def initialize(event_proposal:, event_category: nil) private - def inner_call + def inner_call # rubocop:disable Metrics/AbcSize event = convention.events.new(event_category: event_category, status: "active") event.assign_default_values_from_form_items(event_form.form_items) event.assign_form_response_attributes(event_attributes) event.con_mail_destination ||= "gms" event.title ||= event_proposal.title # in case the form doesn't include it event.length_seconds ||= event_proposal.length_seconds # same deal as title + # Unlike title/length_seconds, this isn't just a missing-value fallback: if the target + # category's form happens to declare a registration_policy form item, the generic + # assign_form_response_attributes call above would have already set event.registration_policy + # to the *same row* as event_proposal.registration_policy (association assignment, not a + # copy). Events and proposals have independent lifecycles, so always rebuild a fresh, + # independent copy here rather than ever sharing a row -- regardless of whether the form + # happened to collect it, and regardless of whether the proposal has one at all. + event.registration_policy = build_own_registration_policy event_proposal.images.blobs.each { |blob| event.images.attach(blob) } event.save! @@ -79,4 +87,14 @@ def proposal_form_item_identifiers def convention @convention ||= event_proposal.convention end + + def build_own_registration_policy + source_policy = event_proposal.registration_policy + # No buckets at all (rather than defaulting to unlimited) -- accepts_signups? is false with + # an empty bucket list, so an event accepted without an explicit policy fails closed + # (blocks signups until someone configures one) instead of silently accepting unlimited + # signups. + return RegistrationPolicy.new unless source_policy + RegistrationPolicy.build_from_hash(source_policy.as_json) + end end diff --git a/app/services/event_change_registration_policy_service.rb b/app/services/event_change_registration_policy_service.rb index ad2199c6705..aa87538892c 100644 --- a/app/services/event_change_registration_policy_service.rb +++ b/app/services/event_change_registration_policy_service.rb @@ -175,8 +175,7 @@ def inner_call apply_simulation_changes(new_signups_by_signup_id) end - event.allow_registration_policy_change = true - event.update!(registration_policy: new_registration_policy) + event.registration_policy.update_from!(new_registration_policy) move_results.each { |move_result| move_result.signup.log_signup_change!(action: "change_registration_policy") } end diff --git a/app/services/event_freeze_bucket_assignments_service.rb b/app/services/event_freeze_bucket_assignments_service.rb index 7d32da2912b..11ece0f5e12 100644 --- a/app/services/event_freeze_bucket_assignments_service.rb +++ b/app/services/event_freeze_bucket_assignments_service.rb @@ -20,10 +20,7 @@ def initialize(event:, whodunit: nil) def inner_call lock_all_runs do Event.transaction do - new_policy = build_frozen_registration_policy - - event.allow_registration_policy_change = true - event.update!(registration_policy: new_policy) + apply_frozen_registration_policy anything_signups_with_preference.each do |signup| signup.update!(bucket_key: signup.requested_bucket_key, updated_by: whodunit) signup.log_signup_change!(action: "freeze_bucket_assignments") @@ -41,21 +38,20 @@ def anything_signups_with_preference end end - def build_frozen_registration_policy + def apply_frozen_registration_policy expand_buckets_count = anything_signups_with_preference.each_with_object({}) do |signup, hash| hash[signup.requested_bucket_key] ||= 0 hash[signup.requested_bucket_key] += 1 end - new_policy = event.registration_policy.dup - new_policy.freeze_no_preference_buckets = true + registration_policy = event.registration_policy + registration_policy.update!(freeze_no_preference_buckets: true) + expand_buckets_count.each do |bucket_key, amount| - new_policy.bucket_with_key(bucket_key).total_slots += amount - new_policy.anything_bucket.total_slots -= amount + registration_policy.bucket_with_key(bucket_key).increment!(:total_slots, amount) + registration_policy.anything_bucket.decrement!(:total_slots, amount) end - - new_policy end def lock_all_runs(&block) diff --git a/app/services/execute_ranked_choice_signup_round_service.rb b/app/services/execute_ranked_choice_signup_round_service.rb index 7b4b56248ea..a1cdf849772 100644 --- a/app/services/execute_ranked_choice_signup_round_service.rb +++ b/app/services/execute_ranked_choice_signup_round_service.rb @@ -170,13 +170,13 @@ def generate_potential_random_choices_for_user_con_profile(user_con_profile, pen def random_signup_eligible_runs @random_signup_eligible_runs ||= begin - eligible_event_ids = Event.connection.select_values(<<~SQL, "random_signup_eligible_event_ids", [convention.id]) - SELECT DISTINCT events.id from events - INNER JOIN ( - SELECT id, jsonb_array_elements(registration_policy->'buckets')->'not_counted' bucket_not_counted - from events where convention_id = $1 - ) bucket_not_counted ON (events.id = bucket_not_counted.id AND bucket_not_counted.bucket_not_counted != 'true') - SQL + counted_registration_policy_ids = RegistrationPolicyBucket.where(counted: true).select(:registration_policy_id) + eligible_event_ids = + Event + .where(convention_id: convention.id) + .where(registration_policy_id: counted_registration_policy_ids) + .distinct + .pluck(:id) Run.where(event_id: eligible_event_ids) end diff --git a/app/services/import_convention_data_service.rb b/app/services/import_convention_data_service.rb index 844b3adfd42..927e488d1da 100644 --- a/app/services/import_convention_data_service.rb +++ b/app/services/import_convention_data_service.rb @@ -475,10 +475,7 @@ def import_store_orders(_convention, product_map, user_con_profile_map) def build_registration_policy(policy_data) return RegistrationPolicy.unlimited unless policy_data - RegistrationPolicy.new( - buckets: (policy_data[:buckets] || []).map { |b| RegistrationPolicy::Bucket.new(**b.transform_keys(&:to_sym)) }, - prevent_no_preference_signups: policy_data[:prevent_no_preference_signups] || false - ) + RegistrationPolicy.build_from_hash(policy_data) end def money_value(money_data) diff --git a/db/migrate/20260615192952_convert_registration_policies_to_tables.rb b/db/migrate/20260615192952_convert_registration_policies_to_tables.rb index 125533ae7b3..ce5878aa965 100644 --- a/db/migrate/20260615192952_convert_registration_policies_to_tables.rb +++ b/db/migrate/20260615192952_convert_registration_policies_to_tables.rb @@ -1,4 +1,9 @@ +# frozen_string_literal: true +# rubocop:disable Metrics/ClassLength class ConvertRegistrationPoliciesToTables < ActiveRecord::Migration[8.1] + SLOT_COUNT_TYPES = %w[minimum preferred total].freeze + SLOT_COUNTED_TYPES = %w[all counted not_counted].freeze + def change create_table :registration_policies do |t| t.boolean :prevent_no_preference_signups, null: false, default: false @@ -13,9 +18,9 @@ def change t.string :key, null: false t.text :name, null: false t.text :description - t.integer :minimum_slots - t.integer :preferred_slots - t.integer :total_slots + t.integer :minimum_slots, null: false, default: 0 + t.integer :preferred_slots, null: false, default: 0 + t.integer :total_slots, null: false, default: 0 t.boolean :slots_limited, default: false, null: false t.boolean :flex, default: false, null: false t.boolean :counted, default: true, null: false @@ -23,96 +28,17 @@ def change t.timestamps - t.index [:registration_policy_id, :key], unique: true + t.index %i[registration_policy_id key], unique: true + t.index %i[registration_policy_id position], unique: true end add_reference :events, :registration_policy, foreign_key: true + add_reference :event_proposals, :registration_policy, foreign_key: true reversible do |dir| dir.up do - data = select_rows("SELECT id, registration_policy FROM events") - data.each do |(event_id, policy_json)| - policy = JSON.parse(policy_json || "{}") - now = Time.zone.now - - registration_policy_id = exec_insert( - "INSERT INTO registration_policies (prevent_no_preference_signups, freeze_no_preference_buckets, created_at, updated_at) VALUES ($1, $2, $3, $4)", - "RegistrationPolicy insert", - [ - !!policy["prevent_no_preference_signups"], - !!policy["freeze_no_preference_buckets"], - now, - now - ], - returning: [:id] - ).rows.first.first - - now = Time.zone.now - bucket_values = (policy["buckets"] || []).each_with_index.map do |bucket, index| - { - registration_policy_id:, - position: index + 1, - key: bucket["key"], - name: bucket["name"] || bucket["key"], - description: bucket["description"], - minimum_slots: bucket["minimum_slots"], - preferred_slots: bucket["preferred_slots"], - total_slots: bucket["total_slots"], - slots_limited: !!bucket["slots_limited"], - flex: !!bucket["anything"], - counted: !bucket["not_counted"], # we're inverting the meaning on this one - expose_attendees: !!bucket["expose_attendees"], - created_at: now, - updated_at: now - } - end - - exec_insert( - <<~SQL.squish, - INSERT INTO registration_policy_buckets - ( - registration_policy_id, - position, - key, - name, - description, - minimum_slots, - preferred_slots, - total_slots, - slots_limited, - flex, - counted, - expose_attendees, - created_at, - updated_at - ) - SELECT - registration_policy_id, - position, - key, - name, - description, - minimum_slots, - preferred_slots, - total_slots, - slots_limited, - flex, - counted, - expose_attendees, - created_at, - updated_at - FROM jsonb_populate_recordset(NULL::registration_policy_buckets, $1::jsonb) - SQL - "RegistrationPolicyBucket bulk insert", - [bucket_values.to_json] - ) - - exec_update( - "UPDATE events SET registration_policy_id = $1 WHERE id = $2", - "Event update", - [registration_policy_id, event_id] - ) - end + migrate_registration_policies_for("events") + migrate_registration_policies_for("event_proposals", skip_if_null: true) end end @@ -120,5 +46,96 @@ def change t.remove :registration_policy, type: :jsonb t.change_null :registration_policy_id, false end + + change_table :event_proposals, bulk: true do |t| + t.remove :registration_policy, type: :jsonb + end + + reversible { |dir| dir.up { drop_registration_policy_functions } } + end + + private + + def migrate_registration_policies_for(table_name, skip_if_null: false) + data = select_rows("SELECT id, registration_policy FROM #{table_name}") + data.each do |(record_id, policy_json)| + next if skip_if_null && policy_json.nil? + + policy = JSON.parse(policy_json || "{}") + + registration_policy_id = insert_registration_policy(policy) + insert_registration_policy_buckets(registration_policy_id, policy["buckets"] || []) + + exec_update( + "UPDATE #{table_name} SET registration_policy_id = $1 WHERE id = $2", + "#{table_name} update", + [registration_policy_id, record_id] + ) + end + end + + def insert_registration_policy(policy) + now = Time.zone.now + exec_insert( + <<~SQL.squish, + INSERT INTO registration_policies + (prevent_no_preference_signups, freeze_no_preference_buckets, created_at, updated_at) + VALUES ($1, $2, $3, $4) + SQL + "RegistrationPolicy insert", + [!!policy["prevent_no_preference_signups"], !!policy["freeze_no_preference_buckets"], now, now], + returning: [:id] + ).rows.first.first + end + + def insert_registration_policy_buckets(registration_policy_id, buckets) + now = Time.zone.now + bucket_values = + buckets.each_with_index.map do |bucket, index| + { + registration_policy_id:, + position: index + 1, + key: bucket["key"], + name: bucket["name"] || bucket["key"], + description: bucket["description"], + minimum_slots: bucket["minimum_slots"] || 0, + preferred_slots: bucket["preferred_slots"] || 0, + total_slots: bucket["total_slots"] || 0, + slots_limited: !!bucket["slots_limited"], + flex: !!bucket["anything"], + counted: !bucket["not_counted"], # we're inverting the meaning on this one + expose_attendees: !!bucket["expose_attendees"], + created_at: now, + updated_at: now + } + end + + exec_insert(<<~SQL.squish, "RegistrationPolicyBucket bulk insert", [bucket_values.to_json]) + INSERT INTO registration_policy_buckets + ( + registration_policy_id, position, key, name, description, minimum_slots, preferred_slots, + total_slots, slots_limited, flex, counted, expose_attendees, created_at, updated_at + ) + SELECT + registration_policy_id, position, key, name, description, minimum_slots, preferred_slots, + total_slots, slots_limited, flex, counted, expose_attendees, created_at, updated_at + FROM jsonb_populate_recordset(NULL::registration_policy_buckets, $1::jsonb) + SQL + end + + def drop_registration_policy_functions + SLOT_COUNT_TYPES.each do |slot_count_type| + execute "DROP FUNCTION IF EXISTS bucket_#{slot_count_type}_slots(registration_policy jsonb, bucket_key text)" + SLOT_COUNTED_TYPES.each do |counted_type| + execute "DROP FUNCTION IF EXISTS #{slot_count_type}_#{counted_type}_slots(registration_policy jsonb)" + end + end + execute "DROP FUNCTION IF EXISTS registration_bucket(registration_policy jsonb, bucket_key text)" + execute "DROP FUNCTION IF EXISTS anything_bucket_keys(registration_policy jsonb)" + execute "DROP FUNCTION IF EXISTS not_counted_bucket_keys(registration_policy jsonb)" + execute "DROP FUNCTION IF EXISTS counted_bucket_keys(registration_policy jsonb)" + execute "DROP FUNCTION IF EXISTS bucket_keys(registration_policy jsonb)" + execute "DROP FUNCTION IF EXISTS registration_policy_buckets(registration_policy jsonb)" end end +# rubocop:enable Metrics/ClassLength diff --git a/db/structure.sql b/db/structure.sql index b6328ec57eb..3939c747fb4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -38,74 +38,6 @@ CREATE EXTENSION IF NOT EXISTS unaccent WITH SCHEMA public; COMMENT ON EXTENSION unaccent IS 'text search dictionary that removes accents'; --- --- Name: anything_bucket_keys(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.anything_bucket_keys(registration_policy jsonb) RETURNS text[] - LANGUAGE sql - AS $$ - SELECT ARRAY_AGG(key) FROM registration_policy_buckets(registration_policy) - WHERE anything = 't' - $$; - - --- --- Name: bucket_keys(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.bucket_keys(registration_policy jsonb) RETURNS text[] - LANGUAGE sql - AS $$ - SELECT ARRAY_AGG(key) FROM registration_policy_buckets(registration_policy) - $$; - - --- --- Name: bucket_minimum_slots(jsonb, text); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.bucket_minimum_slots(registration_policy jsonb, bucket_key text) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT (registration_bucket(registration_policy, bucket_key)->>'minimum_slots')::bigint - $$; - - --- --- Name: bucket_preferred_slots(jsonb, text); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.bucket_preferred_slots(registration_policy jsonb, bucket_key text) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT (registration_bucket(registration_policy, bucket_key)->>'preferred_slots')::bigint - $$; - - --- --- Name: bucket_total_slots(jsonb, text); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.bucket_total_slots(registration_policy jsonb, bucket_key text) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT (registration_bucket(registration_policy, bucket_key)->>'total_slots')::bigint - $$; - - --- --- Name: counted_bucket_keys(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.counted_bucket_keys(registration_policy jsonb) RETURNS text[] - LANGUAGE sql - AS $$ - SELECT ARRAY_AGG(key) FROM registration_policy_buckets(registration_policy) - WHERE not_counted = 'f' - $$; - - -- -- Name: current_scheduled_value(jsonb); Type: FUNCTION; Schema: public; Owner: - -- @@ -148,41 +80,6 @@ CREATE FUNCTION public.event_update_all_runs_timespan_tsrange() RETURNS trigger $$; --- --- Name: minimum_all_slots(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.minimum_all_slots(registration_policy jsonb) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT COALESCE(SUM(minimum_slots), 0) FROM registration_policy_buckets(registration_policy) - $$; - - --- --- Name: minimum_counted_slots(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.minimum_counted_slots(registration_policy jsonb) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT COALESCE(SUM(minimum_slots), 0) FROM registration_policy_buckets(registration_policy) - WHERE not_counted = 'f' - $$; - - --- --- Name: minimum_not_counted_slots(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.minimum_not_counted_slots(registration_policy jsonb) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT COALESCE(SUM(minimum_slots), 0) FROM registration_policy_buckets(registration_policy) - WHERE not_counted = 't' - $$; - - -- -- Name: next_scheduled_value(jsonb); Type: FUNCTION; Schema: public; Owner: - -- @@ -253,92 +150,6 @@ CREATE FUNCTION public.next_scheduled_value_timespan_at(scheduled_value jsonb, t $$; --- --- Name: not_counted_bucket_keys(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.not_counted_bucket_keys(registration_policy jsonb) RETURNS text[] - LANGUAGE sql - AS $$ - SELECT ARRAY_AGG(key) FROM registration_policy_buckets(registration_policy) - WHERE not_counted = 't' - $$; - - --- --- Name: preferred_all_slots(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.preferred_all_slots(registration_policy jsonb) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT COALESCE(SUM(preferred_slots), 0) FROM registration_policy_buckets(registration_policy) - $$; - - --- --- Name: preferred_counted_slots(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.preferred_counted_slots(registration_policy jsonb) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT COALESCE(SUM(preferred_slots), 0) FROM registration_policy_buckets(registration_policy) - WHERE not_counted = 'f' - $$; - - --- --- Name: preferred_not_counted_slots(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.preferred_not_counted_slots(registration_policy jsonb) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT COALESCE(SUM(preferred_slots), 0) FROM registration_policy_buckets(registration_policy) - WHERE not_counted = 't' - $$; - - --- --- Name: registration_bucket(jsonb, text); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.registration_bucket(registration_policy jsonb, bucket_key text) RETURNS jsonb - LANGUAGE sql - AS $$ - SELECT to_jsonb(bucket) FROM ( - SELECT * FROM registration_policy_buckets(registration_policy) - WHERE key = bucket_key - LIMIT 1 - ) bucket - $$; - - --- --- Name: registration_policy_buckets(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.registration_policy_buckets(registration_policy jsonb) RETURNS TABLE(bucket_index bigint, key text, name text, description text, minimum_slots integer, preferred_slots integer, total_slots integer, anything boolean, not_counted boolean, expose_attendees boolean) - LANGUAGE sql - AS $$ - SELECT - row_number() over() AS bucket_index, - bucket->>'key' AS key, - bucket->>'name' AS name, - bucket->>'description' AS description, - (bucket->>'minimum_slots')::int AS minimum_slots, - (bucket->>'preferred_slots')::int AS preferred_slots, - (bucket->>'total_slots')::int AS total_slots, - (bucket->>'anything' IS NOT NULL AND bucket->>'anything' = 'true') AS anything, - (bucket->>'not_counted' IS NOT NULL AND bucket->>'not_counted' = 'true') AS not_counted, - (bucket->>'expose_attendees' IS NOT NULL AND bucket->>'expose_attendees' = 'true') AS expose_attendees - FROM ( - SELECT jsonb_array_elements(registration_policy->'buckets') AS bucket - ) buckets - $$; - - -- -- Name: run_update_timespan_tsrange(); Type: FUNCTION; Schema: public; Owner: - -- @@ -404,41 +215,6 @@ CREATE FUNCTION public.scheduled_value_timespans(scheduled_value jsonb) RETURNS $$; --- --- Name: total_all_slots(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.total_all_slots(registration_policy jsonb) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT COALESCE(SUM(total_slots), 0) FROM registration_policy_buckets(registration_policy) - $$; - - --- --- Name: total_counted_slots(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.total_counted_slots(registration_policy jsonb) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT COALESCE(SUM(total_slots), 0) FROM registration_policy_buckets(registration_policy) - WHERE not_counted = 'f' - $$; - - --- --- Name: total_not_counted_slots(jsonb); Type: FUNCTION; Schema: public; Owner: - --- - -CREATE FUNCTION public.total_not_counted_slots(registration_policy jsonb) RETURNS bigint - LANGUAGE sql - AS $$ - SELECT COALESCE(SUM(total_slots), 0) FROM registration_policy_buckets(registration_policy) - WHERE not_counted = 't' - $$; - - -- -- Name: english_unaccent; Type: TEXT SEARCH CONFIGURATION; Schema: public; Owner: - -- @@ -1456,7 +1232,6 @@ CREATE TABLE public.event_proposals ( length_seconds integer, description text, short_blurb text, - registration_policy jsonb, can_play_concurrently boolean, additional_info jsonb, created_at timestamp without time zone NOT NULL, @@ -1466,7 +1241,8 @@ CREATE TABLE public.event_proposals ( admin_notes text, reminded_at timestamp without time zone, team_mailing_list_name text, - event_category_id bigint NOT NULL + event_category_id bigint NOT NULL, + registration_policy_id bigint ); @@ -2425,9 +2201,9 @@ CREATE TABLE public.registration_policy_buckets ( key character varying NOT NULL, name text NOT NULL, description text, - minimum_slots integer, - preferred_slots integer, - total_slots integer, + minimum_slots integer DEFAULT 0 NOT NULL, + preferred_slots integer DEFAULT 0 NOT NULL, + total_slots integer DEFAULT 0 NOT NULL, slots_limited boolean DEFAULT false NOT NULL, flex boolean DEFAULT false NOT NULL, counted boolean DEFAULT true NOT NULL, @@ -4191,6 +3967,13 @@ CREATE INDEX idx_max_event_provided_tickets_on_ticket_type_id ON public.maximum_ CREATE UNIQUE INDEX idx_on_registration_policy_id_key_b71cb40026 ON public.registration_policy_buckets USING btree (registration_policy_id, key); +-- +-- Name: idx_on_registration_policy_id_position_c9150cdc46; Type: INDEX; Schema: public; Owner: - +-- + +CREATE UNIQUE INDEX idx_on_registration_policy_id_position_c9150cdc46 ON public.registration_policy_buckets USING btree (registration_policy_id, "position"); + + -- -- Name: idx_on_user_con_profile_id_state_priority_7c693e2c51; Type: INDEX; Schema: public; Owner: - -- @@ -4548,6 +4331,13 @@ CREATE INDEX index_event_proposals_on_event_id ON public.event_proposals USING b CREATE INDEX index_event_proposals_on_owner_id ON public.event_proposals USING btree (owner_id); +-- +-- Name: index_event_proposals_on_registration_policy_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_event_proposals_on_registration_policy_id ON public.event_proposals USING btree (registration_policy_id); + + -- -- Name: index_event_ratings_on_event_id; Type: INDEX; Schema: public; Owner: - -- @@ -5543,6 +5333,14 @@ ALTER TABLE ONLY public.ticket_types ADD CONSTRAINT fk_rails_3f5bd3dab9 FOREIGN KEY (event_id) REFERENCES public.events(id); +-- +-- Name: event_proposals fk_rails_3fcd06459b; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.event_proposals + ADD CONSTRAINT fk_rails_3fcd06459b FOREIGN KEY (registration_policy_id) REFERENCES public.registration_policies(id); + + -- -- Name: signup_requests fk_rails_4018e13a21; Type: FK CONSTRAINT; Schema: public; Owner: - -- diff --git a/test/factories/event_proposals.rb b/test/factories/event_proposals.rb index 1eaca7b90a9..fb3d6dd04c8 100644 --- a/test/factories/event_proposals.rb +++ b/test/factories/event_proposals.rb @@ -10,7 +10,6 @@ # description :text # email :text # length_seconds :integer -# registration_policy :jsonb # reminded_at :datetime # short_blurb :text # status :string @@ -24,13 +23,15 @@ # event_category_id :bigint not null # event_id :bigint # owner_id :bigint +# registration_policy_id :bigint # # Indexes # -# index_event_proposals_on_convention_id (convention_id) -# index_event_proposals_on_event_category_id (event_category_id) -# index_event_proposals_on_event_id (event_id) -# index_event_proposals_on_owner_id (owner_id) +# index_event_proposals_on_convention_id (convention_id) +# index_event_proposals_on_event_category_id (event_category_id) +# index_event_proposals_on_event_id (event_id) +# index_event_proposals_on_owner_id (owner_id) +# index_event_proposals_on_registration_policy_id (registration_policy_id) # # Foreign Keys # @@ -38,6 +39,7 @@ # fk_rails_... (event_category_id => event_categories.id) # fk_rails_... (event_id => events.id) # fk_rails_... (owner_id => user_con_profiles.id) +# fk_rails_... (registration_policy_id => registration_policies.id) # # rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective diff --git a/test/factories/registration_policies.rb b/test/factories/registration_policies.rb new file mode 100644 index 00000000000..86d9f04327a --- /dev/null +++ b/test/factories/registration_policies.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true +# rubocop:disable Layout/LineLength, Lint/RedundantCopDisableDirective +# == Schema Information +# +# Table name: registration_policies +# +# id :bigint not null, primary key +# freeze_no_preference_buckets :boolean default(FALSE), not null +# prevent_no_preference_signups :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# +# rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective + +FactoryBot.define { factory :registration_policy } diff --git a/test/factories/registration_policy_buckets.rb b/test/factories/registration_policy_buckets.rb new file mode 100644 index 00000000000..75de04f2613 --- /dev/null +++ b/test/factories/registration_policy_buckets.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true +# rubocop:disable Layout/LineLength, Lint/RedundantCopDisableDirective +# == Schema Information +# +# Table name: registration_policy_buckets +# +# id :bigint not null, primary key +# counted :boolean default(TRUE), not null +# description :text +# expose_attendees :boolean default(FALSE), not null +# flex :boolean default(FALSE), not null +# key :string not null +# minimum_slots :integer default(0), not null +# name :text not null +# position :integer not null +# preferred_slots :integer default(0), not null +# slots_limited :boolean default(FALSE), not null +# total_slots :integer default(0), not null +# created_at :datetime not null +# updated_at :datetime not null +# registration_policy_id :bigint not null +# +# Indexes +# +# idx_on_registration_policy_id_key_b71cb40026 (registration_policy_id,key) UNIQUE +# idx_on_registration_policy_id_position_c9150cdc46 (registration_policy_id,position) UNIQUE +# index_registration_policy_buckets_on_registration_policy_id (registration_policy_id) +# +# rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective + +FactoryBot.define do + factory :registration_policy_bucket do + registration_policy + sequence(:key) { |n| "bucket_#{n}" } + name { "Bucket" } + end +end diff --git a/test/graphql/mutations/update_event_proposal_test.rb b/test/graphql/mutations/update_event_proposal_test.rb index dc6709daf4c..bf33c0be479 100644 --- a/test/graphql/mutations/update_event_proposal_test.rb +++ b/test/graphql/mutations/update_event_proposal_test.rb @@ -6,10 +6,12 @@ class Mutations::UpdateEventProposalTest < ActiveSupport::TestCase let(:convention) { create(:convention) } let(:admin_profile) { create(:user_con_profile, convention:, user: create(:site_admin)) } let(:old_registration_policy) do - RegistrationPolicy.new(buckets: [{ key: "unlimited", slots_limited: false, anything: true }]) + RegistrationPolicy.build_from_hash( + buckets: [{ key: "unlimited", name: "Signups", slots_limited: false, anything: true }] + ) end let(:new_registration_policy) do - RegistrationPolicy.new( + RegistrationPolicy.build_from_hash( buckets: [ { key: "players", name: "Players", slots_limited: true, total_slots: 4 }, { key: "anything", name: "Anything", slots_limited: true, total_slots: 2, anything: true } diff --git a/test/graphql/mutations/update_event_test.rb b/test/graphql/mutations/update_event_test.rb index dfa22476e44..16dc69437cb 100644 --- a/test/graphql/mutations/update_event_test.rb +++ b/test/graphql/mutations/update_event_test.rb @@ -8,10 +8,12 @@ class Mutations::UpdateEventTest < ActiveSupport::TestCase let(:convention) { create(:convention) } let(:admin_profile) { create(:user_con_profile, convention:, user: create(:site_admin)) } let(:old_registration_policy) do - RegistrationPolicy.new(buckets: [{ key: "unlimited", slots_limited: false, anything: true }]) + RegistrationPolicy.build_from_hash( + buckets: [{ key: "unlimited", name: "Signups", slots_limited: false, anything: true }] + ) end let(:new_registration_policy) do - RegistrationPolicy.new( + RegistrationPolicy.build_from_hash( buckets: [ { key: "players", name: "Players", slots_limited: true, total_slots: 4 }, { key: "anything", name: "Anything", slots_limited: true, total_slots: 2, anything: true } diff --git a/test/liquid_drops/convention_drop_test.rb b/test/liquid_drops/convention_drop_test.rb index 4def9564e84..240f72da2c6 100644 --- a/test/liquid_drops/convention_drop_test.rb +++ b/test/liquid_drops/convention_drop_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require "test_helper" describe ConventionDrop do let(:convention) { create(:convention) } @@ -8,15 +8,15 @@ assert_equal convention.name, convention_drop.name end - describe 'with runs that have openings' do + describe "with runs that have openings" do let(:limited_registration_policy) do - { + RegistrationPolicy.build_from_hash( buckets: [ - { key: 'dogs', name: 'dogs', slots_limited: true, total_slots: 3 }, - { key: 'cats', name: 'cats', slots_limited: true, total_slots: 2 }, - { key: 'anything', name: 'flex', slots_limited: true, total_slots: 4, anything: true } + { key: "dogs", name: "dogs", slots_limited: true, total_slots: 3 }, + { key: "cats", name: "cats", slots_limited: true, total_slots: 2 }, + { key: "anything", name: "flex", slots_limited: true, total_slots: 4, anything: true } ] - } + ) end let(:event_with_openings) do create(:event, convention: convention, registration_policy: limited_registration_policy) @@ -24,7 +24,7 @@ let(:run_with_openings) { create(:run, event: event_with_openings) } let(:unlimited_event_with_openings) { create(:event, convention: convention) } let(:unlimited_run_with_openings) { create(:run, event: unlimited_event_with_openings) } - let(:volunteer_event_category) { create(:event_category, convention: convention, name: 'Volunteer event') } + let(:volunteer_event_category) { create(:event_category, convention: convention, name: "Volunteer event") } let(:volunteer_event_with_openings) do create( :event, @@ -44,18 +44,18 @@ run_without_openings end - it 'returns all runs with limited-slot openings' do + it "returns all runs with limited-slot openings" do assert_includes convention_drop.runs_with_openings, run_with_openings assert_includes convention_drop.runs_with_openings, volunteer_run_with_openings - refute_includes convention_drop.runs_with_openings, run_without_openings - refute_includes convention_drop.runs_with_openings, unlimited_run_with_openings + assert_not_includes convention_drop.runs_with_openings, run_without_openings + assert_not_includes convention_drop.runs_with_openings, unlimited_run_with_openings end - it 'returns all non-volunteer runs with limited-slot openings' do + it "returns all non-volunteer runs with limited-slot openings" do assert_includes convention_drop.non_volunteer_runs_with_openings, run_with_openings - refute_includes convention_drop.non_volunteer_runs_with_openings, volunteer_run_with_openings - refute_includes convention_drop.non_volunteer_runs_with_openings, run_without_openings - refute_includes convention_drop.non_volunteer_runs_with_openings, unlimited_run_with_openings + assert_not_includes convention_drop.non_volunteer_runs_with_openings, volunteer_run_with_openings + assert_not_includes convention_drop.non_volunteer_runs_with_openings, run_without_openings + assert_not_includes convention_drop.non_volunteer_runs_with_openings, unlimited_run_with_openings end end end diff --git a/test/models/event_proposal_test.rb b/test/models/event_proposal_test.rb index 1f56576cd67..5a1cfe6defe 100644 --- a/test/models/event_proposal_test.rb +++ b/test/models/event_proposal_test.rb @@ -10,7 +10,6 @@ # description :text # email :text # length_seconds :integer -# registration_policy :jsonb # reminded_at :datetime # short_blurb :text # status :string @@ -24,13 +23,15 @@ # event_category_id :bigint not null # event_id :bigint # owner_id :bigint +# registration_policy_id :bigint # # Indexes # -# index_event_proposals_on_convention_id (convention_id) -# index_event_proposals_on_event_category_id (event_category_id) -# index_event_proposals_on_event_id (event_id) -# index_event_proposals_on_owner_id (owner_id) +# index_event_proposals_on_convention_id (convention_id) +# index_event_proposals_on_event_category_id (event_category_id) +# index_event_proposals_on_event_id (event_id) +# index_event_proposals_on_owner_id (owner_id) +# index_event_proposals_on_registration_policy_id (registration_policy_id) # # Foreign Keys # @@ -38,6 +39,7 @@ # fk_rails_... (event_category_id => event_categories.id) # fk_rails_... (event_id => events.id) # fk_rails_... (owner_id => user_con_profiles.id) +# fk_rails_... (registration_policy_id => registration_policies.id) # # rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective diff --git a/test/models/event_test.rb b/test/models/event_test.rb index 174cfec0c50..d1f82a7ea0a 100644 --- a/test/models/event_test.rb +++ b/test/models/event_test.rb @@ -59,11 +59,11 @@ class EventTest < ActiveSupport::TestCase :event, convention: convention, registration_policy: - RegistrationPolicy.new( + RegistrationPolicy.build_from_hash( buckets: [ - { key: "dogs", slots_limited: true, total_slots: 2 }, - { key: "cats", slots_limited: true, total_slots: 2 }, - { key: "anything", slots_limited: true, total_slots: 2, anything: true } + { key: "dogs", name: "Dogs", slots_limited: true, total_slots: 2 }, + { key: "cats", name: "Cats", slots_limited: true, total_slots: 2 }, + { key: "anything", name: "Anything", slots_limited: true, total_slots: 2, anything: true } ] ) ) @@ -106,4 +106,22 @@ class EventTest < ActiveSupport::TestCase assert_equal ["dogs"], event.bucket_keys_with_pending_signups_or_requests end end + + describe "#apply_registration_policy_change (from the HasRegistrationPolicy concern)" do + it "returns {} and does not yield when given a nil hash" do + yielded = false + changes = event.apply_registration_policy_change(nil) { yielded = true } + + assert_equal({}, changes) + assert_not yielded + end + + it "returns {} and does not yield when the hash describes an equivalent policy" do + yielded = false + changes = event.apply_registration_policy_change(event.registration_policy.as_json) { yielded = true } + + assert_equal({}, changes) + assert_not yielded + end + end end diff --git a/test/models/registration_policy/unlimited_test.rb b/test/models/registration_policy/unlimited_test.rb index 8f9a0bcd284..8e1e8ef0da8 100644 --- a/test/models/registration_policy/unlimited_test.rb +++ b/test/models/registration_policy/unlimited_test.rb @@ -30,9 +30,9 @@ class RegistrationPolicy::UnlimitedTest < ActiveSupport::TestCase end end - it "serializes and deserializes" do - json = subject.to_json - deserialized = RegistrationPolicy.new.from_json(json) - assert_equal subject.buckets, deserialized.buckets + it "round-trips through persistence" do + subject.save! + reloaded = RegistrationPolicy.find(subject.id) + assert_equal subject.buckets.map(&:key), reloaded.buckets.map(&:key) end end diff --git a/test/models/registration_policy_bucket_test.rb b/test/models/registration_policy_bucket_test.rb index f93cd4111a0..5aecc2ce964 100644 --- a/test/models/registration_policy_bucket_test.rb +++ b/test/models/registration_policy_bucket_test.rb @@ -9,12 +9,12 @@ # expose_attendees :boolean default(FALSE), not null # flex :boolean default(FALSE), not null # key :string not null -# minimum_slots :integer +# minimum_slots :integer default(0), not null # name :text not null # position :integer not null -# preferred_slots :integer +# preferred_slots :integer default(0), not null # slots_limited :boolean default(FALSE), not null -# total_slots :integer +# total_slots :integer default(0), not null # created_at :datetime not null # updated_at :datetime not null # registration_policy_id :bigint not null @@ -22,13 +22,137 @@ # Indexes # # idx_on_registration_policy_id_key_b71cb40026 (registration_policy_id,key) UNIQUE +# idx_on_registration_policy_id_position_c9150cdc46 (registration_policy_id,position) UNIQUE # index_registration_policy_buckets_on_registration_policy_id (registration_policy_id) # # rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective require "test_helper" class RegistrationPolicyBucketTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end + describe ".normalize_key" do + it "downcases and replaces non-alphanumeric characters with underscores" do + assert_equal "don_t_care", RegistrationPolicyBucket.normalize_key("Don't Care") + end + end + + describe "#key=" do + it "normalizes the key on assignment" do + bucket = RegistrationPolicyBucket.new(key: "Some Key!") + assert_equal "some_key_", bucket.key + end + end + + describe "anything/not_counted aliases" do + let(:bucket) { build(:registration_policy_bucket) } + + it "anything?/anything= round-trip through the real flex column" do + bucket.anything = true + assert bucket.anything? + assert bucket.flex? + + bucket.anything = false + assert_not bucket.anything? + assert_not bucket.flex? + end + + it "not_counted?/not_counted= round-trip through the real counted column, inverted" do + bucket.not_counted = true + assert bucket.not_counted? + assert_not bucket.counted? + + bucket.not_counted = false + assert_not bucket.not_counted? + assert bucket.counted? + end + end + + describe "#slots_unlimited?/#slots_unlimited=" do + it "is the inverse of slots_limited" do + bucket = build(:registration_policy_bucket, slots_limited: true) + assert_not bucket.slots_unlimited? + + bucket.slots_unlimited = true + assert_not bucket.slots_limited? + end + end + + describe "#available_slots/#full?/#has_available_slots?" do + it "returns nil for available_slots when slots are unlimited" do + bucket = build(:registration_policy_bucket, slots_limited: false) + assert_nil bucket.available_slots([]) + assert bucket.has_available_slots?([]) + assert_not bucket.full?([]) + end + + it "counts only signups that definitely occupy a slot in this bucket" do + bucket = create(:registration_policy_bucket, slots_limited: true, total_slots: 1) + signup = + create( + :signup, + run: create(:run, event: create(:event, registration_policy: bucket.registration_policy)), + bucket_key: bucket.key, + counted: true + ) + + assert_equal 0, bucket.available_slots([signup]) + assert bucket.full?([signup]) + assert_not bucket.has_available_slots?([signup]) + end + end + + describe "#signup_definitely_occupies_slot_in_bucket?" do + let(:bucket) { build(:registration_policy_bucket, key: "pcs") } + + it "returns true for a confirmed Signup with a matching bucket_key" do + signup = build(:signup, state: "confirmed", bucket_key: "pcs", counted: true) + assert bucket.signup_definitely_occupies_slot_in_bucket?(signup) + end + + it "returns false for a Signup that isn't occupying a slot" do + signup = build(:signup, state: "withdrawn", bucket_key: "pcs", counted: true) + assert_not bucket.signup_definitely_occupies_slot_in_bucket?(signup) + end + + it "excludes an uncounted signup from a counted bucket, but not_counted buckets count everyone" do + counted_bucket = build(:registration_policy_bucket, key: "pcs", not_counted: false) + not_counted_bucket = build(:registration_policy_bucket, key: "pcs", not_counted: true) + counted_signup = build(:signup, state: "confirmed", bucket_key: "pcs", counted: true) + uncounted_signup = build(:signup, state: "confirmed", bucket_key: "pcs", counted: false) + + assert counted_bucket.signup_definitely_occupies_slot_in_bucket?(counted_signup) + assert_not counted_bucket.signup_definitely_occupies_slot_in_bucket?(uncounted_signup) + assert not_counted_bucket.signup_definitely_occupies_slot_in_bucket?(counted_signup) + assert not_counted_bucket.signup_definitely_occupies_slot_in_bucket?(uncounted_signup) + end + + it "returns true for a FakeSignup that occupies a slot with a matching bucket_key" do + fake_signup = SignupBucketFinder::FakeSignup.new(state: "confirmed", bucket_key: "pcs", counted: true) + assert bucket.signup_definitely_occupies_slot_in_bucket?(fake_signup) + end + + it "checks state and requested_bucket_key for a SignupRequest" do + pending_request = build(:signup_request, state: "pending", requested_bucket_key: "pcs") + other_state_request = build(:signup_request, state: "withdrawn", requested_bucket_key: "pcs") + + assert bucket.signup_definitely_occupies_slot_in_bucket?(pending_request) + assert_not bucket.signup_definitely_occupies_slot_in_bucket?(other_state_request) + end + + it "raises ArgumentError for an unrecognized object" do + assert_raises(ArgumentError) { bucket.signup_definitely_occupies_slot_in_bucket?(Object.new) } + end + end + + describe "database constraints" do + it "enforces uniqueness of key within a registration policy" do + policy = create(:registration_policy) + create(:registration_policy_bucket, registration_policy: policy, key: "pcs", position: 1) + + assert_raises(ActiveRecord::RecordNotUnique) do + RegistrationPolicyBucket.new(registration_policy: policy, key: "pcs", name: "Duplicate", position: 2).save!( + validate: false + ) + end + end + end end diff --git a/test/models/registration_policy_test.rb b/test/models/registration_policy_test.rb index 894bca93a1c..7292e0719cd 100644 --- a/test/models/registration_policy_test.rb +++ b/test/models/registration_policy_test.rb @@ -1,15 +1,27 @@ -require 'test_helper' +# rubocop:disable Layout/LineLength, Lint/RedundantCopDisableDirective +# == Schema Information +# +# Table name: registration_policies +# +# id :bigint not null, primary key +# freeze_no_preference_buckets :boolean default(FALSE), not null +# prevent_no_preference_signups :boolean default(FALSE), not null +# created_at :datetime not null +# updated_at :datetime not null +# +# rubocop:enable Layout/LineLength, Lint/RedundantCopDisableDirective +require "test_helper" class RegistrationPolicyTest < ActiveSupport::TestCase - describe '#anything_bucket' do - it 'finds the anything bucket' do - anything_bucket = RegistrationPolicy::Bucket.new(key: 'dont_care', anything: true) + describe "#anything_bucket" do + it "finds the anything bucket" do + anything_bucket = RegistrationPolicyBucket.new(key: "dont_care", anything: true) policy = RegistrationPolicy.new( buckets: [ - RegistrationPolicy::Bucket.new(key: 'pcs'), - RegistrationPolicy::Bucket.new(key: 'npcs'), + RegistrationPolicyBucket.new(key: "pcs"), + RegistrationPolicyBucket.new(key: "npcs"), anything_bucket ] ) @@ -18,13 +30,13 @@ class RegistrationPolicyTest < ActiveSupport::TestCase end end - describe '#total_slots' do - it 'only counts counted buckets' do + describe "#total_slots" do + it "only counts counted buckets" do policy = RegistrationPolicy.new( buckets: [ - RegistrationPolicy::Bucket.new(key: 'pcs', total_slots: 2), - RegistrationPolicy::Bucket.new(key: 'npcs', total_slots: 5, not_counted: true) + RegistrationPolicyBucket.new(key: "pcs", total_slots: 2), + RegistrationPolicyBucket.new(key: "npcs", total_slots: 5, not_counted: true) ] ) @@ -32,20 +44,158 @@ class RegistrationPolicyTest < ActiveSupport::TestCase end end - describe 'validations' do - it 'validates that there is only one anything bucket' do + describe "validations" do + it "validates that there is only one anything bucket" do policy = RegistrationPolicy.new( buckets: [ - RegistrationPolicy::Bucket.new(key: 'pcs'), - RegistrationPolicy::Bucket.new(key: 'npcs'), - RegistrationPolicy::Bucket.new(key: 'anything1', anything: true), - RegistrationPolicy::Bucket.new(key: 'anything2', anything: true) + RegistrationPolicyBucket.new(key: "pcs"), + RegistrationPolicyBucket.new(key: "npcs"), + RegistrationPolicyBucket.new(key: "anything1", anything: true), + RegistrationPolicyBucket.new(key: "anything2", anything: true) ] ) - refute policy.valid? + assert_not policy.valid? assert policy.errors.full_messages.first =~ /at most 1 flex bucket/ end end + + describe ".build_from_hash" do + it "builds a detached policy with buckets in array order" do + policy = + RegistrationPolicy.build_from_hash( + "prevent_no_preference_signups" => true, + "buckets" => [{ "key" => "pcs", "name" => "PCs" }, { "key" => "npcs", "name" => "NPCs" }] + ) + + assert_not policy.persisted? + assert_equal true, policy.prevent_no_preference_signups + assert_equal %w[pcs npcs], policy.buckets.map(&:key) + assert policy.buckets.none?(&:persisted?) + end + + it "strips __typename, id, created_at, and updated_at from both the policy and its buckets" do + policy = + RegistrationPolicy.build_from_hash( + "__typename" => "RegistrationPolicyType", + "id" => 999_999, + "created_at" => "2020-01-01T00:00:00Z", + "updated_at" => "2020-01-01T00:00:00Z", + "buckets" => [{ "__typename" => "RegistrationPolicyBucketType", "id" => 999_999, "key" => "pcs" }] + ) + + assert_nil policy.id + assert_nil policy.buckets.first.id + end + end + + describe "#equivalent_to?" do + it "returns false for a nil or non-RegistrationPolicy other" do + policy = RegistrationPolicy.build_from_hash(buckets: [{ key: "pcs" }]) + assert_not policy.equivalent_to?(nil) + assert_not policy.equivalent_to?("not a policy") + end + + it "returns true for an unsaved policy with equivalent buckets and settings" do + hash = { prevent_no_preference_signups: true, buckets: [{ key: "pcs", total_slots: 2 }] } + a = RegistrationPolicy.build_from_hash(hash) + b = RegistrationPolicy.build_from_hash(hash) + + assert a.equivalent_to?(b) + end + + it "returns false when a persisted policy is compared against one with different bucket content" do + policy = create(:registration_policy, buckets: [build(:registration_policy_bucket, key: "pcs", total_slots: 2)]) + changed = RegistrationPolicy.build_from_hash(policy.as_json.merge("buckets" => [{ key: "pcs", total_slots: 3 }])) + + assert_not policy.equivalent_to?(changed) + end + end + + describe "#sync_buckets_from_hash!" do + it "updates matched buckets in place, preserving their row id" do + policy = create(:registration_policy, buckets: [build(:registration_policy_bucket, key: "pcs", total_slots: 2)]) + original_id = policy.buckets.first.id + + policy.sync_buckets_from_hash!([{ key: "pcs", name: "Player characters", total_slots: 5 }]) + policy.reload + + assert_equal [original_id], policy.buckets.map(&:id) + assert_equal 5, policy.buckets.first.total_slots + assert_equal "Player characters", policy.buckets.first.name + end + + it "destroys buckets for keys no longer present" do + policy = + create( + :registration_policy, + buckets: [build(:registration_policy_bucket, key: "pcs"), build(:registration_policy_bucket, key: "npcs")] + ) + + policy.sync_buckets_from_hash!([{ key: "pcs" }]) + policy.reload + + assert_equal ["pcs"], policy.buckets.map(&:key) + end + + it "creates buckets for new keys" do + policy = create(:registration_policy, buckets: [build(:registration_policy_bucket, key: "pcs")]) + + policy.sync_buckets_from_hash!([{ key: "pcs" }, { key: "npcs", name: "NPCs" }]) + policy.reload + + assert_equal %w[pcs npcs], policy.buckets.map(&:key) + end + + it "handles an incoming key matching one being freed up in the same call without a unique-index violation" do + policy = + create( + :registration_policy, + buckets: [build(:registration_policy_bucket, key: "pcs"), build(:registration_policy_bucket, key: "npcs")] + ) + + # 'npcs' is being removed, and a *new* bucket is taking over the 'pcs' key isn't what's + # happening here -- rather, we're removing 'pcs' and reusing its old key-space isn't + # relevant; what matters is a destroy (npcs) and a create/update happening in the same call. + policy.sync_buckets_from_hash!([{ key: "pcs", name: "still here" }]) + policy.reload + + assert_equal ["pcs"], policy.buckets.map(&:key) + end + end + + describe "#update_from!" do + it "applies another policy's top-level attributes and syncs its buckets" do + policy = create(:registration_policy, buckets: [build(:registration_policy_bucket, key: "pcs", total_slots: 2)]) + other = + RegistrationPolicy.build_from_hash( + prevent_no_preference_signups: true, + freeze_no_preference_buckets: true, + buckets: [{ key: "pcs", name: "Player characters", total_slots: 9 }] + ) + + policy.update_from!(other) + policy.reload + + assert_equal true, policy.prevent_no_preference_signups + assert_equal true, policy.freeze_no_preference_buckets + assert_equal 9, policy.buckets.first.total_slots + end + end + + describe "persistence" do + it "round-trips buckets in position order through save and reload" do + policy = + RegistrationPolicy.build_from_hash( + buckets: [{ key: "first", name: "First" }, { key: "second", name: "Second" }, { key: "third", name: "Third" }] + ) + + policy.save! + policy.reload + + assert_equal %w[first second third], policy.buckets.map(&:key) + assert_equal [1, 2, 3], policy.buckets.map(&:position) + end + end end diff --git a/test/presenters/signup_count_presenter_test.rb b/test/presenters/signup_count_presenter_test.rb index 4567152ddf4..79b5bc20856 100644 --- a/test/presenters/signup_count_presenter_test.rb +++ b/test/presenters/signup_count_presenter_test.rb @@ -6,7 +6,9 @@ class SignupCountPresenterTest < ActiveSupport::TestCase describe "with a single-bucket limited event" do let(:registration_policy) do - RegistrationPolicy.new(buckets: [{ key: "attendees", name: "Attendees", slots_limited: true, total_slots: 10 }]) + RegistrationPolicy.build_from_hash( + buckets: [{ key: "attendees", name: "Attendees", slots_limited: true, total_slots: 10 }] + ) end let(:event) { create(:event, convention:, registration_policy:) } let(:the_run) { create(:run, event:) } @@ -76,7 +78,7 @@ class SignupCountPresenterTest < ActiveSupport::TestCase describe "with a multi-bucket event" do let(:registration_policy) do - RegistrationPolicy.new( + RegistrationPolicy.build_from_hash( buckets: [ { key: "dogs", name: "Dogs", slots_limited: true, total_slots: 5 }, { key: "cats", name: "Cats", slots_limited: true, total_slots: 5 }, diff --git a/test/services/create_signup_request_service_test.rb b/test/services/create_signup_request_service_test.rb index a5f3129ba55..3600ea809dc 100644 --- a/test/services/create_signup_request_service_test.rb +++ b/test/services/create_signup_request_service_test.rb @@ -73,7 +73,14 @@ class CreateSignupRequestServiceTest < ActiveSupport::TestCase convention:, registration_policy: RegistrationPolicy.new( - buckets: [RegistrationPolicy::Bucket.new(key: "unlimited", slots_limited: false, not_counted: true)] + buckets: [ + RegistrationPolicyBucket.new( + key: "unlimited", + name: "Unlimited", + slots_limited: false, + not_counted: true + ) + ] ) ) end @@ -99,7 +106,14 @@ class CreateSignupRequestServiceTest < ActiveSupport::TestCase length_seconds: event.length_seconds, registration_policy: RegistrationPolicy.new( - buckets: [RegistrationPolicy::Bucket.new(key: "unlimited", slots_limited: false, not_counted: true)] + buckets: [ + RegistrationPolicyBucket.new( + key: "unlimited", + name: "Unlimited", + slots_limited: false, + not_counted: true + ) + ] ) ) end diff --git a/test/services/create_team_member_service_test.rb b/test/services/create_team_member_service_test.rb index 521173cf35d..9be48d328c7 100644 --- a/test/services/create_team_member_service_test.rb +++ b/test/services/create_team_member_service_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require "test_helper" class CreateTeamMemberServiceTest < ActiveSupport::TestCase let(:convention) { create(:convention, :with_notification_templates) } @@ -19,7 +19,7 @@ class CreateTeamMemberServiceTest < ActiveSupport::TestCase ) end - it 'creates a team_member record' do + it "creates a team_member record" do assert_difference(-> { event.team_members.count }, 1) do result = subject.call! assert_equal user_con_profile, result.team_member.user_con_profile @@ -27,11 +27,11 @@ class CreateTeamMemberServiceTest < ActiveSupport::TestCase end end - describe 'providing tickets' do + describe "providing tickets" do let(:ticket_type) { create(:event_provided_ticket_type, convention: convention) } let(:provide_ticket_type_id) { ticket_type.id } - it 'provides a ticket if requested' do + it "provides a ticket if requested" do assert_difference(-> { Ticket.count }, 1) do result = subject.call! assert_equal user_con_profile, result.ticket.user_con_profile @@ -41,82 +41,83 @@ class CreateTeamMemberServiceTest < ActiveSupport::TestCase end end - describe 'with existing signup' do + describe "with existing signup" do let(:signup) do create( :signup, run: the_run, user_con_profile: user_con_profile, - state: 'confirmed', - bucket_key: 'unlimited', + state: "confirmed", + bucket_key: "unlimited", counted: true ) end before { signup } - it 'converts it to a GM signup' do + it "converts it to a GM signup" do result = subject.call! signup.reload assert_equal [signup], result.converted_signups - refute signup.counted? + assert_not signup.counted? assert_nil signup.bucket_key - assert_equal 'confirmed', signup.state + assert_equal "confirmed", signup.state end - describe 'limited buckets' do + describe "limited buckets" do let(:event) do create( :event, convention: convention, event_category: event_category, - registration_policy: { - buckets: [ - { key: 'dogs', name: 'dogs', slots_limited: true, total_slots: 3 }, - { key: 'cats', name: 'cats', slots_limited: true, total_slots: 2 } - ] - } + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "dogs", name: "dogs", slots_limited: true, total_slots: 3 }, + { key: "cats", name: "cats", slots_limited: true, total_slots: 2 } + ] + ) ) end - describe 'waitlisted' do + describe "waitlisted" do let(:signup) do create( :signup, run: the_run, user_con_profile: user_con_profile, - state: 'waitlisted', - requested_bucket_key: 'dogs', + state: "waitlisted", + requested_bucket_key: "dogs", counted: false ) end - it 'converts it to a confirmed GM signup' do + it "converts it to a confirmed GM signup" do result = subject.call! signup.reload assert_equal [signup], result.converted_signups - refute signup.counted? + assert_not signup.counted? assert_nil signup.bucket_key - assert_equal 'confirmed', signup.state + assert_equal "confirmed", signup.state end end - describe 'blocking someone in the waitlist' do + describe "blocking someone in the waitlist" do let(:signup) do create( :signup, run: the_run, user_con_profile: user_con_profile, - state: 'confirmed', - bucket_key: 'dogs', - requested_bucket_key: 'dogs', + state: "confirmed", + bucket_key: "dogs", + requested_bucket_key: "dogs", counted: true ) end let(:waitlist_signup) do - create(:signup, run: the_run, state: 'waitlisted', requested_bucket_key: 'dogs', counted: false) + create(:signup, run: the_run, state: "waitlisted", requested_bucket_key: "dogs", counted: false) end before do @@ -125,15 +126,15 @@ class CreateTeamMemberServiceTest < ActiveSupport::TestCase :signup, 2, run: the_run, - state: 'confirmed', - bucket_key: 'dogs', - requested_bucket_key: 'dogs', + state: "confirmed", + bucket_key: "dogs", + requested_bucket_key: "dogs", counted: true ) waitlist_signup end - it 'pulls in the waitlisted person' do + it "pulls in the waitlisted person" do result = subject.call! signup.reload waitlist_signup.reload @@ -141,8 +142,8 @@ class CreateTeamMemberServiceTest < ActiveSupport::TestCase assert_equal [signup], result.converted_signups assert_equal [waitlist_signup], result.move_results.map(&:signup) assert waitlist_signup.counted? - assert_equal 'dogs', waitlist_signup.bucket_key - assert_equal 'confirmed', waitlist_signup.state + assert_equal "dogs", waitlist_signup.bucket_key + assert_equal "confirmed", waitlist_signup.state end end end diff --git a/test/services/event_change_registration_policy_service_test.rb b/test/services/event_change_registration_policy_service_test.rb index cdf5901ed84..982f8206aa0 100644 --- a/test/services/event_change_registration_policy_service_test.rb +++ b/test/services/event_change_registration_policy_service_test.rb @@ -7,11 +7,11 @@ class EventChangeRegistrationPolicyServiceTest < ActiveSupport::TestCase let(:event) { create(:event, convention: convention) } let(:the_run) { create(:run, event: event) } let(:new_registration_policy) do - RegistrationPolicy.new( + RegistrationPolicy.build_from_hash( buckets: [ - { key: "dogs", slots_limited: true, total_slots: 1 }, - { key: "cats", slots_limited: true, total_slots: 1 }, - { key: "anything", slots_limited: true, total_slots: 1, anything: true } + { key: "dogs", name: "Dogs", slots_limited: true, total_slots: 1 }, + { key: "cats", name: "Cats", slots_limited: true, total_slots: 1 }, + { key: "anything", name: "Anything", slots_limited: true, total_slots: 1, anything: true } ] ) end @@ -26,10 +26,39 @@ class EventChangeRegistrationPolicyServiceTest < ActiveSupport::TestCase end it "changes the registration policy" do + original_registration_policy_id = event.registration_policy_id result = subject.call assert result.success? - assert_equal new_registration_policy, event.reload.registration_policy + event.reload + assert event.registration_policy.equivalent_to?(new_registration_policy) + assert_equal original_registration_policy_id, event.registration_policy_id + end + + describe "when a bucket's key is unchanged" do + let(:event) do + create( + :event, + convention: convention, + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [{ key: "dogs", name: "Dogs (old)", slots_limited: true, total_slots: 3 }] + ) + ) + end + + it "preserves that bucket's row id while updating its other attributes" do + original_bucket_id = event.registration_policy.buckets.find_by!(key: "dogs").id + + result = subject.call + assert result.success? + + event.reload + dogs_bucket = event.registration_policy.buckets.find_by!(key: "dogs") + assert_equal original_bucket_id, dogs_bucket.id + assert_equal "Dogs", dogs_bucket.name + assert_equal 1, dogs_bucket.total_slots + end end it "does not email the team members if nobody moved" do @@ -92,11 +121,11 @@ class EventChangeRegistrationPolicyServiceTest < ActiveSupport::TestCase :event, convention: convention, registration_policy: - RegistrationPolicy.new( + RegistrationPolicy.build_from_hash( buckets: [ - { key: "dogs", slots_limited: true, total_slots: 2 }, - { key: "cats", slots_limited: true, total_slots: 1 }, - { key: "anything", slots_limited: true, total_slots: 1, anything: true } + { key: "dogs", name: "Dogs", slots_limited: true, total_slots: 2 }, + { key: "cats", name: "Cats", slots_limited: true, total_slots: 1 }, + { key: "anything", name: "Anything", slots_limited: true, total_slots: 1, anything: true } ] ) ) @@ -238,7 +267,7 @@ class EventChangeRegistrationPolicyServiceTest < ActiveSupport::TestCase assert_equal "dogs", signup1.reload.bucket_key assert_equal "dogs", signup2.reload.bucket_key assert_equal "anything", signup3.reload.bucket_key - refute_equal new_registration_policy, event.reload.registration_policy + assert_not event.reload.registration_policy.equivalent_to?(new_registration_policy) end end end @@ -249,8 +278,11 @@ class EventChangeRegistrationPolicyServiceTest < ActiveSupport::TestCase :event, convention: convention, registration_policy: - RegistrationPolicy.new( - buckets: [{ key: "unlimited", slots_limited: false }, { key: "dogs", slots_limited: true, total_slots: 2 }] + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "unlimited", name: "Unlimited", slots_limited: false }, + { key: "dogs", name: "Dogs", slots_limited: true, total_slots: 2 } + ] ) ) end @@ -352,11 +384,11 @@ class EventChangeRegistrationPolicyServiceTest < ActiveSupport::TestCase :event, convention: convention, registration_policy: - RegistrationPolicy.new( + RegistrationPolicy.build_from_hash( buckets: [ - { key: "dogs", slots_limited: true, total_slots: 0 }, - { key: "cats", slots_limited: true, total_slots: 1 }, - { key: "anything", slots_limited: true, total_slots: 1, anything: true } + { key: "dogs", name: "Dogs", slots_limited: true, total_slots: 0 }, + { key: "cats", name: "Cats", slots_limited: true, total_slots: 1 }, + { key: "anything", name: "Anything", slots_limited: true, total_slots: 1, anything: true } ] ) ) diff --git a/test/services/event_freeze_bucket_assignments_service_test.rb b/test/services/event_freeze_bucket_assignments_service_test.rb index a7bc89e91ae..9bac1db3c0b 100644 --- a/test/services/event_freeze_bucket_assignments_service_test.rb +++ b/test/services/event_freeze_bucket_assignments_service_test.rb @@ -8,13 +8,14 @@ class EventFreezeBucketAssignmentsServiceTest < ActiveSupport::TestCase create( :event, convention:, - registration_policy: { - buckets: [ - { key: "dogs", slots_limited: true, total_slots: 1 }, - { key: "cats", slots_limited: true, total_slots: 1 }, - { key: "anything", slots_limited: true, total_slots: 1, anything: true } - ] - } + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "dogs", name: "Dogs", slots_limited: true, total_slots: 1 }, + { key: "cats", name: "Cats", slots_limited: true, total_slots: 1 }, + { key: "anything", name: "Anything", slots_limited: true, total_slots: 1, anything: true } + ] + ) ) end @@ -36,6 +37,9 @@ def create_signup(**attrs) travel(-1.second) { anything_signup } waitlist_signup + original_policy_id = event.registration_policy_id + original_bucket_ids = event.registration_policy.buckets.index_by(&:key).transform_values(&:id) + result = EventFreezeBucketAssignmentsService.new(event:).call! event.reload @@ -47,6 +51,12 @@ def create_signup(**attrs) assert_equal 1, event.registration_policy.bucket_with_key("cats").total_slots assert_equal 0, event.registration_policy.bucket_with_key("anything").total_slots assert event.registration_policy.freeze_no_preference_buckets? + + # Regression guard: this used to reconstruct the whole bucket set via `.dup` (which silently + # loses AR's association cache), so verify every bucket -- not just the touched ones -- + # still exists with its original row id. + assert_equal original_policy_id, event.registration_policy_id + assert_equal(original_bucket_ids, event.registration_policy.buckets.index_by(&:key).transform_values(&:id)) end it "does not work on events with multiple runs" do diff --git a/test/services/event_signup_service_test.rb b/test/services/event_signup_service_test.rb index af57b24b347..acb1d37e456 100644 --- a/test/services/event_signup_service_test.rb +++ b/test/services/event_signup_service_test.rb @@ -101,7 +101,7 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics recipients = ActionMailer::Base.deliveries.flat_map(&:to) assert_includes recipients, email_team_member.user_con_profile.email assert_includes recipients, email_team_member2.user_con_profile.email - refute_includes recipients, no_email_team_member.user_con_profile.email + assert_not_includes recipients, no_email_team_member.user_con_profile.email end end @@ -131,7 +131,7 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics assert result.success? assert result.signup.confirmed? - refute result.signup.counted? + assert_not result.signup.counted? assert_nil result.signup.bucket_key assert_nil result.signup.requested_bucket_key end @@ -203,7 +203,7 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics convention.signup_rounds.update!(maximum_event_signups: "1") other_event = create(:event, convention:, length_seconds: event.length_seconds) - other_run = create(:run, event: other_event, starts_at: the_run.starts_at + event.length_seconds * 2) + other_run = create(:run, event: other_event, starts_at: the_run.starts_at + (event.length_seconds * 2)) other_signup_service = EventSignupService.new(user_con_profile, other_run, requested_bucket_key, user) assert other_signup_service.call.success? @@ -305,13 +305,14 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics create( :event, convention:, - registration_policy: { - buckets: [ - { key: "dogs", name: "dogs", slots_limited: true, total_slots: 3 }, - { key: "cats", name: "cats", slots_limited: true, total_slots: 2 }, - { key: "anything", name: "flex", slots_limited: true, total_slots: 4, anything: true } - ] - } + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "dogs", name: "dogs", slots_limited: true, total_slots: 3 }, + { key: "cats", name: "cats", slots_limited: true, total_slots: 2 }, + { key: "anything", name: "flex", slots_limited: true, total_slots: 4, anything: true } + ] + ) ) end @@ -380,7 +381,7 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics assert_equal 1, ActionMailer::Base.deliveries.size recipients = ActionMailer::Base.deliveries.first.to assert_includes recipients, email_team_member.user_con_profile.email - refute_includes recipients, no_email_team_member.user_con_profile.email + assert_not_includes recipients, no_email_team_member.user_con_profile.email end end @@ -428,7 +429,7 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics assert result.success? assert result.signup.confirmed? assert_nil result.signup.requested_bucket_key - refute_equal "anything", result.signup.bucket_key + assert_not_equal "anything", result.signup.bucket_key end describe "but the registration policy does not allow it" do @@ -436,14 +437,15 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics create( :event, convention:, - registration_policy: { - buckets: [ - { key: "dogs", name: "dogs", slots_limited: true, total_slots: 3 }, - { key: "cats", name: "cats", slots_limited: true, total_slots: 2 }, - { key: "anything", name: "flex", slots_limited: true, total_slots: 4, anything: true } - ], - prevent_no_preference_signups: true - } + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "dogs", name: "dogs", slots_limited: true, total_slots: 3 }, + { key: "cats", name: "cats", slots_limited: true, total_slots: 2 }, + { key: "anything", name: "flex", slots_limited: true, total_slots: 4, anything: true } + ], + prevent_no_preference_signups: true + ) ) end @@ -542,13 +544,14 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics create( :event, convention:, - registration_policy: { - buckets: [ - { key: "pc", name: "PC", slots_limited: true, total_slots: 1 }, - { key: "npc", name: "NPC", slots_limited: true, total_slots: 1, not_counted: true }, - { key: "anything", name: "Flex", slots_limited: true, total_slots: 4, anything: true } - ] - } + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "pc", name: "PC", slots_limited: true, total_slots: 1 }, + { key: "npc", name: "NPC", slots_limited: true, total_slots: 1, not_counted: true }, + { key: "anything", name: "Flex", slots_limited: true, total_slots: 4, anything: true } + ] + ) ) end @@ -560,7 +563,7 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics result = subject.call! assert result.success? assert result.signup.confirmed? - refute result.signup.counted? + assert_not result.signup.counted? assert_equal "npc", result.signup.bucket_key assert_equal "npc", result.signup.requested_bucket_key end @@ -580,7 +583,7 @@ class EventSignupServiceTest < ActiveSupport::TestCase # rubocop:disable Metrics result = subject.call! assert result.success? assert result.signup.confirmed? - refute result.signup.counted? + assert_not result.signup.counted? assert_equal "npc", result.signup.bucket_key assert_equal "npc", result.signup.requested_bucket_key end diff --git a/test/services/event_vacancy_fill_service_test.rb b/test/services/event_vacancy_fill_service_test.rb index 93450a517e6..02ce9e24c27 100644 --- a/test/services/event_vacancy_fill_service_test.rb +++ b/test/services/event_vacancy_fill_service_test.rb @@ -8,13 +8,14 @@ class EventVacancyFillServiceTest < ActiveSupport::TestCase create( :event, convention:, - registration_policy: { - buckets: [ - { key: "dogs", slots_limited: true, total_slots: 1 }, - { key: "cats", slots_limited: true, total_slots: 1 }, - { key: "anything", slots_limited: true, total_slots: 1, anything: true } - ] - } + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "dogs", name: "Dogs", slots_limited: true, total_slots: 1 }, + { key: "cats", name: "Cats", slots_limited: true, total_slots: 1 }, + { key: "anything", name: "Anything", slots_limited: true, total_slots: 1, anything: true } + ] + ) ) end @@ -96,9 +97,7 @@ def create_signup(**attrs) travel(-2.seconds) { no_pref_signup } travel(-1.second) { anything_signup } waitlist_signup - event.allow_registration_policy_change = true - frozen_policy = event.registration_policy.dup.tap { |p| p.assign_attributes(freeze_no_preference_buckets: true) } - event.update!(registration_policy: frozen_policy) + event.registration_policy.update!(freeze_no_preference_buckets: true) cats_result = EventVacancyFillService.new(the_run, "cats").call assert cats_result.success? @@ -110,9 +109,7 @@ def create_signup(**attrs) travel(-2.seconds) { create_signup(state: "confirmed", bucket_key: "cats", requested_bucket_key: nil) } travel(-1.second) { create_signup(state: "confirmed", bucket_key: "anything", requested_bucket_key: nil) } waitlist_no_pref_signup - event.allow_registration_policy_change = true - frozen_policy = event.registration_policy.dup.tap { |p| p.assign_attributes(freeze_no_preference_buckets: true) } - event.update!(registration_policy: frozen_policy) + event.registration_policy.update!(freeze_no_preference_buckets: true) dogs_result = EventVacancyFillService.new(the_run, "dogs").call assert dogs_result.success? @@ -242,14 +239,15 @@ def create_signup(**attrs) create( :event, convention:, - registration_policy: { - buckets: [ - { key: "pc", slots_limited: true, total_slots: 1 }, - { key: "npc", slots_limited: true, total_slots: 1, not_counted: true }, - { key: "spectator", slots_limited: false }, - { key: "anything", slots_limited: true, total_slots: 1, anything: true } - ] - } + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "pc", name: "PC", slots_limited: true, total_slots: 1 }, + { key: "npc", name: "NPC", slots_limited: true, total_slots: 1, not_counted: true }, + { key: "spectator", name: "Spectator", slots_limited: false }, + { key: "anything", name: "Anything", slots_limited: true, total_slots: 1, anything: true } + ] + ) ) end @@ -306,12 +304,13 @@ def create_signup(**attrs) create( :event, convention:, - registration_policy: { - buckets: [ - { key: "green", slots_limited: true, total_slots: 1 }, - { key: "blue", slots_limited: true, total_slots: 1 } - ] - } + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "green", name: "Green", slots_limited: true, total_slots: 1 }, + { key: "blue", name: "Blue", slots_limited: true, total_slots: 1 } + ] + ) ) end let(:bucket_key) { "green" } diff --git a/test/services/event_withdraw_service_test.rb b/test/services/event_withdraw_service_test.rb index dd381a49a2c..db5fde3765f 100644 --- a/test/services/event_withdraw_service_test.rb +++ b/test/services/event_withdraw_service_test.rb @@ -42,7 +42,7 @@ class EventWithdrawServiceTest < ActiveSupport::TestCase recipients = ActionMailer::Base.deliveries.flat_map(&:to) assert_includes recipients, email_team_member.user_con_profile.email assert_includes recipients, email_team_member2.user_con_profile.email - refute_includes recipients, no_email_team_member.user_con_profile.email + assert_not_includes recipients, no_email_team_member.user_con_profile.email end end @@ -71,13 +71,14 @@ class EventWithdrawServiceTest < ActiveSupport::TestCase create( :event, convention:, - registration_policy: { - buckets: [ - { key: "dogs", name: "dogs", slots_limited: true, total_slots: 1 }, - { key: "cats", name: "cats", slots_limited: true, total_slots: 1 }, - { key: "anything", name: "anything", slots_limited: true, total_slots: 1, anything: true } - ] - } + registration_policy: + RegistrationPolicy.build_from_hash( + buckets: [ + { key: "dogs", name: "dogs", slots_limited: true, total_slots: 1 }, + { key: "cats", name: "cats", slots_limited: true, total_slots: 1 }, + { key: "anything", name: "anything", slots_limited: true, total_slots: 1, anything: true } + ] + ) ) end diff --git a/test/services/execute_ranked_choice_signup_round_service_test.rb b/test/services/execute_ranked_choice_signup_round_service_test.rb index 7558cb45f08..2d94d5813fa 100644 --- a/test/services/execute_ranked_choice_signup_round_service_test.rb +++ b/test/services/execute_ranked_choice_signup_round_service_test.rb @@ -5,7 +5,9 @@ let(:convention) { create(:convention, :with_notification_templates) } let(:one_player_registration_policy) do - RegistrationPolicy.new({ buckets: [{ key: "only_one_player", total_slots: 1, slots_limited: true }] }) + RegistrationPolicy.build_from_hash( + buckets: [{ key: "only_one_player", name: "Only one player", total_slots: 1, slots_limited: true }] + ) end let(:event) { create(:event, convention:, registration_policy: one_player_registration_policy) } let(:the_run) { create(:run, event:) } @@ -412,7 +414,7 @@ assert_includes all_recipients, email_team_member2.user_con_profile.email # Team member with no notifications should not receive anything - refute_includes all_recipients, no_email_team_member.user_con_profile.email + assert_not_includes all_recipients, no_email_team_member.user_con_profile.email end end @@ -494,4 +496,26 @@ assert_equal 2, user_b.reload.lottery_number end end + + describe "#random_signup_eligible_runs" do + let(:signup_round) { create(:signup_round, convention:, start: 1.day.ago) } + let(:service) { ExecuteRankedChoiceSignupRoundService.new(signup_round:, whodunit: nil) } + + it "excludes events whose only bucket is not_counted" do + not_counted_registration_policy = + RegistrationPolicy.build_from_hash( + buckets: [{ key: "unlimited", name: "Unlimited", slots_limited: false, not_counted: true }] + ) + not_counted_event = create(:event, convention:, registration_policy: not_counted_registration_policy) + not_counted_run = create(:run, event: not_counted_event) + + counted_event = create(:event, convention:, registration_policy: one_player_registration_policy) + counted_run = create(:run, event: counted_event) + + eligible_runs = service.send(:random_signup_eligible_runs) + + assert_not_includes eligible_runs, not_counted_run + assert_includes eligible_runs, counted_run + end + end end diff --git a/test/services/execute_ranked_choice_signup_service_test.rb b/test/services/execute_ranked_choice_signup_service_test.rb index a1a945050ef..f90623edf92 100644 --- a/test/services/execute_ranked_choice_signup_service_test.rb +++ b/test/services/execute_ranked_choice_signup_service_test.rb @@ -23,7 +23,11 @@ :event, convention:, registration_policy: - RegistrationPolicy.new(buckets: [RegistrationPolicy::Bucket.new(slots_limited: true, total_slots: 0)]) + RegistrationPolicy.new( + buckets: [ + RegistrationPolicyBucket.new(key: "unlimited", name: "Unlimited", slots_limited: true, total_slots: 0) + ] + ) ) the_run = create(:run, event:) signup_ranked_choice = create(:signup_ranked_choice, target_run: the_run) @@ -58,7 +62,11 @@ :event, convention:, registration_policy: - RegistrationPolicy.new(buckets: [RegistrationPolicy::Bucket.new(slots_limited: true, total_slots: 0)]) + RegistrationPolicy.new( + buckets: [ + RegistrationPolicyBucket.new(key: "unlimited", name: "Unlimited", slots_limited: true, total_slots: 0) + ] + ) ) the_run = create(:run, event:) signup_ranked_choice = create(:signup_ranked_choice, target_run: the_run, prioritize_waitlist: true) @@ -77,7 +85,11 @@ :event, convention:, registration_policy: - RegistrationPolicy.new(buckets: [RegistrationPolicy::Bucket.new(slots_limited: true, total_slots: 0)]) + RegistrationPolicy.new( + buckets: [ + RegistrationPolicyBucket.new(key: "unlimited", name: "Unlimited", slots_limited: true, total_slots: 0) + ] + ) ) end @@ -137,7 +149,11 @@ :event, convention:, registration_policy: - RegistrationPolicy.new(buckets: [RegistrationPolicy::Bucket.new(slots_limited: true, total_slots: 0)]) + RegistrationPolicy.new( + buckets: [ + RegistrationPolicyBucket.new(key: "unlimited", name: "Unlimited", slots_limited: true, total_slots: 0) + ] + ) ) the_run = create(:run, event:) signup_ranked_choice = create(:signup_ranked_choice, target_run: the_run) @@ -196,7 +212,11 @@ :event, convention:, registration_policy: - RegistrationPolicy.new(buckets: [RegistrationPolicy::Bucket.new(slots_limited: true, total_slots: 0)]) + RegistrationPolicy.new( + buckets: [ + RegistrationPolicyBucket.new(key: "unlimited", name: "Unlimited", slots_limited: true, total_slots: 0) + ] + ) ) the_run = create(:run, event:) signup_ranked_choice = create(:signup_ranked_choice, target_run: the_run) @@ -222,7 +242,11 @@ :event, convention:, registration_policy: - RegistrationPolicy.new(buckets: [RegistrationPolicy::Bucket.new(slots_limited: true, total_slots: 1)]) + RegistrationPolicy.new( + buckets: [ + RegistrationPolicyBucket.new(key: "unlimited", name: "Unlimited", slots_limited: true, total_slots: 1) + ] + ) ) the_run = create(:run, event:) signup_ranked_choice = create(:signup_ranked_choice, target_run: the_run) @@ -243,7 +267,11 @@ :event, convention:, registration_policy: - RegistrationPolicy.new(buckets: [RegistrationPolicy::Bucket.new(slots_limited: true, total_slots: 1)]) + RegistrationPolicy.new( + buckets: [ + RegistrationPolicyBucket.new(key: "unlimited", name: "Unlimited", slots_limited: true, total_slots: 1) + ] + ) ) the_run = create(:run, event:) signup_ranked_choice = create(:signup_ranked_choice, target_run: the_run) From b9ac283ab74e3a014d186244cb8dfa8667b13201 Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Wed, 15 Jul 2026 13:26:49 -0700 Subject: [PATCH 3/8] Restore plain anything/not_counted readers on RegistrationPolicyBucket RegistrationPolicy::BucketDrop's `delegate :anything, :not_counted, to: :bucket` calls the plain (non-predicate) method names, not anything?/not_counted?. These were dropped during the JSONB->AR migration cleanup after a grep for direct `.anything` call sites came up empty -- it missed this delegate's symbol arguments, so any CMS template referencing bucket.anything/bucket.not_counted crashed with "undefined method 'anything'". Co-Authored-By: Claude Sonnet 5 --- app/models/registration_policy_bucket.rb | 18 ++++++++++++++---- test/models/registration_policy_bucket_test.rb | 12 ++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/app/models/registration_policy_bucket.rb b/app/models/registration_policy_bucket.rb index ec95f308e6b..6b8762f47be 100644 --- a/app/models/registration_policy_bucket.rb +++ b/app/models/registration_policy_bucket.rb @@ -65,9 +65,15 @@ def slots_unlimited=(value) self.slots_limited = !value end - # External API compatibility: GraphQL/Liquid keep calling anything?/not_counted? and - # anything=/not_counted=. Plain methods, not `alias`, because flex?/counted? are generated - # lazily by AR's attribute-methods module and a class-body `alias` can race that. + # External API compatibility: GraphQL calls anything?/not_counted? (it appends the `?` itself), + # but RegistrationPolicy::BucketDrop's `delegate :anything, :not_counted, to: :bucket` calls the + # plain, non-predicate names -- so both forms need to exist. Plain methods, not `alias`, because + # flex?/counted? are generated lazily by AR's attribute-methods module and a class-body `alias` + # can race that. + def anything + flex + end + def anything? flex? end @@ -76,8 +82,12 @@ def anything=(value) self.flex = value end + def not_counted # rubocop:disable Naming/PredicateMethod + !counted + end + def not_counted? - !counted? + not_counted end def not_counted=(value) diff --git a/test/models/registration_policy_bucket_test.rb b/test/models/registration_policy_bucket_test.rb index 5aecc2ce964..a519a7f7b4a 100644 --- a/test/models/registration_policy_bucket_test.rb +++ b/test/models/registration_policy_bucket_test.rb @@ -64,6 +64,18 @@ class RegistrationPolicyBucketTest < ActiveSupport::TestCase assert_not bucket.not_counted? assert bucket.counted? end + + it "exposes plain (non-predicate) anything/not_counted readers for RegistrationPolicy::BucketDrop's delegate" do + bucket.anything = true + bucket.not_counted = true + assert bucket.anything + assert bucket.not_counted + + bucket.anything = false + bucket.not_counted = false + assert_not bucket.anything + assert_not bucket.not_counted + end end describe "#slots_unlimited?/#slots_unlimited=" do From 9bd74e9e099ec6a8413c3660d36c85b19c36a171 Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Wed, 15 Jul 2026 15:55:24 -0700 Subject: [PATCH 4/8] Fix cloned event proposals sharing registration_policy row with source Creating a new proposal via "copy from an old proposal" routed registration_policy through the generic form-response clone path, which returns the source's live RegistrationPolicy record and assigns it directly -- pointing the new proposal's FK at the same row (and buckets) as the source. Since EventProposal has dependent: :destroy on registration_policy, destroying either proposal would destroy the other's policy out from under it. Build an independent copy via RegistrationPolicy.build_from_hash instead, same pattern already used in AcceptEventProposalService. Co-Authored-By: Claude Sonnet 5 --- .rubocop_todo.yml | 2 + .../mutations/create_event_proposal.rb | 29 ++++++- .../mutations/create_event_proposal_test.rb | 82 +++++++++++++++++++ 3 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 test/graphql/mutations/create_event_proposal_test.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 72083d7564f..90e8925252e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -36,6 +36,7 @@ GraphQL/UnnecessaryArgumentCamelize: Lint/MissingCopEnableDirective: Exclude: - 'app/graphql/types/mutation_type.rb' + - 'test/graphql/mutations/create_event_proposal_test.rb' - 'test/graphql/mutations/update_event_proposal_test.rb' - 'test/graphql/mutations/update_event_test.rb' @@ -51,6 +52,7 @@ Metrics/BlockLength: Exclude: - 'config/environments/production.rb' - 'config/initializers/doorkeeper.rb' + - 'test/graphql/mutations/create_event_proposal_test.rb' - 'test/liquid_drops/convention_drop_test.rb' - 'test/models/event_test.rb' - 'test/models/registration_policy_bucket_test.rb' diff --git a/app/graphql/mutations/create_event_proposal.rb b/app/graphql/mutations/create_event_proposal.rb index c5d3258a3b2..5019e1ef6bc 100644 --- a/app/graphql/mutations/create_event_proposal.rb +++ b/app/graphql/mutations/create_event_proposal.rb @@ -1,9 +1,19 @@ # frozen_string_literal: true class Mutations::CreateEventProposal < Mutations::BaseMutation - field :event_proposal, Types::EventProposalType, null: false + description "Create a new event proposal, optionally cloning form values from an existing one" - argument :clone_event_proposal_id, ID, required: false, camelize: true - argument :event_category_id, ID, required: false, camelize: true + field :event_proposal, Types::EventProposalType, null: false, description: "The newly-created event proposal" + + argument :clone_event_proposal_id, + ID, + required: false, + camelize: true, + description: "The ID of an existing event proposal to copy compatible form values from" + argument :event_category_id, + ID, + required: false, + camelize: true, + description: "The ID of the event category the new proposal belongs to" authorize_create_convention_associated_model :event_proposals @@ -35,6 +45,19 @@ def clone_attributes_from_event_proposal_id(id, event_proposal) event_proposal.event_category.event_proposal_form ) clone_attributes = template_proposal.read_form_response_attributes_for_form_items(compatible_items) + + # registration_policy is a belongs_to association, not a plain form value -- + # read_form_response_attribute returns the template's actual persisted row, and assigning it + # directly would point the new proposal's FK at the SAME policy/buckets as the template's (and, + # since both have dependent: :destroy, risk one proposal's destroy cascading into the other's + # policy). Build an independent copy instead, same as + # AcceptEventProposalService#build_own_registration_policy. + if clone_attributes["registration_policy"] + clone_attributes["registration_policy"] = RegistrationPolicy.build_from_hash( + clone_attributes["registration_policy"].as_json + ) + end + event_proposal.assign_form_response_attributes(clone_attributes) end diff --git a/test/graphql/mutations/create_event_proposal_test.rb b/test/graphql/mutations/create_event_proposal_test.rb new file mode 100644 index 00000000000..537ce441eb0 --- /dev/null +++ b/test/graphql/mutations/create_event_proposal_test.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true +# rubocop:disable GraphQL/ObjectDescription +require "test_helper" + +class Mutations::CreateEventProposalTest < ActiveSupport::TestCase + let(:convention) { create(:convention, accepting_proposals: true) } + let(:event_category) { create(:event_category, convention:) } + let(:profile) { create(:user_con_profile, convention:) } + let(:source_registration_policy) do + RegistrationPolicy.build_from_hash( + buckets: [{ key: "unlimited", name: "Signups", slots_limited: false, anything: true }] + ) + end + let(:template_proposal) do + create( + :event_proposal, + convention:, + event_category:, + owner: profile, + registration_policy: source_registration_policy + ) + end + + # Add a registration_policy form item so it's a "compatible item" to clone + before do + form = event_category.event_proposal_form + section = form.form_sections.create!(title: "Section") + section.form_items.create!( + item_type: "registration_policy", + identifier: "registration_policy", + properties: { + "identifier" => "registration_policy", + "presets" => [], + "allow_custom" => true + } + ) + end + + CREATE_EVENT_PROPOSAL_MUTATION = <<~GRAPHQL + mutation TestCreateEventProposal($eventCategoryId: ID!, $cloneEventProposalId: ID) { + createEventProposal( + input: { eventCategoryId: $eventCategoryId, cloneEventProposalId: $cloneEventProposalId } + ) { + event_proposal { id } + } + } + GRAPHQL + + describe "cloning from an existing proposal with a registration policy" do + let(:new_proposal) do + result = + execute_graphql_query( + CREATE_EVENT_PROPOSAL_MUTATION, + user_con_profile: profile, + variables: { + "eventCategoryId" => event_category.id.to_s, + "cloneEventProposalId" => template_proposal.id.to_s + } + ) + + EventProposal.find(result["data"]["createEventProposal"]["event_proposal"]["id"]) + end + + it "copies the registration policy into a new, independent row" do + assert new_proposal.registration_policy.present? + assert_not_equal template_proposal.registration_policy_id, new_proposal.registration_policy_id + end + + it "copies bucket content equivalently, not by reference" do + assert new_proposal.registration_policy.equivalent_to?(template_proposal.registration_policy) + assert_not_equal( + template_proposal.registration_policy.buckets.map(&:id), + new_proposal.registration_policy.buckets.map(&:id) + ) + end + + it "does not cascade-destroy the template's policy when the clone is destroyed" do + new_proposal.destroy! + assert RegistrationPolicy.exists?(template_proposal.reload.registration_policy_id) + end + end +end From 3c50aeac13f0b5ddd73a2e7d2208427cd23b466e Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Thu, 16 Jul 2026 09:01:22 -0700 Subject: [PATCH 5/8] Add unique index on registration_policy_id for events and event_proposals DB-level backstop for the "one policy, one owner" invariant -- every registration_policy association is belongs_to with dependent: :destroy on the owning side, which only makes sense if a policy is never shared. Catches the same bug class as the recent create_event_proposal clone fix even if a future code path reintroduces it. Fixed one pre-existing local-data violation (two event proposals sharing a policy row, left over from the clone bug) by giving the newer proposal an independent copy before adding the constraint. Updated two test files that relied on `let`-memoized RegistrationPolicy instances shared across multiple events within a single test -- harmless under the old JSONB-embedded design, but a real collision now that policies are real rows with a unique FK. Co-Authored-By: Claude Sonnet 5 --- .rubocop_todo.yml | 1 + app/models/event.rb | 2 +- app/models/event_proposal.rb | 2 +- ...policy_id_to_events_and_event_proposals.rb | 10 ++++++++++ db/structure.sql | 5 +++-- test/factories/event_proposals.rb | 2 +- test/factories/events.rb | 6 +++--- test/liquid_drops/convention_drop_test.rb | 5 ++++- test/models/event_proposal_test.rb | 20 +++++++++++++++---- test/models/event_test.rb | 9 ++++++++- ...ranked_choice_signup_round_service_test.rb | 5 ++++- 11 files changed, 52 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20260716155031_add_unique_index_on_registration_policy_id_to_events_and_event_proposals.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 90e8925252e..2e645054f49 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -161,6 +161,7 @@ Style/FrozenStringLiteralComment: - 'config/initializers/doorkeeper_openid_connect.rb' - 'lib/intercode/virtual_host_constraint.rb' - 'test/factories/event_proposals.rb' + - 'test/factories/events.rb' - 'test/factories/oauth_applications.rb' - 'test/liquid_drops/convention_drop_test.rb' - 'test/models/event_proposal_test.rb' diff --git a/app/models/event.rb b/app/models/event.rb index 2adcbec364a..03c14af0001 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -38,7 +38,7 @@ # index_events_on_convention_id (convention_id) # index_events_on_event_category_id (event_category_id) # index_events_on_owner_id (owner_id) -# index_events_on_registration_policy_id (registration_policy_id) +# index_events_on_registration_policy_id (registration_policy_id) UNIQUE # index_events_on_title_vector (title_vector) USING gin # index_events_on_updated_by_id (updated_by_id) # diff --git a/app/models/event_proposal.rb b/app/models/event_proposal.rb index e1227afff67..30a39c1cf3a 100644 --- a/app/models/event_proposal.rb +++ b/app/models/event_proposal.rb @@ -32,7 +32,7 @@ # index_event_proposals_on_event_category_id (event_category_id) # index_event_proposals_on_event_id (event_id) # index_event_proposals_on_owner_id (owner_id) -# index_event_proposals_on_registration_policy_id (registration_policy_id) +# index_event_proposals_on_registration_policy_id (registration_policy_id) UNIQUE # # Foreign Keys # diff --git a/db/migrate/20260716155031_add_unique_index_on_registration_policy_id_to_events_and_event_proposals.rb b/db/migrate/20260716155031_add_unique_index_on_registration_policy_id_to_events_and_event_proposals.rb new file mode 100644 index 00000000000..166ecb38666 --- /dev/null +++ b/db/migrate/20260716155031_add_unique_index_on_registration_policy_id_to_events_and_event_proposals.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true +class AddUniqueIndexOnRegistrationPolicyIdToEventsAndEventProposals < ActiveRecord::Migration[8.1] + def change + remove_index :events, :registration_policy_id + add_index :events, :registration_policy_id, unique: true + + remove_index :event_proposals, :registration_policy_id + add_index :event_proposals, :registration_policy_id, unique: true + end +end diff --git a/db/structure.sql b/db/structure.sql index 3939c747fb4..ad59bec0cd5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4335,7 +4335,7 @@ CREATE INDEX index_event_proposals_on_owner_id ON public.event_proposals USING b -- Name: index_event_proposals_on_registration_policy_id; Type: INDEX; Schema: public; Owner: - -- -CREATE INDEX index_event_proposals_on_registration_policy_id ON public.event_proposals USING btree (registration_policy_id); +CREATE UNIQUE INDEX index_event_proposals_on_registration_policy_id ON public.event_proposals USING btree (registration_policy_id); -- @@ -4384,7 +4384,7 @@ CREATE INDEX index_events_on_owner_id ON public.events USING btree (owner_id); -- Name: index_events_on_registration_policy_id; Type: INDEX; Schema: public; Owner: - -- -CREATE INDEX index_events_on_registration_policy_id ON public.events USING btree (registration_policy_id); +CREATE UNIQUE INDEX index_events_on_registration_policy_id ON public.events USING btree (registration_policy_id); -- @@ -6092,6 +6092,7 @@ ALTER TABLE ONLY public.cms_files_pages SET search_path TO "$user", public; INSERT INTO "schema_migrations" (version) VALUES +('20260716155031'), ('20260615192952'), ('20260601000000'), ('20260524175914'), diff --git a/test/factories/event_proposals.rb b/test/factories/event_proposals.rb index fb3d6dd04c8..49f6add42a5 100644 --- a/test/factories/event_proposals.rb +++ b/test/factories/event_proposals.rb @@ -31,7 +31,7 @@ # index_event_proposals_on_event_category_id (event_category_id) # index_event_proposals_on_event_id (event_id) # index_event_proposals_on_owner_id (owner_id) -# index_event_proposals_on_registration_policy_id (registration_policy_id) +# index_event_proposals_on_registration_policy_id (registration_policy_id) UNIQUE # # Foreign Keys # diff --git a/test/factories/events.rb b/test/factories/events.rb index 012e5788830..56f0cdfd59d 100644 --- a/test/factories/events.rb +++ b/test/factories/events.rb @@ -37,7 +37,7 @@ # index_events_on_convention_id (convention_id) # index_events_on_event_category_id (event_category_id) # index_events_on_owner_id (owner_id) -# index_events_on_registration_policy_id (registration_policy_id) +# index_events_on_registration_policy_id (registration_policy_id) UNIQUE # index_events_on_title_vector (title_vector) USING gin # index_events_on_updated_by_id (updated_by_id) # @@ -55,10 +55,10 @@ convention sequence(:title) { |n| "Event #{n}" } - status { 'active' } + status { "active" } registration_policy { RegistrationPolicy.unlimited } length_seconds { 4.hours } - con_mail_destination { 'event_email' } + con_mail_destination { "event_email" } after(:build) { |event| event.event_category ||= build(:event_category, convention: event.convention) } end diff --git a/test/liquid_drops/convention_drop_test.rb b/test/liquid_drops/convention_drop_test.rb index 240f72da2c6..906795f62ac 100644 --- a/test/liquid_drops/convention_drop_test.rb +++ b/test/liquid_drops/convention_drop_test.rb @@ -9,7 +9,10 @@ end describe "with runs that have openings" do - let(:limited_registration_policy) do + # Deliberately not `let` (memoized) -- each event needs its own independent RegistrationPolicy + # row, and this file creates multiple events that would otherwise collide on the new unique + # index on events.registration_policy_id. + def limited_registration_policy RegistrationPolicy.build_from_hash( buckets: [ { key: "dogs", name: "dogs", slots_limited: true, total_slots: 3 }, diff --git a/test/models/event_proposal_test.rb b/test/models/event_proposal_test.rb index 5a1cfe6defe..988c134554f 100644 --- a/test/models/event_proposal_test.rb +++ b/test/models/event_proposal_test.rb @@ -31,7 +31,7 @@ # index_event_proposals_on_event_category_id (event_category_id) # index_event_proposals_on_event_id (event_id) # index_event_proposals_on_owner_id (owner_id) -# index_event_proposals_on_registration_policy_id (registration_policy_id) +# index_event_proposals_on_registration_policy_id (registration_policy_id) UNIQUE # # Foreign Keys # @@ -46,7 +46,19 @@ require "test_helper" class EventProposalTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end + let(:convention) { create(:convention) } + let(:event_proposal) do + create( + :event_proposal, + convention:, + registration_policy: RegistrationPolicy.build_from_hash(buckets: [{ key: "unlimited", name: "Signups" }]) + ) + end + + describe "registration_policy_id uniqueness" do + it "rejects a second event proposal pointing at the same registration_policy row" do + other_proposal = build(:event_proposal, convention:, registration_policy: event_proposal.registration_policy) + assert_raises(ActiveRecord::RecordNotUnique) { other_proposal.save!(validate: false) } + end + end end diff --git a/test/models/event_test.rb b/test/models/event_test.rb index d1f82a7ea0a..1d5484dd2b7 100644 --- a/test/models/event_test.rb +++ b/test/models/event_test.rb @@ -37,7 +37,7 @@ # index_events_on_convention_id (convention_id) # index_events_on_event_category_id (event_category_id) # index_events_on_owner_id (owner_id) -# index_events_on_registration_policy_id (registration_policy_id) +# index_events_on_registration_policy_id (registration_policy_id) UNIQUE # index_events_on_title_vector (title_vector) USING gin # index_events_on_updated_by_id (updated_by_id) # @@ -124,4 +124,11 @@ class EventTest < ActiveSupport::TestCase assert_not yielded end end + + describe "registration_policy_id uniqueness" do + it "rejects a second event pointing at the same registration_policy row" do + other_event = build(:event, convention:, registration_policy: event.registration_policy) + assert_raises(ActiveRecord::RecordNotUnique) { other_event.save!(validate: false) } + end + end end diff --git a/test/services/execute_ranked_choice_signup_round_service_test.rb b/test/services/execute_ranked_choice_signup_round_service_test.rb index 2d94d5813fa..ac78cf33aaa 100644 --- a/test/services/execute_ranked_choice_signup_round_service_test.rb +++ b/test/services/execute_ranked_choice_signup_round_service_test.rb @@ -4,7 +4,10 @@ include ActiveJob::TestHelper let(:convention) { create(:convention, :with_notification_templates) } - let(:one_player_registration_policy) do + # Deliberately not `let` (memoized) -- each event needs its own independent RegistrationPolicy + # row, and several tests create multiple events that would otherwise collide on the new unique + # index on events.registration_policy_id. + def one_player_registration_policy RegistrationPolicy.build_from_hash( buckets: [{ key: "only_one_player", name: "Only one player", total_slots: 1, slots_limited: true }] ) From 54ea738bafb54c8d79cd357368c66e85aa83d4b8 Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Thu, 16 Jul 2026 09:54:20 -0700 Subject: [PATCH 6/8] Fix createEvent/createFillerEvent crash and lost registration_policy on proposal import createEvent and createFillerEvent had no registration_policy handling at all, so any event category with a registration_policy form item -- which is the normal case -- made both mutations unusable: submitting a value crashed with ActiveRecord::AssociationTypeMismatch (assigning a raw Hash to a belongs_to, with no accepts_nested_attributes_for), and omitting it failed the NOT NULL constraint. Build a real RegistrationPolicy explicitly before the generic form-response assignment, same as the other mutations; build_from_hash(nil) already returns an empty, fail-closed policy when nothing is submitted. Also fixed ImportConventionDataService#import_event_proposals, which never read a registration_policy from import data at all (only import_events did), silently dropping it on import. Since registration_policy_id is nullable for proposals (unlike events), only build one when the source data actually provides it, rather than fabricating a default that wasn't there. Found while auditing every Event/EventProposal creation and update path for registration_policy handling after the recent unique-index/clone-sharing fix. Co-Authored-By: Claude Sonnet 5 --- .rubocop_todo.yml | 9 ++ app/graphql/mutations/create_event.rb | 18 ++- app/graphql/mutations/create_filler_event.rb | 20 +++- .../import_convention_data_service.rb | 9 ++ test/graphql/mutations/create_event_test.rb | 111 ++++++++++++++++++ .../mutations/create_filler_event_test.rb | 74 ++++++++++++ .../import_convention_data_service_test.rb | 47 ++++++++ 7 files changed, 281 insertions(+), 7 deletions(-) create mode 100644 test/graphql/mutations/create_event_test.rb create mode 100644 test/graphql/mutations/create_filler_event_test.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2e645054f49..7039ef7182d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -13,6 +13,11 @@ GraphQL/ArgumentDescription: - 'app/graphql/types/ability_type.rb' - 'app/graphql/types/query_type.rb' +# Offense count: 1 +GraphQL/ExtractInputType: + Exclude: + - 'app/graphql/mutations/create_filler_event.rb' + # Offense count: 14 # Configuration parameters: MaxFields, Prefixes. # Prefixes: is, has, with, avg, min, max @@ -37,6 +42,8 @@ Lint/MissingCopEnableDirective: Exclude: - 'app/graphql/types/mutation_type.rb' - 'test/graphql/mutations/create_event_proposal_test.rb' + - 'test/graphql/mutations/create_event_test.rb' + - 'test/graphql/mutations/create_filler_event_test.rb' - 'test/graphql/mutations/update_event_proposal_test.rb' - 'test/graphql/mutations/update_event_test.rb' @@ -53,6 +60,8 @@ Metrics/BlockLength: - 'config/environments/production.rb' - 'config/initializers/doorkeeper.rb' - 'test/graphql/mutations/create_event_proposal_test.rb' + - 'test/graphql/mutations/create_event_test.rb' + - 'test/graphql/mutations/create_filler_event_test.rb' - 'test/liquid_drops/convention_drop_test.rb' - 'test/models/event_test.rb' - 'test/models/registration_policy_bucket_test.rb' diff --git a/app/graphql/mutations/create_event.rb b/app/graphql/mutations/create_event.rb index fa8ee0ac403..5aae7dc40b1 100644 --- a/app/graphql/mutations/create_event.rb +++ b/app/graphql/mutations/create_event.rb @@ -1,9 +1,14 @@ # frozen_string_literal: true class Mutations::CreateEvent < Mutations::BaseMutation - field :event, Types::EventType, null: false + description "Create a new event" - argument :event, Types::EventInputType, required: true - argument :signed_image_blob_ids, [ID], required: false + field :event, Types::EventType, null: false, description: "The newly-created event" + + argument :event, Types::EventInputType, required: true, description: "The event attributes" + argument :signed_image_blob_ids, + [ID], + required: false, + description: "Signed blob IDs for images to attach to this event" authorize_create_convention_associated_model :events @@ -12,6 +17,13 @@ def resolve(signed_image_blob_ids: nil, **args) form_response_attrs = JSON.parse(event_attrs.delete("form_response_attrs_json")) event = convention.events.new(event_attrs) + # registration_policy is a belongs_to association, not a plain form value -- assigning a raw + # hash through the generic form-response path below would raise + # ActiveRecord::AssociationTypeMismatch (there's no accepts_nested_attributes_for). Build a + # real RegistrationPolicy explicitly instead; build_from_hash(nil) already returns an empty, + # fail-closed policy when the client doesn't submit one, which registration_policy_id being + # NOT NULL requires regardless. + event.registration_policy = RegistrationPolicy.build_from_hash(form_response_attrs.delete("registration_policy")) event.assign_form_response_attributes( event.filter_form_response_attributes_for_assignment( form_response_attrs, diff --git a/app/graphql/mutations/create_filler_event.rb b/app/graphql/mutations/create_filler_event.rb index 702c9802c06..c0a57992caa 100644 --- a/app/graphql/mutations/create_filler_event.rb +++ b/app/graphql/mutations/create_filler_event.rb @@ -1,10 +1,15 @@ # frozen_string_literal: true class Mutations::CreateFillerEvent < Mutations::BaseMutation - field :event, Types::EventType, null: false + description "Create a new filler event (a single-run event with no signup process, e.g. an ongoing activity)" - argument :event, Types::EventInputType, required: true - argument :run, Types::RunInputType, required: false - argument :signed_image_blob_ids, [ID], required: false + field :event, Types::EventType, null: false, description: "The newly-created event" + + argument :event, Types::EventInputType, required: true, description: "The event attributes" + argument :run, Types::RunInputType, required: false, description: "The initial run to create for this event" + argument :signed_image_blob_ids, + [ID], + required: false, + description: "Signed blob IDs for images to attach to this event" authorize_create_convention_associated_model :events @@ -14,6 +19,13 @@ def resolve(signed_image_blob_ids: nil, **args) form_response_attrs = JSON.parse(event_attrs.delete("form_response_attrs_json")) event = convention.events.new(event_attrs) + # registration_policy is a belongs_to association, not a plain form value -- assigning a raw + # hash through the generic form-response path below would raise + # ActiveRecord::AssociationTypeMismatch (there's no accepts_nested_attributes_for). Build a + # real RegistrationPolicy explicitly instead; build_from_hash(nil) already returns an empty, + # fail-closed policy when the client doesn't submit one, which registration_policy_id being + # NOT NULL requires regardless. + event.registration_policy = RegistrationPolicy.build_from_hash(form_response_attrs.delete("registration_policy")) event.assign_form_response_attributes( event.filter_form_response_attributes_for_assignment( form_response_attrs, diff --git a/app/services/import_convention_data_service.rb b/app/services/import_convention_data_service.rb index 927e488d1da..03ad24fb619 100644 --- a/app/services/import_convention_data_service.rb +++ b/app/services/import_convention_data_service.rb @@ -345,11 +345,20 @@ def import_event_proposals(convention, event_map, user_con_profile_map) # ruboco proposal = convention.event_proposals.new(event_category: category, owner: profile, status: ep[:status]) proposal.event = event_map[ep[:event_id]] if ep[:event_id] && event_map[ep[:event_id]] + proposal.registration_policy = build_proposal_registration_policy(ep) proposal.assign_form_response_attributes(ep[:form_response_attributes] || {}) proposal.save! end end + # Unlike events, registration_policy_id is nullable on event proposals -- a proposal genuinely + # may not have one yet, so only build one if the import data actually provides it, rather than + # fabricating an unlimited/empty default that wasn't part of the source data. + def build_proposal_registration_policy(proposal_data) + return nil unless proposal_data[:registration_policy] + RegistrationPolicy.build_from_hash(proposal_data[:registration_policy]) + end + def import_runs(convention, event_map) room_map = convention.rooms.index_by(&:name) run_indexes = Hash.new(-1) diff --git a/test/graphql/mutations/create_event_test.rb b/test/graphql/mutations/create_event_test.rb new file mode 100644 index 00000000000..e4478f68c83 --- /dev/null +++ b/test/graphql/mutations/create_event_test.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true +# rubocop:disable GraphQL/ObjectDescription +require "test_helper" + +class Mutations::CreateEventTest < ActiveSupport::TestCase + let(:convention) { create(:convention) } + let(:admin_profile) { create(:user_con_profile, convention:, user: create(:site_admin)) } + let(:event_category) { create(:event_category, convention:) } + + before do + form = event_category.event_form + section = form.form_sections.create!(title: "Section") + section.form_items.create!( + item_type: "free_text", + identifier: "title", + properties: { + "lines" => 1, + "caption" => "Title", + "required" => true + } + ) + section.form_items.create!( + item_type: "timespan", + identifier: "length_seconds", + properties: { + "caption" => "Event Length", + "required" => true + } + ) + section.form_items.create!( + item_type: "registration_policy", + identifier: "registration_policy", + properties: { + "presets" => [], + "allow_custom" => true, + "required" => false + } + ) + end + + CREATE_EVENT_MUTATION = <<~GRAPHQL + mutation TestCreateEvent($eventCategoryId: ID!, $formResponseAttrsJson: String!) { + createEvent(input: { event: { eventCategoryId: $eventCategoryId, form_response_attrs_json: $formResponseAttrsJson } }) { + event { id } + } + } + GRAPHQL + + def create_event(form_response_attrs) + result = + execute_graphql_query( + CREATE_EVENT_MUTATION, + user_con_profile: admin_profile, + variables: { + "eventCategoryId" => event_category.id.to_s, + "formResponseAttrsJson" => form_response_attrs.to_json + } + ) + + Event.find(result["data"]["createEvent"]["event"]["id"]) + end + + describe "with a registration_policy submitted" do + it "builds an independent registration policy with the submitted buckets" do + event = + create_event( + "title" => "A New Event", + "length_seconds" => 3600, + "registration_policy" => { + "buckets" => [{ "key" => "unlimited", "name" => "Signups", "slots_limited" => false, "anything" => true }] + } + ) + + assert event.registration_policy.present? + assert_equal ["unlimited"], event.registration_policy.buckets.map(&:key) + end + end + + describe "without a registration_policy submitted" do + it "defaults to an empty, fail-closed registration policy" do + event = create_event("title" => "Another New Event", "length_seconds" => 3600) + + assert event.registration_policy.present? + assert_equal [], event.registration_policy.buckets.to_a + assert_not event.registration_policy.accepts_signups? + end + end + + describe "creating multiple events" do + it "gives each event its own independent registration_policy row" do + event1 = + create_event( + "title" => "Event One", + "length_seconds" => 3600, + "registration_policy" => { + "buckets" => [{ "key" => "unlimited", "name" => "Signups", "slots_limited" => false, "anything" => true }] + } + ) + event2 = + create_event( + "title" => "Event Two", + "length_seconds" => 3600, + "registration_policy" => { + "buckets" => [{ "key" => "unlimited", "name" => "Signups", "slots_limited" => false, "anything" => true }] + } + ) + + assert_not_equal event1.registration_policy_id, event2.registration_policy_id + end + end +end diff --git a/test/graphql/mutations/create_filler_event_test.rb b/test/graphql/mutations/create_filler_event_test.rb new file mode 100644 index 00000000000..57ac91f7a02 --- /dev/null +++ b/test/graphql/mutations/create_filler_event_test.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true +# rubocop:disable GraphQL/ObjectDescription +require "test_helper" + +class Mutations::CreateFillerEventTest < ActiveSupport::TestCase + let(:convention) { create(:convention) } + let(:admin_profile) { create(:user_con_profile, convention:, user: create(:site_admin)) } + let(:event_category) { create(:event_category, convention:) } + + before do + form = event_category.event_form + section = form.form_sections.create!(title: "Section") + section.form_items.create!( + item_type: "free_text", + identifier: "title", + properties: { + "lines" => 1, + "caption" => "Title", + "required" => true + } + ) + section.form_items.create!( + item_type: "timespan", + identifier: "length_seconds", + properties: { + "caption" => "Event Length", + "required" => true + } + ) + section.form_items.create!( + item_type: "registration_policy", + identifier: "registration_policy", + properties: { + "presets" => [], + "allow_custom" => true, + "required" => false + } + ) + end + + CREATE_FILLER_EVENT_MUTATION = <<~GRAPHQL + mutation TestCreateFillerEvent($eventCategoryId: ID!, $formResponseAttrsJson: String!) { + createFillerEvent(input: { event: { eventCategoryId: $eventCategoryId, form_response_attrs_json: $formResponseAttrsJson } }) { + event { id } + } + } + GRAPHQL + + describe "with a registration_policy submitted" do + it "builds an independent registration policy with the submitted buckets" do + form_response_attrs = { + "title" => "A Filler Event", + "length_seconds" => 3600, + "registration_policy" => { + "buckets" => [{ "key" => "unlimited", "name" => "Signups", "slots_limited" => false, "anything" => true }] + } + } + + result = + execute_graphql_query( + CREATE_FILLER_EVENT_MUTATION, + user_con_profile: admin_profile, + variables: { + "eventCategoryId" => event_category.id.to_s, + "formResponseAttrsJson" => form_response_attrs.to_json + } + ) + + event = Event.find(result["data"]["createFillerEvent"]["event"]["id"]) + assert event.registration_policy.present? + assert_equal ["unlimited"], event.registration_policy.buckets.map(&:key) + end + end +end diff --git a/test/services/import_convention_data_service_test.rb b/test/services/import_convention_data_service_test.rb index 61e91138560..a713e2e6f74 100644 --- a/test/services/import_convention_data_service_test.rb +++ b/test/services/import_convention_data_service_test.rb @@ -518,6 +518,53 @@ class ImportConventionDataServiceTest < ActiveSupport::TestCase end end + describe "event proposals" do + let(:data) do + base_data.merge( + cms_content_set: "standard", + convention: base_data[:convention].merge(event_categories: standard_event_categories), + users: [{ email: "proposer@example.com", first_name: "Prop", last_name: "Oser" }], + user_con_profiles: [{ user_email: "proposer@example.com", first_name: "Prop", last_name: "Oser" }], + event_proposals: [ + { + event_category_name: "LARP", + status: "proposed", + owner_email: "proposer@example.com", + form_response_attributes: { + title: "A Proposed LARP" + }, + registration_policy: { + buckets: [{ key: "unlimited", name: "Signups", slots_limited: false, anything: true }] + } + }, + { + event_category_name: "LARP", + status: "proposed", + owner_email: "proposer@example.com", + form_response_attributes: { + title: "Another Proposed LARP" + } + } + ] + ) + end + + before { ImportConventionDataService.new(data:).call! } + + it "imports the registration policy with buckets when provided" do + convention = Convention.find_by!(domain: "importtest.example.com") + proposal = convention.event_proposals.find_by!(title: "A Proposed LARP") + assert proposal.registration_policy.present? + assert_equal ["unlimited"], proposal.registration_policy.buckets.map(&:key) + end + + it "leaves registration_policy nil when not provided, rather than fabricating a default" do + convention = Convention.find_by!(domain: "importtest.example.com") + proposal = convention.event_proposals.find_by!(title: "Another Proposed LARP") + assert_nil proposal.registration_policy_id + end + end + describe "signups" do before do data = From 8a1da729ee55d6fd7c1714b0ecce25df2791398f Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Thu, 16 Jul 2026 14:50:50 -0700 Subject: [PATCH 7/8] Replace HasRegistrationPolicy's block-based API with a plain return value apply_registration_policy_change wrapped a caller-supplied block between capturing the old policy's JSON and reading the new one, so the "how do we actually apply this" logic (in-place update for EventProposal, full signup simulation via EventChangeRegistrationPolicyService for Event) lived at the call site while the comparison/snapshot bookkeeping lived in the concern. Correct, but harder to read than necessary: understanding what a mutation does required jumping into the concern to see when the block runs relative to the snapshot. Replaced with #registration_policy_change_for, which returns a HasRegistrationPolicy::Change (or nil if there's nothing to apply) holding the detached new policy and the old policy's JSON. Each mutation now applies the change and builds its own diff hash inline, in linear top-to-bottom code, at the cost of each mutation writing that one-line hash itself instead of having the concern build it. Co-Authored-By: Claude Sonnet 5 --- app/graphql/mutations/update_event.rb | 11 +++--- .../mutations/update_event_proposal.rb | 19 +++++----- .../concerns/has_registration_policy.rb | 36 ++++++++++++------- test/models/event_test.rb | 23 ++++++------ 4 files changed, 53 insertions(+), 36 deletions(-) diff --git a/app/graphql/mutations/update_event.rb b/app/graphql/mutations/update_event.rb index 89c63fc36ef..3a204424966 100644 --- a/app/graphql/mutations/update_event.rb +++ b/app/graphql/mutations/update_event.rb @@ -28,10 +28,13 @@ def resolve(**args) private def apply_registration_policy(event, registration_policy_attributes, bucket_key_mappings) - event.apply_registration_policy_change(registration_policy_attributes) do |new_registration_policy| - EventChangeRegistrationPolicyService.new(event, new_registration_policy, current_user, bucket_key_mappings).call! - event.reload - end + change = event.registration_policy_change_for(registration_policy_attributes) + return {} unless change + + EventChangeRegistrationPolicyService.new(event, change.new_policy, current_user, bucket_key_mappings).call! + event.reload + + { "registration_policy" => [change.old_json, event.registration_policy.as_json] } end def apply_form_response_attrs(event, form_response_attrs) diff --git a/app/graphql/mutations/update_event_proposal.rb b/app/graphql/mutations/update_event_proposal.rb index c8b4bcd9789..0cc6cb24423 100644 --- a/app/graphql/mutations/update_event_proposal.rb +++ b/app/graphql/mutations/update_event_proposal.rb @@ -31,16 +31,19 @@ def resolve(**args) private def apply_registration_policy(event_proposal, registration_policy_attributes) - event_proposal.apply_registration_policy_change(registration_policy_attributes) do |new_registration_policy| - ActiveRecord::Base.transaction do - if event_proposal.registration_policy - event_proposal.registration_policy.update_from!(new_registration_policy) - else - event_proposal.registration_policy = new_registration_policy - event_proposal.save! - end + change = event_proposal.registration_policy_change_for(registration_policy_attributes) + return {} unless change + + ActiveRecord::Base.transaction do + if event_proposal.registration_policy + event_proposal.registration_policy.update_from!(change.new_policy) + else + event_proposal.registration_policy = change.new_policy + event_proposal.save! end end + + { "registration_policy" => [change.old_json, event_proposal.registration_policy.as_json] } end def apply_form_response_attrs(event_proposal, form_response_attrs) diff --git a/app/models/concerns/has_registration_policy.rb b/app/models/concerns/has_registration_policy.rb index 569f52480c8..4eb305d02d9 100644 --- a/app/models/concerns/has_registration_policy.rb +++ b/app/models/concerns/has_registration_policy.rb @@ -2,20 +2,30 @@ module HasRegistrationPolicy extend ActiveSupport::Concern - # Builds a detached proposed RegistrationPolicy from `hash`, and -- if it actually differs - # from the current one -- captures an eager "before" JSON snapshot (the caller's block may - # update the current policy's rows in place rather than replacing them, so capturing - # afterward would show the already-updated state), yields the proposal for the caller to - # apply however is appropriate, and returns a {"registration_policy" => [old, new]} change - # hash for FormResponseChange logging. Returns {} if there's nothing to apply. - def apply_registration_policy_change(hash) - return {} unless hash + # A proposed registration policy change that differs from the current one, returned by + # #registration_policy_change_for. `new_policy` is a detached, unsaved RegistrationPolicy the + # caller applies however is appropriate for their model (in-place update, first-time + # assignment, running signup simulation, etc.). `old_json` is captured up front, since applying + # the change may mutate the owner's registration_policy row in place rather than replacing it -- + # reading it again afterward would already show the new state. + class Change + attr_reader :new_policy, :old_json - new_registration_policy = RegistrationPolicy.build_from_hash(hash) - return {} if registration_policy&.equivalent_to?(new_registration_policy) + def initialize(new_policy:, old_json:) + @new_policy = new_policy + @old_json = old_json + end + end + + # Builds a detached proposed RegistrationPolicy from `hash` and returns a Change describing it, + # or nil if `hash` is blank or describes a policy equivalent to the current one (nothing to + # apply). + def registration_policy_change_for(hash) + return nil unless hash + + new_policy = RegistrationPolicy.build_from_hash(hash) + return nil if registration_policy&.equivalent_to?(new_policy) - old_registration_policy_json = registration_policy&.as_json - yield new_registration_policy - { "registration_policy" => [old_registration_policy_json, registration_policy.as_json] } + Change.new(new_policy:, old_json: registration_policy&.as_json) end end diff --git a/test/models/event_test.rb b/test/models/event_test.rb index 1d5484dd2b7..4b8d13fc9a6 100644 --- a/test/models/event_test.rb +++ b/test/models/event_test.rb @@ -107,21 +107,22 @@ class EventTest < ActiveSupport::TestCase end end - describe "#apply_registration_policy_change (from the HasRegistrationPolicy concern)" do - it "returns {} and does not yield when given a nil hash" do - yielded = false - changes = event.apply_registration_policy_change(nil) { yielded = true } + describe "#registration_policy_change_for (from the HasRegistrationPolicy concern)" do + it "returns nil when given a nil hash" do + assert_nil event.registration_policy_change_for(nil) + end - assert_equal({}, changes) - assert_not yielded + it "returns nil when the hash describes an equivalent policy" do + assert_nil event.registration_policy_change_for(event.registration_policy.as_json) end - it "returns {} and does not yield when the hash describes an equivalent policy" do - yielded = false - changes = event.apply_registration_policy_change(event.registration_policy.as_json) { yielded = true } + it "returns a Change with the detached new policy and the old policy's JSON when the hash differs" do + new_hash = { buckets: [{ key: "unlimited", name: "Signups", slots_limited: false, anything: true }] } + change = event.registration_policy_change_for(new_hash) - assert_equal({}, changes) - assert_not yielded + assert change.new_policy.equivalent_to?(RegistrationPolicy.build_from_hash(new_hash)) + assert_not change.new_policy.persisted? + assert_equal event.registration_policy.as_json, change.old_json end end From 649ba589d4cb4d0fee326055096ed7d221c37d55 Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Thu, 16 Jul 2026 15:10:54 -0700 Subject: [PATCH 8/8] Address PR review: trim overwritten comments, YARD-document HasRegistrationPolicy::Change Condensed several comments that had grown into decision logs rather than usable documentation. Notably: moved the IGNORED_HASH_KEYS rationale out of a standalone comment block and into a short inline note inside build_from_hash (the only place a developer would actually look for it); removed an unnecessary comment on `positioned on: :registration_policy` (the method name already says what it does); and clarified that sync_buckets_from_hash!'s key-based bucket matching is about this API's own write-identity model, not signups' bucket_key -- so it isn't automatically resolved by the deferred bucket_key->FK conversion on signups unless that work also redesigns this API around ids. Also replaced HasRegistrationPolicy::Change's prose comment and plain attr_reader with YARD @!attribute/@param/@return annotations. Co-Authored-By: Claude Sonnet 5 --- app/graphql/mutations/create_event.rb | 7 +-- .../mutations/create_event_proposal.rb | 7 +-- app/graphql/mutations/create_filler_event.rb | 7 +-- .../concerns/has_registration_policy.rb | 16 +++--- app/models/registration_policy.rb | 50 +++++-------------- app/models/registration_policy_bucket.rb | 8 +-- app/services/accept_event_proposal_service.rb | 14 ++---- 7 files changed, 27 insertions(+), 82 deletions(-) diff --git a/app/graphql/mutations/create_event.rb b/app/graphql/mutations/create_event.rb index 5aae7dc40b1..4ba55f9de6b 100644 --- a/app/graphql/mutations/create_event.rb +++ b/app/graphql/mutations/create_event.rb @@ -17,12 +17,7 @@ def resolve(signed_image_blob_ids: nil, **args) form_response_attrs = JSON.parse(event_attrs.delete("form_response_attrs_json")) event = convention.events.new(event_attrs) - # registration_policy is a belongs_to association, not a plain form value -- assigning a raw - # hash through the generic form-response path below would raise - # ActiveRecord::AssociationTypeMismatch (there's no accepts_nested_attributes_for). Build a - # real RegistrationPolicy explicitly instead; build_from_hash(nil) already returns an empty, - # fail-closed policy when the client doesn't submit one, which registration_policy_id being - # NOT NULL requires regardless. + # Build explicitly -- a raw hash would crash here (no accepts_nested_attributes_for). event.registration_policy = RegistrationPolicy.build_from_hash(form_response_attrs.delete("registration_policy")) event.assign_form_response_attributes( event.filter_form_response_attributes_for_assignment( diff --git a/app/graphql/mutations/create_event_proposal.rb b/app/graphql/mutations/create_event_proposal.rb index 5019e1ef6bc..f46ae952102 100644 --- a/app/graphql/mutations/create_event_proposal.rb +++ b/app/graphql/mutations/create_event_proposal.rb @@ -46,12 +46,7 @@ def clone_attributes_from_event_proposal_id(id, event_proposal) ) clone_attributes = template_proposal.read_form_response_attributes_for_form_items(compatible_items) - # registration_policy is a belongs_to association, not a plain form value -- - # read_form_response_attribute returns the template's actual persisted row, and assigning it - # directly would point the new proposal's FK at the SAME policy/buckets as the template's (and, - # since both have dependent: :destroy, risk one proposal's destroy cascading into the other's - # policy). Build an independent copy instead, same as - # AcceptEventProposalService#build_own_registration_policy. + # Build an independent copy -- sharing the template's row risks a dependent: :destroy cascade. if clone_attributes["registration_policy"] clone_attributes["registration_policy"] = RegistrationPolicy.build_from_hash( clone_attributes["registration_policy"].as_json diff --git a/app/graphql/mutations/create_filler_event.rb b/app/graphql/mutations/create_filler_event.rb index c0a57992caa..c878ec9d0b8 100644 --- a/app/graphql/mutations/create_filler_event.rb +++ b/app/graphql/mutations/create_filler_event.rb @@ -19,12 +19,7 @@ def resolve(signed_image_blob_ids: nil, **args) form_response_attrs = JSON.parse(event_attrs.delete("form_response_attrs_json")) event = convention.events.new(event_attrs) - # registration_policy is a belongs_to association, not a plain form value -- assigning a raw - # hash through the generic form-response path below would raise - # ActiveRecord::AssociationTypeMismatch (there's no accepts_nested_attributes_for). Build a - # real RegistrationPolicy explicitly instead; build_from_hash(nil) already returns an empty, - # fail-closed policy when the client doesn't submit one, which registration_policy_id being - # NOT NULL requires regardless. + # Build explicitly -- a raw hash would crash here (no accepts_nested_attributes_for). event.registration_policy = RegistrationPolicy.build_from_hash(form_response_attrs.delete("registration_policy")) event.assign_form_response_attributes( event.filter_form_response_attributes_for_assignment( diff --git a/app/models/concerns/has_registration_policy.rb b/app/models/concerns/has_registration_policy.rb index 4eb305d02d9..0825709c664 100644 --- a/app/models/concerns/has_registration_policy.rb +++ b/app/models/concerns/has_registration_policy.rb @@ -2,13 +2,12 @@ module HasRegistrationPolicy extend ActiveSupport::Concern - # A proposed registration policy change that differs from the current one, returned by - # #registration_policy_change_for. `new_policy` is a detached, unsaved RegistrationPolicy the - # caller applies however is appropriate for their model (in-place update, first-time - # assignment, running signup simulation, etc.). `old_json` is captured up front, since applying - # the change may mutate the owner's registration_policy row in place rather than replacing it -- - # reading it again afterward would already show the new state. + # A proposed registration policy change, returned by #registration_policy_change_for. class Change + # @!attribute [r] new_policy + # @return [RegistrationPolicy] a detached, unsaved policy for the caller to apply + # @!attribute [r] old_json + # @return [Hash, nil] the current policy's JSON, captured before applying the change attr_reader :new_policy, :old_json def initialize(new_policy:, old_json:) @@ -17,9 +16,8 @@ def initialize(new_policy:, old_json:) end end - # Builds a detached proposed RegistrationPolicy from `hash` and returns a Change describing it, - # or nil if `hash` is blank or describes a policy equivalent to the current one (nothing to - # apply). + # @param hash [Hash, nil] a raw registration policy hash (e.g. from form response attrs) + # @return [Change, nil] nil if hash is blank or describes a policy equivalent to the current one def registration_policy_change_for(hash) return nil unless hash diff --git a/app/models/registration_policy.rb b/app/models/registration_policy.rb index 1e2f50a03b0..20697f47504 100644 --- a/app/models/registration_policy.rb +++ b/app/models/registration_policy.rb @@ -21,16 +21,7 @@ class RegistrationPolicy < ApplicationRecord class_name: "RegistrationPolicyBucket", inverse_of: :registration_policy, dependent: :destroy - # These exist for the inverse_of relationship with Event/EventProposal's belongs_to. The - # intended cleanup direction is the other way around (Event/EventProposal's belongs_to has - # dependent: :destroy, so destroying an event/proposal destroys its own policy along with it). - # :nullify rather than :restrict_with_exception here deliberately: when an event's own destroy - # cascades into destroying its policy, Rails processes that as an after_destroy callback on the - # *already-destroyed* event -- so a :restrict_with_exception check at that point would - # (incorrectly) still see this association as needing to be checked and block the legitimate - # cascade. :nullify only matters if something destroys a policy directly while an unrelated - # event/proposal still references it, which -- since events.registration_policy_id is NOT NULL - # -- surfaces immediately as a loud NotNullViolation instead of silently orphaning the FK. + # :nullify avoids blocking an event's own destroy cascade. has_many :events, inverse_of: :registration_policy, dependent: :nullify has_many :event_proposals, inverse_of: :registration_policy, dependent: :nullify @@ -49,21 +40,13 @@ def self.unlimited ) end - # Explicit builder for callers constructing a policy from a plain hash (GraphQL args, import - # data, etc). Deliberately NOT hooked into `new`/`assign_attributes` -- RegistrationPolicy - # behaves like an ordinary AR model everywhere else; call sites that need hash construction - # call this by name instead of relying on implicit coercion. - # Keys that must never flow into `new(...)` here: `__typename` can come in from parsed - # form_response_attrs_json built from GraphQL query results (Apollo Client echoes it onto every - # object for cache normalization). `id`/`created_at`/`updated_at` aren't in any GraphQL type - # today, but if they ever were, letting one through would be a serious bug -- `new(id: ...)` - # would make this supposedly-detached, unsaved policy carry the same id as a real persisted - # row, corrupting association/dirty-tracking behavior. Stripped here rather than tolerated by - # the models themselves, which otherwise behave like ordinary AR models with no - # unknown-attribute tolerance. + # Explicit builder for hash-based construction (GraphQL args, import data) -- deliberately not + # hooked into `new`/`assign_attributes`. IGNORED_HASH_KEYS = %w[__typename id created_at updated_at].freeze def self.build_from_hash(hash) + # Strip __typename (an Apollo Client cache-normalization artifact) and id/created_at/updated_at + # (would let this detached policy alias a real persisted row). hash = hash.to_h.stringify_keys.except(*IGNORED_HASH_KEYS) bucket_hashes = hash.delete("buckets") || [] new(hash).tap do |policy| @@ -93,16 +76,12 @@ def update_from!(other) sync_buckets_from_hash!(other.buckets.map(&:attributes)) end - # Updates this policy's buckets in place to match `bucket_hashes`, matching existing rows by - # `key` (never rewriting `key` itself -- it's not user-editable anywhere, so a key - # disappearing/appearing is always a removal/creation, never a rename). Destroys removed - # buckets FIRST, before updating/creating the rest, to avoid a transient collision on the - # `key` unique index if an incoming key happens to match one being freed up in the same call -- - # this ordering is still our responsibility, since it's about `key`, not `position`. Explicit - # `position:` values are set on every create/update to match `bucket_hashes`' array order; the - # `positioned` gem (see RegistrationPolicyBucket) handles reassigning positions safely without - # collisions, so no equivalent ordering care is needed for it. Callers are responsible for - # wrapping this in a transaction alongside any other related writes. + # Matches buckets by key (never rewritten in place); destroys removed keys before + # creating/updating the rest to avoid a transient key collision (positions are reassigned safely + # by the `positioned` gem, so no equivalent care is needed there). Caller wraps this in a + # transaction. This key-based matching is about this API's own write-identity model, not + # signups' bucket_key -- it isn't resolved by the deferred bucket_key->FK conversion on signups + # unless that work also redesigns this API around ids. def sync_buckets_from_hash!(bucket_hashes) bucket_hashes = bucket_hashes.map { |hash| hash.to_h.stringify_keys } desired_keys = bucket_hashes.map { |hash| RegistrationPolicyBucket.normalize_key(hash["key"]) } @@ -176,11 +155,8 @@ def as_json(options = {}) } end - # Explicit value-equality check, deliberately NOT `==` -- every call site already has two - # actual RegistrationPolicy instances in hand (build_from_hash always runs before any - # comparison), so there's no need to accept a raw hash here, and naming this explicitly avoids - # the implicit-everywhere reach of overriding `==` (assert_equal, Array#include?, case/when, - # uniq, etc. would all silently pick up value semantics otherwise). + # Deliberately not `==` -- avoids silently changing assert_equal/Array#include?/case-when/uniq + # semantics everywhere. def equivalent_to?(other) return false unless other.is_a?(RegistrationPolicy) return false unless prevent_no_preference_signups? == other.prevent_no_preference_signups? diff --git a/app/models/registration_policy_bucket.rb b/app/models/registration_policy_bucket.rb index 6b8762f47be..de02d24e04a 100644 --- a/app/models/registration_policy_bucket.rb +++ b/app/models/registration_policy_bucket.rb @@ -30,10 +30,6 @@ class RegistrationPolicyBucket < ApplicationRecord belongs_to :registration_policy, inverse_of: :buckets - # Reuses this codebase's existing convention for an ordered-list-within-a-scope column (see - # SignupRankedChoice's `positioned on: %i[user_con_profile state], column: :priority`), rather - # than hand-rolling position management. Scope is inferred from the belongs_to; column - # defaults to `position`, matching our column name. positioned on: :registration_policy COMPARABLE_ATTRIBUTES = %w[ @@ -146,9 +142,7 @@ def as_json(_options = {}) } end - # Explicit value-equality check, deliberately NOT `==` -- see RegistrationPolicy#equivalent_to? - # for the rationale (overriding `==` reaches implicitly into assert_equal, Array#include?, - # case/when, uniq, etc). + # Deliberately not `==` -- see RegistrationPolicy#equivalent_to? for why. def equivalent_to?(other) return equivalent_to?(other.bucket) if other.is_a?(RegistrationPolicy::BucketDrop) return false unless other.is_a?(RegistrationPolicyBucket) diff --git a/app/services/accept_event_proposal_service.rb b/app/services/accept_event_proposal_service.rb index b3676654079..001b9a76aa1 100644 --- a/app/services/accept_event_proposal_service.rb +++ b/app/services/accept_event_proposal_service.rb @@ -25,13 +25,8 @@ def inner_call # rubocop:disable Metrics/AbcSize event.con_mail_destination ||= "gms" event.title ||= event_proposal.title # in case the form doesn't include it event.length_seconds ||= event_proposal.length_seconds # same deal as title - # Unlike title/length_seconds, this isn't just a missing-value fallback: if the target - # category's form happens to declare a registration_policy form item, the generic - # assign_form_response_attributes call above would have already set event.registration_policy - # to the *same row* as event_proposal.registration_policy (association assignment, not a - # copy). Events and proposals have independent lifecycles, so always rebuild a fresh, - # independent copy here rather than ever sharing a row -- regardless of whether the form - # happened to collect it, and regardless of whether the proposal has one at all. + # Always rebuild an independent copy -- the generic assignment above may have shared the + # proposal's row directly. event.registration_policy = build_own_registration_policy event_proposal.images.blobs.each { |blob| event.images.attach(blob) } @@ -90,10 +85,7 @@ def convention def build_own_registration_policy source_policy = event_proposal.registration_policy - # No buckets at all (rather than defaulting to unlimited) -- accepts_signups? is false with - # an empty bucket list, so an event accepted without an explicit policy fails closed - # (blocks signups until someone configures one) instead of silently accepting unlimited - # signups. + # Empty (not unlimited) -- fails closed, blocking signups until someone configures a real policy. return RegistrationPolicy.new unless source_policy RegistrationPolicy.build_from_hash(source_policy.as_json) end