-
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): [DATATRANS] Implement card payments #5028
feat(connector): [DATATRANS] Implement card payments #5028
Conversation
d2edc7e
to
95ebb7b
Compare
reason: response.reason, | ||
code: error_code, | ||
message: error_message.clone(), | ||
reason: Some(error_message), |
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.
Remove the some
here as mentioned in the previous comment.
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.
reason is Option here
and
error_message is 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.
Ok
.unwrap_or(consts::NO_ERROR_CODE.to_string()); | ||
let error_message = response | ||
.error | ||
.as_ref() | ||
.and_then(|e| e.message.clone()) | ||
.map(|e| e.message.clone()) |
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.
Move this unwrap_or
to the message
field below. And remove the some
used for reason
}, | ||
}; | ||
|
||
#[derive(Default, Debug, Clone, Serialize, Deserialize)] | ||
pub struct DatatransErrorResponse { | ||
pub error: Option<DatatransError>, |
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.
Why is this made option?
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.
Removed That
#[derive(Serialize, Deserialize, Clone, Debug)] | ||
#[serde(rename_all = "camelCase")] | ||
pub struct PlainCardDetails { | ||
#[serde(rename = "type", default = "default_res_type")] |
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.
This hard coding should've been done in the try_from.
pub enum CardType { | ||
#[serde(rename = "card")] | ||
PLAIN(PlainCardDetails), | ||
PlainCardDetails, | ||
ALIAS, | ||
NetworkTOKEN, | ||
DeviceTOKEN, | ||
} |
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.
Remove this unused enum
_ => common_enums::AttemptStatus::Failure, | ||
}, | ||
DataTransCaptureResponse::Error(error) => { | ||
if error.message == *"already settled" { |
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.
if error.message == *"already settled" { | |
if error.message == "already settled".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.
warning: this creates an owned instance just for comparison
--> crates/router/src/connector/datatrans/transformers.rs:517:37
|
517 | if error.message == "already settled".to_string() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: *"already settled"
Was getting this warning while running the
cargo clippy --all-features --all-targets
{ | ||
"transaction already canceled" => common_enums::AttemptStatus::Voided, | ||
_ => common_enums::AttemptStatus::Failure, | ||
if error.message == *"transaction already canceled" { |
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.
if error.message == *"transaction already canceled" { | |
if error.message == "transaction already canceled".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.
Please maintain constants for such strings
We also need to implement the new amount conversion framework. |
54fed06
to
30f1c61
Compare
5cee8bd
to
a622ab6
Compare
Ok(format!( | ||
"{}v1/transactions/{}", | ||
self.base_url(connectors), | ||
connector_payment_id | ||
)) |
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.
here
{ | ||
"transaction already canceled" => common_enums::AttemptStatus::Voided, | ||
_ => common_enums::AttemptStatus::Failure, | ||
if error.message == *"transaction already canceled" { |
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 maintain constants for such strings
28afc6f
to
abd9739
Compare
5bcafa8
to
dd2b801
Compare
029c4b1
dd2b801
to
029c4b1
Compare
@@ -23,6 +24,7 @@ const connectorDetails = { | |||
paypal: paypalConnectorDetails, | |||
stripe: stripeConnectorDetails, | |||
trustpay: trustpayConnectorDetails, | |||
datatrans:datatransConnectorDetails |
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.
- Was this connector tested?
- Since this is a new connector, tests against other connectors needs to be run as well just to see if it affects other connectors although it shouldn't be. Was that done?
- Looks like lints were not fixed either
- And yes, creds are not updated
* 'main' of github.com:juspay/hyperswitch: (25 commits) fix(logs): ignore request headers while logging (#5273) feat(webhooks): add support for custom outgoing webhook http headers (#5275) fix(payment_methods): set `requires_cvv` to false when either `connector_mandate_details` or `network_transaction_id` is present during MITs (#5331) chore: create justfile for running commands for v1 and v2 migrations (#5325) fix(routing): do not update `perform_session_flow_routing` output if the `SessionRoutingChoice` is none (#5336) fix(database): modified_at updated for every state change for Payment Attempts (#5312) feat(mca): Added recipient connector call for open banking connectors (#3758) chore(version): 2024.07.16.0 refactor(connector): [Mifinity] add a field language_preference in payment request for mifinity payment method data (#5326) fix(router): store `customer_acceptance` in payment_attempt, use it in confirm flow for delayed authorizations like external 3ds flow (#5308) feat(proxy): add support to pass proxy bypass urls from configs (#5322) Docs: Updating Error codes in API-ref (#5296) feat(core): [Payouts] Add retrieve flow for payouts (#4936) fix(connector): [AUTHORIZEDOTNET] Populate error reason for failure transactions (#5319) chore(version): 2024.07.15.0 feat(logging): Emit a setup error when a restricted keys are used for logging default keys (#5185) feat(payment_methods): add support to migrate existing customer PMs from processor to hyperswitch (#5306) feat(connector): [DATATRANS] Implement card payments (#5028) chore: making of function create_encrypted_data (#5251) fix(payments): populate merchant order ref id in list (#5310) ...
Type of Change
Description
Implemented Card Payments for Datatrans
Additional Changes
Motivation and Context
#5027
How did you test it?
Attached Postman Collection
postman/collection-json/datatrans.postman_collection.json
Checklist
cargo +nightly fmt --all
cargo clippy