Feature/rea2 60 build api key auth and idempotency foundation#52
Feature/rea2 60 build api key auth and idempotency foundation#52bomberkill wants to merge 4 commits intodevelopfrom
Conversation
- add normalized payment session, transaction and webhook models - implement Flutterwave primary and Stripe fallback providers - add payment session creation and lookup endpoints - add secure provider webhook endpoints with status normalization - activate workspace plan or credits on confirmed payments - document payment endpoints in OpenAPI - add backend integration coverage for sessions and webhooks
- move billing source of truth to organization - add organization plan, credits, verification status and senderId - add contact channel validation flags - support experimental plan quotas for sms and email - enforce billing and sender rules for campaign and direct sends - expose workspace billing summary in API and settings UI
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a Public API foundation featuring API key authentication and idempotency management, while refactoring the billing system to support currency-based wallet balances and tiered message pricing. It also implements contact unsubscription checks during campaign launches and updates the database schema to include API keys, idempotency records, and quota tracking timestamps. Feedback highlights critical race conditions in the entitlement services where usage counters are updated non-atomically, which could lead to undercounting and billing discrepancies.
| ...(channel === "sms" | ||
| ? { | ||
| smsQuotaPeriodStartedAt: smsPeriodStart, | ||
| smsQuotaUsed: currentUsage + units, | ||
| } | ||
| : { | ||
| emailQuotaUsed: { | ||
| increment: units, | ||
| }, | ||
| emailQuotaPeriodStartedAt: emailPeriodStart, | ||
| emailQuotaUsed: currentUsage + units, | ||
| }), |
There was a problem hiding this comment.
The update of smsQuotaUsed and emailQuotaUsed using currentUsage + units is not atomic and is prone to race conditions. When multiple requests occur simultaneously, they may all read the same currentUsage value, leading to undercounted usage. Furthermore, since the billing calculation (chargeAmountMinor) also depends on this potentially stale currentUsage, users could be undercharged. Using Prisma's increment operation for the usage counters is safer for the increment case. However, to fully prevent billing discrepancies, a row-level lock (e.g., SELECT ... FOR UPDATE) on the organization record is recommended during the entitlement check and reservation process.
...(channel === "sms"
? {
smsQuotaPeriodStartedAt: smsPeriodStart,
smsQuotaUsed: isSameInstant(organization.smsQuotaPeriodStartedAt, smsPeriodStart)
? { increment: units }
: units,
}
: {
emailQuotaPeriodStartedAt: emailPeriodStart,
emailQuotaUsed: isSameInstant(organization.emailQuotaPeriodStartedAt, emailPeriodStart)
? { increment: units }
: units,
}),
There was a problem hiding this comment.
@bomberkill I found it usefull. can you implement this fix ?
| ...(campaign.channel === "sms" | ||
| ? { | ||
| smsQuotaPeriodStartedAt: smsPeriodStart, | ||
| smsQuotaUsed: currentUsage + eligibleTargetCount, | ||
| } | ||
| : { | ||
| emailQuotaUsed: { | ||
| increment: eligibleTargetCount, | ||
| }, | ||
| emailQuotaPeriodStartedAt: emailPeriodStart, | ||
| emailQuotaUsed: currentUsage + eligibleTargetCount, | ||
| }), |
There was a problem hiding this comment.
Similar to the issue in MessagingEntitlementsService, updating usage counters with currentUsage + eligibleTargetCount is not atomic. In a concurrent environment, this can lead to incorrect quota tracking and billing. Even within a transaction, standard isolation levels may not prevent race conditions between the read and the write. Using atomic increments and row-level locking is recommended for sensitive billing logic. This approach also aligns with repository guidelines to use bulk database actions for performance and consistency during bulk operations like campaign launches.
...(campaign.channel === "sms"
? {
smsQuotaPeriodStartedAt: smsPeriodStart,
smsQuotaUsed: isSameInstant(organization.smsQuotaPeriodStartedAt, smsPeriodStart)
? { increment: eligibleTargetCount }
: eligibleTargetCount,
}
: {
emailQuotaPeriodStartedAt: emailPeriodStart,
emailQuotaUsed: isSameInstant(organization.emailQuotaPeriodStartedAt, emailPeriodStart)
? { increment: eligibleTargetCount }
: eligibleTargetCount,
}),References
- For bulk operations, fetch all necessary data in a single query and use bulk database actions instead of iterating and querying for each item individually to avoid performance issues and database overload.
There was a problem hiding this comment.
@copilot can you sugeeest me a commit to fix it ?
There was a problem hiding this comment.
To address the atomicity issue in request-campaign-launch.usecase.ts, you should use Prisma's atomic update syntax. This ensures the database handles the increment as a single, atomic operation, preventing race conditions even under high concurrency. Here is the suggested change for the tx.organization.update call:
| ...(campaign.channel === "sms" | |
| ? { | |
| smsQuotaPeriodStartedAt: smsPeriodStart, | |
| smsQuotaUsed: currentUsage + eligibleTargetCount, | |
| } | |
| : { | |
| emailQuotaUsed: { | |
| increment: eligibleTargetCount, | |
| }, | |
| emailQuotaPeriodStartedAt: emailPeriodStart, | |
| emailQuotaUsed: currentUsage + eligibleTargetCount, | |
| }), | |
| ...(campaign.channel === "sms" | |
| ? { | |
| smsQuotaPeriodStartedAt: smsPeriodStart, | |
| smsQuotaUsed: { increment: eligibleTargetCount }, | |
| } | |
| : { | |
| emailQuotaPeriodStartedAt: emailPeriodStart, | |
| emailQuotaUsed: { increment: eligibleTargetCount }, | |
| }), |
There was a problem hiding this comment.
@bomberkill For bulk operations, fetch all necessary data in a single query and use bulk database actions instead of iterating and querying for each item individually to avoid performance issues and database overload.
|
okay i will fix all these issues and implement your suggestions in the next
issue, because i have already started to work on it.
…On Tue, 7 Apr 2026, 20:19 Belrick Stephane, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In packages/core/src/services/request-campaign-launch.usecase.ts
<#52 (comment)>:
> + ...(campaign.channel === "sms"
+ ? {
+ smsQuotaPeriodStartedAt: smsPeriodStart,
+ smsQuotaUsed: currentUsage + eligibleTargetCount,
+ }
: {
- emailQuotaUsed: {
- increment: eligibleTargetCount,
- },
+ emailQuotaPeriodStartedAt: emailPeriodStart,
+ emailQuotaUsed: currentUsage + eligibleTargetCount,
}),
@bomberkill <https://github.com/bomberkill> For bulk operations, fetch
all necessary data in a single query and use bulk database actions instead
of iterating and querying for each item individually to avoid performance
issues and database overload.
—
Reply to this email directly, view it on GitHub
<#52 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AUDHFYKZO5CLX2IDDQBLRVL4UVID7AVCNFSM6AAAAACXPYZXDCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DANZQGY4DQNRTG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
2 participants
No description provided.