-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(connector): [Itau Bank] Add payment and sync flow for Pix #5342
Conversation
}) | ||
} | ||
_ => Err( | ||
errors::ConnectorError::NotImplemented("Payment methods".to_string()) |
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.
errors::ConnectorError::NotImplemented("Payment methods".to_string()) | |
errors::ConnectorError::NotImplemented("Payment method".to_string()) |
devedor, | ||
}) | ||
} | ||
_ => Err( |
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 not use _=>
instead do a match on all payment method types?
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.
Please add cypress test cases
), | ||
( | ||
headers::USER_AGENT.to_string(), | ||
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36".to_string().into(), |
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.
Should not be hard_coded, can take this from browser_info, if a required field
), | ||
( | ||
headers::ACCEPT.to_string(), | ||
"text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.".to_string().into(), |
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.
Does all these types are required to mentioned?
), | ||
( | ||
headers::ACCEPT.to_string(), | ||
"text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.".to_string().into(), |
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.
We probably don't need all these types. And this doesn't seem to have JSON included here?
@@ -414,6 +414,7 @@ pub struct PaymentsSyncData { | |||
pub payment_method_type: Option<storage_enums::PaymentMethodType>, | |||
pub currency: storage_enums::Currency, | |||
pub payment_experience: Option<common_enums::PaymentExperience>, | |||
pub browser_info: Option<BrowserInformation>, |
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.
Let's not use this browser information object, let's hard-code the user agent instead.
CC: @Narayanbhat166
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.
Yes, we should attach the user agent of our reqwest client here and not the one that we receive from the user
let req = Some( | ||
services::RequestBuilder::new() | ||
.method(services::Method::Post) | ||
.headers(types::RefreshTokenType::get_headers(self, req, connectors)?) |
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 attach default headers as well?
) -> CustomResult<String, errors::ConnectorError> { | ||
Err(errors::ConnectorError::NotImplemented("get_url method".to_string()).into()) | ||
Ok(format!( | ||
"{}itau-ep9-gtw-pix-recebimentos-ext-v2/v2/cob", |
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.
Hard-coding this part of the URL can require us significant build time if and when we are required to update this URL. Can we have this part of the URL being read from configs?
card_pm: { | ||
ZeroAuthMandate: { | ||
Response: { | ||
status: 500, |
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.
Is this intentional? 4xx is expected here if I'm not wrong. Also, this can be picked from defaults which is in commons.js
country_code: "+91", | ||
}, | ||
}, | ||
currency: "BRL", |
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.
Since currency is passed in payment intent, this can be removed.
break; | ||
default: | ||
verifyReturnUrl(redirection_url, expected_url, true); | ||
// expected_redirection can be used here to handle other payment methods |
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.
please check the lints
switch (payment_method_type) { | ||
case "pix": | ||
fetchAndParseImageData(redirection_url).then((qrCodeData) => { | ||
console.log("Imaage_data>>>>>>>>>", qrCodeData) |
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.
console.log()
can be removed as it is no longer necessary.
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.
Since these are minor changes, Im putting them in the subsequent pr for refund
* 'main' of github.com:juspay/hyperswitch: (27 commits) refactor(connector): added amount conversion framework for billwerk (#4972) feat(connector): [Itaubank] Add refund and rsync flow (#5420) feat: create additional columns in organization table (#5380) refactor(merchant_id): create domain type for `merchant_id` (#5408) fix(euclid): remove business_profile routing feature flag (#5430) feat: add create retrieve and update api endpoints for organization resource (#5361) chore(version): 2024.07.24.0 refactor(connector): [Itaubank] add dynamic fields for pix (#5419) Feat(connector): [WELLSFARGO] Add template code (#5333) fix(connector): [Datatrans] Handling for 4-Digit YYYY input and Correct 3DS Routing to no_3ds (#5410) refactor(connector): add amount conversion framework to volt (#4985) chore(users): email templates footer icon style enhance (#5375) feat(customer): customer v2 refactor for customer create end point (#5350) chore(version): 2024.07.23.0 fix(router): store `network_transaction_id` in stripe `authorize` flow (#5399) ci: handle packages to run are being empty (#5403) chore: add customer, shipping and billing details to payment_response for payment list api (#5401) refactor(dashboard_metadata): alter query for merchant scoped metadata (#5397) refactor(connector): Add billing_country in klarna dynamic fields (#5373) feat(connector): [Itau Bank] Add payment and sync flow for Pix (#5342) ...
Type of Change
Description
Add payment and sync flow for Itau Bank connector
Additional Changes
Motivation and Context
How did you test it?
Through Cypress:
Checklist
cargo +nightly fmt --all
cargo clippy