Skip to content

feat(manual-update):add amount captured#13151

Open
Nithin1506200 wants to merge 1 commit into
mainfrom
manual-update-amount
Open

feat(manual-update):add amount captured#13151
Nithin1506200 wants to merge 1 commit into
mainfrom
manual-update-amount

Conversation

@Nithin1506200

@Nithin1506200 Nithin1506200 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

  • Update amount captured in manual-update api to update Amount captured also

  • Automatic Calculation: Derives captured amount from attempt's amount_to_capture

  • Validation: Ensures data integrity with comprehensive checks

  • Idempotency Protection: Prevents duplicate updates if amount already captured

NOTE not applicable for multi capture

When update_amount_captured=true:

  • Calculates amount_captured = amount_to_capture from payment attempt
  • Calculates amount_capturable = current_capturable - amount_captured
  • Prevents providing amount_capturable directly when flag is true
  • Validates terminal statuses (Charged, PartialCharged, etc.)
  • Prevents update if amount_captured already exists on payment intent (> 0)

use case

in any case where capture is initiated via hs but we didnt receive Webhooks this feature will be useful

Test case

Make a manual capture EG: Adyen which has processing status

{
    "payment_id": "pay_COKyWZOaVnS77fEkOuls",
    "merchant_id": "merchant_1776936367",
    "status": "processing",
    "amount": 123,
    "net_amount": 123,
    "shipping_cost": null,
    "amount_capturable": 123,
    "amount_received": null,
    "processor_merchant_id": "merchant_1776936367",
    "initiator": null,
    "sdk_authorization": "cHJvZmlsZV9pZD1wcm9fUkpFTzhCZjlicmZEZVNSdGZOTHMscHVibGlzaGFibGVfa2V5PXBrX2Rldl84NTU3NzNlZDBmZjA0Y2UyOTAwNzBhOTIyOWY2Y2RjOCxjbGllbnRfc2VjcmV0PXBheV9DT0t5V1pPYVZuUzc3ZkVrT3Vsc19zZWNyZXRfQ2dxbHVpY2VITVk3QWVMUnNlazQsY3VzdG9tZXJfaWQ9bml0aHh4ZGlubixwYXltZW50X2lkPXBheV9DT0t5V1pPYVZuUzc3ZkVrT3Vscw==",
    "connector": "adyen",
    "state_metadata": null,
    "client_secret": "pay_COKyWZOaVnS77fEkOuls_secret_CgqluiceHMY7AeLRsek4",
    "created": "2026-04-29T12:46:47.076Z",
    "modified_at": "2026-04-29T12:47:04.051Z",
    "currency": "EUR",
    "customer_id": "nithxxdinn",
    "customer": {
        "id": "nithxxdinn",
        "name": null,
        "email": "hello@gmail.com",
        "phone": null,
        "phone_country_code": null,
        "customer_document_details": null
    },
    "description": "hellow world",
    "refunds": null,
    "disputes": null,
    "mandate_id": null,
    "mandate_data": null,
    "setup_future_usage": "off_session",
    "off_session": null,
    "capture_on": null,
    "capture_method": "manual",
    "payment_method": "wallet",
    "payment_method_data": {
        "wallet": {
            "google_pay": {
                "last4": "Discover 9319",
                "card_network": "VISA",
                "type": "CARD",
                "card_exp_month": "01",
                "card_exp_year": "30",
                "auth_code": "095690",
                "email": null
            }
        },
        "billing": null
    },
    "payment_token": null,
    "shipping": null,
    "billing": {
        "address": {
            "city": "Fasdf",
            "country": "US",
            "line1": "Fasdf",
            "line2": "Fasdf",
            "line3": null,
            "zip": "560095",
            "state": null,
            "first_name": "Sakil",
            "last_name": "Mostak",
            "origin_zip": null
        },
        "phone": null,
        "email": "test@gmail.com"
    },
    "order_details": null,
    "email": "hello@gmail.com",
    "name": null,
    "phone": null,
    "return_url": "https://www.google.com/",
    "authentication_type": "three_ds",
    "statement_descriptor_name": null,
    "statement_descriptor_suffix": null,
    "next_action": null,
    "cancellation_reason": null,
    "error_code": null,
    "error_message": null,
    "unified_code": null,
    "unified_message": null,
    "error_details": null,
    "payment_experience": null,
    "payment_method_type": "google_pay",
    "connector_label": null,
    "business_country": null,
    "business_label": "default",
    "business_sub_label": null,
    "allowed_payment_method_types": null,
    "manual_retry_allowed": null,
    "connector_transaction_id": "JBZG2W9MJK33BWV5",
    "frm_message": null,
    "metadata": {
        "capture_delay_hours": null
    },
    "connector_metadata": null,
    "connector_response_metadata": null,
    "feature_metadata": {
        "redirect_response": null,
        "search_tags": null,
        "apple_pay_recurring_details": null,
        "pix_additional_details": null,
        "boleto_additional_details": null,
        "pix_automatico_additional_details": null
    },
    "reference_id": "pay_COKyWZOaVnS77fEkOuls_1",
    "payment_link": null,
    "profile_id": "pro_RJEO8Bf9brfDeSRtfNLs",
    "surcharge_details": null,
    "attempt_count": 1,
    "merchant_decision": null,
    "merchant_connector_id": "mca_2YeKNZcBerurPhduQ4aG",
    "incremental_authorization_allowed": false,
    "authorization_count": null,
    "incremental_authorizations": null,
    "external_authentication_details": null,
    "external_3ds_authentication_attempted": false,
    "expires_on": "2026-04-29T13:01:47.076Z",
    "fingerprint": null,
    "browser_info": {
        "language": "en-US",
        "time_zone": 330,
        "ip_address": "192.168.1.1",
        "user_agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/114.0.0.0 Safari/537.36",
        "color_depth": 24,
        "java_enabled": false,
        "screen_width": 1920,
        "accept_header": "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8",
        "screen_height": 1080,
        "java_script_enabled": true
    },
    "payment_channel": null,
    "payment_method_id": "pm_nFuLTzZsfX7yrY82gsH9",
    "network_transaction_id": "766480594892094",
    "payment_method_status": null,
    "updated": "2026-04-29T12:47:04.051Z",
    "split_payments": null,
    "frm_metadata": null,
    "extended_authorization_applied": null,
    "extended_authorization_last_applied_at": null,
    "request_extended_authorization": null,
    "capture_before": null,
    "merchant_order_reference_id": null,
    "order_tax_amount": null,
    "connector_mandate_id": null,
    "card_discovery": null,
    "force_3ds_challenge": false,
    "force_3ds_challenge_trigger": false,
    "issuer_error_code": null,
    "issuer_error_message": null,
    "is_iframe_redirection_enabled": null,
    "whole_connector_response": null,
    "enable_partial_authorization": null,
    "enable_overcapture": null,
    "is_overcapture_enabled": null,
    "network_details": null,
    "is_stored_credential": null,
    "mit_category": null,
    "billing_descriptor": null,
    "tokenization": null,
    "partner_merchant_identifier_details": null,
    "payment_method_tokenization_details": null,
    "installment_options": null,
    "installment_data": null
}

req

curl --location --request PUT 'http://localhost:8080/payments/pay_COKyWZOaVnS77fEkOuls/manual-update' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'api-key: test_admin' \
--header 'x-merchant-id: merchant_1776936367' \
--data '{
    "attempt_id": "pay_COKyWZOaVnS77fEkOuls_1",
    "merchant_id": "merchant_1776936367",
    "attempt_status": "charged",
  "update_amount_captured":true
}'

response

{
    "payment_id": "pay_COKyWZOaVnS77fEkOuls",
    "attempt_id": "pay_COKyWZOaVnS77fEkOuls_1",
    "merchant_id": "merchant_1776936367",
    "attempt_status": "charged",
    "error_code": null,
    "error_message": null,
    "error_reason": null,
    "connector_transaction_id": "JBZG2W9MJK33BWV5",
    "amount_capturable": 0,
    "amount_captured": 123
}

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

@Nithin1506200 Nithin1506200 requested review from a team as code owners July 3, 2026 07:22
@semanticdiff-com

semanticdiff-com Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review changes with  SemanticDiff

Changed Files
File Status
  crates/hyperswitch_domain_models/src/payments/payment_intent.rs  70% smaller
  crates/diesel_models/src/payment_intent.rs  70% smaller
  crates/router/src/core/payments.rs  43% smaller
  crates/api_models/src/payments.rs  0% smaller

@Nithin1506200 Nithin1506200 self-assigned this Jul 3, 2026
@XyneSpaces

Copy link
Copy Markdown

⚠️ Missing #[instrument] on async hot-path function

The new update_amount_captured async function in crates/router/src/core/payments.rs (~11780-11973) lacks #[instrument(skip_all, fields(...))]. This is on the payment core path and should be instrumented for observability.

Add #[instrument(skip_all, fields(payment_id = %payment_intent.id))] or similar to ensure tracing coverage.

@XyneSpaces

Copy link
Copy Markdown

Review Findings

[should-fix] Validation timing

The validation for update_amount_captured being true while amount_capturable is also provided happens AFTER the payment_intent is fetched from the database (line ~11832). Consider moving this validation earlier to avoid unnecessary DB calls when the request is invalid.

[should-fix] amount_capturable calculation consistency

When update_amount_captured is true, the code calculates new_capturable = payment_attempt.amount_capturable - captured (line ~11878). However, payment_attempt.amount_capturable may have been modified by previous operations. Verify this calculation aligns with the expected semantics when multiple partial captures exist.

[nit] Error message formatting

The error message at line ~11876 uses {:?} formatting for MinorUnit, which may expose internal struct representation. Consider using a display format if available.

@XyneSpaces

Copy link
Copy Markdown

⚠️ Race condition risk in amount_captured validation

The validation on line ~11854 checks if amount_captured > MinorUnit::zero() to prevent updates when already captured. However, this read-modify-write pattern isn't atomic. Concurrent manual update calls could race:

  1. Request A reads amount_captured = 0
  2. Request B reads amount_captured = 0
  3. Both requests pass validation
  4. Both requests update amount_captured

Consider wrapping this in a database-level check or acquiring a row-level lock on the payment intent during the update operation.

// Current pattern (raceable):
if let Some(existing) = payment_intent.amount_captured {
    if existing > MinorUnit::zero() { return Err(...); }
}

// Safer: include in the UPDATE WHERE clause

@Nithin1506200 Nithin1506200 force-pushed the manual-update-amount branch from 81df059 to 5e4d1ad Compare July 3, 2026 11:23
Comment on lines +11863 to +11868
if let Some(existing_captured) = payment_intent.amount_captured {
if existing_captured > MinorUnit::zero() {
return Err(errors::ApiErrorResponse::InvalidRequestData {
message: "Cannot update amount_captured: payment intent already has amount_captured > 0".to_string(),
}.into());
}

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.

please link an issue that this only helps in case of single capture, manual multiple capture will not work with it

@XyneSpaces

Copy link
Copy Markdown

🚨 Validation logic allows bypassing amount_capturable check through None path

The validation at lines 11835-11871 checks if amount_capturable.is_some() but the error is only returned when the value is Some. If attempt_status is also None, all validations are skipped and calculated_amount_captured proceeds with the update.

This creates a path where update_amount_captured=true with amount_capturable=None and attempt_status=None bypasses all validation and updates the captured amount. Ensure validation is comprehensive regardless of which fields are present.

@XyneSpaces

Copy link
Copy Markdown

⚠️ Missing #[instrument] attribute on async function handling manual payment updates

The update_payment function at lines ~11780 in crates/router/src/core/payments.rs handles sensitive payment operations (updating captured amounts) but lacks tracing instrumentation. This makes it difficult to trace manual payment updates in production.

Add #[instrument(skip_all, fields(payment_id, merchant_id))] to enable observability for this critical operation.

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.

4 participants