Skip to content

Conversation

@ignaciosantise
Copy link
Collaborator

Summary

Renames API key terminology from "merchant" to "partner" throughout the POS app for consistency with platform branding. Updates environment variables, secure storage keys, store methods, UI text, and error messages.

Changes

  • Environment variable: EXPO_PUBLIC_DEFAULT_MERCHANT_API_KEYEXPO_PUBLIC_DEFAULT_PARTNER_API_KEY
  • Secure storage key: merchant_api_keypartner_api_key
  • Store state/methods: isMerchantApiKeySetisPartnerApiKeySet, and related methods
  • UI labels and placeholders updated to use "partner API key"
  • Migration function handles upgrade path for existing users

Test plan

  • All existing tests pass (145 tests)
  • Settings store tests verify API key storage/retrieval
  • Payment service tests verify API key validation
  • Settings screen renders correctly with new labels
  • Migration function correctly transfers keys from old to new storage name

🤖 Generated with Claude Code

Update API key naming throughout POS app to use 'partner' terminology for consistency with platform branding. Changes include environment variables, secure storage keys, store state/methods, UI text, and documentation. Adds migration function to handle upgrade path for existing users.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings January 30, 2026 14:57
@vercel
Copy link

vercel bot commented Jan 30, 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 Jan 30, 2026 7:10pm

Request Review

@claude
Copy link

claude bot commented Jan 30, 2026

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


Code Review Complete

Reviewed PR #369: Rename merchant API key to partner API key

Summary

PR size: 15 files, 291 lines changed (163 additions, 128 deletions) - within acceptable limits.

Comprehensive review completed across all changed files, including migration logic, secure storage implementations, settings store, and UI components.

Found 3 issue(s)

Issue 1: User-facing text still references "merchant settings"

ID: settings-merchant-terminology-a4f2
File: dapps/pos-app/app/settings.tsx:160, 357, 369
Severity: LOW
Category: code_quality

Context:

  • Pattern: Three user-facing strings still use "merchant settings" instead of "partner settings"
  • Risk: Inconsistent terminology in UI/UX after renaming to "partner"
  • Impact: Users see mixed terminology (partner API key but merchant settings)
  • Trigger: Visible when biometric authentication prompt appears and in PIN modal subtitles

Recommendation: Update user-facing strings for consistency:

// Line 160
`Use ${biometricLabel} to change partner settings`

// Line 357
subtitle="Enter your PIN to save partner settings"

// Line 369
subtitle="Set a 4-digit PIN to protect partner settings"

Issue 2: Migration runs on every app start

ID: layout-migration-performance-b8c1
File: dapps/pos-app/app/_layout.tsx:77-88
Severity: MEDIUM
Category: performance

Context:

  • Pattern: migratePartnerApiKey() called in useEffect with empty dependency array on every app mount
  • Risk: Unnecessary async operations and secure storage reads/writes on every app start
  • Impact: Slight performance degradation on app launch; migration should be one-time operation
  • Trigger: Every time the root layout component mounts

Recommendation: Track migration completion to prevent repeated execution:

// In secure storage
const MIGRATION_COMPLETED_KEY = "migration_partner_api_key_completed";

export async function migratePartnerApiKey(): Promise<void> {
  const migrationCompleted = storage.getItem<boolean>(MIGRATION_COMPLETED_KEY);
  if (migrationCompleted) return;

  const OLD_KEY = "merchant_api_key";
  const NEW_KEY = "partner_api_key";

  const oldValue = await SecureStore.getItemAsync(OLD_KEY);
  if (oldValue) {
    const newValue = await SecureStore.getItemAsync(NEW_KEY);
    if (!newValue) {
      await SecureStore.setItemAsync(NEW_KEY, oldValue);
    }
    await SecureStore.deleteItemAsync(OLD_KEY);
  }
  
  storage.setItem(MIGRATION_COMPLETED_KEY, true);
}

Issue 3: Comment reference outdated

ID: payment-comment-terminology-c7d3
File: dapps/pos-app/services/payment.ts:10
Severity: LOW
Category: code_quality

Context:

  • Pattern: JSDoc comment says "Get merchant API headers" but function now retrieves partner API key
  • Risk: Misleading documentation for future developers
  • Impact: Minor confusion when reading code; doesn't affect runtime behavior
  • Trigger: N/A - documentation only

Recommendation: Update JSDoc to reflect new terminology:

/**
 * Get partner API headers for authenticated requests
 * @returns Headers object with Api-Key, Merchant-Id, and SDK headers
 * @throws Error if API key or merchant ID is missing
 */

Positive Observations

  • Clean, consistent renaming across codebase
  • Migration logic handles edge cases (existing new key, missing old key)
  • Proper secure storage usage on both native and web platforms
  • All tests updated to reflect new terminology
  • Error messages appropriately updated

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 PR renames API key terminology from "merchant" to "partner" throughout the POS app for consistency with platform branding. The changes affect environment variables, secure storage keys, store methods, UI text, and error messages, with a migration function to handle the upgrade path for existing users.

Changes:

  • Renamed environment variable from EXPO_PUBLIC_DEFAULT_MERCHANT_API_KEY to EXPO_PUBLIC_DEFAULT_PARTNER_API_KEY
  • Updated secure storage key from merchant_api_key to partner_api_key with migration support
  • Renamed store state and methods (e.g., isMerchantApiKeySetisPartnerApiKeySet)
  • Updated all UI labels, placeholders, and error messages to use "partner API key" terminology
  • Added migratePartnerApiKey() function to preserve existing users' API keys during upgrade

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
dapps/pos-app/utils/secure-storage.web.ts Added migration function and renamed storage key constant
dapps/pos-app/utils/secure-storage.ts Added migration function and renamed storage key constant
dapps/pos-app/utils/merchant-config.ts Renamed environment variable and config method
dapps/pos-app/store/useSettingsStore.ts Renamed state properties and methods, updated env default handling
dapps/pos-app/services/payment.web.ts Updated variable names and error message
dapps/pos-app/services/payment.ts Updated variable names and error message
dapps/pos-app/hooks/use-merchant-flow.ts Renamed all state and method references
dapps/pos-app/app/settings.tsx Updated UI labels and references
dapps/pos-app/app/index.tsx Updated state property reference
dapps/pos-app/app/_layout.tsx Added migration execution and state synchronization
dapps/pos-app/tests/utils/store-helpers.ts Updated test helper storage keys
dapps/pos-app/tests/store/useSettingsStore.test.ts Renamed test descriptions and assertions
dapps/pos-app/tests/services/payment.test.ts Updated storage keys and error message assertions
dapps/pos-app/AGENTS.md Updated documentation for environment variable and code examples
dapps/pos-app/.env.example Updated environment variable name

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

- Add migration completion tracking to avoid running on every app start
- Move migration to onRehydrateStorage to fix race condition with defaults
- Update user-facing text from "merchant settings" to "partner settings"
- Update JSDoc comment in payment.ts for new terminology
- Add test coverage for migration function (5 test cases)
- Simplify _layout.tsx by removing redundant migration call

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ignaciosantise
Copy link
Collaborator Author

PR Review Feedback Addressed

All issues from the code review have been addressed in commit 7724eb5:

Issue 1: User-facing text still references "merchant settings"

Status: ✅ Fixed

  • Updated biometric prompt: Use {biometricLabel} to change partner settings
  • Updated PIN verify subtitle: Enter your PIN to save partner settings
  • Note: PIN setup subtitle kept as "merchant settings" per user preference

Issue 2: Migration runs on every app start

Status: ✅ Fixed

  • Added migration_partner_api_key_completed flag in MMKV storage
  • Migration now returns early if flag is set (fast sync check)
  • Moved migration to onRehydrateStorage to fix race condition with defaults

Issue 3: Comment reference outdated in payment.ts

Status: ✅ Fixed

  • Updated JSDoc to: "Get API headers for authenticated requests"
  • Updated @throws to reference "partner API key"

Additional Improvements

  • Added test coverage for migration function (5 test cases)
  • Simplified _layout.tsx by moving migration logic to store
  • Fixed potential race condition between migration and env defaults

ignaciosantise and others added 2 commits January 30, 2026 15:41
Reverted user-facing PIN and biometric prompt text to use "merchant settings"
instead of "partner settings" per user preference.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add an info log entry to track when the partner API key migration
from the old merchant_api_key is performed. This helps with debugging
and monitoring app updates in production.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.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.

2 participants