-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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): remove default cases for Authorizedotnet, Braintree and Fiserv Connector #2796
Conversation
Co-authored-by: chikke srujan <[email protected]>
…into default-case
| api::PaymentMethodData::GiftCard(_) | ||
| api::PaymentMethodData::CardToken(_) => { | ||
Err(errors::ConnectorError::NotImplemented( | ||
utils::get_unimplemented_payment_method_error_message("braintree"), |
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.
This is supposed to be fiserv
.
| enums::CaptureMethod::ManualMultiple | ||
| enums::CaptureMethod::Scheduled => Self::Final, |
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.
ManualMultiple and Scheduled is not supported by the connector. In such cases throw error
| SyncStatus::Voided | ||
| SyncStatus::CouldNotVoid | ||
| SyncStatus::GeneralError | ||
| SyncStatus::FDSPendingReview => Self::Failure, |
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.
Why FDSPendingReview, SettledSuccessfully, AuthorizedPendingCapture, CapturedPendingSettlement, Voided should be mapped to Failure?
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.
@deepanshu-iiitu will handle the status mapping modifications in the pull request where he is addressing the connector audit fixes.
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.
Changes required
.map(|c| AuthorizationIndicator { | ||
authorization_indicator: c.into(), | ||
}); | ||
let authorization_indicator_type = match item.router_data.request.capture_method { |
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.
Why do we need this change?
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.
I refactored the CaptureMethod from from
to try_from
, in order to throw the error for ManualMultiple
and Scheduled
. this was a relative change for that
Type of Change
Description
Resolves 4376
Issue: defaults case in match shouldn't be handled using "_"
Solution: "_" will never let the developer know impact of his changes incase of new addition of enum variants. By having the all cases in match compiler itself will throw errors wherever new enums needs to be handled.
(In this PR it is done for Authorizrdotnet, Braintree and Fiserv Connector)
Additional Changes
Motivation and Context
How did you test it?
Do payment create for Authorizedotnet, Braintree and Fiserv Connector for any payment method which is not implemented it should throw the error
here is payment request for PIX PM
Checklist
cargo +nightly fmt --all
cargo clippy