✨️ server: add activity and push notification on card decline#622
✨️ server: add activity and push notification on card decline#622
Conversation
🦋 Changeset detectedLatest commit: 0b0e25b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds server-side decline handling: classifies decline reasons, persists declined transaction bodies, routes frozen/non-active/error flows through new decline handlers, triggers push notifications for select decline types, extends activity schema to include "requested"/declined fields, and adds tests for declines and notifications. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CardSystem as Card System
participant PandaHook as Panda Hook
participant DB as Database
participant Notif as Notification Service
Client->>CardSystem: submit transaction
CardSystem->>PandaHook: webhook / payload
PandaHook->>DB: fetch card & transaction
alt card FROZEN or not ACTIVE
DB-->>PandaHook: card status != ACTIVE
PandaHook->>PandaHook: getDeclineReason()
PandaHook->>PandaHook: handleRejectedTransactionSync()
PandaHook-->>CardSystem: 403 / decline response
else processing error or explicit decline
PandaHook->>PandaHook: getDeclineReason()
PandaHook->>DB: updateTransactionRecord(...) with declined body
DB-->>PandaHook: OK
PandaHook->>PandaHook: handleDeclinedTransaction()
PandaHook->>Notif: sendDeclinedNotification()
Notif-->>Client: push notification delivered
else success
PandaHook-->>CardSystem: success response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @aguxez, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the server's transaction handling by introducing a mechanism to process declined card transactions. The immediate user-facing change is the implementation of push notifications, which will inform users instantly about rejected purchases. Although the foundational code for recording these declined transactions as user activity has been added, this specific database logging functionality is temporarily disabled, pending the development of corresponding user interface elements. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @server/hooks/panda.ts:
- Around line 955-998: Remove the large commented-out DB logic inside
handleDeclinedTransaction and track the work in your issue tracker: create an
issue describing the pending UI changes needed to handle declined transactions
and include its ID; then replace the commented block with a single-line comment
in handleDeclinedTransaction referencing that issue (e.g., "See ISSUE-1234:
enable declined-transaction persistence once UI supports it"). Ensure the rest
of the function (push notification and error capture) remains unchanged.
- Line 965: Replace the existing comment "// TODO: Enable once UI has proper
designs to handle declined transactions in activity" with the coding-guideline
compliant format: use uppercase tag, a single space, and a fully lowercase
comment body (e.g. "// TODO enable once ui has proper designs to handle declined
transactions in activity"); update the line containing that TODO comment in the
server/hooks/panda.ts hook to remove the colon and capitalize only the TODO tag.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.changeset/honest-peas-stand.mdserver/hooks/panda.tsserver/test/hooks/panda.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/.changeset/*.md
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Use a lowercase sentence in the imperative present tense for changeset summaries
Files:
.changeset/honest-peas-stand.md
server/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/server.mdc)
server/**/*.ts: Usec.varobject to pass strongly-typed data between Hono middleware and route handlers; do not usec.set
All request validation (headers, body, params) must be handled by@hono/valibot-validatormiddleware; do not perform manual validation inside route handlers
Use Hono's built-in error handling by throwingnew HTTPException()for expected errors; unhandled errors will be caught and logged automatically
Enforce Node.js best practices using ESLintplugin:n/recommendedconfiguration
Enforce Drizzle ORM best practices using ESLintplugin:drizzle/allconfiguration, including requiringwhereclauses forupdateanddeleteoperations
Use Drizzle ORM query builder for all database interactions; do not write raw SQL queries unless absolutely unavoidable
All authentication and authorization logic must be implemented in Hono middleware
Do not accessprocess.envdirectly in application code; load all configuration and secrets once at startup and pass them through dependency injection or context
Avoid long-running, synchronous operations; useasync/awaitcorrectly and be mindful of CPU-intensive tasks to prevent blocking the event loop
Files:
server/hooks/panda.tsserver/test/hooks/panda.test.ts
**/*.{js,ts,tsx,jsx,sol}
📄 CodeRabbit inference engine (AGENTS.md)
Follow linter/formatter (eslint, prettier, solhint) strictly with high strictness level. No
anytype.
Files:
server/hooks/panda.tsserver/test/hooks/panda.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Omit redundant type names in variable declarations - let the type system explain itself
**/*.{ts,tsx}: Use PascalCase for TypeScript types and interfaces
Use valibot for all runtime validation of API inputs, environment variables, and other data; define schemas once and reuse them
Infer TypeScript types from valibot schemas usingtype User = v.Input<typeof UserSchema>instead of manually defining interfaces
Files:
server/hooks/panda.tsserver/test/hooks/panda.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Omit contextual names - don't repeat class/module names in members
Omit meaningless words like 'data', 'state', 'manager', 'engine', 'value' from variable and function names unless they add disambiguation
**/*.{ts,tsx,js,jsx}: Prefer function declarations for all multi-line functions; use function expressions or arrow functions only for single-line implementations
Preferconstfor all variable declarations by default; only useletif the variable's value will be reassigned
Declare each variable on its own line with its ownconstorletkeyword, not multiple declarations on one line
Use camelCase for TypeScript variables and functions
Always useimport type { ... }for type imports
Use relative paths for all imports within the project; avoid tsconfig path aliases
Follow eslint-plugin-import order: react, external libraries, then relative paths
Use object and array destructuring to access and use properties
Use object method shorthand syntax when a function is a property of an object
Prefer optional chaining (?.), nullish coalescing (??), object and array spreading (...), andfor...ofloops over traditional syntax
Do not use abbreviations or cryptic names; write out full words likeerror,parameters,requestinstead oferr,params,req
UseNumber.parseInt()instead of the globalparseInt()function when parsing numbers
All classes called withnewmust use PascalCase
UseBuffer.from(),Buffer.alloc(), orBuffer.allocUnsafe()instead of the deprecatednew Buffer()
Use@ts-expect-errorinstead of@ts-ignore; follow it immediately with a single-line lowercase comment explaining why the error is expected, without separators like-or:
Do not include the type in a variable's name; let the static type system do its job (e.g., useconst user: Usernotconst userObject: User)
Do not repeat the name of a class or module within its members; omit contextual names (e.g., use `class User { getProfil...
Files:
server/hooks/panda.tsserver/test/hooks/panda.test.ts
server/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
server/**/*.{ts,tsx}: Server API: implement schema-first approach using OpenAPI via hono with validation via valibot middleware
Server database: drizzle schema is source of truth. Migrations required. No direct database access in handlers - usec.var.db
Files:
server/hooks/panda.tsserver/test/hooks/panda.test.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
For files with a single
defaultexport, name the file identically to the export; for files with multiple exports, use camelCase with a strong preference for a single word
Files:
server/hooks/panda.tsserver/test/hooks/panda.test.ts
🧠 Learnings (2)
📚 Learning: 2025-12-31T00:23:55.034Z
Learnt from: cruzdanilo
Repo: exactly/exa PR: 610
File: .changeset/ready-experts-fly.md:1-2
Timestamp: 2025-12-31T00:23:55.034Z
Learning: In the exactly/exa repository, allow and require empty changeset files (containing only --- separators) when changes are not user-facing and do not warrant a version bump. This is needed because CI runs changeset status --since origin/main and requires a changeset file to exist. Ensure such empty changesets are used only for non-user-facing changes and document the rationale in the commit or changelog notes.
Applied to files:
.changeset/honest-peas-stand.md
📚 Learning: 2025-12-23T19:58:16.574Z
Learnt from: CR
Repo: exactly/exa PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-23T19:58:16.574Z
Learning: Zero config local dev environment: no `.env` files, mock all external services
Applied to files:
server/test/hooks/panda.test.ts
🧬 Code graph analysis (2)
server/hooks/panda.ts (1)
server/utils/onesignal.ts (1)
sendPushNotification(7-25)
server/test/hooks/panda.test.ts (1)
server/database/schema.ts (1)
transactions(36-43)
🔇 Additional comments (5)
server/test/hooks/panda.test.ts (2)
2-2: LGTM!Import adjustments for mocks are clean and improve organization.
Also applies to: 7-7
1352-1439: Test stubs properly scaffolded for future implementation.The two test cases for declined transaction handling are well-structured and align with the commented-out database logic in
server/hooks/panda.ts. Usingit.todois appropriate while waiting for UI designs.server/hooks/panda.ts (2)
533-533: LGTM!The call to
handleDeclinedTransactionis correctly placed after the mutex is released, and the type cast is necessary due to TypeScript's union type handling.
988-998: LGTM!Push notification implementation properly formats the transaction details and includes error handling consistent with the rest of the codebase.
.changeset/honest-peas-stand.md (1)
1-5: LGTM!Changeset properly documents the feature addition with appropriate version bump and clear description.
118a340 to
dc0ac8a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #622 +/- ##
==========================================
+ Coverage 69.11% 69.38% +0.26%
==========================================
Files 208 208
Lines 7155 7159 +4
Branches 2269 2267 -2
==========================================
+ Hits 4945 4967 +22
+ Misses 2020 2002 -18
Partials 190 190
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2b6c4a8 to
64325f3
Compare
64325f3 to
b3814d0
Compare
b3814d0 to
8c11c8c
Compare
8c11c8c to
77c26c2
Compare
98e690f to
a16d92d
Compare
a16d92d to
685fbf3
Compare
685fbf3 to
4bcf82f
Compare
4bcf82f to
bbacd86
Compare
4ee1569 to
e91e988
Compare
280933f to
517e2cc
Compare
517e2cc to
2a6fdab
Compare
| const { spend } = payload.body; | ||
| const transactionId = payload.body.id ?? payload.id; | ||
| return { transactionId, spend }; |
There was a problem hiding this comment.
🟡 Duplicate declined records and notifications when body.id is undefined for "requested" action
When the "requested" webhook has body.id as undefined (allowed by the schema at server/hooks/panda.ts:148), the validateTransactionId fallback uses payload.id (the webhook event ID) as the transaction primary key. Later, the "created" declined webhook arrives with body.id set to the actual transaction ID — a different key. Both rejectTx invocations insert separate records with isNewRecord = true, causing a duplicate push notification and two declined activity entries in the user's feed.
Detailed trace of the duplicate flow
- "requested" fails (e.g. InsufficientAccountLiquidity at line 424) →
rejectTxcalled fire-and-forget at line 462 withpayload.body.id = undefined. validateTransactionIdatserver/hooks/panda.ts:1156returnstransactionId = payload.id(e.g."abcdef-123456").updateTransactionRecordINSERTs a row withid = "abcdef-123456",isNewRecord = true→ notification sent.- "created" declined webhook arrives with
body.id = "31eaa81e-..."(actual tx ID). handleDeclinedTransaction→rejectTx→validateTransactionIdreturnstransactionId = "31eaa81e-..."(different key).updateTransactionRecordINSERTs another row withid = "31eaa81e-...",isNewRecord = true→ second notification sent.
The xmax = 0 guard at server/hooks/panda.ts:1121 only prevents duplicates for the same primary key. With two different keys, both upserts are treated as new records.
Impact: The user receives two push notifications for a single decline event, and the activity feed shows two duplicate declined entries.
Was this helpful? React with 👍 or 👎 to provide feedback.
2a6fdab to
914e2c0
Compare
914e2c0 to
d907571
Compare
| } catch (error: unknown) { | ||
| captureException(error, { level: "error" }); | ||
| return; | ||
| } | ||
|
|
||
| if (isNewRecord && NOTIFICATION_TRIGGERING_REASONS.has(reason)) { | ||
| await sendDeclinedNotification(account, spend, reason).catch((error: unknown) => { | ||
| captureException(error, { level: "error" }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🚩 Notification suppressed for existing transaction records regardless of reason change
The notification logic at server/hooks/panda.ts:1314 sends notifications only when isNewRecord && NOTIFICATION_TRIGGERING_REASONS.has(reason). This means if a transaction record already exists (e.g., from a prior rejectTx call during the "requested" phase with a non-triggering reason like "transaction declined"), a subsequent decline event with a triggering reason (e.g., "insufficient funds") will NOT send a notification because isNewRecord is false.
Scenario: (1) "requested" fails with "bad collection" → rejectTx inserts record, reason maps to "transaction declined" (not in triggering set), no notification. (2) "created" arrives with status "declined" and declinedReason="insufficient_funds" → rejectTx updates existing record, isNewRecord=false → no notification despite the reason being triggering.
In practice this edge case may be rare (most InsufficientAccountLiquidity errors would trigger notifications in step 1), but it's a design limitation worth understanding.
Was this helpful? React with 👍 or 👎 to provide feedback.
d907571 to
b2de9c4
Compare
b2de9c4 to
abf0b03
Compare
Greptile Summaryadds comprehensive support for declined card transactions with activity tracking and push notifications. the implementation handles three decline scenarios: frozen cards (checked upfront on key changes:
the implementation correctly prevents duplicate notifications using postgres Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Card Processor
participant Panda as Panda Webhook
participant DB as Database
participant Push as Push Notification
participant Activity as Activity API
Client->>Panda: POST /hooks/panda (transaction requested)
alt Card FROZEN
Panda->>DB: rejectTx (insert/update with declined)
Panda->>Push: sendPushNotification (frozen card)
Panda-->>Client: 403 frozen card
else Card ACTIVE
Panda->>Panda: authorize transaction
Panda-->>Client: 200 ok
end
Client->>Panda: POST /hooks/panda (transaction declined)
Panda->>DB: handleDeclinedTransaction
Panda->>DB: updateTransactionRecord (upsert)
alt isNewRecord && triggering reason
Panda->>Push: sendDeclinedNotification
end
Panda-->>Client: 200 ok
Client->>Activity: GET /activity
Activity->>DB: query transactions
DB-->>Activity: transaction payloads
Activity->>Activity: parse PandaActivity
alt declined transaction found
Activity-->>Client: declined activity with reason
else normal transaction
Activity-->>Client: normal activity
end
Last reviewed commit: abf0b03 |
abf0b03 to
0b0e25b
Compare
Greptile Summaryadds comprehensive declined transaction tracking with push notifications and activity feed integration.
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Panda as Panda Webhook
participant Hook as panda.ts Hook
participant DB as Database
participant OneSignal as Push Notification
User->>Panda: Card Transaction Request
Panda->>Hook: POST /hooks/panda (action: requested)
alt Card Status Check
Hook->>DB: Query card status
alt Card is FROZEN
Hook->>DB: Insert declined tx (frozen_card)
Hook->>OneSignal: Send notification (frozen card)
Hook-->>Panda: 403 frozen card
else Card is not ACTIVE
Hook-->>Panda: 403 card not active
end
end
alt Transaction Processing
Hook->>Hook: Assess risk & validate
Hook->>Hook: Execute onchain transaction
alt Transaction Fails (InsufficientAccountLiquidity)
Hook->>DB: Insert declined tx (insufficient funds)
Hook->>OneSignal: Send notification (insufficient funds)
Hook-->>Panda: 557 error
else Transaction Succeeds
Hook->>DB: Insert successful tx
Hook-->>Panda: 200 OK
end
end
alt Declined Transaction Webhook
Panda->>Hook: POST /hooks/panda (action: created/updated, status: declined)
Hook->>DB: Query card & account
Hook->>DB: Upsert declined tx record
alt First declined event for this tx
Hook->>OneSignal: Send notification
end
Hook-->>Panda: 200 OK
end
User->>Hook: GET /api/activity
Hook->>DB: Query transactions
Hook->>Hook: Parse PandaActivity (declined status)
Hook-->>User: Activity feed with declined tx
Last reviewed commit: 0b0e25b |
| state: "", | ||
| icon: activity.body.spend.enrichedMerchantIcon, |
There was a problem hiding this comment.
field order changed from icon, state to state, icon - keep original field ordering to minimize diff noise
| state: "", | |
| icon: activity.body.spend.enrichedMerchantIcon, | |
| icon: activity.body.spend.enrichedMerchantIcon, | |
| state: "", |
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!
There was a problem hiding this comment.
🚩 Pre-existing field name mismatch: timestamps vs timestamp in activity endpoint
The PandaActivity schema at server/api/activity.ts:335 defines the borrows field as array(nullable(object({ timestamp: optional(bigint()), events: array(Borrow) }))) — note timestamp (singular). However, the activity endpoint at server/api/activity.ts:262 constructs borrow objects with timestamps (plural):
return {
events: b.events,
timestamps: b.blockNumber && timestamps.get(b.blockNumber),
};Because valibot's object() strips unknown keys, the timestamps property is silently dropped during parsing, and timestamp is always undefined. This means borrow?.timestamp at line 383 is always undefined, causing blockTimestamp to always fall back to BigInt(Math.floor(new Date(createdAt).getTime() / 1000)) in CreditActivity and InstallmentsActivity. This is pre-existing (not introduced by this PR), but it means borrow rate calculations for panda activities use the less accurate createdAt-derived timestamp instead of the actual block timestamp. The PR's test change at server/test/api/activity.test.ts:183 correctly uses timestamp (singular), so tests pass — but they don't exercise the production code path that constructs the borrow data in the endpoint handler.
Was this helpful? React with 👍 or 👎 to provide feedback.
closes #114
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores