feat(matching): prevent auto suggestions when manual match exists + comprehensive test scenarios

Matching Service: - Fetch manual matches at start of runMatching() (suggestionId: null) - Build manualBlockedPairs set with both directions (A:B and B:A) - Skip recorder candidates if manual match exists between dancer and recorder - Ensures no duplicate matches on /matches page - Manual match = user-controlled, algorithm respects user decisions Documentation (docs/TODO.md): - Add comprehensive matching system test scenarios (S1-S16) - Document 4 critical gaps (P0): ratings/stats, admin middleware, participant validation - Document 3 high priority items (P1): cleanup conflicts, rate limiting, notifications - Document 6 medium priority items (P2): audit endpoints, zombie cleanup, reminders - List 11 edge cases for team discussion (E1-E11) - Clear priority ranking and questions for team Critical Findings: - recordingsDone/recordingsReceived fields exist but NEVER updated (fairness broken!) - Admin endpoints lack security middleware - Inconsistent event participant validation across endpoints Test Coverage: - S1-S7: Implemented (basic flow, collisions, limits, manual vs auto) - S10: NOT IMPLEMENTED - ratings/stats system (CRITICAL!) - S11: Partially implemented - audit trail exists, API endpoints missing - S14: Partially implemented - recorder-only auth works, admin middleware missing - S15-S16: NOT IMPLEMENTED - security, notifications
2025-11-30 15:53:00 +01:00
parent f45cadae7d
commit 25236222de
2 changed files with 469 additions and 0 deletions
--- a/backend/src/services/matching.js
+++ b/backend/src/services/matching.js
@@ -273,6 +273,23 @@ async function runMatching(eventId) {
  // Build division-to-slot map from schedule config
  const divisionSlotMap = buildDivisionSlotMap(event?.scheduleConfig);

+  // 0a. Get manual matches - we won't create auto suggestions for these pairs
+  const manualMatches = await prisma.match.findMany({
+    where: {
+      eventId,
+      suggestionId: null, // Manual matches don't have a suggestionId
+      status: { in: ['pending', 'accepted', 'completed'] },
+    },
+    select: { user1Id: true, user2Id: true },
+  });
+
+  // Build set of blocked pairs (both directions)
+  const manualBlockedPairs = new Set();
+  for (const m of manualMatches) {
+    manualBlockedPairs.add(`${m.user1Id}:${m.user2Id}`);
+    manualBlockedPairs.add(`${m.user2Id}:${m.user1Id}`); // both directions
+  }
+
  // 1. Get all participants with their heats and user info
  const participants = await prisma.eventParticipant.findMany({
    where: { eventId },
@@ -416,6 +433,9 @@ async function runMatching(eventId) {
        // Skip self
        if (recorder.userId === dancer.userId) continue;

+        // Skip if manual match exists between dancer and recorder
+        if (manualBlockedPairs.has(`${dancer.userId}:${recorder.userId}`)) continue;
+
        // Check assignment limit
        const currentCount = recorderAssignmentCount.get(recorder.userId) || 0;
        if (currentCount >= MAX_RECORDINGS_PER_PERSON) continue;
--- a/docs/TODO.md
+++ b/docs/TODO.md
@@ -34,6 +34,7 @@
  5. **Phase 5:** Edge Cases (TC17-18) - Sanity checks
 - **Impact:** Ensures matching algorithm works correctly end-to-end, verifies critical bug fixes
 - **Status:** Test plan documented, implementation pending
+- **Extended Scenarios:** See comprehensive test scenarios below

 ### Recently Completed (2025-11-29)
 - 3-Tier Account System (BASIC/SUPPORTER/COMFORT) with fairness algorithm
@@ -48,6 +49,454 @@

 ---

+## Matching System - Comprehensive Test Scenarios
+
+**Last Updated:** 2025-11-30
+**Status:** Documented for team discussion
+
+### Implementation Status
+
+#### ✅ Implemented Scenarios
+- **S1-S3:** Basic flow, collision detection, limits (covered by existing tests)
+- **S7.1-7.2:** Manual match blocks auto suggestions (implemented 2025-11-30)
+- **S12:** Multi-heat collision detection (existing logic)
+- **S14.1:** Only recorder can accept/reject (implemented in MVP)
+
+#### 🔴 Critical Gaps (P0 - Before Production)
+
+1. **S10: Ratings & Stats System** - **CRITICAL**
+   - Fields `recordingsDone`/`recordingsReceived` exist but NEVER updated
+   - Fairness algorithm depends on these stats - currently broken!
+   - Need: `statsApplied` flag on Match model
+   - Need: Auto-increment stats after both users rate (only for auto matches)
+
+2. **S14.2: Admin Middleware** - **SECURITY**
+   - Admin endpoints not protected: `/admin/events/:slug/run-now`, `/admin/matching-runs`
+   - Need: `requireAdmin` middleware
+
+3. **S14.3: Event Participant Validation** - **SECURITY**
+   - Inconsistent checks across endpoints
+   - Need: Audit all suggestion/match endpoints for participant validation
+
+#### ⚠️ High Priority (P1 - First Month)
+
+4. **E9/S13.2: Manual match created AFTER auto suggestion**
+   - Current: Manual blocks only NEW auto suggestions, old pending remain
+   - Need: Cleanup conflicting pending auto suggestions when manual match created
+
+5. **S15.1-15.2: Rate Limiting & Spam Protection**
+   - Max pending outgoing requests (20)
+   - Rate limit manual match requests (10/minute)
+
+6. **S16.1: Socket Notifications**
+   - Real-time notification when new suggestion created
+
+#### 📋 Medium Priority (P2 - Q1 2025)
+
+7. **S11.3-11.4: Matching Run Details API**
+   - Endpoint: `GET /matching-runs/:id/suggestions`
+   - Filters: `onlyAssigned`, `includeNotFound`
+
+8. **S15.3: Zombie Matches Cleanup**
+   - Auto-cancel pending matches older than 30 days
+
+9. **S16.3: Email Reminders**
+   - Reminder before event for accepted recording assignments
+
+### Test Scenarios by Category
+
+<details>
+<summary><b>S1: BASIC FLOW</b> ✅ Implemented</summary>
+
+#### S1.1: Happy path - basic assignment
+- **Given:** Event with deadline, user A has heat H1, user B no conflict
+- **When:** Matching runs after deadline
+- **Then:** Creates suggestion B records A in H1 (pending)
+
+#### S1.2: Recorder accepts suggestion
+- **Given:** Pending suggestion: B records A
+- **When:** B clicks Accept
+- **Then:** Status→accepted, creates Match (type: auto), both see on /matches
+
+#### S1.3: Recorder rejects suggestion
+- **Given:** Pending suggestion: B records A
+- **When:** B clicks Reject
+- **Then:** Status→rejected, A sees read-only, next run may assign different recorder
+
+</details>
+
+<details>
+<summary><b>S2: TIME COLLISION DETECTION</b> ✅ Implemented</summary>
+
+#### S2.1: Can't record while dancing
+- **Given:** A has heat H1, B has heat H1 (same slot)
+- **Then:** B NOT candidate to record A in H1
+
+#### S2.2: Buffer before - preparation time
+- **Given:** A has heat H5, B has heat H6 (next)
+- **When:** HEAT_BUFFER_BEFORE = 1
+- **Then:** B NOT candidate (needs H5 for prep)
+
+#### S2.3: Buffer after - rest time
+- **Given:** A has heat H5, B has heat H4 (previous)
+- **When:** HEAT_BUFFER_AFTER = 1
+- **Then:** B NOT candidate (needs H5 for rest)
+
+#### S2.4: Schedule config - divisions in same slot
+- **Given:** Schedule config: slot 1 = [Open, Newcomer]; A has Open H3, B has Newcomer H3
+- **Then:** B can't record A (same slot despite different divisions)
+
+</details>
+
+<details>
+<summary><b>S3: LIMITS & CONSTRAINTS</b> ✅ Implemented</summary>
+
+#### S3.1: Max 3 recordings per person
+- **Given:** B already assigned to record 3 others
+- **Then:** B NOT candidate for dancer A
+
+#### S3.2: Opt-out from recording
+- **Given:** B checked "I don't want to record others"
+- **Then:** B completely excluded as potential recorder
+
+#### S3.3: No self-recording
+- **Given:** A has heat H1
+- **Then:** A can't be recorder for themselves
+
+</details>
+
+<details>
+<summary><b>S4: FAIRNESS & TIER SYSTEM</b> ✅ Implemented (but stats not updating!)</summary>
+
+#### S4.1: BASIC user - normal frequency
+- **Given:** C (BASIC): done=5, received=0; A,B (BASIC): done=0, received=0
+- **Then:** C has priority (fairness debt = -5)
+
+#### S4.2: SUPPORTER user - reduced frequency (-10)
+- **Given:** A (SUPPORTER) vs B (BASIC), both 0/0
+- **Then:** B has priority (A has -10 penalty)
+
+#### S4.3: COMFORT user - significantly reduced (-50)
+- **Given:** A (COMFORT), B (SUPPORTER), C (BASIC), all 0/0
+- **Then:** Priority: C > B > A
+
+#### S4.4: Location preference
+- **Given:** Dancer A in LA,US; B (LA,US), C (NYC,US), D (London,UK)
+- **Then:** Priority: B > C > D
+
+</details>
+
+<details>
+<summary><b>S5: INCREMENTAL MATCHING</b> ✅ Implemented</summary>
+
+#### S5.1: Accepted suggestions preserved
+- **Given:** Run 1 created suggestion B→A (pending), B accepted
+- **When:** Run 2
+- **Then:** A-B suggestion NOT deleted or overwritten
+
+#### S5.2: Pending suggestions regenerated
+- **Given:** Run 1 created B→A (pending), B didn't accept
+- **When:** Run 2 (new users joined)
+- **Then:** Pending A-B deleted, new suggestion may assign C→A
+
+#### S5.3: Accepted suggestions block slots
+- **Given:** B accepted recording A in heat H5
+- **When:** Run 2 for dancer D (also has H5)
+- **Then:** B NOT candidate (busy in H5)
+
+</details>
+
+<details>
+<summary><b>S6: SCHEDULER</b> ✅ Implemented</summary>
+
+#### S6.1: Waits until deadline
+- **Given:** Event has registrationDeadline in 2 hours
+- **Then:** Scheduler does NOT run matching (waits)
+
+#### S6.2: Runs after deadline
+- **Given:** registrationDeadline passed 1 minute ago
+- **Then:** Runs matching (run 1/5)
+
+#### S6.3: 5 runs at 5-minute intervals
+- **Then:** Run 1 (immediately), Run 2 (+5min), Run 3 (+10min), Run 4 (+15min), Run 5 (+20min), STOP
+
+#### S6.4: Audit trail
+- **Given:** Matching ran
+- **Then:** Admin sees table of runs with timestamps and stats (total/pending/accepted/rejected/not_found)
+
+</details>
+
+<details>
+<summary><b>S7: MANUAL vs AUTO MATCHES</b> ✅ S7.1-7.4 Implemented</summary>
+
+#### S7.1: Manual match blocks auto suggestion ✅
+- **Given:** A sent manual match request to B (pending)
+- **When:** Matching algorithm runs
+- **Then:** Does NOT create auto suggestion A↔B or B↔A
+
+#### S7.2: All manual statuses block ✅
+- **Given:** Manual match A↔B in status: pending/accepted/completed
+- **Then:** All cases block auto suggestion
+
+#### S7.3: /matches shows both types with badges ✅
+- **Given:** A has manual match with B (pending), auto suggestion with C (pending - C records A)
+- **Then:** A sees 2 items: B (badge "Manual"), C (badge "Auto")
+
+#### S7.4: Auto pending - redirect to Records ✅
+- **Given:** A has auto suggestion with C (pending, C records A)
+- **Then:** Button "Go to Records" → /events/X/chat?tab=records (no Accept/Reject for dancer)
+
+</details>
+
+<details>
+<summary><b>S10: RATINGS & STATS</b> 🔴 NOT IMPLEMENTED - CRITICAL!</summary>
+
+#### S10.1: Auto match completed → stats updated exactly once
+- **Given:** Auto Match A↔B (suggestionId != null, statsApplied=false), both rated
+- **When:** Ratings endpoint called
+- **Then:**
+  - `recordingsDone++` for recorder
+  - `recordingsReceived++` for dancer
+  - `match.status = 'completed'`
+  - `match.statsApplied = true`
+
+#### S10.2: Only one rated → no stats
+- **Given:** Auto Match A↔B, only A rated
+- **Then:** `statsApplied` stays false, stats don't change
+
+#### S10.3: Manual match completion → no stats update
+- **Given:** Match A↔B (suggestionId=null), both rated
+- **Then:** Stats don't change (manual matches don't affect fairness)
+
+#### S10.4: Rating edit → no double counting
+- **Given:** Auto Match A↔B has `statsApplied=true`, user edits rating
+- **Then:** Stats don't change (already applied)
+
+**Implementation needed:**
+```javascript
+// Match model
+statsApplied: Boolean @default(false)
+
+// After both ratings (pseudocode):
+if (bothRated && !match.statsApplied && match.suggestionId) {
+  // Increment stats
+  // Set statsApplied = true
+}
+```
+
+</details>
+
+<details>
+<summary><b>S11: AUDIT & origin_run_id</b> ✅ Partially Implemented</summary>
+
+#### S11.1: New suggestions get correct origin_run_id ✅
+- **Given:** Run #5 runs matching
+- **Then:** Each new recording_suggestion has origin_run_id = 5
+
+#### S11.2: Existing origin_run_id preserved ✅
+- **Given:** Suggestion S1 from run #3 (accepted/completed)
+- **When:** Run #4
+- **Then:** S1 keeps origin_run_id=3 (not deleted)
+
+#### S11.3: Get suggestions by run 🔴 NOT IMPLEMENTED
+- **Need:** `GET /matching-runs/:id/suggestions`
+- **Then:** Returns only suggestions with origin_run_id=:id
+
+#### S11.4: Filters work correctly 🔴 NOT IMPLEMENTED
+- **Need:** Query params `onlyAssigned`, `includeNotFound`
+- **Then:** Filters recorder presence and NOT_FOUND status
+
+</details>
+
+<details>
+<summary><b>S12: MULTIPLE HEATS & COMPLEX COLLISIONS</b> ✅ Implemented</summary>
+
+#### S12.1: Same dancer in multiple heats
+- **Given:** Dancer A in H10 and H12 (Novice J&J), B no conflict
+- **Then:** B can record A in both heats (if under limit 3)
+
+#### S12.2: Recorder records multiple dancers in different slots
+- **Given:** Recorder B has zero own heats, A has H5 (slot 2), C has H8 (slot 3)
+- **Then:** B can be assigned to both A and C (different slots)
+
+#### S12.3: Same recorder can't record two people in same slot
+- **Given:** B accepted recording A in Novice H10 (slot 2), D has Intermediate H10 (also slot 2)
+- **When:** Next matching run for D
+- **Then:** B excluded (busySlot recording A)
+
+</details>
+
+<details>
+<summary><b>S13: MANUAL vs AUTO - FURTHER INTERACTIONS</b> ⚠️ Edge Case</summary>
+
+#### S13.1: Manual before matching ✅ DONE
+- See S7.1
+
+#### S13.2: Manual created AFTER auto suggestion ⚠️ EDGE CASE
+- **Given:** Run #1 created auto suggestion A↔B (pending), then user A creates manual match with B
+- **When:** Run #2
+- **Then:**
+  - **Current behavior:** Manual blocks NEW auto suggestions, old pending remains
+  - **Recommended:** Cleanup conflicting pending auto suggestions when manual created
+
+**Recommended implementation:**
+```javascript
+// routes/matches.js - POST /
+// After creating manual match:
+await prisma.recordingSuggestion.deleteMany({
+  where: {
+    eventId,
+    OR: [
+      { heatId: { in: user1HeatIds }, recorderId: user2Id },
+      { heatId: { in: user2HeatIds }, recorderId: user1Id },
+    ],
+    status: 'pending', // only pending, don't touch accepted
+  }
+});
+```
+
+</details>
+
+<details>
+<summary><b>S14: PERMISSIONS & VISIBILITY</b> ⚠️ Partially Implemented</summary>
+
+#### S14.1: Only recorder can update status ✅
+- **Given:** Suggestion B records A (pending), A tries PUT /suggestions/:id/status
+- **Then:** 403 Forbidden
+
+#### S14.2: Admin-only endpoints 🔴 NOT SECURE
+- **Given:** Regular user tries POST /admin/events/:slug/run-now or GET /admin/matching-runs
+- **Then:** Should get 403, but **middleware missing!**
+
+**Need to implement:**
+```javascript
+// middleware/auth.js
+const requireAdmin = (req, res, next) => {
+  if (req.user.role !== 'admin') {
+    return res.status(403).json({ error: 'Admin access required' });
+  }
+  next();
+};
+```
+
+#### S14.3: Event participant visibility ⚠️ INCONSISTENT
+- **Need to audit:** Do all suggestion/match endpoints verify user is event participant?
+- **Locations to check:**
+  - GET /api/matches?eventSlug=X
+  - GET /api/events/:slug/matching/suggestions
+  - Frontend: /events/:slug/chat?tab=records
+
+</details>
+
+<details>
+<summary><b>S15: SECURITY & ABUSE PREVENTION</b> 🔴 NOT IMPLEMENTED</summary>
+
+#### S15.1: Rate limiting on manual match requests
+- **Given:** User A sends 100 manual requests in 1 minute
+- **Then:** 429 Too Many Requests after 10 requests/minute
+
+#### S15.2: Max pending requests limit
+- **Given:** User A has 20 pending outgoing requests
+- **Then:** Can't send more (limit 20)
+
+#### S15.3: Zombie matches cleanup
+- **Given:** Pending match older than 30 days
+- **Then:** Auto-cancelled by cron job
+
+</details>
+
+<details>
+<summary><b>S16: NOTIFICATIONS & REAL-TIME</b> 🔴 NOT IMPLEMENTED</summary>
+
+#### S16.1: Socket event for new suggestion
+- **Given:** Matching created suggestion B records A, B is online
+- **Then:** B receives socket event → toast notification
+
+#### S16.2: Push notification for offline users
+- **Given:** B offline when suggestion created
+- **Then:** Email/push: "You have a new recording assignment"
+
+#### S16.3: Reminder before event
+- **Given:** B accepted recording A, event in 2 days
+- **Then:** Email: "Don't forget to record @userA on Saturday"
+
+</details>
+
+### Edge Cases for Discussion
+
+#### E1: User has 10 heats, no one can record
+- **Question:** Show all as NOT_FOUND? Prioritize which heats are most important?
+
+#### E2: Recorder wants to un-accept
+- **Current:** No "un-accept" option
+- **Question:** Add cancel feature for accepted suggestions?
+
+#### E3: Manual match rejected, then matching runs
+- **Current:** Rejected manual still blocks auto suggestions
+- **Question:** Should rejected manual stop blocking?
+
+#### E4: User joins AFTER all 5 scheduler runs
+- **Current:** Won't get auto suggestions (scheduler stopped)
+- **Question:** Allow admin to manually trigger additional run?
+
+#### E5: Timezone for deadline
+- **Current:** Server timezone
+- **Question:** Use event/organizer timezone?
+
+#### E6: Load balancing - one recorder has best scores
+- **Current:** Can get max 3 assignments
+- **Question:** Distribute more evenly even if scores worse?
+
+#### E7: Dancer rejects auto suggestion
+- **Current:** Dancer CAN'T reject (only recorder in MVP)
+- **Question:** Allow dancers to reject in future?
+
+#### E8: Event without schedule config
+- **Current:** Fallback to division-based slots
+- **Question:** Show admin warning that matching may be less efficient?
+
+#### E9: Manual match created AFTER auto suggestion exists
+- See S13.2 above
+
+#### E10: User joins BETWEEN scheduler runs
+- **Current:** ✅ Works - each run regenerates pending suggestions
+- **Status:** No issue
+
+#### E11: Recorder accepted but didn't actually record
+- **Question:** Timeout (7 days after event → expired)? "Report issue" button? Admin override?
+
+### Priority Ranking
+
+#### P0 - CRITICAL (Before Production)
+1. ✅ Manual blocks auto suggestions (DONE 2025-11-30)
+2. 🔴 **Implement ratings and stats system** (S10 - fairness broken without this!)
+3. 🔴 **Admin middleware** (S14.2 - security)
+4. 🔴 **Event participant validation** (S14.3 - security)
+
+#### P1 - HIGH (First Month of Production)
+5. ⚠️ Manual match cleanup on conflict (S13.2 / E9)
+6. ⚠️ Rate limiting and spam protection (S15.1, S15.2)
+7. ⚠️ Socket notifications for new suggestions (S16.1)
+
+#### P2 - MEDIUM (Q1 2025)
+8. 📋 Endpoint /matching-runs/:id/suggestions with filters (S11.3, S11.4)
+9. 📋 Zombie matches cleanup (S15.3)
+10. 📋 Email reminders (S16.3)
+
+#### P3 - LOW (Nice to Have)
+11. 📋 Report issue for dancers (E11)
+12. 📋 Admin manual override for suggestions
+13. 📋 Analytics dashboard (success rate, acceptance rate)
+
+### Questions for Team
+
+1. **Is ratings system in roadmap?** If yes - THIS IS THE MOST IMPORTANT (fairness is broken without it)
+2. **Do we have existing admin middleware?** If not - must add ASAP for security
+3. **Which edge case (E1-E11) is most likely in production?**
+4. **Should we implement S13.2 cleanup before launch?** (manual match after auto suggestion)
+
+---
+
 ## Security Audit Findings

 ### Critical Issues (Must Fix Before Production)