Skip to content

Sync affiliate counts on application status changes#202

Closed
morganschp wants to merge 1 commit into
profullstack:masterfrom
morganschp:fix-affiliate-application-count
Closed

Sync affiliate counts on application status changes#202
morganschp wants to merge 1 commit into
profullstack:masterfrom
morganschp:fix-affiliate-application-count

Conversation

@morganschp
Copy link
Copy Markdown
Contributor

Fixes #201.

Summary

  • track the previous affiliate application status before approve/reject updates
  • increment total_affiliates when an application becomes approved
  • decrement total_affiliates when an approved application is rejected, clamped at zero
  • add focused PATCH route tests for increment, decrement, clamping, and no-op same-status transitions

Validation

  • ./node_modules/.bin/vitest run src/app/api/affiliates/offers/[id]/applications/route.test.ts
  • ./node_modules/.bin/eslint src/app/api/affiliates/offers/[id]/applications/route.ts src/app/api/affiliates/offers/[id]/applications/route.test.ts
  • ./node_modules/.bin/prettier --check src/app/api/affiliates/offers/[id]/applications/route.ts src/app/api/affiliates/offers/[id]/applications/route.test.ts
  • ./node_modules/.bin/tsc --noEmit --pretty false
  • git diff --check -- src/app/api/affiliates/offers/[id]/applications/route.ts src/app/api/affiliates/offers/[id]/applications/route.test.ts

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 23, 2026

Greptile Summary

This PR adds total_affiliates counter synchronisation to the affiliate application approve/reject flow. It introduces a helper updateAffiliateCountForStatusChange that computes a delta based on the previous and next status, then applies it to the offer row, and adds a focused test suite covering increment, decrement, clamp-at-zero, and no-op transitions.

  • The PATCH handler now fetches the application's current status before writing the update, so the delta can be computed correctly; both a decrement and an early-return no-op path are covered.
  • total_affiliates was added to the GET handler's offer select but is still absent from the PATCH handler's offer select (line 105), meaning offer.total_affiliates is always undefined in production and the counter resets to 0 or 1 on every action — the previous review comment on this remains unaddressed.
  • The count update runs after the application row has already been mutated and only logs a warning on failure, leaving the counter silently out of sync if the update fails.

Confidence Score: 3/5

Not safe to merge: the offer select in the PATCH handler still omits total_affiliates, so the counter will be set to 0 or 1 on every approve/reject regardless of its real value.

The PATCH handler reads the offer row with .select("id, seller_id") (line 105), leaving total_affiliates out. Every call to updateAffiliateCountForStatusChange therefore receives undefined as currentCount, which the || 0 fallback silently converts to 0 — so every approval writes 1 and every rejection writes 0, permanently destroying the real counter. This bug was noted in the previous review pass and is still present in the current diff.

route.ts line 105 — the offer select must include total_affiliates before this change can function correctly

Important Files Changed

Filename Overview
src/app/api/affiliates/offers/[id]/applications/route.ts Adds updateAffiliateCountForStatusChange helper and pre-fetch of existingApplication, but the PATCH offer select still omits total_affiliates so the helper always receives undefined and miscounts
src/app/api/affiliates/offers/[id]/applications/route.test.ts New test file covering increment/decrement/clamp/no-op paths; mock always returns total_affiliates regardless of the select columns actually used, masking the production mis-select

Reviews (3): Last reviewed commit: "Sync affiliate count on application stat..." | Re-trigger Greptile

Comment on lines +19 to +27
const totalAffiliates = Math.max(0, (currentCount || 0) + delta);

const { error } = await admin
.from("affiliate_offers")
.update({
total_affiliates: totalAffiliates,
updated_at: new Date().toISOString(),
})
.eq("id", offerId);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Read-modify-write race on total_affiliates

Two concurrent PATCH requests that both approve different applications for the same offer will both read the same total_affiliates snapshot, compute count + 1 independently, and both write the same resulting value — so one increment is silently lost. A database-level atomic update (e.g. a Postgres total_affiliates + 1 expression or an RPC with FOR UPDATE row lock) would eliminate this window.

Comment on lines +73 to +88
total_affiliates: options.totalAffiliates,
},
error: null,
});
}
return query(table, { data: null, error: null });
}

if (table === "affiliate_applications") {
if (applicationCalls++ === 0) {
return query(table, {
data: {
id: "app-1",
status: options.previousStatus,
},
error: null,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Mock returns total_affiliates regardless of the select fields, masking the production bug

mockPatchQueries always includes total_affiliates in the offer data no matter what columns are actually selected by the route. As a result all four tests pass even when the route's offer query omits total_affiliates, giving a false green signal. Tightening the mock (e.g. by only returning fields that match the select string, or by adding an assertion that the select call includes total_affiliates) would let the test suite catch this class of regression.

@ralyodio ralyodio closed this May 23, 2026
@ralyodio ralyodio reopened this May 23, 2026
@ralyodio ralyodio closed this May 23, 2026
@ralyodio ralyodio reopened this May 23, 2026
@ralyodio ralyodio closed this May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: affiliate approval changes leave offer counts stale

2 participants