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

refactor(connector): Use connector_response_reference_id for Shift4 #2492

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

daniloff200
Copy link
Contributor

@daniloff200 daniloff200 commented Oct 8, 2023

Type of Change

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

Description

fixes #2316

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

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

@daniloff200 daniloff200 requested a review from a team as a code owner October 8, 2023 00:13
@github-actions github-actions bot added the S-conventions-not-followed Status: This PR does not follow contributing guidelines label Oct 8, 2023
@daniloff200 daniloff200 force-pushed the fix-2316 branch 3 times, most recently from 5a0dc69 to 966fdea Compare October 8, 2023 11:55
@daniloff200 daniloff200 changed the title Use connector_request_reference_id as reference to the connector for … refactor(connector): Use connector_request_reference_id for Shift4 Oct 8, 2023
@github-actions github-actions bot removed the S-conventions-not-followed Status: This PR does not follow contributing guidelines label Oct 8, 2023
@daniloff200 daniloff200 force-pushed the fix-2316 branch 3 times, most recently from 4285b57 to f011034 Compare October 8, 2023 12:44
@srujanchikke srujanchikke added A-connector-integration Area: Connector integration S-waiting-on-review Status: This PR has been implemented and needs to be reviewed C-refactor Category: Refactor hacktoberfest Issues that are up for grabs for Hacktoberfest participants labels Oct 8, 2023
@srujanchikke
Copy link
Contributor

srujanchikke commented Oct 8, 2023

Hey @daniloff200 ,

Please use the following cargo command -

cargo clippy --all-features

ref - Cargo commands, and fix the errors thrown by clippy for the checks to pass .

Please run cargo +nightly fmt to fix failing formatting check

@daniloff200 daniloff200 force-pushed the fix-2316 branch 2 times, most recently from 3cedd88 to a270a9d Compare October 8, 2023 13:24
@daniloff200
Copy link
Contributor Author

hi, @srujanchikke . Thanks for your review! I've executed the commands

@daniloff200 daniloff200 force-pushed the fix-2316 branch 10 times, most recently from 0da02d6 to 71ccad7 Compare October 10, 2023 19:41
@daniloff200
Copy link
Contributor Author

hi, @srujanchikke , could you help me with an issue, I have here? I've tried various fixes, but, got a problem with a Some (as here -> https://github.com/juspay/hyperswitch/actions/runs/6459469231/job/17538567259?pr=2492) or with borrowed value.
I'm kinda new to Rust, and tried different approaches, but, it doesn't work

@daniloff200 daniloff200 force-pushed the fix-2316 branch 3 times, most recently from 2f91fec to 7fc6db9 Compare October 18, 2023 17:59
Copy link
Contributor

@srujanchikke srujanchikke left a comment

Choose a reason for hiding this comment

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

As this PR addressing #2347 , i am reassigning the issue , So could you please comment on this issue #2347

@@ -671,6 +671,8 @@ impl<F>
types::PaymentsResponseData,
>,
) -> Result<Self, Self::Error> {
let token_id = types::ResponseId::ConnectorTransactionId(item.response.token.id.clone());
Copy link
Contributor

Choose a reason for hiding this comment

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

Shift4ThreeDsResponse don't have field id , Remove token_id

@@ -686,7 +688,7 @@ impl<F>
..item.data.request
},
response: Ok(types::PaymentsResponseData::TransactionResponse {
resource_id: types::ResponseId::NoResponseId,
resource_id: token_id,
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert this change too

@srujanchikke srujanchikke changed the title refactor(connector): Use connector_request_reference_id for Shift4 refactor(connector): Use connector_response_reference_id for Shift4 Oct 19, 2023
@daniloff200 daniloff200 force-pushed the fix-2316 branch 6 times, most recently from 20a28dd to 408a75a Compare October 20, 2023 09:12
@daniloff200
Copy link
Contributor Author

@srujanchikke can you review my pr please? I've managed to fix all issues

Copy link
Contributor

@srujanchikke srujanchikke left a comment

Choose a reason for hiding this comment

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

minor changes required , other than that looks good to me !

@@ -671,6 +671,7 @@ impl<F>
types::PaymentsResponseData,
>,
) -> Result<Self, Self::Error> {
let token_id = Some(item.response.token.id.clone());
Copy link
Contributor

Choose a reason for hiding this comment

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

This token id is not connector response id , it is shift4 3ds token id . revert the changes from shift4 try_from .

Copy link
Contributor

@srujanchikke srujanchikke left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@SanchithHegde SanchithHegde left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Thanks for the PR, @daniloff200!

@SanchithHegde SanchithHegde added this pull request to the merge queue Oct 21, 2023
@SanchithHegde SanchithHegde added hacktoberfest-accepted Pull requests accepted as Hacktoberfest contributions and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Oct 21, 2023
Merged via the queue into juspay:main with commit 83f0062 Oct 21, 2023
10 checks passed
@daniloff200 daniloff200 deleted the fix-2316 branch October 21, 2023 12:47
@swangi-kumari
Copy link
Contributor

Hey @daniloff200 ,
Thanks a bunch for all your contributions! We've got a little something for you to show our appreciation.
Just take a moment to fill out this form, and get ready for some awesome swag coming your way.
Thanks!

@daniloff200
Copy link
Contributor Author

daniloff200 commented Oct 23, 2023

@swangi-kumari hey!
Thanks for the link, I've submitted the form. One question: can it be the issue, if I live in Ukraine?
because previous year, for example, the Digital Ocean were not shipping swag here, because of the military actions here

@VedantKhairnar
Copy link
Contributor

image
Hey @daniloff200 ,
We are using DHL courier service and they do provide service in Ukraine as shown in the above image.
If we face any issues, we will reach out to you. If you have any queries, pls let us know.
Thanks.

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-refactor Category: Refactor hacktoberfest Issues that are up for grabs for Hacktoberfest participants hacktoberfest-accepted Pull requests accepted as Hacktoberfest contributions
Projects
None yet
5 participants