Skip to content

refactor(router): add amount conversion framework to netcetera #6238

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

refactor(router): add amount conversion framework to netcetera #6238

wants to merge 11 commits into from

Conversation

hex1c
Copy link
Contributor

@hex1c hex1c commented Oct 5, 2024

Type of Change

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

Description

Minor refactoring to standardize Netcetera connector to use the conversion framework to reduce potential inconsistencies in managing amounts fixes #5930

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

@hex1c hex1c requested review from a team as code owners October 5, 2024 06:43
@semanticdiff-com SemanticDiff.com
Copy link

semanticdiff-com bot commented Oct 5, 2024 •
edited
Loading

Review changes with  SemanticDiff

Changed Files
File Status
  crates/router/src/types/api/authentication.rs  86% smaller
  crates/router/src/types/api.rs  86% smaller
  crates/router/tests/connectors/netcetera.rs  71% smaller
  crates/router/src/connector/netcetera.rs  40% smaller
  crates/router/src/connector/netcetera/transformers.rs  8% smaller
  crates/router/src/connector/netcetera/netcetera_types.rs  0% smaller

@hex1c hex1c changed the title Netcetra refactor #5930 refactor(router): add amount conversion framework to netcetera Oct 5, 2024
@swangi-kumari swangi-kumari added C-refactor Category: Refactor A-connector-integration Area: Connector integration labels Oct 14, 2024
hrithikesh026
hrithikesh026 reviewed Oct 15, 2024
View reviewed changes
crates/hyperswitch_domain_models/src/router_request_types/authentication.rs Outdated
@@ -91,6 +91,8 @@ pub struct ConnectorAuthenticationRequestData {
pub threeds_method_comp_ind: api_models::payments::ThreeDsCompletionIndicator,
pub three_ds_requestor_url: String,
pub webhook_url: String,

pub minor_amount: Option<MinorUnit>
Copy link
Contributor

@hrithikesh026 hrithikesh026 Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

wouldn't amount be a better name for this field?
The type itself will indicate that the amount is of type MinorUnit.

And there is already an amount field in this struct. Please use that field itself

Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't amount be a better name for this field? The type itself will indicate that the amount is of type MinorUnit.

And there is already an amount field in this struct. Please use that field itself

Sorry for the confusion here. I wanted you to change type of the amount field here from Option<i64> to Option<MinorUnit> and then use it.

@hrithikesh026
Copy link
Contributor

Please run cargo +nightly fmt --all once and commit.

@hex1c hex1c force-pushed the netcetra-refactor-#5930 branch from 2845513 to aa4e09b Compare October 15, 2024 12:59
@hex1c
Copy link
Contributor Author

hex1c commented Oct 15, 2024

@hrithikesh026 review comments done!

@gorakhnathy7 gorakhnathy7 added the hacktoberfest Issues that are up for grabs for Hacktoberfest participants label Oct 15, 2024
@deepanshu-iiitu
Copy link
Contributor

deepanshu-iiitu commented Oct 17, 2024 •
edited
Loading

@hex1c
Please run the following command to fix the formatting of this pr

cargo +nightly fmt

@hex1c
Copy link
Contributor Author

hex1c commented Oct 17, 2024

@deepanshu-iiitu sorry missed the formatting after the last refactor. Formatted the code now :)

hrithikesh026
hrithikesh026 previously approved these changes Oct 22, 2024
View reviewed changes
@hex1c hex1c force-pushed the netcetra-refactor-#5930 branch from 252c6ad to a13d14f Compare October 22, 2024 12:37
@hex1c hex1c force-pushed the netcetra-refactor-#5930 branch from a13d14f to 8cd3afd Compare October 22, 2024 12:39
@hex1c
Copy link
Contributor Author

hex1c commented Oct 22, 2024

@hrithikesh026 there was a compilation issue fixed it now

hrithikesh026
hrithikesh026 previously approved these changes Oct 23, 2024
View reviewed changes
dracarys18
dracarys18 previously approved these changes Oct 24, 2024
View reviewed changes
deepanshu-iiitu
deepanshu-iiitu requested changes Oct 24, 2024
View reviewed changes
@deepanshu-iiitu
Copy link
Contributor

Hey @hex1c
Can you work on the above suggested changes

@gorakhnathy7 gorakhnathy7 added the hacktoberfest-accepted Pull requests accepted as Hacktoberfest contributions label Oct 31, 2024
@gorakhnathy7
Copy link
Collaborator

Hey @hex1c
Kindly look into the comments, and make the changes!

@hex1c hex1c dismissed stale reviews from dracarys18 and hrithikesh026 via eaf7ec0 November 10, 2024 06:26
@hex1c
Copy link
Contributor Author

hex1c commented Nov 10, 2024

@deepanshu-iiitu made the changes kindly review it and let me know if there are any changes.

@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Nov 19, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 19, 2024
@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Nov 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepanshu-iiitu deepanshu-iiitu deepanshu-iiitu approved these changes

@dracarys18 dracarys18 dracarys18 approved these changes

@NishantJoshi00 NishantJoshi00 Awaiting requested review from NishantJoshi00

@hrithikesh026 hrithikesh026 Awaiting requested review from hrithikesh026

Assignees

@hex1c hex1c

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[REFACTOR]: [NETCETERA] Add amount conversion framework to Netcetera
6 participants
@hex1c @hrithikesh026 @deepanshu-iiitu @gorakhnathy7 @dracarys18 @swangi-kumari