Skip to content

fix: Validate Razorpay order_id before initiating checkout - #1167

Open
kadhir-17 wants to merge 1 commit into
masterfrom
razorpay-order-id-android-sdk
Open

fix: Validate Razorpay order_id before initiating checkout#1167
kadhir-17 wants to merge 1 commit into
masterfrom
razorpay-order-id-android-sdk

Conversation

@kadhir-17

Copy link
Copy Markdown

When students initiated payments without a valid Razorpay Order ID, money was deducted from their bank account and immediately auto-refunded by Razorpay due to a missing order intent.

To prevent failed checkouts and unexpected auto-refunds, added a pre-checkout validation check ensuring the Order ID is present, non-empty, and strictly follows Razorpay's "order_" prefix convention. If invalid or missing, the SDK now halts checkout immediately, dismisses the loading spinner, and directs the user to the payment failure screen.

Todo:

https://app.basecamp.com/4160028/buckets/48707509/card_tables/cards/10267968559

@claude

claude Bot commented Sep 3, 2026

Copy link
Copy Markdown

Claude finished @kadhir-17's task in 1m 12s —— View job


Claude finished reviewing this PR

Todo List:

  • Gather context (read diff and relevant file)
  • Understand the request
  • Perform code review (Android specifics, code quality, security, performance, testing, docs)
  • Post final review feedback

Summary

This is a small, well-targeted fix: RazorpayPaymentGateway.startPayment() now rejects checkout when order.orderId is null/empty or doesn't start with order_, calling onPaymentError(...) instead. I traced the callback chain into OrderConfirmActivity.onPaymentError(String) (store/src/main/java/in/testpress/store/ui/OrderConfirmActivity.java:399-404) and confirmed it does hide the progress bar and route to PaymentFailureActivity as the PR description claims — the fix behaves as intended and correctly prevents the auto-refund scenario described.

Observations

  1. No test coverage added (store/src/main/java/in/testpress/store/razorpay/RazorpayPaymentGateway.kt:21-28) — there's no existing test file for RazorpayPaymentGateway, and this PR doesn't add one either. Given this fixes a real money-impacting bug, a small unit test (e.g. asserting onPaymentError fires for null/empty/malformed order IDs, and that Checkout.open is not invoked) would guard against regressions. Not blocking, but worth considering.

  2. Minor nit — magic string duplicated logic: the "order_" prefix check is inline; if this convention needs to change or be reused elsewhere later, a small companion constant (e.g. private const val ORDER_ID_PREFIX = "order_") would be marginally cleaner, but it's a single-use case today so this is optional.

  3. Comment verbosity: lines 23-24 are two separate comments explaining the same rationale — could be combined into one line, but this is purely stylistic.

  4. Edge case (very minor): an order ID of exactly "order_" (no suffix) would pass this check. Unlikely in practice since Razorpay always appends an ID, so not worth adding extra validation for.

No security, threading, or architecture concerns — the change is synchronous, UI-thread-only (consistent with the rest of this class), and doesn't introduce any leaks or nullability issues (isNullOrEmpty() safely smart-casts the platform-type String! before startsWith is called).

Overall: solid, minimal fix for a real bug. Recommend merging; test coverage is the only thing I'd suggest adding, either in this PR or a fast follow-up.

Fix this →

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.

1 participant