fix(connector): [STRIPE] Retrieving Connect Account Id from Mandate Metadata in MITs #8326
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review changes with Changed Files
|
deepanshu-iiitu
requested changes
Jun 12, 2025
crates/storage_impl/src/errors.rs
Outdated
| @@ -193,6 +193,8 @@ pub enum ConnectorError { | |||
| InvalidDataFormat { field_name: &'static str }, | |||
| #[error("Payment Method data / Payment Method Type / Payment Experience Mismatch ")] | |||
| MismatchedPaymentData, | |||
| #[error("transfer_account_id/application_fees/charge_type sent in request doesn't match with the values used during mandate creation ")] | |||
There was a problem hiding this comment.
Use generic field name instead of transfer_account_id/application_fees/charge_type
There was a problem hiding this comment.
Do you mean to say i should make the error message Dynamic and at the connector level populate the message?
There was a problem hiding this comment.
like we do for FlowNotSupported
deepanshu-iiitu
requested changes
Jun 13, 2025
deepanshu-iiitu
requested changes
Jun 13, 2025
| @@ -290,6 +290,8 @@ pub enum ApiErrorResponse { | |||
| InvalidPlatformOperation, | |||
| #[error(error_type = ErrorType::InvalidRequestError, code = "IR_45", message = "External vault failed during processing with connector")] | |||
| ExternalVaultFailed, | |||
| #[error(error_type = ErrorType::InvalidRequestError, code = "IR_46", message = "Fields like {fields} doesn't match with the ones used during mandate creation")] | |||
There was a problem hiding this comment.
Suggested change
| #[error(error_type = ErrorType::InvalidRequestError, code = "IR_46", message = "Fields like {fields} doesn't match with the ones used during mandate creation")] | |
| #[error(error_type = ErrorType::InvalidRequestError, code = "IR_46", message = "Field {fields} doesn't match with the ones used during mandate creation")] |
| @@ -651,6 +653,9 @@ impl ErrorSwitch<api_models::errors::types::ApiErrorResponse> for ApiErrorRespon | |||
| Self::ExternalVaultFailed => { | |||
| AER::BadRequest(ApiError::new("IR", 45, "External Vault failed while processing with connector.", None)) | |||
| }, | |||
| Self::MandatePaymentDataMismatch { fields} => { | |||
| AER::BadRequest(ApiError::new("IR", 46, format!("Fields like {fields} doesn't match with the ones used during mandate creation"), Some(Extra {fields: Some(fields.to_owned()), ..Default::default()}))) //FIXME: error message | |||
There was a problem hiding this comment.
Suggested change
| AER::BadRequest(ApiError::new("IR", 46, format!("Fields like {fields} doesn't match with the ones used during mandate creation"), Some(Extra {fields: Some(fields.to_owned()), ..Default::default()}))) //FIXME: error message | |
| AER::BadRequest(ApiError::new("IR", 46, format!("Field {fields} doesn't match with the ones used during mandate creation"), Some(Extra {fields: Some(fields.to_owned()), ..Default::default()}))) //FIXME: error message |
deepanshu-iiitu
requested changes
Jun 13, 2025
crates/storage_impl/src/errors.rs
Outdated
| @@ -193,6 +193,8 @@ pub enum ConnectorError { | |||
| InvalidDataFormat { field_name: &'static str }, | |||
| #[error("Payment Method data / Payment Method Type / Payment Experience Mismatch ")] | |||
| MismatchedPaymentData, | |||
| #[error("Fields like {fields} doesn't match with the ones used during mandate creation")] | |||
There was a problem hiding this comment.
Suggested change
| #[error("Fields like {fields} doesn't match with the ones used during mandate creation")] | |
| #[error("Field {fields} doesn't match with the ones used during mandate creation")] |
deepanshu-iiitu
previously approved these changes
Jun 13, 2025
jagan-jaya
previously approved these changes
Jun 16, 2025
sahkal
reviewed
Jun 16, 2025
| }) | ||
| .and_then(|secret_value| { | ||
| let json_value = secret_value.clone().expose(); | ||
| serde_json::from_value::<Self>(json_value).ok() |
There was a problem hiding this comment.
can we not ignore error here, instead log the error if not populate
| @@ -600,6 +600,9 @@ impl From<errors::ApiErrorResponse> for StripeErrorCode { | |||
| errors::ApiErrorResponse::AddressNotFound => Self::AddressNotFound, | |||
| errors::ApiErrorResponse::NotImplemented { .. } => Self::Unauthorized, | |||
| errors::ApiErrorResponse::FlowNotSupported { .. } => Self::InternalServerError, | |||
| errors::ApiErrorResponse::MandatePaymentDataMismatch { .. } => { | |||
| Self::InternalServerError | |||
There was a problem hiding this comment.
This should be 4xx if we are checking some request data to our internal saved data
Comment on lines
7011
to
7025
| .get_amount_as_i64() | ||
| != 0 | ||
| { | ||
| return Err(errors::ApiErrorResponse::InvalidDataValue { | ||
| field_name: "split_payments.stripe_split_payment.application_fees", | ||
| }); | ||
| } | ||
| } | ||
| api::Amount::Value(amount) => { | ||
| if stripe_split_payment.application_fees.get_amount_as_i64() > amount.into() { | ||
| if stripe_split_payment | ||
| .application_fees | ||
| .as_ref() | ||
| .map_or(MinorUnit::zero(), |amount| *amount) | ||
| .get_amount_as_i64() | ||
| > amount.into() |
There was a problem hiding this comment.
can we please try removing get_amount_as_i64() function from here, also not use it any of the future use cases
bsayak03
dismissed stale reviews from jagan-jaya and deepanshu-iiitu
via
2edd8d0
sahkal
previously approved these changes
Jun 16, 2025
sahkal
reviewed
Jun 16, 2025
crates/common_utils/src/types.rs
Outdated
| @@ -386,6 +386,14 @@ impl AmountConvertor for MinorUnitForConnector { | |||
| #[diesel(sql_type = sql_types::BigInt)] | |||
| pub struct MinorUnit(i64); | |||
|
|
|||
| use std::num::NonZeroI64; | |||
There was a problem hiding this comment.
nit: move this import to top of the file
jagan-jaya
approved these changes
Jun 16, 2025
deepanshu-iiitu
approved these changes
Jun 16, 2025
sahkal
approved these changes
Jun 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of Change
Description
In the CIT call, we store the stripe split payment details inside Mandate Metadata and during the MIT calls we retrieve the split payments data from there and use it in the MIT calls. And if the merchant sends split payment details again in the MIT call, we match it. If its the same we move forward. If different we throw an error.
Additional Changes
Motivation and Context
How did you test it?
Case 1 : In MIT call donot pass Split Object
Do a CIT Call in Stripe :
cURL :
Response:
Do a MIT call now (without passing any split payment object now) :
cURL:
Response:
Case 2: Pass same Split Payment object in MIT
CIT ->
cURL:
Response:
MIT ->
cURL :
Response:
Case 3: Pass Split Payment object in MIT but different from the one which was sent in CIT (conflicted case)
CIT ->
cURL :
Response:
MIT (application_fees passed is different than the one sent in MIT) ->
cURL:
Response:
Checklist
cargo +nightly fmt --allcargo clippy