Skip to content

fix: sync-wallet failing scenario test#415

Open
emarc99 wants to merge 2 commits into
safetrustcr:mainfrom
emarc99:fix/failing-test-sync-wallet
Open

fix: sync-wallet failing scenario test#415
emarc99 wants to merge 2 commits into
safetrustcr:mainfrom
emarc99:fix/failing-test-sync-wallet

Conversation

@emarc99

@emarc99 emarc99 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Changes

  1. Webhook Handler: Updated the UPSERT_WALLET query in sync-wallet.handler.js to resolve conflicts on the unique wallet address constraint using PostgreSQL's EXCLUDED prefix (EXCLUDED.user_id and EXCLUDED.is_primary).

  2. Karate Test Suite:

    • Refactored is_primary assertions in sync-wallet.feature to robustly match database-specific boolean variations ('t', 'true', or true).
    • Updated expected response body in get_one_rest.feature to match the standardized auth middleware 401 response layout.

Summary by CodeRabbit

  • Bug Fixes
    • Improved wallet synchronization so matching wallet addresses update the associated user and primary-wallet status correctly.
    • Updated the apartment “unauthorized” response expectations when no Authorization token is provided, aligning with the current API error payload.
  • Tests
    • Relaxed wallet-related assertions to accept multiple valid representations of the primary-wallet flag.

- Modify `sync-wallet.handler.js` to resolve wallet address conflicts using `EXCLUDED.user_id` and `EXCLUDED.is_primary`.
- Update `sync-wallet.feature` to support different database boolean formats in assertions (`'t'`, `'true'`, or `true`).
- Fix `get_one_rest.feature` to align with the standardized auth middleware 401 response structure.
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0b519008-fc0d-4611-843f-995369154d98

📥 Commits

Reviewing files that changed from the base of the PR and between 2d35bb2 and 87bc459.

📒 Files selected for processing (1)
  • webhook/src/routes/auth/sync-wallet.handler.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • webhook/src/routes/auth/sync-wallet.handler.js

📝 Walkthrough

Walkthrough

Updated Karate expectations for an apartment 401 response and a sync-wallet upsert path. The wallet handler now writes conflict rows from incoming values, and the feature test accepts multiple truthy is_primary forms.

Changes

Apartment unauthorized response

Layer / File(s) Summary
401 payload assertion
tests/karate/features/apartments/get_one_rest.feature
GET /api/apartments/:id now expects error: 'Unauthorized' and message: 'Missing or malformed Authorization header' when no token is sent.

Wallet upsert behavior

Layer / File(s) Summary
Conflict update uses incoming values
webhook/src/routes/auth/sync-wallet.handler.js
The wallet_address conflict update sets user_id, is_primary, and chain_type from EXCLUDED values during upsert.
Primary flag matcher accepts truthy values
tests/karate/features/auth/sync-wallet.feature
The inserted-wallet scenario accepts is_primary as "true", "t", or boolean true in the database row.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • sotoJ24

Poem

A rabbit hopped through auth at dawn,
Found one token gone, then bounded on.
Wallets twitched, and truthy flags gave cheer,
“Hop!” said the tests — “we’re green this year.” 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The 401 assertion change in get_one_rest.feature is unrelated to issue #402 and goes beyond the sync-wallet fix. Move the auth error-response update to a separate PR or link the issue that requires it.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main sync-wallet fix.
Linked Issues check ✅ Passed The upsert now uses EXCLUDED values for user_id, chain_type, and is_primary, and the Karate test verifies the reassigned user_id as requested.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/karate/features/auth/sync-wallet.feature (1)

26-37: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the upsert test actually prove is_primary changes.

Lines 28 and 32 both use false, so this scenario still passes if the conflict branch ignores EXCLUDED.is_primary. Seed the existing row with the opposite value and assert the stored flag after the POST.

Suggested test change
-    * db.execute("INSERT INTO public.user_wallets (user_id, wallet_address, chain_type, is_primary) VALUES ('tenant-456', '" + stellarAddress + "', 'STELLAR', false)")
+    * db.execute("INSERT INTO public.user_wallets (user_id, wallet_address, chain_type, is_primary) VALUES ('tenant-456', '" + stellarAddress + "', 'STELLAR', true)")
...
     * def rows = db.query("SELECT * FROM public.user_wallets WHERE wallet_address = '" + stellarAddress + "'")
     And match rows[0].user_id == testUid
+    And match rows[0].is_primary == '#? _ == "false" || _ == "f" || _ == false'
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/karate/features/auth/sync-wallet.feature` around lines 26 - 37, The
sync-wallet upsert scenario is not currently verifying that the conflict path
updates is_primary. In the sync-wallet feature’s “Upsert same address updates
user_id and is_primary” scenario, seed the existing user_wallets row with the
opposite is_primary value from the request, then assert after the POST that the
stored row for stellarAddress reflects the new flag along with the updated
user_id. This should be done in the same scenario using the existing db.execute,
/api/auth/sync-wallet request, and db.query assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@webhook/src/routes/auth/sync-wallet.handler.js`:
- Line 14: The conflict handling in the wallet sync upsert is not updating
chain_type, so an existing wallet row can keep a stale chain classification
after being reassigned. Update the ON CONFLICT DO UPDATE branch in
sync-wallet.handler.js so the upsert in the wallet sync query also sets
chain_type alongside user_id, is_primary, and updated_at, using the same
conflict path keyed by wallet_address.

---

Outside diff comments:
In `@tests/karate/features/auth/sync-wallet.feature`:
- Around line 26-37: The sync-wallet upsert scenario is not currently verifying
that the conflict path updates is_primary. In the sync-wallet feature’s “Upsert
same address updates user_id and is_primary” scenario, seed the existing
user_wallets row with the opposite is_primary value from the request, then
assert after the POST that the stored row for stellarAddress reflects the new
flag along with the updated user_id. This should be done in the same scenario
using the existing db.execute, /api/auth/sync-wallet request, and db.query
assertions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9b04ab33-fd5c-4fa5-8034-746400a2e4f5

📥 Commits

Reviewing files that changed from the base of the PR and between e05201b and 2d35bb2.

📒 Files selected for processing (3)
  • tests/karate/features/apartments/get_one_rest.feature
  • tests/karate/features/auth/sync-wallet.feature
  • webhook/src/routes/auth/sync-wallet.handler.js

Comment thread webhook/src/routes/auth/sync-wallet.handler.js Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.

🧪[fix-test] POST /api/auth/sync-wallet — 1 failing scenario in sync-wallet.feature

1 participant