-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(connector): [SANTANDER] Added Authorize, PSync, Void, Refund & RSync Flows for Pix QR Code Bank Transfer #8463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
80eb4d9
to
0a06ad9
Compare
router_data.request.integrity_object = Some(response_integrity_object); | ||
router_data | ||
}) | ||
.change_context(errors::ConnectorError::ResponseHandlingFailed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested fix (NIT)
Apply change_context before calling Map()
new_router_data
.change_context(errors::ConnectorError::ResponseHandlingFailed)
.map(|mut router_data| {
router_data.request.integrity_object = Some(response_integrity_object);
router_data
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no map only works on Result and Options and once we do change_context the Result is unwrapped so we wont be able to use .map then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Result is not upwrapped unless you apply ?
.change_context(errors::ConnectorError::ResponseHandlingFailed)
will only change the error type. It will still be Result
.change_context() does NOT unwrap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, made the relevant changes
router_data.request.integrity_object = Some(response_integrity_object); | ||
router_data | ||
}) | ||
.change_context(errors::ConnectorError::ResponseHandlingFailed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
router_data.request.integrity_object = Some(response_integrity_object); | ||
router_data | ||
}) | ||
.change_context(errors::ConnectorError::ResponseHandlingFailed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
) -> CustomResult<String, errors::ConnectorError> { | ||
Err(errors::ConnectorError::NotImplemented("get_url method".to_string()).into()) | ||
let connector_metadata = req.request.connector_metadata.clone(); | ||
let end_to_end_id = match &connector_metadata { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
We can use .and_then() instead of nested match cases
pub struct SantanderValue { | ||
pub original: StringMajorUnit, | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SantanderValue
and SantanderValueResponse
both are same structs
can we use a single struct instead
@@ -78,6 +78,7 @@ pub struct PaymentsAuthorizeData { | |||
pub additional_payment_method_data: Option<AdditionalPaymentData>, | |||
pub merchant_account_id: Option<Secret<String>>, | |||
pub merchant_config_currency: Option<storage_enums::Currency>, | |||
pub santander_pix_qr_expiration_time: Option<api_models::payments::SantanderBillingType>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we confirm with core folks if we should be passing connector specific config here
@SanchithHegde
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing this
Add test cases in PR description |
Cannot be tested because we are still awaiting on creds from Zurich team |
crates/hyperswitch_connectors/src/connectors/santander/transformers.rs
Outdated
Show resolved
Hide resolved
crates/payment_methods/src/configs/payment_connector_required_fields.rs
Outdated
Show resolved
Hide resolved
b4373fd
@@ -18,6 +18,8 @@ dummy_connector = [ | |||
] | |||
|
|||
[dependencies] | |||
chrono = "0.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bsayak03 @awasthi21 Prefer to use the time
crate over chrono
since we're using it everywhere else in our codebase...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, i'll replace it in the next PR
Type of Change
Description
Additional Changes
Motivation and Context
Ref Doc : https://developer.santander.com.br/api/documentacao/pix-visao-geral#/api/documentacao/pix-visao-geral/
https://developer.santander.com.br/api/user-guide/pix-visao-geral
How did you test it?
This PR cannot be tested because we are still awaiting on creds
Checklist
cargo +nightly fmt --all
cargo clippy