diff --git a/.gitignore b/.gitignore index f07b67855..71bac6175 100644 --- a/.gitignore +++ b/.gitignore @@ -58,3 +58,6 @@ vendor/bundle /yarn-error.log yarn-debug.log* .yarn-integrity + +# Git worktrees +.worktrees diff --git a/app/models/waiting_list.rb b/app/models/waiting_list.rb index 39868d135..92fe00d6e 100644 --- a/app/models/waiting_list.rb +++ b/app/models/waiting_list.rb @@ -9,7 +9,9 @@ class WaitingList < ApplicationRecord scope :with_notes_and_their_authors, -> { includes(member: { member_notes: :author }) } def self.add(invitation, auto_rsvp = true) - create(invitation: invitation, auto_rsvp: auto_rsvp) + find_or_create_by(invitation: invitation) do |waiting_list| + waiting_list.auto_rsvp = auto_rsvp + end end def self.students(workshop) diff --git a/db/migrate/20260211140256_add_unique_index_to_waiting_lists_invitation_id.rb b/db/migrate/20260211140256_add_unique_index_to_waiting_lists_invitation_id.rb new file mode 100644 index 000000000..c7e1bb8d1 --- /dev/null +++ b/db/migrate/20260211140256_add_unique_index_to_waiting_lists_invitation_id.rb @@ -0,0 +1,54 @@ +class AddUniqueIndexToWaitingListsInvitationId < ActiveRecord::Migration[8.1] + def up + # Clean up duplicate waiting list entries + duplicate_count = say_with_time "Cleaning up duplicate waiting list entries" do + duplicate_invitation_ids = WaitingList + .group(:invitation_id) + .having('COUNT(*) > 1') + .pluck(:invitation_id) + + duplicate_count = duplicate_invitation_ids.count + say "Found #{duplicate_count} invitation_ids with duplicates" + + if duplicate_count > 0 + # For each duplicate set, keep oldest and delete the rest + duplicate_invitation_ids.each do |invitation_id| + entries = WaitingList + .where(invitation_id: invitation_id) + .order(:created_at) + + # Get IDs to delete (all except first/oldest) + ids_to_delete = entries[1..].map(&:id) + deleted_count = ids_to_delete.size + + say " Invitation #{invitation_id}: deleting #{deleted_count} duplicate(s), keeping entry ##{entries.first.id}" + + # Use delete_all for performance and to avoid callbacks + WaitingList.where(id: ids_to_delete).delete_all + end + end + + duplicate_count + end + + # Add unique constraint (remove existing non-unique index first if it exists) + say_with_time "Adding unique index on waiting_lists.invitation_id" do + begin + remove_index :waiting_lists, :invitation_id + rescue StandardError => e + say " Note: Could not remove existing index (#{e.message})" + end + + add_index :waiting_lists, :invitation_id, unique: true + end + end + + def down + begin + remove_index :waiting_lists, :invitation_id + rescue StandardError => e + say "Could not remove index: #{e.message}" + end + # Note: Cannot restore deleted duplicate entries + end +end diff --git a/db/schema.rb b/db/schema.rb index 5041374a8..c2583f1ff 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_01_29_175110) do +ActiveRecord::Schema[8.1].define(version: 2026_02_11_140256) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -19,15 +19,15 @@ create_enum "dietary_restriction_enum", ["vegan", "vegetarian", "pescetarian", "halal", "gluten_free", "dairy_free", "other"] create_table "activities", force: :cascade do |t| - t.string "trackable_type" - t.bigint "trackable_id" - t.string "owner_type" - t.bigint "owner_id" + t.datetime "created_at", precision: nil, null: false t.string "key" + t.bigint "owner_id" + t.string "owner_type" t.text "parameters" - t.string "recipient_type" t.bigint "recipient_id" - t.datetime "created_at", precision: nil, null: false + t.string "recipient_type" + t.bigint "trackable_id" + t.string "trackable_type" t.datetime "updated_at", precision: nil, null: false t.index ["owner_id", "owner_type"], name: "index_activities_on_owner_id_and_owner_type" t.index ["owner_type", "owner_id"], name: "index_activities_on_owner_type_and_owner_id" @@ -38,70 +38,70 @@ end create_table "addresses", id: :serial, force: :cascade do |t| + t.string "city" + t.datetime "created_at", precision: nil + t.text "directions" t.string "flat" - t.string "street" + t.string "latitude" + t.string "longitude" t.string "postal_code" t.integer "sponsor_id" - t.datetime "created_at", precision: nil + t.string "street" t.datetime "updated_at", precision: nil - t.string "city" - t.string "latitude" - t.string "longitude" - t.text "directions" end create_table "announcements", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil + t.integer "created_by_id" t.datetime "expires_at", precision: nil t.text "message" - t.integer "created_by_id" - t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["created_by_id"], name: "index_announcements_on_created_by_id" end create_table "attendance_warnings", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil t.integer "member_id" t.integer "sent_by_id" - t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["member_id"], name: "index_attendance_warnings_on_member_id" t.index ["sent_by_id"], name: "index_attendance_warnings_on_sent_by_id" end create_table "auth_services", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil t.integer "member_id" t.string "provider" t.string "uid" - t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["member_id"], name: "index_auth_services_on_member_id" end create_table "bans", id: :serial, force: :cascade do |t| - t.integer "member_id" - t.datetime "expires_at", precision: nil - t.string "reason" - t.text "note" t.integer "added_by_id" t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.boolean "permanent", default: false + t.datetime "expires_at", precision: nil t.text "explanation" + t.integer "member_id" + t.text "note" + t.boolean "permanent", default: false + t.string "reason" + t.datetime "updated_at", precision: nil t.index ["added_by_id"], name: "index_bans_on_added_by_id" t.index ["member_id"], name: "index_bans_on_member_id" end create_table "chapters", id: :serial, force: :cascade do |t| - t.string "name" + t.boolean "active", default: true t.string "city" t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil + t.text "description" t.string "email" + t.string "image" + t.string "name" t.string "slug" - t.boolean "active", default: true t.string "time_zone", default: "London", null: false - t.text "description" - t.string "image" + t.datetime "updated_at", precision: nil end create_table "chapters_events", id: :serial, force: :cascade do |t| @@ -119,26 +119,26 @@ end create_table "contacts", id: :serial, force: :cascade do |t| - t.integer "sponsor_id" t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.string "name" - t.string "surname" t.string "email" t.boolean "mailing_list_consent", default: false + t.string "name" + t.integer "sponsor_id" + t.string "surname" t.string "token", null: false + t.datetime "updated_at", precision: nil t.index ["email", "sponsor_id"], name: "index_contacts_on_email_and_sponsor_id", unique: true t.index ["sponsor_id"], name: "index_contacts_on_sponsor_id" end create_table "course_invitations", id: :serial, force: :cascade do |t| + t.boolean "attended" + t.boolean "attending" t.integer "course_id" + t.datetime "created_at", precision: nil t.integer "member_id" - t.boolean "attending" - t.boolean "attended" t.text "note" t.string "token" - t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["course_id"], name: "index_course_invitations_on_course_id" t.index ["member_id"], name: "index_course_invitations_on_member_id" @@ -146,122 +146,122 @@ create_table "course_tutors", id: :serial, force: :cascade do |t| t.integer "course_id" - t.integer "tutor_id" t.datetime "created_at", precision: nil + t.integer "tutor_id" t.datetime "updated_at", precision: nil t.index ["course_id"], name: "index_course_tutors_on_course_id" t.index ["tutor_id"], name: "index_course_tutors_on_tutor_id" end create_table "courses", id: :serial, force: :cascade do |t| - t.string "title" - t.string "short_description" - t.text "description" - t.integer "tutor_id" + t.integer "chapter_id" + t.datetime "created_at", precision: nil t.datetime "date_and_time", precision: nil + t.text "description" + t.datetime "ends_at", precision: nil t.integer "seats", default: 0 + t.string "short_description" t.string "slug" - t.string "url" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil t.integer "sponsor_id" t.string "ticket_url" - t.integer "chapter_id" - t.datetime "ends_at", precision: nil + t.string "title" + t.integer "tutor_id" + t.datetime "updated_at", precision: nil + t.string "url" t.index ["chapter_id"], name: "index_courses_on_chapter_id" t.index ["sponsor_id"], name: "index_courses_on_sponsor_id" t.index ["tutor_id"], name: "index_courses_on_tutor_id" end create_table "delayed_jobs", id: :serial, force: :cascade do |t| - t.integer "priority", default: 0, null: false t.integer "attempts", default: 0, null: false + t.datetime "created_at", precision: nil + t.datetime "failed_at", precision: nil t.text "handler", null: false t.text "last_error" - t.datetime "run_at", precision: nil t.datetime "locked_at", precision: nil - t.datetime "failed_at", precision: nil t.string "locked_by" + t.integer "priority", default: 0, null: false t.string "queue" - t.datetime "created_at", precision: nil + t.datetime "run_at", precision: nil t.datetime "updated_at", precision: nil t.index ["priority", "run_at"], name: "delayed_jobs_priority" end create_table "eligibility_inquiries", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil t.integer "member_id" t.integer "sent_by_id" - t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["member_id"], name: "index_eligibility_inquiries_on_member_id" t.index ["sent_by_id"], name: "index_eligibility_inquiries_on_sent_by_id" end create_table "events", id: :serial, force: :cascade do |t| - t.string "name" - t.text "description" + t.boolean "announce_only" + t.string "audience" + t.text "coach_description" + t.string "coach_questionnaire" + t.integer "coach_spaces" + t.boolean "confirmation_required", default: false + t.datetime "created_at", precision: nil t.datetime "date_and_time", precision: nil + t.text "description" + t.boolean "display_coaches" + t.boolean "display_students" + t.string "email" t.datetime "ends_at", precision: nil - t.integer "venue_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.string "slug" - t.text "schedule" - t.integer "coach_spaces" - t.integer "student_spaces" - t.string "coach_questionnaire" - t.string "student_questionnaire" - t.text "coach_description" + t.string "external_url" t.string "info" - t.boolean "announce_only" - t.string "url" - t.string "email" t.boolean "invitable", default: false - t.string "tito_url" + t.string "name" + t.text "schedule" t.boolean "show_faq" - t.boolean "display_students" - t.boolean "display_coaches" - t.string "external_url" - t.boolean "confirmation_required", default: false + t.string "slug" + t.string "student_questionnaire" + t.integer "student_spaces" t.boolean "surveys_required", default: false - t.string "audience" - t.boolean "virtual", default: false, null: false t.string "time_zone", default: "London", null: false + t.string "tito_url" + t.datetime "updated_at", precision: nil + t.string "url" + t.integer "venue_id" + t.boolean "virtual", default: false, null: false t.index ["date_and_time"], name: "index_events_on_date_and_time" t.index ["slug"], name: "index_events_on_slug", unique: true t.index ["venue_id"], name: "index_events_on_venue_id" end create_table "feedback_requests", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil t.integer "member_id" - t.integer "workshop_id" - t.string "token" t.boolean "submited" - t.datetime "created_at", precision: nil + t.string "token" t.datetime "updated_at", precision: nil + t.integer "workshop_id" t.index ["member_id"], name: "index_feedback_requests_on_member_id" t.index ["workshop_id"], name: "index_feedback_requests_on_workshop_id" end create_table "feedbacks", id: :serial, force: :cascade do |t| - t.integer "tutorial_id" - t.text "request" t.integer "coach_id" - t.text "suggestions" t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil t.integer "rating" + t.text "request" + t.text "suggestions" + t.integer "tutorial_id" + t.datetime "updated_at", precision: nil t.integer "workshop_id" t.index ["coach_id"], name: "index_feedbacks_on_coach_id" t.index ["tutorial_id"], name: "index_feedbacks_on_tutorial_id" end create_table "friendly_id_slugs", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil + t.string "scope" t.string "slug", null: false t.integer "sluggable_id", null: false t.string "sluggable_type", limit: 50 - t.string "scope" - t.datetime "created_at", precision: nil t.index ["slug", "sluggable_type", "scope"], name: "index_friendly_id_slugs_on_slug_and_sluggable_type_and_scope", unique: true t.index ["slug", "sluggable_type"], name: "index_friendly_id_slugs_on_slug_and_sluggable_type" t.index ["sluggable_id"], name: "index_friendly_id_slugs_on_sluggable_id" @@ -270,8 +270,8 @@ create_table "group_announcements", id: :serial, force: :cascade do |t| t.integer "announcement_id" - t.integer "group_id" t.datetime "created_at", precision: nil + t.integer "group_id" t.datetime "updated_at", precision: nil t.index ["announcement_id"], name: "index_group_announcements_on_announcement_id" t.index ["group_id"], name: "index_group_announcements_on_group_id" @@ -279,23 +279,23 @@ create_table "groups", id: :serial, force: :cascade do |t| t.integer "chapter_id" - t.string "name" - t.text "description" t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil + t.text "description" t.string "mailing_list_id" + t.string "name" + t.datetime "updated_at", precision: nil t.index ["chapter_id"], name: "index_groups_on_chapter_id" end create_table "invitations", id: :serial, force: :cascade do |t| - t.integer "event_id" t.boolean "attending" + t.datetime "created_at", precision: nil + t.integer "event_id" t.integer "member_id" - t.string "role" t.text "note" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil + t.string "role" t.string "token" + t.datetime "updated_at", precision: nil t.boolean "verified" t.integer "verified_by_id" t.index ["event_id"], name: "index_invitations_on_event_id" @@ -304,115 +304,115 @@ end create_table "jobs", id: :serial, force: :cascade do |t| - t.string "title" - t.text "description" - t.string "location" - t.datetime "expiry_date", precision: nil - t.string "email" - t.string "link_to_job" - t.integer "created_by_id" t.boolean "approved", default: false - t.boolean "submitted", default: false - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.string "company" t.integer "approved_by_id" - t.string "company_website" + t.string "company" t.string "company_address" t.string "company_postcode" + t.string "company_website" + t.datetime "created_at", precision: nil + t.integer "created_by_id" + t.text "description" + t.string "email" + t.datetime "expiry_date", precision: nil + t.string "link_to_job" + t.string "location" t.datetime "published_on", precision: nil t.boolean "remote" t.integer "salary" t.string "slug" t.integer "status", default: 0 + t.boolean "submitted", default: false + t.string "title" + t.datetime "updated_at", precision: nil t.index ["created_by_id"], name: "index_jobs_on_created_by_id" end create_table "meeting_invitations", id: :serial, force: :cascade do |t| - t.integer "meeting_id" + t.boolean "attended", default: false t.boolean "attending" + t.datetime "created_at", precision: nil + t.integer "meeting_id" t.integer "member_id" - t.string "role" t.text "note" + t.string "role" t.string "token" - t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil - t.boolean "attended", default: false t.index ["meeting_id"], name: "index_meeting_invitations_on_meeting_id" t.index ["member_id"], name: "index_meeting_invitations_on_member_id" end create_table "meeting_talks", id: :serial, force: :cascade do |t| - t.integer "meeting_id" - t.string "title" - t.string "description" t.text "abstract" - t.integer "speaker_id" t.datetime "created_at", precision: nil + t.string "description" + t.integer "meeting_id" + t.integer "speaker_id" + t.string "title" t.datetime "updated_at", precision: nil t.index ["meeting_id"], name: "index_meeting_talks_on_meeting_id" t.index ["speaker_id"], name: "index_meeting_talks_on_speaker_id" end create_table "meetings", id: :serial, force: :cascade do |t| - t.datetime "date_and_time", precision: nil - t.integer "venue_id" t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.string "name" + t.datetime "date_and_time", precision: nil t.text "description" - t.string "slug" + t.datetime "ends_at", precision: nil t.boolean "invitable" + t.boolean "invites_sent", default: false + t.string "name" + t.string "slug" t.integer "spaces" t.integer "sponsor_id" - t.boolean "invites_sent", default: false - t.datetime "ends_at", precision: nil + t.datetime "updated_at", precision: nil + t.integer "venue_id" t.index ["slug"], name: "index_meetings_on_slug", unique: true t.index ["venue_id"], name: "index_meetings_on_venue_id" end create_table "member_email_deliveries", id: :serial, force: :cascade do |t| - t.bigint "member_id" - t.text "subject" + t.text "bcc", default: [], array: true t.text "body" - t.text "to", default: [], array: true t.text "cc", default: [], array: true - t.text "bcc", default: [], array: true t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.bigint "member_id" t.string "member_type" + t.text "subject" + t.text "to", default: [], array: true + t.datetime "updated_at", null: false t.index ["member_id"], name: "index_member_email_deliveries_on_member_id" end create_table "member_notes", id: :serial, force: :cascade do |t| - t.integer "member_id" t.integer "author_id" - t.text "note" t.datetime "created_at", precision: nil + t.integer "member_id" + t.text "note" t.datetime "updated_at", precision: nil t.index ["author_id"], name: "index_member_notes_on_author_id" t.index ["member_id"], name: "index_member_notes_on_member_id" end create_table "members", id: :serial, force: :cascade do |t| - t.string "name" - t.string "surname" - t.string "email" t.string "about_you" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.boolean "unsubscribed" - t.boolean "can_log_in", default: false, null: false - t.string "mobile" - t.boolean "received_coach_welcome_email", default: false - t.boolean "received_student_welcome_email", default: false - t.string "pronouns" t.datetime "accepted_toc_at", precision: nil - t.datetime "opt_in_newsletter_at", precision: nil + t.boolean "can_log_in", default: false, null: false + t.datetime "created_at", precision: nil t.enum "dietary_restrictions", default: [], array: true, enum_type: "dietary_restriction_enum" - t.string "other_dietary_restrictions" + t.string "email" t.integer "how_you_found_us" t.string "how_you_found_us_other_reason" + t.string "mobile" + t.string "name" + t.datetime "opt_in_newsletter_at", precision: nil + t.string "other_dietary_restrictions" + t.string "pronouns" + t.boolean "received_coach_welcome_email", default: false + t.boolean "received_student_welcome_email", default: false + t.string "surname" + t.boolean "unsubscribed" + t.datetime "updated_at", precision: nil t.index ["email"], name: "index_members_on_email", unique: true end @@ -430,63 +430,63 @@ end create_table "permissions", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil t.string "name" - t.string "resource_type" t.integer "resource_id" - t.datetime "created_at", precision: nil + t.string "resource_type" t.datetime "updated_at", precision: nil t.index ["name", "resource_type", "resource_id"], name: "index_permissions_on_name_and_resource_type_and_resource_id" t.index ["name"], name: "index_permissions_on_name" end create_table "roles", id: :serial, force: :cascade do |t| - t.string "name" - t.string "description" t.datetime "created_at", precision: nil + t.string "description" + t.string "name" t.datetime "updated_at", precision: nil end create_table "sponsors", id: :serial, force: :cascade do |t| - t.string "name" - t.text "description" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil + t.text "accessibility_info" t.string "avatar" - t.string "website" - t.integer "seats", default: 15 + t.datetime "created_at", precision: nil + t.text "description" t.string "image_cache" - t.integer "number_of_coaches" - t.text "accessibility_info" t.integer "level", default: 1, null: false + t.string "name" + t.integer "number_of_coaches" + t.integer "seats", default: 15 + t.datetime "updated_at", precision: nil + t.string "website" end create_table "sponsorships", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil t.integer "event_id" + t.string "level" t.integer "sponsor_id" - t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil - t.string "level" t.index ["event_id"], name: "index_sponsorships_on_event_id" t.index ["sponsor_id"], name: "index_sponsorships_on_sponsor_id" end create_table "subscriptions", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil t.integer "group_id" t.integer "member_id" - t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["group_id"], name: "index_subscriptions_on_group_id" t.index ["member_id"], name: "index_subscriptions_on_member_id" end create_table "taggings", id: :serial, force: :cascade do |t| + t.string "context", limit: 128 + t.datetime "created_at", precision: nil t.integer "tag_id" - t.string "taggable_type" t.integer "taggable_id" - t.string "tagger_type" + t.string "taggable_type" t.integer "tagger_id" - t.string "context", limit: 128 - t.datetime "created_at", precision: nil + t.string "tagger_type" t.string "tenant", limit: 128 t.index ["context"], name: "index_taggings_on_context" t.index ["tag_id", "taggable_id", "taggable_type", "context", "tagger_id", "tagger_type"], name: "taggings_idx", unique: true @@ -507,78 +507,78 @@ end create_table "testimonials", id: :serial, force: :cascade do |t| + t.datetime "created_at", precision: nil t.integer "member_id" t.text "text" - t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil t.index ["member_id"], name: "index_testimonials_on_member_id" end create_table "tutorials", id: :serial, force: :cascade do |t| - t.string "title" + t.datetime "created_at", precision: nil t.text "description" + t.string "title" + t.datetime "updated_at", precision: nil t.string "url" t.integer "workshop_id" - t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil t.index ["workshop_id"], name: "index_tutorials_on_workshop_id" end create_table "waiting_lists", id: :serial, force: :cascade do |t| - t.integer "invitation_id" t.boolean "auto_rsvp", default: true t.datetime "created_at", precision: nil + t.integer "invitation_id" t.datetime "updated_at", precision: nil - t.index ["invitation_id"], name: "index_waiting_lists_on_invitation_id" + t.index ["invitation_id"], name: "index_waiting_lists_on_invitation_id", unique: true end create_table "workshop_invitations", id: :serial, force: :cascade do |t| - t.integer "workshop_id" - t.integer "member_id" - t.boolean "attending" t.boolean "attended" - t.text "note" + t.boolean "attending" + t.boolean "automated_rsvp" t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil - t.string "token" - t.string "role" + t.integer "last_overridden_by_id" + t.integer "member_id" + t.text "note" t.datetime "reminded_at", precision: nil + t.string "role" t.datetime "rsvp_time", precision: nil - t.boolean "automated_rsvp" + t.string "token" t.text "tutorial" - t.integer "last_overridden_by_id" + t.datetime "updated_at", precision: nil + t.integer "workshop_id" t.index ["member_id"], name: "index_workshop_invitations_on_member_id" t.index ["token"], name: "index_workshop_invitations_on_token", unique: true t.index ["workshop_id"], name: "index_workshop_invitations_on_workshop_id" end create_table "workshop_sponsors", id: :serial, force: :cascade do |t| - t.integer "sponsor_id" - t.integer "workshop_id" - t.boolean "host", default: false t.datetime "created_at", precision: nil + t.boolean "host", default: false + t.integer "sponsor_id" t.datetime "updated_at", precision: nil + t.integer "workshop_id" t.index ["sponsor_id"], name: "index_workshop_sponsors_on_sponsor_id" t.index ["workshop_id"], name: "index_workshop_sponsors_on_workshop_id" end create_table "workshops", id: :serial, force: :cascade do |t| - t.string "title" - t.text "description" - t.datetime "date_and_time", precision: nil + t.integer "chapter_id" + t.integer "coach_spaces", default: 0 t.datetime "created_at", precision: nil - t.datetime "updated_at", precision: nil + t.datetime "date_and_time", precision: nil + t.text "description" + t.datetime "ends_at", precision: nil t.boolean "invitable", default: true - t.string "sign_up_url" - t.integer "chapter_id" t.datetime "rsvp_closes_at", precision: nil t.datetime "rsvp_opens_at", precision: nil - t.datetime "ends_at", precision: nil - t.boolean "virtual", default: false - t.integer "student_spaces", default: 0 - t.integer "coach_spaces", default: 0 + t.string "sign_up_url" t.string "slack_channel" t.string "slack_channel_link" + t.integer "student_spaces", default: 0 + t.string "title" + t.datetime "updated_at", precision: nil + t.boolean "virtual", default: false t.index ["chapter_id"], name: "index_workshops_on_chapter_id" t.index ["date_and_time"], name: "index_workshops_on_date_and_time" end diff --git a/docs/plans/2026-02-11-fix-duplicate-waiting-list-entries-design.md b/docs/plans/2026-02-11-fix-duplicate-waiting-list-entries-design.md new file mode 100644 index 000000000..c4cf9edde --- /dev/null +++ b/docs/plans/2026-02-11-fix-duplicate-waiting-list-entries-design.md @@ -0,0 +1,114 @@ +# Fix Duplicate Waiting List Entries + +**Issue:** [#2479](https://github.com/codebar/planner/issues/2479) +**Date:** 2026-02-11 + +## Problem + +Duplicate entries appear in workshop waiting lists. The same member shows up multiple times in the waiting list display, with each entry pointing to the same invitation and member. These duplicates are counted in the summary statistics. + +## Root Cause + +1. **No uniqueness constraint:** The `waiting_lists` table has no unique constraint on `invitation_id` +2. **Non-idempotent code:** `WaitingList.add()` blindly creates a new record without checking for existing entries +3. **Non-idempotent controller:** `WaitingListsController#create` calls `WaitingList.add()` on every POST, with no duplicate-submission protection + +Duplicates occur through: +- Double-clicking the "Join the waiting list" button +- Browser back button + form resubmission +- Network retries +- Page refresh after submission + +## Solution + +### 1. Make `WaitingList.add()` Idempotent + +Change from `create` to `find_or_create_by`: + +**Before:** +```ruby +def self.add(invitation, auto_rsvp = true) + create(invitation: invitation, auto_rsvp: auto_rsvp) +end +``` + +**After:** +```ruby +def self.add(invitation, auto_rsvp = true) + find_or_create_by(invitation: invitation) do |waiting_list| + waiting_list.auto_rsvp = auto_rsvp + end +end +``` + +This approach: +- Returns existing record if one already exists +- Creates new record only if none exists +- Sets `auto_rsvp` only on creation (first call wins) +- Is fully idempotent + +### 2. Add Database Constraint + +Add unique index in migration: + +```ruby +add_index :waiting_lists, :invitation_id, unique: true +``` + +Provides defense-in-depth - prevents duplicates even if code bypasses the model. + +### 3. Clean Up Existing Duplicates + +Migration must remove duplicates before adding constraint: + +**Strategy:** +- Find invitation_ids with multiple waiting list entries +- Keep the oldest entry (earliest `created_at`) +- Delete newer duplicates + +**Rationale for keeping oldest:** +- Preserves original join time (fairest for waiting list position) +- Maintains user's original `auto_rsvp` choice +- Respects chronological order + +## Testing + +### Model Tests +- Verify idempotency: calling `add()` twice returns same record +- Verify count: only one waiting list entry exists after multiple calls +- Verify `auto_rsvp` unchanged on subsequent calls + +### Controller Tests +- Verify double-submission creates only one entry + +### Migration Tests +- Verify duplicates are removed correctly +- Verify oldest entry is retained + +## Edge Cases + +### Race Conditions +Database unique constraint catches simultaneous inserts. `find_or_create_by` handles `ActiveRecord::RecordNotUnique` by retrying the find. + +### Existing Invitation on Waiting List +When user already on waiting list tries to join again: +- `@invitation.save` updates invitation attributes (tutorial, note) +- `WaitingList.add()` returns existing entry (no-op) +- Result: Invitation updates, waiting list unchanged ✓ + +### Migration Rollback +Down migration removes unique index. Cannot restore deleted duplicates (acceptable - they were bugs). + +## Files Changed + +- `app/models/waiting_list.rb` - Make `add()` idempotent +- `db/migrate/YYYYMMDDHHMMSS_add_unique_index_to_waiting_lists.rb` - Migration +- `spec/models/waiting_list_spec.rb` - Add idempotency tests +- `spec/controllers/waiting_lists_controller_spec.rb` - Add double-submission test + +## Impact + +- Prevents future duplicates +- Fixes existing duplicates in production database +- No changes to UI or user experience +- No breaking changes to existing code diff --git a/docs/plans/2026-02-11-fix-duplicate-waiting-list-entries.md b/docs/plans/2026-02-11-fix-duplicate-waiting-list-entries.md new file mode 100644 index 000000000..1b0ecf525 --- /dev/null +++ b/docs/plans/2026-02-11-fix-duplicate-waiting-list-entries.md @@ -0,0 +1,389 @@ +# Fix Duplicate Waiting List Entries Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Prevent duplicate waiting list entries by making `WaitingList.add()` idempotent and adding database constraints. + +**Architecture:** Use `find_or_create_by` instead of `create` in `WaitingList.add()` to ensure idempotency. Add unique database constraint on `invitation_id`. Clean up existing duplicates in migration. + +**Tech Stack:** Rails 8.1, RSpec, PostgreSQL + +**Related Issue:** [#2479](https://github.com/codebar/planner/issues/2479) + +--- + +## Task 1: Add Model Tests for Idempotency + +**Files:** +- Modify: `spec/models/waiting_list_spec.rb` + +**Step 1: Write failing test for idempotent behavior** + +Add in the existing `context '#add'` block: + +```ruby +it 'is idempotent - returns existing record when called twice' do + invitation = Fabricate(:workshop_invitation, workshop: workshop) + + first_call = WaitingList.add(invitation) + second_call = WaitingList.add(invitation) + + expect(first_call.id).to eq(second_call.id) + expect(WaitingList.by_workshop(workshop).count).to eq(1) +end +``` + +**Step 2: Write failing test for auto_rsvp preservation** + +Add after the previous test: + +```ruby +it 'does not change auto_rsvp on subsequent calls' do + invitation = Fabricate(:workshop_invitation, workshop: workshop) + + first_entry = WaitingList.add(invitation, true) + second_entry = WaitingList.add(invitation, false) + + expect(second_entry.reload.auto_rsvp).to eq(true) +end +``` + +**Step 3: Run tests to verify they fail** + +Run: `bundle exec rspec spec/models/waiting_list_spec.rb` + +Expected: 2 new failures in the `#add` context - "expected 1, got 2" and similar + +**Step 4: Commit failing tests** + +```bash +git add spec/models/waiting_list_spec.rb +git commit -m "test: add failing tests for WaitingList idempotency" +``` + +--- + +## Task 2: Make WaitingList.add() Idempotent + +**Files:** +- Modify: `app/models/waiting_list.rb:11-13` + +**Step 1: Update add method to use find_or_create_by** + +Replace the `add` method: + +```ruby +def self.add(invitation, auto_rsvp = true) + find_or_create_by(invitation: invitation) do |waiting_list| + waiting_list.auto_rsvp = auto_rsvp + end +end +``` + +**Step 2: Run tests to verify they pass** + +Run: `bundle exec rspec spec/models/waiting_list_spec.rb` + +Expected: All 8 tests pass (6 existing + 2 new) + +**Step 3: Commit implementation** + +```bash +git add app/models/waiting_list.rb +git commit -m "fix: make WaitingList.add() idempotent + +Use find_or_create_by instead of create to prevent duplicate +waiting list entries for the same invitation. First call wins +for auto_rsvp setting. + +Refs #2479" +``` + +--- + +## Task 3: Add Controller Test for Double-Submission + +**Files:** +- Modify: `spec/controllers/waiting_lists_controller_spec.rb` + +**Step 1: Check if controller spec exists** + +Run: `ls spec/controllers/waiting_lists_controller_spec.rb` + +If not found, create it. Otherwise, add to existing file. + +**Step 2: Write comprehensive test for idempotent controller action** + +If file doesn't exist, create with: + +```ruby +require 'rails_helper' + +RSpec.describe WaitingListsController, type: :controller do + let(:workshop) { Fabricate(:workshop) } + let(:invitation) { Fabricate(:workshop_invitation, workshop: workshop) } + + describe 'POST #create' do + it 'is idempotent when called multiple times' do + expect { + post :create, params: { invitation_id: invitation.token } + }.to change { WaitingList.count }.by(1) + + expect(response).to have_http_status(:redirect) + expect(flash[:notice]).to eq('You have been added to the waiting list') + + # Second call should not create duplicate + expect { + post :create, params: { invitation_id: invitation.token } + }.not_to change { WaitingList.count } + + expect(response).to have_http_status(:redirect) + expect(flash[:notice]).to eq('You have been added to the waiting list') + expect(WaitingList.where(invitation: invitation).count).to eq(1) + end + end +end +``` + +If file exists, add the test to existing `describe 'POST #create'` block or create new one. + +**Step 3: Run controller test** + +Run: `bundle exec rspec spec/controllers/waiting_lists_controller_spec.rb` + +Expected: Test passes (implementation already makes it work) + +**Step 4: Commit controller test** + +```bash +git add spec/controllers/waiting_lists_controller_spec.rb +git commit -m "test: add controller test for idempotent waiting list creation" +``` + +--- + +## Task 4: Create Migration to Clean Duplicates and Add Constraint + +**Files:** +- Create: `db/migrate/YYYYMMDDHHMMSS_add_unique_index_to_waiting_lists_invitation_id.rb` + +**Step 1: Generate migration** + +Run: `bundle exec rails generate migration AddUniqueIndexToWaitingListsInvitationId` + +This creates a timestamped file in `db/migrate/` + +**Step 2: Write migration to clean duplicates safely** + +Edit the generated migration file: + +```ruby +class AddUniqueIndexToWaitingListsInvitationId < ActiveRecord::Migration[8.0] + def up + say_with_time "Cleaning up duplicate waiting list entries" do + # Find invitation_ids with duplicates + duplicates = WaitingList + .select('invitation_id, COUNT(*) as duplicate_count') + .group(:invitation_id) + .having('COUNT(*) > 1') + + duplicate_count = duplicates.count + say "Found #{duplicate_count} invitation_ids with duplicates" + + return if duplicate_count.zero? + + # For each duplicate set, keep oldest and delete the rest + duplicates.each do |dup| + entries = WaitingList + .where(invitation_id: dup.invitation_id) + .order(:created_at) + + # Get IDs to delete (all except first/oldest) + ids_to_delete = entries[1..].map(&:id) + deleted_count = ids_to_delete.size + + say " Invitation #{dup.invitation_id}: deleting #{deleted_count} duplicate(s), keeping entry ##{entries.first.id}" + + # Use delete_all for performance and to avoid callbacks + WaitingList.where(id: ids_to_delete).delete_all + end + + duplicate_count + end + + # Add unique constraint + add_index :waiting_lists, :invitation_id, unique: true + end + + def down + remove_index :waiting_lists, :invitation_id + # Note: Cannot restore deleted duplicate entries + end +end +``` + +**Step 3: Run migration and verify output** + +Run: `bundle exec rake db:migrate` + +Expected output should show: +- "Cleaning up duplicate waiting list entries" +- Count of duplicate invitation_ids found +- For each duplicate: which entries are being deleted +- "add_index(:waiting_lists, :invitation_id)" + +**Step 4: Verify schema updated** + +Run: `grep -A 5 'create_table "waiting_lists"' db/schema.rb` + +Expected: Shows `index ["invitation_id"], name: "index_waiting_lists_on_invitation_id", unique: true` + +**Step 5: Verify no duplicates remain** + +Run: `bundle exec rails runner 'puts WaitingList.group(:invitation_id).having("COUNT(*) > 1").count'` + +Expected: Returns `0` (no duplicates) + +**Step 6: Commit migration** + +```bash +git add db/migrate/*_add_unique_index_to_waiting_lists_invitation_id.rb db/schema.rb +git commit -m "feat: add unique constraint to waiting_lists.invitation_id + +Migration cleans up existing duplicates (keeping oldest entry) +before adding unique index to prevent future duplicates. + +Refs #2479" +``` + +--- + +## Task 5: Run Full Test Suite and Verify Linting + +**Files:** +- None (verification only) + +**Step 1: Run all waiting list related tests** + +Run: `bundle exec rspec spec/models/waiting_list_spec.rb spec/controllers/waiting_lists_controller_spec.rb` + +Expected: All tests pass + +**Step 2: Run feature tests for workshop management** + +Run: `bundle exec rspec spec/features/admin/manage_workshop_attendances_spec.rb` + +Expected: All tests pass + +**Step 3: Run tests that use WaitingList.add** + +Run: `bundle exec rspec spec/support/shared_examples/behaves_like_an_invitation_route.rb spec/models/workshop_spec.rb` + +Expected: All tests pass + +**Step 4: Run RuboCop on changed files** + +Run: `bundle exec rubocop app/models/waiting_list.rb spec/models/waiting_list_spec.rb spec/controllers/waiting_lists_controller_spec.rb` + +Expected: No offenses detected + +**Step 5: Fix any linting issues if found** + +If RuboCop reports issues, fix them and commit: + +```bash +git add +git commit -m "style: fix rubocop offenses" +``` + +--- + +## Task 6: Manual Verification + +**Files:** +- None (manual testing) + +**Step 1: Start Rails console** + +Run: `bundle exec rails console` + +**Step 2: Verify idempotency in console** + +```ruby +# Test idempotency +workshop = Workshop.first || Fabricate(:workshop) +invitation = WorkshopInvitation.first || Fabricate(:workshop_invitation, workshop: workshop) + +wl1 = WaitingList.add(invitation) +wl2 = WaitingList.add(invitation) + +puts "Same record? #{wl1.id == wl2.id}" # Should be true +puts "Count: #{WaitingList.where(invitation: invitation).count}" # Should be 1 +``` + +Expected: Both checks pass + +**Step 3: Verify unique constraint prevents duplicates** + +```ruby +# Try to create duplicate directly (should fail) +begin + WaitingList.create!(invitation: invitation, auto_rsvp: true) + puts "ERROR: Duplicate was created!" +rescue ActiveRecord::RecordNotUnique => e + puts "GOOD: Unique constraint prevented duplicate" +end +``` + +Expected: "GOOD: Unique constraint prevented duplicate" + +**Step 4: Exit console** + +Type: `exit` + +--- + +## Testing Summary + +After completing all tasks, you should have: + +- ✅ Model tests verifying idempotency (2 new tests) +- ✅ Controller test verifying double-submission protection with full response checks (1 new test) +- ✅ Migration that safely cleans duplicates with logging and adds unique constraint +- ✅ All existing tests still passing +- ✅ RuboCop clean +- ✅ Manual verification that constraints work + +## Next Steps + +After implementation is complete: + +1. Use `@superpowers:verification-before-completion` to verify all tests pass +2. Create pull request following project conventions +3. Reference issue #2479 in PR description + +## Production Deployment Notes + +**Pre-deployment:** +- Review migration output in staging first +- Migration is safe to run - uses `delete_all` for performance and logs all deletions +- Expected downtime: None (migration is fast, <1 second for typical data) + +**Post-deployment:** +- Monitor logs for any `ActiveRecord::RecordNotUnique` exceptions (shouldn't occur but worth checking) +- Verify waiting list functionality works correctly in production +- Check Rollbar/error tracking for any unexpected issues + +**Rollback plan:** +- Code rollback: Safe - old code will continue working (no duplicates can be created anymore) +- Database rollback: Run `rake db:rollback` to remove unique index +- Note: Deleted duplicate entries cannot be restored (acceptable - they were bugs) + +## Notes + +- The unique constraint provides defense-in-depth at the database level +- `find_or_create_by` handles race conditions gracefully by rescuing `RecordNotUnique` and retrying +- First call wins for `auto_rsvp` setting (won't be overwritten on subsequent calls) +- Migration keeps oldest entry when cleaning duplicates (fairest for waiting list position) +- Migration uses `delete_all` instead of `destroy` for performance (avoids callbacks) +- Migration logs all deletions for audit trail diff --git a/spec/controllers/waiting_lists_controller_spec.rb b/spec/controllers/waiting_lists_controller_spec.rb new file mode 100644 index 000000000..89a5d391e --- /dev/null +++ b/spec/controllers/waiting_lists_controller_spec.rb @@ -0,0 +1,34 @@ +RSpec.describe WaitingListsController do + let(:workshop) { Fabricate(:workshop) } + let(:invitation) { Fabricate(:workshop_invitation, workshop: workshop) } + + describe 'POST #create' do + it 'creates a waiting list entry on first submission' do + expect do + post :create, params: { invitation_id: invitation.token } + end.to change(WaitingList, :count).by(1) + end + + it 'displays success message on first submission' do + post :create, params: { invitation_id: invitation.token } + + expect(response).to redirect_to(invitation_path(invitation)) + expect(flash[:notice]).to eq('You have been added to the waiting list') + end + + it 'does not create duplicate on second submission' do + post :create, params: { invitation_id: invitation.token } + + expect do + post :create, params: { invitation_id: invitation.token } + end.not_to change(WaitingList, :count) + end + + it 'maintains idempotency' do + post :create, params: { invitation_id: invitation.token } + post :create, params: { invitation_id: invitation.token } + + expect(WaitingList.where(invitation: invitation).count).to eq(1) + end + end +end diff --git a/spec/models/waiting_list_spec.rb b/spec/models/waiting_list_spec.rb index 5cb78a0df..caa38d4e7 100644 --- a/spec/models/waiting_list_spec.rb +++ b/spec/models/waiting_list_spec.rb @@ -1,56 +1,75 @@ RSpec.describe WaitingList do let(:workshop) { Fabricate(:workshop) } - context 'scopes' do - context '#by_workshop' do + describe 'scopes' do + describe '#by_workshop' do it 'is empty when there are not invitations in the waiting list' do - expect(WaitingList.by_workshop(workshop)).to eq([]) + expect(described_class.by_workshop(workshop)).to eq([]) end it 'is returns the waiting list entries when there are any' do - invitations = 2.times.map { Fabricate(:workshop_invitation, workshop: workshop) } - invitations.each { |invitation| WaitingList.add(invitation) } + invitations = Array.new(2) { Fabricate(:workshop_invitation, workshop: workshop) } + invitations.each { |invitation| described_class.add(invitation) } - expect(WaitingList.by_workshop(workshop).map(&:invitation)).to match_array(invitations) + expect(described_class.by_workshop(workshop).map(&:invitation)).to match_array(invitations) end end - context '#next_spot' do + describe '#next_spot' do it 'returns the next spot to be allocated' do invitation = Fabricate(:workshop_invitation, workshop: workshop) - WaitingList.add(invitation) + described_class.add(invitation) - expect(WaitingList.next_spot(workshop, 'Student').invitation).to eq(invitation) + expect(described_class.next_spot(workshop, 'Student').invitation).to eq(invitation) end end end - context '#add' do + describe '#add' do it 'is adds an invitation to the waiting list' do invitation = Fabricate(:workshop_invitation, workshop: workshop) - WaitingList.add(invitation) + described_class.add(invitation) - expect(WaitingList.by_workshop(workshop).map(&:invitation)).to eq([invitation]) + expect(described_class.by_workshop(workshop).map(&:invitation)).to eq([invitation]) + end + + it 'is idempotent - returns existing record when called twice' do + invitation = Fabricate(:workshop_invitation, workshop: workshop) + + first_call = described_class.add(invitation) + second_call = described_class.add(invitation) + + expect(first_call.id).to eq(second_call.id) + expect(described_class.by_workshop(workshop).count).to eq(1) + end + + it 'does not change auto_rsvp on subsequent calls' do + invitation = Fabricate(:workshop_invitation, workshop: workshop) + + described_class.add(invitation, true) + second_entry = described_class.add(invitation, false) + + expect(second_entry.reload.auto_rsvp).to be(true) end end - context '#coaches_for' do + describe '#coaches_for' do it 'returns waitlisted coaches for a specific workshop' do coach = Fabricate(:coach) invitation = Fabricate(:coach_workshop_invitation, workshop: workshop, member: coach) - coach_invitation = WaitingList.add(invitation) + coach_invitation = described_class.add(invitation) - expect(WaitingList.coaches_for(workshop)).to eq([coach_invitation]) + expect(described_class.coaches_for(workshop)).to eq([coach_invitation]) end it 'returns waitlisted students for a specific workshop' do student = Fabricate(:student) - + invitation = Fabricate(:student_workshop_invitation, workshop: workshop, member: student) - student_invitation = WaitingList.add(invitation) + student_invitation = described_class.add(invitation) - expect(WaitingList.students_for(workshop)).to eq([student_invitation]) + expect(described_class.students_for(workshop)).to eq([student_invitation]) end end end