Fix referral rate limits for already-invited recipients#440
Conversation
Greptile SummaryThis PR fixes a bug where already-invited recipients were counted against hourly and daily invite quotas, preventing legitimate new invitations. It also adds a 500-error guard when the deduplication DB query fails, preventing duplicate emails from bypassing deduplication on transient errors.
Confidence Score: 5/5Safe to merge — the reordering and error-guard are logically correct and the new tests exercise the changed paths. The deduplication query is now correctly positioned before quota checks, error handling was added to prevent silent bypass on DB failure, and both changed guards use the filtered email count. No regressions were introduced on the existing paths. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[POST /api/referrals] --> B[Auth check]
B --> C[Validate & normalize emails]
C --> D[createServiceClient]
D --> E["Query referrals WHERE referred_email IN validEmails\n(dedup check)"]
E -->|error| F[500 Failed to check existing invitations]
E -->|ok| G["Build alreadyInvited Set\nnewValidEmails = validEmails − alreadyInvited"]
G -->|newValidEmails empty| H[400 All already invited]
G -->|newValidEmails not empty| I["Hourly count query\ncount + newValidEmails.length > 10?"]
I -->|yes| J[429 Too many invites]
I -->|no| K["Daily count query\ncount + newValidEmails.length > 50?"]
K -->|yes| L[429 Daily limit reached]
K -->|no| M[Fetch profile]
M --> N[Send emails to newValidEmails]
N --> O[Insert successful rows into referrals]
O --> P[200 OK]
Reviews (2): Last reviewed commit: "fix(referrals): fail closed on dedupe lo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it("only counts new recipients toward the hourly invite limit", async () => { | ||
| const existingInviteLookup = vi.fn().mockResolvedValue({ | ||
| data: [{ referred_email: "existing@test.com" }], | ||
| error: null, | ||
| }); | ||
| mocks.mockCreateServiceClient.mockReturnValue({ | ||
| from: vi.fn(() => ({ | ||
| select: vi.fn(() => ({ | ||
| eq: vi.fn(() => ({ | ||
| gte: vi.fn().mockResolvedValue({ count: 9, error: null }), | ||
| in: existingInviteLookup, | ||
| })), | ||
| })), | ||
| })), | ||
| }); | ||
|
|
||
| const insertReferrals = vi.fn().mockReturnValue({ | ||
| select: vi.fn().mockResolvedValue({ | ||
| data: [{ id: "ref1", referred_email: "new@test.com", status: "pending" }], | ||
| error: null, | ||
| }), | ||
| }); | ||
| const authSupabase = { | ||
| from: vi.fn((table: string) => { | ||
| if (table === "profiles") { | ||
| return { | ||
| select: vi.fn(() => ({ | ||
| eq: vi.fn(() => ({ | ||
| single: vi.fn().mockResolvedValue({ | ||
| data: { | ||
| referral_code: "testuser", | ||
| username: "testuser", | ||
| full_name: "Test User", | ||
| }, | ||
| error: null, | ||
| }), | ||
| })), | ||
| })), | ||
| }; | ||
| } | ||
|
|
||
| if (table === "referrals") { | ||
| return { insert: insertReferrals }; | ||
| } | ||
|
|
||
| return {}; | ||
| }), | ||
| }; | ||
| mocks.mockGetAuthContext.mockResolvedValue({ | ||
| user: { id: "user1" }, | ||
| supabase: authSupabase, | ||
| }); | ||
|
|
||
| const res = await POST( | ||
| makePostRequest({ emails: ["existing@test.com", "new@test.com"] }) | ||
| ); | ||
|
|
||
| expect(res.status).toBe(200); | ||
| expect(existingInviteLookup).toHaveBeenCalledWith( | ||
| "referred_email", | ||
| ["existing@test.com", "new@test.com"] | ||
| ); | ||
| expect(mocks.mockSendEmail).toHaveBeenCalledTimes(1); | ||
| expect(mocks.mockSendEmail).toHaveBeenCalledWith({ | ||
| to: "new@test.com", | ||
| subject: "Join ugig.net", | ||
| html: "<p>Join</p>", | ||
| text: "Join", | ||
| }); | ||
| expect(insertReferrals).toHaveBeenCalledWith([ | ||
| { | ||
| referrer_id: "user1", | ||
| referred_email: "new@test.com", | ||
| referral_code: "testuser", | ||
| status: "pending", | ||
| }, | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
The new test covers the hourly boundary (9 existing + 1 new = 10, passes the > 10 guard), but there is no equivalent test for the daily limit. The fix applies the same newValidEmails.length substitution to both guards; a user at 49/50 daily invites submitting 1 existing + 1 new should also be admitted. Without a paired test, a future refactor that accidentally reverts only the daily guard would go undetected.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Addressed in Validation: focused 4/4 tests; full pre-commit passed with 1,634 tests, TypeScript, lint (0 errors), and a 185-route production build. |
2 participants
Summary
Fixes #439.
Validation
npm run test:run -- src/app/api/referrals/route.dedupe.test.ts(3/3)npm run test:run(complete suite)npm run type-checknpm run lint -- src/app/api/referrals/route.ts src/app/api/referrals/route.dedupe.test.ts(0 errors; existing repository warnings only)npm run build(185 pages/routes generated)git diff --check