Skip to content

feat(pos-app): unify merchant API into single client#395

Merged
ignaciosantise merged 3 commits intomainfrom
feat/unify-merchant-api
Feb 27, 2026
Merged

feat(pos-app): unify merchant API into single client#395
ignaciosantise merged 3 commits intomainfrom
feat/unify-merchant-api

Conversation

@ignaciosantise
Copy link
Collaborator

@ignaciosantise ignaciosantise commented Feb 26, 2026

Summary

Consolidate Payment and Merchant APIs to use a single base URL (EXPO_PUBLIC_API_URL), eliminating the separate EXPO_PUBLIC_MERCHANT_API_URL. The Merchant Portal API retains its own authentication (EXPO_PUBLIC_MERCHANT_PORTAL_API_KEY) since it currently has a separate auth layer from the Payment API.

  • Delete redundant merchant-client.ts — reuse shared apiClient from client.ts (same base URL)
  • Extract getApiHeaders() to client.ts as shared helper for payment endpoints
  • Native transactions use apiClient with merchant portal API key (x-api-key header)
  • Web proxy keeps EXPO_PUBLIC_MERCHANT_PORTAL_API_KEY server-side; client only sends x-merchant-id
  • Remove EXPO_PUBLIC_MERCHANT_API_URL from .env.example (no longer needed)
  • Fix incomplete test mock in payment.test.ts after getApiHeaders() module migration
  • Add npm test to AGENTS.md pre-commit checklist + mock maintenance guidance
  • Add TODOs for future auth unification once Merchants API aligns with Payment API
graph LR
    subgraph "Before"
        PA1["Payment API"] -->|"EXPO_PUBLIC_API_URL<br/>Api-Key (partner)"| API1["Backend"]
        MA1["Merchant API"] -->|"EXPO_PUBLIC_MERCHANT_API_URL<br/>x-api-key (portal)"| API2["Backend"]
    end

    subgraph "After"
        PA2["Payment API"] -->|"EXPO_PUBLIC_API_URL<br/>Api-Key (partner)"| API3["Backend"]
        MA2["Merchant API"] -->|"EXPO_PUBLIC_API_URL<br/>x-api-key (portal)"| API3
    end
Loading

Test plan

  • All 178 tests pass (npm test)
  • Verify native Activity screen fetches transactions with merchant portal key
  • Verify web Activity screen fetches transactions via proxy (API key server-side)
  • Confirm payment flow (start payment, poll status) still works with partner API key

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings February 26, 2026 19:32
@vercel
Copy link

vercel bot commented Feb 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
react-native-examples Ready Ready Preview, Comment Feb 27, 2026 5:08pm

Request Review

@claude
Copy link

claude bot commented Feb 26, 2026

Claude finished @ignaciosantise's task in 3m 27s —— View job


PR Review: feat(pos-app): unify merchant API into single client

  • Gather context and read changed files
  • Run breaking changes subagent
  • Run data classification subagent
  • Analyze code for issues
  • Post final review

Found 6 issue(s)

Issue 1: Missing timeout on external fetch in Vercel proxy

ID: transactions-proxy-fetch-timeout-d2f1
File: dapps/pos-app/api/transactions.ts:63
Severity: HIGH
Category: performance

Context:

  • Pattern: fetch(...) to upstream API has no AbortController timeout; the native apiClient already has a 30s default timeout but the proxy's external call does not
  • Risk: If upstream API hangs, Vercel function blocks until its own platform timeout (default 10s), masking slow-upstream from caller and exhausting serverless concurrency
  • Impact: Degraded Activity screen on web under slow upstream; hard-to-diagnose timeouts return as 500 rather than a clear timeout error
  • Trigger: Any upstream API slowness on the web platform

Recommendation:

const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 10000);
const response = await fetch(`${normalizedBaseUrl}${endpoint}`, {
  signal: controller.signal,
  headers: { ... },
});
clearTimeout(timeoutId);

Fix this →


Issue 2: Security model shift not documented — API key now client-forwarded to proxy

ID: brk-api-transactions-require-client-apikey-f5d9
File: dapps/pos-app/api/transactions.ts:19-26
Severity: HIGH
Category: breaking_change

Context:

  • Pattern: The proxy previously read EXPO_PUBLIC_MERCHANT_PORTAL_API_KEY from the Vercel server environment; it now requires the client to forward the partner API key as x-api-key. Returns HTTP 400 if absent
  • Risk: Any existing Vercel deployment where only EXPO_PUBLIC_MERCHANT_PORTAL_API_KEY was configured (without per-user API key in secure storage) will break silently after deploy — users see a 400 on the Activity screen
  • Impact: Web deployments that relied on the server-side secret will stop fetching transactions until each user configures their API key in Settings
  • Trigger: First web deployment of this PR to an environment where EXPO_PUBLIC_MERCHANT_PORTAL_API_KEY was the sole credential source

Recommendation: Add a migration note in AGENTS.md or README that operators should remove the old server env vars and ensure EXPO_PUBLIC_DEFAULT_PARTNER_API_KEY is set for default configuration. The security implication (API key is now visible in browser DevTools network tab) should be explicitly acknowledged in docs.


Issue 3: merchantId not URL-encoded in native transactions endpoint path

ID: transactions-native-merchantid-urlencode-c4a7
File: dapps/pos-app/services/transactions.ts:55
Severity: MEDIUM
Category: code_quality

Context:

  • Pattern: \/merchants/${merchantId}/payments`— noencodeURIComponent. The proxy (api/transactions.ts:61) correctly uses encodeURIComponent(merchantId)`
  • Risk: If a merchant ID contains /, ?, #, or space characters (e.g. from URL params on web), the URL will be malformed, producing a 404 or routing to the wrong path
  • Impact: Native requests fail for any non-alphanumeric merchant ID; silent discrepancy with the web proxy path
  • Trigger: Merchant ID with special characters — possible since IDs are accepted via settings/URL params

Recommendation:

const endpoint = `/merchants/${encodeURIComponent(merchantId)}/payments${queryString ? `?${queryString}` : ""}`;

Fix this →


Issue 4: AGENTS.md reports stale versionCode; app.json not incremented for this feature

ID: agents-md-versioncode-stale-increment-b3e8
File: dapps/pos-app/AGENTS.md:604 / dapps/pos-app/app.json
Severity: LOW
Category: code_quality

Context:

  • Pattern: AGENTS.md updated in this PR says "Current version code: 16", but app.json already has versionCode: 17. Additionally, AGENTS.md states "For every new feature or change, you MUST update the Android version code" — app.json was not changed in this PR
  • Risk: Installing new APK builds over previous ones will fail if versionCode is not incremented
  • Impact: Android release builds from this PR cannot be side-loaded over existing installs
  • Trigger: Android release build after this PR merges

Recommendation: Increment versionCode in app.json from 17 → 18, and update AGENTS.md to say "Current version code: 18".
Fix this →


Issue 5: console.error in Vercel serverless function

ID: api-transactions-console-error-logs-a8f5
File: dapps/pos-app/api/transactions.ts:83
Severity: LOW
Category: code_quality

Context:

  • Pattern: console.error("Transactions proxy error:", error) — AGENTS.md explicitly states to remove all console.log()/console.error() before committing
  • Risk: Error object could contain request context details in its stack trace (e.g. URL fragments with merchant ID). Vercel logs are visible to deployment operators
  • Trigger: Any fetch or JSON parse failure in the proxy

Recommendation: Remove the console.error per AGENTS.md guidelines. The error is already returned to the client via the res.status(500).json(...) response.


Issue 6: Redundant, weaker merchantId guard in native transactions.ts

ID: transactions-native-redundant-merchantid-check-f2a3
File: dapps/pos-app/services/transactions.ts:22-25
Severity: LOW
Category: code_quality

Context:

  • Pattern: if (!merchantId) throw at line 22-25 duplicates validation already performed by getApiHeaders() at line 56 (which also checks for whitespace-only strings). The early guard does not check .trim(), creating a gap: a whitespace-only merchant ID passes line 22 but fails line 56 with a different error message
  • Risk: Inconsistent error messages for whitespace merchantId vs null; could confuse callers or tests relying on specific error strings
  • Impact: Minor UX inconsistency; whitespace ID throws "Merchant ID is not configured" from getApiHeaders rather than from the early guard

Recommendation: Either remove the early check (rely on getApiHeaders) or align it: if (!merchantId || merchantId.trim().length === 0) to match getApiHeaders.

Data classification: No issues found — no hardcoded secrets, credentials correctly read from secure storage at runtime.

Breaking changes summary: Deletion of merchant-client.ts is clean (no remaining internal imports). Key deployment concern is Issue 2 above — web deployments must migrate from server-side EXPO_PUBLIC_MERCHANT_PORTAL_API_KEY to per-user API key in Settings.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request consolidates the separate Merchant Portal API integration into the unified Payment API, eliminating code duplication and simplifying credential management across both native and web platforms.

Changes:

  • Extracted getApiHeaders() to shared client module to eliminate duplication
  • Updated both native and web transaction services to use unified API client with consistent authentication
  • Modified Vercel proxy to forward unified auth headers (Api-Key, Merchant-Id, SDK headers) to the backend API
  • Removed obsolete merchant-specific client, environment variables, and documentation references
  • Fixed test mock to preserve real getApiHeaders() after module export migration

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
dapps/pos-app/services/transactions.web.ts Web version now passes API key via header to proxy instead of relying on server-side config
dapps/pos-app/services/transactions.ts Native version switched from merchant-client to unified apiClient with getApiHeaders()
dapps/pos-app/services/payment.ts Removed duplicate getApiHeaders() implementation, now imports from client module
dapps/pos-app/services/merchant-client.ts Deleted entire duplicate API client implementation
dapps/pos-app/services/client.ts Added exported getApiHeaders() function with authentication logic
dapps/pos-app/api/transactions.ts Proxy updated to forward unified auth headers instead of separate merchant portal key
dapps/pos-app/__tests__/services/payment.test.ts Fixed mock to use jest.requireActual() pattern to preserve getApiHeaders()
dapps/pos-app/AGENTS.md Updated documentation to reflect unified API and added test maintenance guidance
dapps/pos-app/.env.example Removed obsolete EXPO_PUBLIC_MERCHANT_API_URL and EXPO_PUBLIC_MERCHANT_PORTAL_API_KEY

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Consolidate to single base URL (EXPO_PUBLIC_API_URL) for both Payment and
Merchant APIs, removing EXPO_PUBLIC_MERCHANT_API_URL. Delete redundant
merchant-client.ts and reuse apiClient. Extract getApiHeaders() to shared
client module for payment endpoints.

Merchant endpoints retain separate auth via EXPO_PUBLIC_MERCHANT_PORTAL_API_KEY
since the Merchants API has its own auth layer. Added TODOs for when APIs
unify auth. Fix incomplete test mock after module export migration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ignaciosantise ignaciosantise merged commit d490e05 into main Feb 27, 2026
6 checks passed
@ignaciosantise ignaciosantise deleted the feat/unify-merchant-api branch February 27, 2026 17:55
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.

2 participants