diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 4de021227b5..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 @@ -36,6 +41,11 @@ GraphQL/UnnecessaryArgumentCamelize: 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' # Offense count: 1 Lint/NoReturnInBeginEndBlocks: @@ -49,7 +59,22 @@ Metrics/BlockLength: Exclude: - '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' + - '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 +85,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 +141,7 @@ Rails/HasAndBelongsToMany: Rails/HasManyOrHasOneDependent: Exclude: + - 'app/models/event.rb' - 'app/models/page.rb' Rails/HelperInstanceVariable: @@ -114,6 +152,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 +164,53 @@ 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/events.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/create_event.rb b/app/graphql/mutations/create_event.rb index fa8ee0ac403..4ba55f9de6b 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,8 @@ 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) + # 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( form_response_attrs, diff --git a/app/graphql/mutations/create_event_proposal.rb b/app/graphql/mutations/create_event_proposal.rb index c5d3258a3b2..f46ae952102 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,14 @@ 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) + + # 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 + ) + end + event_proposal.assign_form_response_attributes(clone_attributes) end diff --git a/app/graphql/mutations/create_filler_event.rb b/app/graphql/mutations/create_filler_event.rb index 702c9802c06..c878ec9d0b8 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,8 @@ 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) + # 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( form_response_attrs, 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..3a204424966 100644 --- a/app/graphql/mutations/update_event.rb +++ b/app/graphql/mutations/update_event.rb @@ -28,15 +28,13 @@ 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! + 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" => [old_registration_policy.as_json, new_registration_policy.as_json] } + { "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 4d4af2eb5f3..0cc6cb24423 100644 --- a/app/graphql/mutations/update_event_proposal.rb +++ b/app/graphql/mutations/update_event_proposal.rb @@ -1,38 +1,71 @@ # 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) + 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) 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..0825709c664 --- /dev/null +++ b/app/models/concerns/has_registration_policy.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true +module HasRegistrationPolicy + extend ActiveSupport::Concern + + # 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:) + @new_policy = new_policy + @old_json = old_json + end + end + + # @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 + + new_policy = RegistrationPolicy.build_from_hash(hash) + return nil if registration_policy&.equivalent_to?(new_policy) + + Change.new(new_policy:, old_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 ebe2df48656..03c14af0001 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) UNIQUE +# 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 @@ -54,6 +56,7 @@ class Event < ApplicationRecord include AgeRestrictions include EventEmail include FormResponse + include HasRegistrationPolicy include MarkdownIndexing include OrderByTitle include PgSearch::Model @@ -114,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" @@ -197,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 @@ -264,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..30a39c1cf3a 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) UNIQUE # # 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..20697f47504 100644 --- a/app/models/registration_policy.rb +++ b/app/models/registration_policy.rb @@ -1,17 +1,36 @@ # 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 + # :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 + + 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 +40,77 @@ def self.unlimited ) end - attr_reader :buckets - - def initialize(attributes = {}) - super - @buckets ||= [] + # 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| + 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 + + # 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"]) } + 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 +130,58 @@ 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) + # 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 - 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 new file mode 100644 index 00000000000..de02d24e04a --- /dev/null +++ b/app/models/registration_policy_bucket.rb @@ -0,0 +1,155 @@ +# 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 +class RegistrationPolicyBucket < ApplicationRecord + belongs_to :registration_policy, inverse_of: :buckets + + 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 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 + + def anything=(value) + self.flex = value + end + + def not_counted # rubocop:disable Naming/PredicateMethod + !counted + end + + def not_counted? + not_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 + + # 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) + 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..001b9a76aa1 100644 --- a/app/services/accept_event_proposal_service.rb +++ b/app/services/accept_event_proposal_service.rb @@ -18,13 +18,16 @@ 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 + # 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) } event.save! @@ -79,4 +82,11 @@ def proposal_form_item_identifiers def convention @convention ||= event_proposal.convention end + + def build_own_registration_policy + source_policy = event_proposal.registration_policy + # 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 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..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) @@ -475,10 +484,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 new file mode 100644 index 00000000000..ce5878aa965 --- /dev/null +++ b/db/migrate/20260615192952_convert_registration_policies_to_tables.rb @@ -0,0 +1,141 @@ +# 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 + 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, 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 + t.boolean :expose_attendees, default: false, null: false + + t.timestamps + + 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 + migrate_registration_policies_for("events") + migrate_registration_policies_for("event_proposals", skip_if_null: true) + end + end + + change_table :events, bulk: true do |t| + 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/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 93553a7c008..ad59bec0cd5 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 ); @@ -1544,7 +1320,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 +1329,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 +2158,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 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, + 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 +3206,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 +3764,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 +3960,20 @@ 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_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: - -- @@ -4437,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 UNIQUE 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: - -- @@ -4479,6 +4380,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 UNIQUE 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 +4730,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 +5133,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: - -- @@ -5410,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: - -- @@ -6161,6 +6092,8 @@ ALTER TABLE ONLY public.cms_files_pages SET search_path TO "$user", public; INSERT INTO "schema_migrations" (version) VALUES +('20260716155031'), +('20260615192952'), ('20260601000000'), ('20260524175914'), ('20260519000000'), diff --git a/test/factories/event_proposals.rb b/test/factories/event_proposals.rb index 1eaca7b90a9..49f6add42a5 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) UNIQUE # # 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/events.rb b/test/factories/events.rb index d75dca46049..56f0cdfd59d 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) UNIQUE +# 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 @@ -53,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/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/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 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/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..906795f62ac 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,18 @@ assert_equal convention.name, convention_drop.name end - describe 'with runs that have openings' do - let(:limited_registration_policy) do - { + describe "with runs that have openings" 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 }, - { 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 +27,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 +47,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..988c134554f 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) UNIQUE # # Foreign Keys # @@ -38,13 +39,26 @@ # 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 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 452572672ef..4b8d13fc9a6 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) UNIQUE +# 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 @@ -57,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 } ] ) ) @@ -104,4 +106,30 @@ class EventTest < ActiveSupport::TestCase assert_equal ["dogs"], event.bucket_keys_with_pending_signups_or_requests end end + + 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 + + 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 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 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 + + 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/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 new file mode 100644 index 00000000000..a519a7f7b4a --- /dev/null +++ b/test/models/registration_policy_bucket_test.rb @@ -0,0 +1,170 @@ +# 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 +require "test_helper" + +class RegistrationPolicyBucketTest < ActiveSupport::TestCase + 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 + + 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 + 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..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,8 +4,13 @@ include ActiveJob::TestHelper 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 }] }) + # 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 }] + ) end let(:event) { create(:event, convention:, registration_policy: one_player_registration_policy) } let(:the_run) { create(:run, event:) } @@ -412,7 +417,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 +499,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) 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 =