From b17dbed745b538fdb0fe8f5c7147ec9e5cb32675 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Feb 2026 14:03:39 +0100 Subject: [PATCH 1/8] docs: add design for fixing duplicate waiting list entries Design addresses issue #2479 by: - Making WaitingList.add() idempotent with find_or_create_by - Adding unique database constraint on invitation_id - Cleaning up existing duplicates in migration --- ...x-duplicate-waiting-list-entries-design.md | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 docs/plans/2026-02-11-fix-duplicate-waiting-list-entries-design.md 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 From aa1895d1027d829095435185b3f7b2bd7f2f33db Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Feb 2026 14:04:11 +0100 Subject: [PATCH 2/8] chore: add .worktrees to gitignore Prevents accidental commit of worktree directories --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) 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 From bca37191557a44b1e809a93d484a18f4102cb926 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Feb 2026 14:06:15 +0100 Subject: [PATCH 3/8] docs: add implementation plan for fixing duplicate waiting list entries --- ...2-11-fix-duplicate-waiting-list-entries.md | 328 ++++++++++++++++++ 1 file changed, 328 insertions(+) create mode 100644 docs/plans/2026-02-11-fix-duplicate-waiting-list-entries.md 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..2c8d9195a --- /dev/null +++ b/docs/plans/2026-02-11-fix-duplicate-waiting-list-entries.md @@ -0,0 +1,328 @@ +# 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 after the existing `#add` test context (around line 28): + +```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:32 spec/models/waiting_list_spec.rb:41` + +Expected: 2 failures - "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 file with basic structure + +**Step 2: Write 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 + 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 +``` + +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** + +Edit the generated migration file: + +```ruby +class AddUniqueIndexToWaitingListsInvitationId < ActiveRecord::Migration[8.0] + def up + # Find and clean up duplicate waiting list entries + duplicate_invitation_ids = WaitingList + .group(:invitation_id) + .having('COUNT(*) > 1') + .pluck(:invitation_id) + + # For each duplicate set, keep the oldest and delete the rest + duplicate_invitation_ids.each do |invitation_id| + waiting_list_entries = WaitingList + .where(invitation_id: invitation_id) + .order(:created_at) + + # Keep first (oldest), destroy the rest + waiting_list_entries[1..].each(&:destroy) + end + + # Add unique constraint + add_index :waiting_lists, :invitation_id, unique: true + end + + def down + remove_index :waiting_lists, :invitation_id + end +end +``` + +**Step 3: Run migration** + +Run: `bundle exec rake db:migrate` + +Expected: Migration runs successfully + +**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: 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 + +**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 + +--- + +## Task 6: Verify Linting + +**Files:** +- None (verification only) + +**Step 1: Run RuboCop on changed files** + +Run: `bundle exec rubocop app/models/waiting_list.rb spec/models/waiting_list_spec.rb` + +Expected: No offenses detected + +**Step 2: Fix any linting issues if found** + +If RuboCop reports issues, fix them and re-run. + +**Step 3: Commit linting fixes if needed** + +```bash +git add +git commit -m "style: fix rubocop offenses" +``` + +--- + +## Task 7: Manual Verification (Optional) + +**Files:** +- None (manual testing) + +**Step 1: Start Rails console** + +Run: `bundle exec rails console` + +**Step 2: Test idempotency in console** + +```ruby +workshop = Workshop.first || Fabricate(:workshop) +invitation = WorkshopInvitation.first || Fabricate(:workshop_invitation, workshop: workshop) + +# Add to waiting list twice +wl1 = WaitingList.add(invitation) +wl2 = WaitingList.add(invitation) + +# Verify same record +wl1.id == wl2.id # => true +WaitingList.where(invitation: invitation).count # => 1 +``` + +Expected: Returns true and 1 + +**Step 3: 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 (1 new test) +- ✅ Migration that cleans duplicates and adds unique constraint +- ✅ All existing tests still passing +- ✅ RuboCop clean + +## 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 + +## Notes + +- The unique constraint provides defense-in-depth at the database level +- `find_or_create_by` handles race conditions gracefully +- First call wins for `auto_rsvp` setting (won't be overwritten) +- Migration keeps oldest entry when cleaning duplicates (fairest for waiting list position) From 3985796eef9ed43edc79dbc2b97404866661ffc7 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Feb 2026 14:10:43 +0100 Subject: [PATCH 4/8] docs: improve implementation plan based on review - Remove hard-coded line numbers - Make controller test more comprehensive (check responses and flash messages) - Improve migration safety (logging, delete_all, verification step) - Combine test and linting verification into single task - Make manual verification required instead of optional - Add production deployment and rollback notes --- ...2-11-fix-duplicate-waiting-list-entries.md | 163 ++++++++++++------ spec/models/waiting_list_spec.rb | 19 ++ 2 files changed, 131 insertions(+), 51 deletions(-) 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 index 2c8d9195a..1b0ecf525 100644 --- a/docs/plans/2026-02-11-fix-duplicate-waiting-list-entries.md +++ b/docs/plans/2026-02-11-fix-duplicate-waiting-list-entries.md @@ -19,7 +19,7 @@ **Step 1: Write failing test for idempotent behavior** -Add after the existing `#add` test context (around line 28): +Add in the existing `context '#add'` block: ```ruby it 'is idempotent - returns existing record when called twice' do @@ -50,9 +50,9 @@ end **Step 3: Run tests to verify they fail** -Run: `bundle exec rspec spec/models/waiting_list_spec.rb:32 spec/models/waiting_list_spec.rb:41` +Run: `bundle exec rspec spec/models/waiting_list_spec.rb` -Expected: 2 failures - "expected 1, got 2" and similar +Expected: 2 new failures in the `#add` context - "expected 1, got 2" and similar **Step 4: Commit failing tests** @@ -110,9 +110,9 @@ Refs #2479" Run: `ls spec/controllers/waiting_lists_controller_spec.rb` -If not found: Create file with basic structure +If not found, create it. Otherwise, add to existing file. -**Step 2: Write test for idempotent controller action** +**Step 2: Write comprehensive test for idempotent controller action** If file doesn't exist, create with: @@ -125,9 +125,20 @@ RSpec.describe WaitingListsController, type: :controller do describe 'POST #create' do it 'is idempotent when called multiple times' do - post :create, params: { invitation_id: invitation.token } - post :create, params: { invitation_id: invitation.token } + 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 @@ -162,27 +173,42 @@ Run: `bundle exec rails generate migration AddUniqueIndexToWaitingListsInvitatio This creates a timestamped file in `db/migrate/` -**Step 2: Write migration to clean duplicates** +**Step 2: Write migration to clean duplicates safely** Edit the generated migration file: ```ruby class AddUniqueIndexToWaitingListsInvitationId < ActiveRecord::Migration[8.0] def up - # Find and clean up duplicate waiting list entries - duplicate_invitation_ids = WaitingList - .group(:invitation_id) - .having('COUNT(*) > 1') - .pluck(:invitation_id) - - # For each duplicate set, keep the oldest and delete the rest - duplicate_invitation_ids.each do |invitation_id| - waiting_list_entries = WaitingList - .where(invitation_id: invitation_id) - .order(:created_at) - - # Keep first (oldest), destroy the rest - waiting_list_entries[1..].each(&:destroy) + 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 @@ -191,15 +217,20 @@ class AddUniqueIndexToWaitingListsInvitationId < ActiveRecord::Migration[8.0] def down remove_index :waiting_lists, :invitation_id + # Note: Cannot restore deleted duplicate entries end end ``` -**Step 3: Run migration** +**Step 3: Run migration and verify output** Run: `bundle exec rake db:migrate` -Expected: Migration runs successfully +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** @@ -207,7 +238,13 @@ 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: Commit migration** +**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 @@ -221,7 +258,7 @@ Refs #2479" --- -## Task 5: Run Full Test Suite +## Task 5: Run Full Test Suite and Verify Linting **Files:** - None (verification only) @@ -244,24 +281,15 @@ Run: `bundle exec rspec spec/support/shared_examples/behaves_like_an_invitation_ Expected: All tests pass ---- - -## Task 6: Verify Linting +**Step 4: Run RuboCop on changed files** -**Files:** -- None (verification only) - -**Step 1: Run RuboCop on changed files** - -Run: `bundle exec rubocop app/models/waiting_list.rb spec/models/waiting_list_spec.rb` +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 2: Fix any linting issues if found** +**Step 5: Fix any linting issues if found** -If RuboCop reports issues, fix them and re-run. - -**Step 3: Commit linting fixes if needed** +If RuboCop reports issues, fix them and commit: ```bash git add @@ -270,7 +298,7 @@ git commit -m "style: fix rubocop offenses" --- -## Task 7: Manual Verification (Optional) +## Task 6: Manual Verification **Files:** - None (manual testing) @@ -279,24 +307,37 @@ git commit -m "style: fix rubocop offenses" Run: `bundle exec rails console` -**Step 2: Test idempotency in console** +**Step 2: Verify idempotency in console** ```ruby +# Test idempotency workshop = Workshop.first || Fabricate(:workshop) invitation = WorkshopInvitation.first || Fabricate(:workshop_invitation, workshop: workshop) -# Add to waiting list twice wl1 = WaitingList.add(invitation) wl2 = WaitingList.add(invitation) -# Verify same record -wl1.id == wl2.id # => true -WaitingList.where(invitation: invitation).count # => 1 +puts "Same record? #{wl1.id == wl2.id}" # Should be true +puts "Count: #{WaitingList.where(invitation: invitation).count}" # Should be 1 ``` -Expected: Returns true and 1 +Expected: Both checks pass -**Step 3: Exit console** +**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` @@ -307,10 +348,11 @@ Type: `exit` After completing all tasks, you should have: - ✅ Model tests verifying idempotency (2 new tests) -- ✅ Controller test verifying double-submission protection (1 new test) -- ✅ Migration that cleans duplicates and adds unique constraint +- ✅ 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 @@ -320,9 +362,28 @@ After implementation is complete: 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 -- First call wins for `auto_rsvp` setting (won't be overwritten) +- `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/models/waiting_list_spec.rb b/spec/models/waiting_list_spec.rb index 5cb78a0df..4fd4c531e 100644 --- a/spec/models/waiting_list_spec.rb +++ b/spec/models/waiting_list_spec.rb @@ -32,6 +32,25 @@ expect(WaitingList.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 = 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 + + it 'does not change auto_rsvp on subsequent calls' do + invitation = Fabricate(:workshop_invitation, workshop: workshop) + + WaitingList.add(invitation, true) + second_entry = WaitingList.add(invitation, false) + + expect(second_entry.reload.auto_rsvp).to be(true) + end end context '#coaches_for' do From c8e21957fdcd9eef41e279f2a963ada94287cc71 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Feb 2026 14:37:23 +0100 Subject: [PATCH 5/8] 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 --- app/models/waiting_list.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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) From 57105d071d6a1f5b3dfc9376b6f281a36ab872c7 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Feb 2026 14:42:25 +0100 Subject: [PATCH 6/8] test: add controller test for idempotent waiting list creation Add comprehensive test to verify double-submission protection at the controller level. Test confirms that multiple identical POST requests result in only a single waiting list entry. --- .../waiting_lists_controller_spec.rb | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 spec/controllers/waiting_lists_controller_spec.rb 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 From 2465f660df6502b575481d632ab7034cf0ff4f0a Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Feb 2026 15:06:24 +0100 Subject: [PATCH 7/8] 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 --- ...ue_index_to_waiting_lists_invitation_id.rb | 54 +++ db/schema.rb | 366 +++++++++--------- 2 files changed, 237 insertions(+), 183 deletions(-) create mode 100644 db/migrate/20260211140256_add_unique_index_to_waiting_lists_invitation_id.rb 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 From 4a7499ec71b3d44c79f25c313217efc920f831e4 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 11 Feb 2026 15:26:34 +0100 Subject: [PATCH 8/8] style: fix rubocop offenses in waiting list tests - Use described_class instead of WaitingList class name - Use describe instead of context for method testing - Replace .times.map with Array.new pattern - Use Array#new with block instead of times.map - Remove useless variable assignment - Use `be` matcher instead of `eq` for boolean - Fix trailing whitespace --- spec/models/waiting_list_spec.rb | 46 ++++++++++++++++---------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/spec/models/waiting_list_spec.rb b/spec/models/waiting_list_spec.rb index 4fd4c531e..caa38d4e7 100644 --- a/spec/models/waiting_list_spec.rb +++ b/spec/models/waiting_list_spec.rb @@ -1,75 +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 = WaitingList.add(invitation) - second_call = WaitingList.add(invitation) + first_call = described_class.add(invitation) + second_call = described_class.add(invitation) expect(first_call.id).to eq(second_call.id) - expect(WaitingList.by_workshop(workshop).count).to eq(1) + 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) - WaitingList.add(invitation, true) - second_entry = WaitingList.add(invitation, false) + 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