Skip to content
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): accept connector_transaction_id in error_response of connector flows for Trustpay #3060

Merged
merged 4 commits into from
Dec 7, 2023

Conversation

AkshayaFoiger
Copy link
Contributor

@AkshayaFoiger AkshayaFoiger commented Dec 5, 2023

Type of Change

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

Description

Resolves #3543

How did you test it?

  1. Create a failed payment with cards in trustpay
    Test card number 4200 0000 0000 002
    for further test cards, refer here
    Expected behavior : connector_transaction_id should be populated
Screenshot 2023-12-05 at 8 04 40 PM Screenshot 2023-12-05 at 8 04 48 PM
  1. Create a bank redirect payment with trustpay and fail the payment in redirect. Expected behavior: connector_transaction_id should be populated (Note: For some types error, connector transaction id will not be sent by the connector)
Screenshot 2023-12-05 at 8 05 06 PM

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
  • I added a CHANGELOG entry if applicable

@AkshayaFoiger AkshayaFoiger added A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels Dec 5, 2023
@AkshayaFoiger AkshayaFoiger requested a review from a team as a code owner December 5, 2023 14:33
@AkshayaFoiger AkshayaFoiger self-assigned this Dec 5, 2023
}

#[derive(Deserialize)]
pub struct TrustPayTransactionStatusErrorResponse {
pub status: i64,
pub payment_description: String,
pub instance_id: Option<String>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This struct is being used in PSync are we getting instance_id in PSync also?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we are getting instance_id in PSync response

Screenshot 2023-12-06 at 11 32 06 AM

response
.payment_information
.references
.payment_request_id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for bank redirects do we get payment_request_id instead of instance_id?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we use bank payment_request_id instead of instance_id for bank redirects. Ref page.no 333 of trustpay.rs

@@ -1637,16 +1647,19 @@ pub struct Errors {
}

#[derive(Default, Debug, Serialize, Deserialize, PartialEq)]
#[serde(rename_all = "camelCase")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm TrustPayTransactionStatusErrorResponse also requires camelCase

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the struct TrustPayTransactionStatusErrorResponse is required to be a camelCase
Screenshot 2023-12-06 at 11 49 36 AM

pub struct TrustpayErrorResponse {
pub status: i64,
pub description: Option<String>,
pub errors: Option<Vec<Errors>>,
pub instance_id: Option<String>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't get instance_id for bank redirects failures right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes instance_id is not provided during all error scenarios.

@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Dec 7, 2023
Merged via the queue into main with commit f53b090 Dec 7, 2023
14 of 16 checks passed
@Gnanasundari24 Gnanasundari24 deleted the connector_transaction_id/trustpay branch December 7, 2023 09:38
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement
Projects
No open projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

5 participants