Fix/e2e issues#200
Merged
Merged
Conversation
…on timer for dispatched orders
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThe pull request implements automated delivery confirmation via a new hourly cron job that identifies overdue deliveries (>72 hours), auto-confirms eligible orders, sends notification warnings at 24h/48h thresholds, and triggers merchant tier checks and payout jobs. Additionally, the frontend catalogue page is refactored to support infinite scrolling using a paginated API endpoint, and merchant profile editing is integrated with immediate state updates. Changes
Sequence DiagramsequenceDiagram
participant Cron as Cron Scheduler
participant AdminCron as AdminCronService
participant DB as Prisma/Database
participant OrderSvc as OrderService
participant NotifSvc as NotificationTriggerService
participant VerifSvc as VerificationService
participant Queue as Payout Queue
Cron->>AdminCron: handleUnconfirmedDeliveries() hourly
AdminCron->>DB: Query orders older than 72h (status: DISPATCHED/IN_TRANSIT)
DB-->>AdminCron: Return overdue deliveries
loop For each eligible order
AdminCron->>OrderSvc: autoConfirmDelivery(orderId)
OrderSvc->>DB: Fetch order with buyer/merchant
DB-->>OrderSvc: Return order details
OrderSvc->>DB: Transactional update: status→DELIVERED, close disputes
OrderSvc->>DB: Create orderTracking entry
OrderSvc->>NotifSvc: triggerOrderAutoConfirmed(buyerId, merchantId, metadata)
NotifSvc-->>OrderSvc: Notification enqueued
OrderSvc->>OrderSvc: Transition DELIVERED→COMPLETED
OrderSvc->>VerifSvc: checkAndUpgradeTier(merchantId)
VerifSvc-->>OrderSvc: Tier check result (errors logged)
alt Payment method is ESCROW
OrderSvc->>Queue: Enqueue payout job
Queue-->>OrderSvc: Job queued
end
OrderSvc-->>AdminCron: { success: true }
end
loop For warning candidates (24h/48h remaining)
AdminCron->>DB: Query upcoming auto-confirmations
AdminCron->>NotifSvc: triggerAutoConfirmationWarning(buyerId, orderRef, hoursRemaining)
NotifSvc-->>AdminCron: Warning notification enqueued
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable poems in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
apps/web/src/lib/api-client.ts (1)
133-133: Tighten paginated response typing to avoidanyleaks.Line 133 and Line 163 use
anyin the return shape, which weakens safety formeta.page/limit/totalconsumers.♻️ Proposed typing update
-import type { ApiResponse, ApiError } from "@swifta/shared"; +import type { ApiResponse, ApiError, PaginatedResponse } from "@swifta/shared"; -async requestPaginated<T>(endpoint: string, config: RequestConfig = {}): Promise<any> { +async requestPaginated<T>(endpoint: string, config: RequestConfig = {}): Promise<PaginatedResponse<T>> { @@ - if (!text) return {}; - return JSON.parse(text); + if (!text) return { success: true, data: [], meta: { page: 1, limit: 0, total: 0 } } as PaginatedResponse<T>; + return JSON.parse(text) as PaginatedResponse<T>; } -getPaginated<T>(endpoint: string, config?: RequestConfig): Promise<ApiResponse<T[]> & { meta: any }> { +getPaginated<T>(endpoint: string, config?: RequestConfig): Promise<PaginatedResponse<T>> { return this.requestPaginated<T>(endpoint, { ...config, method: "GET" }); }Also applies to: 163-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/api-client.ts` at line 133, The requestPaginated function currently returns Promise<any>, leaking unsafe types for paginated consumers; tighten its typing by introducing a generic paginated response interface (e.g. PaginatedResponse<T> with data: T[] and meta: { page: number; limit: number; total: number }) and change requestPaginated<T>(...) to return Promise<PaginatedResponse<T>> (and adjust any internal uses that currently assume any), update any other occurrences around lines referenced (e.g. the second return at lines ~163-165) to use the new PaginatedResponse<T> type so consumers can safely access meta.page/limit/total.apps/web/src/app/(dashboard)/buyer/catalogue/page.tsx (2)
31-32: Debounce/defer search before feeding the infinite query key.Line 52-53 uses raw
searchQuery, so every keystroke starts a new query chain and increases request churn.💡 Lightweight approach with
useDeferredValue-import React, { useState, useEffect, useCallback, useRef, useMemo } from "react"; +import React, { useState, useEffect, useCallback, useRef, useMemo, useDeferredValue } from "react"; @@ const [searchQuery, setSearchQuery] = useState(""); +const deferredSearchQuery = useDeferredValue(searchQuery); @@ - queryKey: ["catalogue", searchQuery, activeCategory], - queryFn: ({ pageParam = 1 }) => productApi.getCatalogue(searchQuery, activeCategory, pageParam as number, 20), + queryKey: ["catalogue", deferredSearchQuery, activeCategory], + queryFn: ({ pageParam = 1 }) => productApi.getCatalogue(deferredSearchQuery, activeCategory, pageParam as number, 20),Also applies to: 52-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dashboard)/buyer/catalogue/page.tsx around lines 31 - 32, Replace direct usage of searchQuery in the infinite query key with a deferred or debounced value: create a deferredSearchQuery (e.g., via React's useDeferredValue or a small debounce hook) derived from searchQuery and use deferredSearchQuery instead of searchQuery wherever the infinite query key and fetch function reference it (symbols: searchQuery, setSearchQuery, the infinite query hook invocation). Ensure the query key and any effects that depend on the search string use deferredSearchQuery so keystrokes don't trigger immediate new query chains, and keep activeCategory usage unchanged.
67-67: Type the observer ref explicitly.Line 67 should use an element ref type to preserve DOM safety and avoid
null-only inference in observer usage.✅ Minimal fix
-const observerTarget = useRef(null); +const observerTarget = useRef<HTMLDivElement | null>(null);Also applies to: 78-80, 230-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dashboard)/buyer/catalogue/page.tsx at line 67, The observer ref currently declared as const observerTarget = useRef(null) lacks an explicit element type; change it to a typed ref like useRef<HTMLDivElement | null>(null) (or useRef<HTMLElement | null>(null) if not a div) so DOM operations are type-safe, and apply the same explicit typing to the other observer refs referenced in this file (the refs around lines 78-80 and 230) to avoid null-only inference when using the ref in IntersectionObserver callbacks and JSX refs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/modules/admin/admin-cron.service.ts`:
- Around line 18-67: The scheduled job handleUnconfirmedDeliveries runs on every
instance and must be guarded by a distributed lock or leader election to avoid
duplicate processing; add a short-lived leader/lock acquisition step at the
start of handleUnconfirmedDeliveries (e.g., obtain a Redis/DB advisory lock or
use your cluster leader-election mechanism) and return immediately if the lock
cannot be acquired, then release the lock at the end or on error; key places to
update are the `@Cron-decorated` handleUnconfirmedDeliveries method and the
surrounding flow that calls prisma.order.findMany and
orderService.autoConfirmDelivery so only the lock-holding instance performs the
queries/notifications.
- Around line 56-65: The warning loop over warningCandidates in
admin-cron.service.ts should isolate failures per order: wrap the per-order
logic inside a try/catch so a single
notifications.triggerAutoConfirmationWarning failure (calls in the for loop that
slice order.id and call triggerAutoConfirmationWarning) does not abort the whole
pass; on error, log the order id and error (using the service logger or
this.logger) and continue to the next order. Ensure both branches (48h and 24h)
remain unchanged except for being inside the try block and that the catch does
not rethrow.
In `@apps/backend/src/modules/notification/notification-trigger.service.ts`:
- Line 632: Add the two new notification kinds to the shared NotificationType
enum (add ORDER_AUTO_CONFIRM_WARNING = "ORDER_AUTO_CONFIRM_WARNING" and
ORDER_AUTO_CONFIRMED = "ORDER_AUTO_CONFIRMED") and update uses in
notification-trigger.service.ts so the emitted types use
NotificationType.ORDER_AUTO_CONFIRM_WARNING and
NotificationType.ORDER_AUTO_CONFIRMED instead of raw strings where those
literals are emitted; ensure imports reference the NotificationType enum and
replace any remaining raw-string occurrences.
In `@apps/backend/src/modules/order/order.service.ts`:
- Around line 997-1005: The call to this.notifications.triggerOrderAutoConfirmed
can throw and currently prevents subsequent payout/tier logic from running; wrap
the call in a try/catch around the invocation of
this.notifications.triggerOrderAutoConfirmed(order.buyerId, order.merchantId,
{...}) so any exception is caught, logged (or reported) and ignored, allowing
the method to continue executing the remaining steps (e.g., order completion,
payout and tier checks) after the notification attempt; ensure the catch does
not rethrow and includes contextual info (order.id) when logging.
- Around line 978-994: The current transaction unconditionally updates order by
id, allowing concurrent workers to re-process and skipping an orderEvent; change
the update to be conditional on current status (only DISPATCHED or IN_TRANSIT)
using a conditional update (e.g., tx.order.updateMany or tx.order.update with a
where that includes status in [DISPATCHED, IN_TRANSIT]) and check affected count
to abort if zero so only one worker proceeds, then within the same tx create the
orderTracking record and an orderEvent record for the
DISPATCHED/IN_TRANSIT→DELIVERED transition and set disputeWindowEndsAt as before
to ensure the transition is atomic and fully audited.
In `@apps/web/src/lib/api-client.ts`:
- Around line 133-157: The paginated request handler requestPaginated currently
parses and returns JSON regardless of HTTP status, which hides 4xx/5xx errors
and skips auth-refresh logic; update requestPaginated to mirror the standard
request() flow: after await performRequest(), if response.status === 401 call
the same auth-refresh logic used by request() (retry performRequest once after
refreshing), then read response.text(), parse JSON, and if response.ok return
the data but if not throw an error containing the parsed body and status so
callers receive proper rejection; reference the functions/blocks
requestPaginated, performRequest and the existing request() auth-refresh/error
handling when implementing.
---
Nitpick comments:
In `@apps/web/src/app/`(dashboard)/buyer/catalogue/page.tsx:
- Around line 31-32: Replace direct usage of searchQuery in the infinite query
key with a deferred or debounced value: create a deferredSearchQuery (e.g., via
React's useDeferredValue or a small debounce hook) derived from searchQuery and
use deferredSearchQuery instead of searchQuery wherever the infinite query key
and fetch function reference it (symbols: searchQuery, setSearchQuery, the
infinite query hook invocation). Ensure the query key and any effects that
depend on the search string use deferredSearchQuery so keystrokes don't trigger
immediate new query chains, and keep activeCategory usage unchanged.
- Line 67: The observer ref currently declared as const observerTarget =
useRef(null) lacks an explicit element type; change it to a typed ref like
useRef<HTMLDivElement | null>(null) (or useRef<HTMLElement | null>(null) if not
a div) so DOM operations are type-safe, and apply the same explicit typing to
the other observer refs referenced in this file (the refs around lines 78-80 and
230) to avoid null-only inference when using the ref in IntersectionObserver
callbacks and JSX refs.
In `@apps/web/src/lib/api-client.ts`:
- Line 133: The requestPaginated function currently returns Promise<any>,
leaking unsafe types for paginated consumers; tighten its typing by introducing
a generic paginated response interface (e.g. PaginatedResponse<T> with data: T[]
and meta: { page: number; limit: number; total: number }) and change
requestPaginated<T>(...) to return Promise<PaginatedResponse<T>> (and adjust any
internal uses that currently assume any), update any other occurrences around
lines referenced (e.g. the second return at lines ~163-165) to use the new
PaginatedResponse<T> type so consumers can safely access meta.page/limit/total.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c053a1fe-29f1-493a-9d7c-766850784cc1
📒 Files selected for processing (17)
apps/backend/src/modules/admin/admin-cron.service.tsapps/backend/src/modules/admin/admin.module.tsapps/backend/src/modules/notification/notification-trigger.service.tsapps/backend/src/modules/order/order.service.tsapps/backend/test-paystack.jsapps/backend/test-resend.jsapps/backend/test-script.jsapps/web/src/app/(dashboard)/buyer/catalogue/page.tsxapps/web/src/components/buyer/cart/cart-view.tsxapps/web/src/components/merchant/profile/merchant-profile-view.tsxapps/web/src/lib/api-client.tsapps/web/src/lib/api/product.api.tsbuild_output.txtbuild_output_2.txtfinal_build_output.txtfinal_lint_output.txtlint_output.txt
💤 Files with no reviewable changes (3)
- apps/backend/test-resend.js
- apps/backend/test-paystack.js
- apps/backend/test-script.js
Comment on lines
+18
to
+67
| @Cron(CronExpression.EVERY_HOUR) | ||
| async handleUnconfirmedDeliveries() { | ||
| this.logger.log("Scanning for unconfirmed deliveries requiring action..."); | ||
|
|
||
| const seventyTwoHoursAgo = new Date(Date.now() - 72 * 60 * 60 * 1000); | ||
| const fortyEightHoursAgo = new Date(Date.now() - 48 * 60 * 60 * 1000); | ||
| const twentyFourHoursAgo = new Date(Date.now() - 24 * 60 * 60 * 1000); | ||
|
|
||
| // 1. Auto-confirm orders older than 72h | ||
| const overdueOrders = await this.prisma.order.findMany({ | ||
| where: { | ||
| status: { in: [OrderStatus.DISPATCHED, OrderStatus.IN_TRANSIT] }, | ||
| updatedAt: { lt: seventyTwoHoursAgo }, | ||
| }, | ||
| }); | ||
|
|
||
| for (const order of overdueOrders) { | ||
| try { | ||
| await this.orderService.autoConfirmDelivery(order.id); | ||
| this.logger.log(`Auto-confirmed order ${order.id} (over 72h since update)`); | ||
| } catch (err: unknown) { | ||
| this.logger.error(`Failed to auto-confirm order ${order.id}: ${err instanceof Error ? err.message : String(err)}`); | ||
| } | ||
| } | ||
|
|
||
| // 2. 48h & 24h Warnings | ||
| // For simplicity, we scan orders in the warning windows and trigger if not already warned (mocking warning status via logs for now, or could use metadata) | ||
| const warningCandidates = await this.prisma.order.findMany({ | ||
| where: { | ||
| status: { in: [OrderStatus.DISPATCHED, OrderStatus.IN_TRANSIT] }, | ||
| updatedAt: { | ||
| lt: twentyFourHoursAgo, | ||
| gt: seventyTwoHoursAgo, | ||
| }, | ||
| }, | ||
| select: { id: true, buyerId: true, updatedAt: true }, | ||
| }); | ||
|
|
||
| for (const order of warningCandidates) { | ||
| const hoursSinceUpdate = (Date.now() - order.updatedAt.getTime()) / (1000 * 60 * 60); | ||
|
|
||
| if (hoursSinceUpdate >= 48 && hoursSinceUpdate < 49) { | ||
| // 48h Warning | ||
| await this.notifications.triggerAutoConfirmationWarning(order.buyerId, order.id.slice(0, 8).toUpperCase(), 24); | ||
| } else if (hoursSinceUpdate >= 24 && hoursSinceUpdate < 25) { | ||
| // 24h Warning | ||
| await this.notifications.triggerAutoConfirmationWarning(order.buyerId, order.id.slice(0, 8).toUpperCase(), 48); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Coordinate this cron across instances to prevent duplicate processing.
At Line 18, this job will run on every app instance. Without a distributed lock/leader election, duplicate warnings and repeated auto-confirm attempts are likely in multi-replica deployments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/src/modules/admin/admin-cron.service.ts` around lines 18 - 67,
The scheduled job handleUnconfirmedDeliveries runs on every instance and must be
guarded by a distributed lock or leader election to avoid duplicate processing;
add a short-lived leader/lock acquisition step at the start of
handleUnconfirmedDeliveries (e.g., obtain a Redis/DB advisory lock or use your
cluster leader-election mechanism) and return immediately if the lock cannot be
acquired, then release the lock at the end or on error; key places to update are
the `@Cron-decorated` handleUnconfirmedDeliveries method and the surrounding flow
that calls prisma.order.findMany and orderService.autoConfirmDelivery so only
the lock-holding instance performs the queries/notifications.
Comment on lines
+56
to
+65
| for (const order of warningCandidates) { | ||
| const hoursSinceUpdate = (Date.now() - order.updatedAt.getTime()) / (1000 * 60 * 60); | ||
|
|
||
| if (hoursSinceUpdate >= 48 && hoursSinceUpdate < 49) { | ||
| // 48h Warning | ||
| await this.notifications.triggerAutoConfirmationWarning(order.buyerId, order.id.slice(0, 8).toUpperCase(), 24); | ||
| } else if (hoursSinceUpdate >= 24 && hoursSinceUpdate < 25) { | ||
| // 24h Warning | ||
| await this.notifications.triggerAutoConfirmationWarning(order.buyerId, order.id.slice(0, 8).toUpperCase(), 48); | ||
| } |
Contributor
There was a problem hiding this comment.
Isolate warning failures per order to avoid aborting the entire warning pass.
Unlike the overdue auto-confirm loop, the warning loop has no try/catch. A single enqueue failure can stop processing remaining warning candidates.
🧩 Proposed fix
for (const order of warningCandidates) {
const hoursSinceUpdate = (Date.now() - order.updatedAt.getTime()) / (1000 * 60 * 60);
-
- if (hoursSinceUpdate >= 48 && hoursSinceUpdate < 49) {
- // 48h Warning
- await this.notifications.triggerAutoConfirmationWarning(order.buyerId, order.id.slice(0, 8).toUpperCase(), 24);
- } else if (hoursSinceUpdate >= 24 && hoursSinceUpdate < 25) {
- // 24h Warning
- await this.notifications.triggerAutoConfirmationWarning(order.buyerId, order.id.slice(0, 8).toUpperCase(), 48);
- }
+ try {
+ if (hoursSinceUpdate >= 48 && hoursSinceUpdate < 49) {
+ await this.notifications.triggerAutoConfirmationWarning(
+ order.buyerId,
+ order.id.slice(0, 8).toUpperCase(),
+ 24,
+ );
+ } else if (hoursSinceUpdate >= 24 && hoursSinceUpdate < 25) {
+ await this.notifications.triggerAutoConfirmationWarning(
+ order.buyerId,
+ order.id.slice(0, 8).toUpperCase(),
+ 48,
+ );
+ }
+ } catch (err: unknown) {
+ this.logger.error(
+ `Failed to send auto-confirm warning for order ${order.id}: ${err instanceof Error ? err.message : String(err)}`,
+ );
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/src/modules/admin/admin-cron.service.ts` around lines 56 - 65,
The warning loop over warningCandidates in admin-cron.service.ts should isolate
failures per order: wrap the per-order logic inside a try/catch so a single
notifications.triggerAutoConfirmationWarning failure (calls in the for loop that
slice order.id and call triggerAutoConfirmationWarning) does not abort the whole
pass; on error, log the order id and error (using the service logger or
this.logger) and continue to the next order. Ensure both branches (48h and 24h)
remain unchanged except for being inside the try block and that the catch does
not rethrow.
| ) { | ||
| await this.addJob( | ||
| buyerId, | ||
| "ORDER_AUTO_CONFIRM_WARNING", |
Contributor
There was a problem hiding this comment.
Register and consume the new notification types via NotificationType enum.
At Line 632, Line 648, and Line 658, new types are emitted as raw strings, while packages/shared/src/enums/notification-type.enum.ts:1-30 does not include these members. This can cause unsupported/unmatched notification handling downstream.
✅ Proposed fix
- "ORDER_AUTO_CONFIRM_WARNING",
+ NotificationType.ORDER_AUTO_CONFIRM_WARNING,
...
- "ORDER_AUTO_CONFIRMED",
+ NotificationType.ORDER_AUTO_CONFIRMED,
...
- "ORDER_AUTO_CONFIRMED",
+ NotificationType.ORDER_AUTO_CONFIRMED,// packages/shared/src/enums/notification-type.enum.ts
ORDER_AUTO_CONFIRM_WARNING = "ORDER_AUTO_CONFIRM_WARNING",
ORDER_AUTO_CONFIRMED = "ORDER_AUTO_CONFIRMED",Also applies to: 648-648, 658-658
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/src/modules/notification/notification-trigger.service.ts` at
line 632, Add the two new notification kinds to the shared NotificationType enum
(add ORDER_AUTO_CONFIRM_WARNING = "ORDER_AUTO_CONFIRM_WARNING" and
ORDER_AUTO_CONFIRMED = "ORDER_AUTO_CONFIRMED") and update uses in
notification-trigger.service.ts so the emitted types use
NotificationType.ORDER_AUTO_CONFIRM_WARNING and
NotificationType.ORDER_AUTO_CONFIRMED instead of raw strings where those
literals are emitted; ensure imports reference the NotificationType enum and
replace any remaining raw-string occurrences.
Comment on lines
+978
to
+994
| await this.prisma.$transaction(async (tx) => { | ||
| await tx.order.update({ | ||
| where: { id: orderId }, | ||
| data: { | ||
| status: OrderStatus.DELIVERED, | ||
| disputeWindowEndsAt: new Date(), // Auto-confirmation bypasses dispute window | ||
| }, | ||
| }); | ||
|
|
||
| await tx.orderTracking.create({ | ||
| data: { | ||
| orderId, | ||
| status: OrderStatus.DELIVERED, | ||
| note: "Delivery auto-confirmed after 72 hours of no response.", | ||
| }, | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Make DELIVERED transition atomic and audit-complete.
At Line 979, updating by id only allows overlapping workers to re-process the same order and duplicate side effects. Also, this path skips creating an orderEvent for the DISPATCHED/IN_TRANSIT → DELIVERED transition.
🔒 Proposed fix
+ let deliveredTransitionApplied = false;
await this.prisma.$transaction(async (tx) => {
- await tx.order.update({
- where: { id: orderId },
+ const updated = await tx.order.updateMany({
+ where: {
+ id: orderId,
+ status: { in: [OrderStatus.DISPATCHED, OrderStatus.IN_TRANSIT] },
+ },
data: {
status: OrderStatus.DELIVERED,
disputeWindowEndsAt: new Date(), // Auto-confirmation bypasses dispute window
},
});
+ if (updated.count === 0) return;
+ deliveredTransitionApplied = true;
await tx.orderTracking.create({
data: {
orderId,
status: OrderStatus.DELIVERED,
note: "Delivery auto-confirmed after 72 hours of no response.",
},
});
+
+ await tx.orderEvent.create({
+ data: {
+ orderId,
+ fromStatus: order.status as OrderStatus,
+ toStatus: OrderStatus.DELIVERED,
+ triggeredBy: "SYSTEM",
+ metadata: { action: "auto_confirmed_system_delivered" },
+ },
+ });
});
+
+ if (!deliveredTransitionApplied) {
+ this.logger.warn(`Order ${orderId} already processed by another worker; skipping.`);
+ return { success: false, skipped: true };
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/src/modules/order/order.service.ts` around lines 978 - 994, The
current transaction unconditionally updates order by id, allowing concurrent
workers to re-process and skipping an orderEvent; change the update to be
conditional on current status (only DISPATCHED or IN_TRANSIT) using a
conditional update (e.g., tx.order.updateMany or tx.order.update with a where
that includes status in [DISPATCHED, IN_TRANSIT]) and check affected count to
abort if zero so only one worker proceeds, then within the same tx create the
orderTracking record and an orderEvent record for the
DISPATCHED/IN_TRANSIT→DELIVERED transition and set disputeWindowEndsAt as before
to ensure the transition is atomic and fully audited.
Comment on lines
+997
to
+1005
| await this.notifications.triggerOrderAutoConfirmed( | ||
| order.buyerId, | ||
| order.merchantId, | ||
| { | ||
| orderId: order.id, | ||
| reference: order.id.slice(0, 8).toUpperCase(), | ||
| amountKobo: order.totalAmountKobo, | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
Prevent notification enqueue failures from blocking completion/payout.
If triggerOrderAutoConfirmed fails at Line 997, the method exits before Line 1008 onward, leaving the order stuck at DELIVERED and skipping payout/tier checks.
🛡️ Proposed fix
- await this.notifications.triggerOrderAutoConfirmed(
- order.buyerId,
- order.merchantId,
- {
- orderId: order.id,
- reference: order.id.slice(0, 8).toUpperCase(),
- amountKobo: order.totalAmountKobo,
- },
- );
+ try {
+ await this.notifications.triggerOrderAutoConfirmed(
+ order.buyerId,
+ order.merchantId,
+ {
+ orderId: order.id,
+ reference: order.id.slice(0, 8).toUpperCase(),
+ amountKobo: order.totalAmountKobo,
+ },
+ );
+ } catch (error: unknown) {
+ this.logger.error(
+ `Failed to enqueue auto-confirm notifications for order ${orderId}: ${error instanceof Error ? error.message : String(error)}`,
+ );
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/src/modules/order/order.service.ts` around lines 997 - 1005, The
call to this.notifications.triggerOrderAutoConfirmed can throw and currently
prevents subsequent payout/tier logic from running; wrap the call in a try/catch
around the invocation of
this.notifications.triggerOrderAutoConfirmed(order.buyerId, order.merchantId,
{...}) so any exception is caught, logged (or reported) and ignored, allowing
the method to continue executing the remaining steps (e.g., order completion,
payout and tier checks) after the notification attempt; ensure the catch does
not rethrow and includes contextual info (order.id) when logging.
Comment on lines
+133
to
+157
| async requestPaginated<T>(endpoint: string, config: RequestConfig = {}): Promise<any> { | ||
| const { params, ...options } = config; | ||
|
|
||
| let url = `${this.baseUrl}${endpoint.startsWith("/") ? endpoint : `/${endpoint}`}`; | ||
| if (params) { | ||
| const searchParams = new URLSearchParams(params); | ||
| url += `?${searchParams.toString()}`; | ||
| } | ||
|
|
||
| const headers = new Headers(options.headers); | ||
| if (!headers.has("Content-Type") && !(options.body instanceof FormData)) { | ||
| headers.set("Content-Type", "application/json"); | ||
| } | ||
|
|
||
| const performRequest = async (): Promise<Response> => { | ||
| return fetch(url, { ...options, headers, credentials: "include" }); | ||
| }; | ||
|
|
||
| let response = await performRequest(); | ||
|
|
||
| const text = await response.text(); | ||
| if (!text) return {}; | ||
|
|
||
| return JSON.parse(text); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify parity gap between request() and requestPaginated()
rg -n "async request<|async requestPaginated<|response.status === 401|if \\(!response\\.ok\\)" apps/web/src/lib/api-client.tsRepository: onerandomdevv/Swifta
Length of output: 368
🏁 Script executed:
cat -n apps/web/src/lib/api-client.ts | sed -n '37,110p'Repository: onerandomdevv/Swifta
Length of output: 2774
🏁 Script executed:
cat -n apps/web/src/lib/api-client.ts | sed -n '133,175p'Repository: onerandomdevv/Swifta
Length of output: 1822
Paginated requests skip auth-refresh and HTTP error rejection.
Lines 153-156 return parsed JSON regardless of response status, unlike lines 95-97 in request(). This silently resolves 401/4xx/5xx responses and breaks query error states downstream.
🔧 Proposed fix (align paginated flow with standard request behavior)
async requestPaginated<T>(endpoint: string, config: RequestConfig = {}): Promise<any> {
let response = await performRequest();
+ // Handle 401 Unauthorized - Attempt Token Refresh
+ if (
+ response.status === 401 &&
+ this.refreshToken &&
+ !endpoint.includes("/auth/login") &&
+ !endpoint.includes("/auth/refresh")
+ ) {
+ if (this.isRefreshing) {
+ const success = await new Promise<boolean>((resolve) => {
+ this.subscribeTokenRefresh((s) => resolve(s));
+ });
+ if (success) {
+ response = await performRequest();
+ }
+ } else {
+ this.isRefreshing = true;
+ try {
+ const success = await this.refreshToken();
+ this.isRefreshing = false;
+ this.onTokenRefreshed(success);
+ if (success) {
+ response = await performRequest();
+ } else {
+ this.onUnauthorized?.();
+ throw await this.handleError(response);
+ }
+ } catch (error) {
+ this.isRefreshing = false;
+ this.onTokenRefreshed(false);
+ this.onUnauthorized?.();
+ throw error;
+ }
+ }
+ }
+
+ if (!response.ok) {
+ throw await this.handleError(response);
+ }
+
const text = await response.text();
if (!text) return {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async requestPaginated<T>(endpoint: string, config: RequestConfig = {}): Promise<any> { | |
| const { params, ...options } = config; | |
| let url = `${this.baseUrl}${endpoint.startsWith("/") ? endpoint : `/${endpoint}`}`; | |
| if (params) { | |
| const searchParams = new URLSearchParams(params); | |
| url += `?${searchParams.toString()}`; | |
| } | |
| const headers = new Headers(options.headers); | |
| if (!headers.has("Content-Type") && !(options.body instanceof FormData)) { | |
| headers.set("Content-Type", "application/json"); | |
| } | |
| const performRequest = async (): Promise<Response> => { | |
| return fetch(url, { ...options, headers, credentials: "include" }); | |
| }; | |
| let response = await performRequest(); | |
| const text = await response.text(); | |
| if (!text) return {}; | |
| return JSON.parse(text); | |
| } | |
| async requestPaginated<T>(endpoint: string, config: RequestConfig = {}): Promise<any> { | |
| const { params, ...options } = config; | |
| let url = `${this.baseUrl}${endpoint.startsWith("/") ? endpoint : `/${endpoint}`}`; | |
| if (params) { | |
| const searchParams = new URLSearchParams(params); | |
| url += `?${searchParams.toString()}`; | |
| } | |
| const headers = new Headers(options.headers); | |
| if (!headers.has("Content-Type") && !(options.body instanceof FormData)) { | |
| headers.set("Content-Type", "application/json"); | |
| } | |
| const performRequest = async (): Promise<Response> => { | |
| return fetch(url, { ...options, headers, credentials: "include" }); | |
| }; | |
| let response = await performRequest(); | |
| // Handle 401 Unauthorized - Attempt Token Refresh | |
| if ( | |
| response.status === 401 && | |
| this.refreshToken && | |
| !endpoint.includes("/auth/login") && | |
| !endpoint.includes("/auth/refresh") | |
| ) { | |
| if (this.isRefreshing) { | |
| const success = await new Promise<boolean>((resolve) => { | |
| this.subscribeTokenRefresh((s) => resolve(s)); | |
| }); | |
| if (success) { | |
| response = await performRequest(); | |
| } | |
| } else { | |
| this.isRefreshing = true; | |
| try { | |
| const success = await this.refreshToken(); | |
| this.isRefreshing = false; | |
| this.onTokenRefreshed(success); | |
| if (success) { | |
| response = await performRequest(); | |
| } else { | |
| this.onUnauthorized?.(); | |
| throw await this.handleError(response); | |
| } | |
| } catch (error) { | |
| this.isRefreshing = false; | |
| this.onTokenRefreshed(false); | |
| this.onUnauthorized?.(); | |
| throw error; | |
| } | |
| } | |
| } | |
| if (!response.ok) { | |
| throw await this.handleError(response); | |
| } | |
| const text = await response.text(); | |
| if (!text) return {}; | |
| return JSON.parse(text); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/api-client.ts` around lines 133 - 157, The paginated request
handler requestPaginated currently parses and returns JSON regardless of HTTP
status, which hides 4xx/5xx errors and skips auth-refresh logic; update
requestPaginated to mirror the standard request() flow: after await
performRequest(), if response.status === 401 call the same auth-refresh logic
used by request() (retry performRequest once after refreshing), then read
response.text(), parse JSON, and if response.ok return the data but if not throw
an error containing the parsed body and status so callers receive proper
rejection; reference the functions/blocks requestPaginated, performRequest and
the existing request() auth-refresh/error handling when implementing.
This was referenced Mar 16, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores