-
Notifications
You must be signed in to change notification settings - Fork 2.1k
refactor(connector): added amount conversion framework for powertranz #6239
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
base: main
Are you sure you want to change the base?
Conversation
Review changes with SemanticDiff. Analyzed 4 of 4 files. Overall, the semantic diff is 18% smaller than the GitHub diff.
|
Hey @sahil9001 Can you please link the issue to this PR. |
@ImSagnik007 is there any way to test this out locally? A command or something? |
hey @sahil9001 you can do cargo run, to build your branch locally |
Hi @sahil9001 |
Hey @deepanshu-iiitu , are my changes correct? I mean do I only need to remove the unused imports? |
@sahil9001 There are still some errors that need to be fixed. |
Hey @sahil9001 Kindly look into the changes to resolve them, so that it can be merged within time. |
sure , i will make the changes tomorrow |
Signed-off-by: Sahil Silare <[email protected]>
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 do the amount conversion for the Capture and Refund flow as well
pub struct Powertranz; | ||
#[derive(Clone)] | ||
pub struct Powertranz { | ||
amount_converter: &'static (dyn AmountConvertor<Output = StringMajorUnit> + Sync), |
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 use FloatMajorUnit
for conversion
impl Powertranz { | ||
pub fn new() -> &'static Self { | ||
&Self { | ||
amount_converter: &StringMajorUnitForConnector, |
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.
Use FloatMajorUnitForConnector
here
@@ -20,6 +20,20 @@ use crate::{ | |||
utils::{self, CardData, PaymentsAuthorizeRequestData, RouterData as _}, | |||
}; | |||
|
|||
pub struct PowertranzRouterData<T> { | |||
pub amount: StringMajorUnit, |
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 use FloatMajorUnit
for conversion
impl<T> From<(StringMajorUnit, T)> for PowertranzRouterData<T> { | ||
fn from((amount, item): (StringMajorUnit, T)) -> Self { | ||
Self { | ||
amount, | ||
router_data: item, | ||
} | ||
} | ||
} |
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 use FloatMajorUnit
for conversion
total_amount: utils::to_currency_base_unit_asf64( | ||
item.request.amount, | ||
item.request.currency, | ||
item.router_data.request.amount, | ||
item.router_data.request.currency, | ||
)?, |
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 use the amount from PowertranzRouterData instead on converting at place
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 update to the latest main branch as well
Hey @sahil9001 Can you please look into the comments, and make the changes! |
Type of Change
Description
Additional Changes
Motivation and Context
Closes #6136
How did you test it?
Checklist
cargo +nightly fmt --all
cargo clippy