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): [coinbase] currency unit conversion #2658

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

DhairyaMajmudar
Copy link

@DhairyaMajmudar DhairyaMajmudar commented Oct 20, 2023

Type of Change

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

Description

This pull request introduces the get_currecny_unit from ConnectorCommon trait for CoinBase. This function allows connectors to declare their accepted currency unit as either "Base" or "Minor" .For coinbase it accepts currency as Base.

Additional Changes

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

How did you test it?

We need to create a payment using coinbase and test the currency unit from connector response and connector dashboard.

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

@DhairyaMajmudar DhairyaMajmudar requested a review from a team as a code owner October 20, 2023 17:28
@DhairyaMajmudar
Copy link
Author

@swangi-kumari @VedantKhairnar pls. take a look at my PR and suggest the changes needed.

@SanchithHegde SanchithHegde linked an issue Oct 22, 2023 that may be closed by this pull request
2 tasks
@SanchithHegde SanchithHegde added A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement hacktoberfest Issues that are up for grabs for Hacktoberfest participants S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Oct 22, 2023
@SanchithHegde
Copy link
Member

@DhairyaMajmudar Could you address the failing CI checks?

@swangi-kumari
Copy link
Contributor

Hello @DhairyaMajmudar ,

Pls address the CI fails

Run cargo +nightly fmt for formatting.

and cargo run --features openapi -- generate-openapi-spec for validating generated openApi spec file. And commit it.

@DhairyaMajmudar
Copy link
Author

Hello @DhairyaMajmudar ,

Pls address the CI fails

Run cargo +nightly fmt for formatting.

and cargo run --features openapi -- generate-openapi-spec for validating generated openApi spec file. And commit it.

Sure I will do the same
Thank you

@DhairyaMajmudar
Copy link
Author

@swangi-kumari ma'am I have done the suggested changes even though its showing some run fails regarding formating and pr message.

@deepanshu-iiitu
Copy link
Contributor

Hi @DhairyaMajmudar
Please address the PR comments as tomorrow is the last day of Hacktoberfest.

@DhairyaMajmudar
Copy link
Author

Hi @DhairyaMajmudar Please address the PR comments as tomorrow is the last day of Hacktoberfest.

Very Sorry sir for my inactivity I will surely follow do the req. changes and commit it in the pr.

@DhairyaMajmudar
Copy link
Author

@SamraatBansal I have made the suggested changes pls. review them.

@swangi-kumari
Copy link
Contributor

Hello @DhairyaMajmudar ,

Pls address the CI fails

Run cargo +nightly fmt for formatting.
Run cargo clippy and addressed lints thrown by it.

fn try_from(item: &types::PaymentsAuthorizeRouterData) -> Result<Self, Self::Error> {
get_crypto_specific_payment_data(item)
fn try_from(item:&CoinbseRouterData<&types::PaymentsAuthorizeRouterData) -> Result<Self, Self::Error> {
get_crypto_specific_payment_data(item.RouterData)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @DhairyaMajmudar

You have to change the definition of get_crypto_specific_payment_data such that it starts accepting CoinbseRouterData<&types::PaymentsAuthorizeRouterData> as an argument rather than passing item.router_data.

Please refer to this for more context similar change was made. https://github.com/juspay/hyperswitch/pull/2645/files

Copy link
Author

Choose a reason for hiding this comment

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

I am a bit confused in this change can you pls. perform this task to remove the confusion

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @DhairyaMajmudar
I cannot perform this task for you, I have referred you with an already merged PR you just have to follow it and replicate it.

Basically you just have to change the type of the argument which the function is accepting.

@swangi-kumari
Copy link
Contributor

Hey @DhairyaMajmudar ,
Thanks for your interest in contributing to hyperswitch.
Let us know if you need any assistance from our end.
Also, even if hacktoberfest is over, we should celebrate open source everyday and we are open for more contributions from you.
We would still be rewarding folks with goodies even if the PR gets merged post hacktoberfest.
May the Source be with you!

@AkshayaFoiger AkshayaFoiger removed the S-waiting-on-author Status: This PR is incomplete or needs to address review comments label Nov 2, 2023
@AkshayaFoiger AkshayaFoiger added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed S-waiting-on-author Status: This PR is incomplete or needs to address review comments and removed S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels Nov 2, 2023
@swangi-kumari
Copy link
Contributor

Hey @DhairyaMajmudar ,
Are you still working on this issue?
Please let us know if you need any help

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 hacktoberfest Issues that are up for grabs for Hacktoberfest participants S-waiting-on-author Status: This PR is incomplete or needs to address review comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE]: [Coinbase] Currency Unit Conversion
6 participants