feat(contributors): add manual tier upgrade for enrolled champions#3121
feat(contributors): add manual tier upgrade for enrolled champions#3121bturcotte520 merged 3 commits intomainfrom
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // Prefer the explicit membership link over the email-derived match, same as enrollContributorChampion. | ||
| const linkedKiloUserId = row.linkedKiloUserId ?? row.linkedUserId; | ||
|
|
||
| const creditGranted = await db.transaction(async tx => { |
There was a problem hiding this comment.
WARNING: Concurrent upgrades can double-grant top-up credits
The current tier and credit differential are computed before the transaction acquires the FOR UPDATE lock. If two admins submit the same upgrade concurrently, both requests can read the old tier, then serialize at this lock and each grant the same differential. Re-read the membership/current tier after acquiring the lock (or condition the update on the old tier) before computing/granting the top-up.
| counts_as_selfservice: false, | ||
| dbOrTx: tx, | ||
| }); | ||
| granted = result.success; |
There was a problem hiding this comment.
WARNING: Successful upgrade top-ups do not advance the grant timestamp
When a contributor tier with no prior credits, such as contributor, is manually upgraded and this call grants the immediate top-up, credits_last_granted_at stays NULL. For memberships with linked_kilo_user_id set, the next credit refresh sees the row as due immediately and can grant the new monthly amount again. Mirror enrollment by setting credits_last_granted_at when the top-up succeeds.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Reviewed by gpt-5.5-2026-04-23 · 815,973 tokens |
…set on tier upgrade
| if (!membership?.enrolled_tier) throw new Error('Contributor membership not found'); | ||
|
|
||
| const lockedCurrentCreditUsd = | ||
| TIER_CREDIT_USD[membership.enrolled_tier as ContributorTier] ?? 0; |
There was a problem hiding this comment.
Suggest to use parseContributorTier for safety in case enrolled_tier has invalid data.
e.g.
const lockedCurrentTier = parseContributorTier(membership.enrolled_tier);
if (!lockedCurrentTier) throw new Error(`Invalid enrolled_tier "${membership.enrolled_tier}" in DB`);
const lockedCurrentCreditUsd = TIER_CREDIT_USD[lockedCurrentTier];
| counts_as_selfservice: false, | ||
| dbOrTx: tx, | ||
| }); | ||
| granted = result.success; |
There was a problem hiding this comment.
The bot's concern has been addressed about the positive scenario, but if the grant fails somethings would still get committed in the transaction. I'd suggest to reject transaction in case of grant failure so that it can be retried cleanly.
suggestion (replacing current granted = result.success :
if (!result.success) {
throw new Error('Failed to grant top-up credit; rolling back tier upgrade');
}
granted = true;
| .where(eq(contributor_champion_memberships.contributor_id, input.contributorId)) | ||
| .limit(1); | ||
|
|
||
| if (!membership?.enrolled_tier) throw new Error('Contributor membership not found'); |
There was a problem hiding this comment.
Would it make sense to separate the error logs for missing membership or not enrolled?
like:
if (!membership) throw new Error('Contributor membership not found');
if (!membership.enrolled_tier) throw new Error('Contributor is not currently enrolled');
2 participants
Allow existing champions to be upgraded