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

fix(mandate): make payment_method_data optional for mandate scenario #1032

Merged
merged 5 commits into from
May 8, 2023

Conversation

jagan-jaya
Copy link
Contributor

Type of Change

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

Description

For Recurring mandate payments we don't need payment_method_data, currently we are storing all card_details in locker so payment_method_data can be built from card_details in locker for card_payment but in other_payment_methods(google_pay, apple_pay) it won't be possible, so we can ignore the payment_method_data in all recurring mandate payments.

Additional Changes

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

Motivation and Context

Closes #1011

How did you test it?

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed submitted code

@jagan-jaya jagan-jaya added A-core Area: Core flows S-waiting-on-review Status: This PR has been implemented and needs to be reviewed A-payment-methods Area: Payment Methods R-waiting-on-L1 Review: Waiting on L1 reviewer labels May 2, 2023
@jagan-jaya jagan-jaya added this to the May 2023 Release milestone May 2, 2023
@jagan-jaya jagan-jaya self-assigned this May 2, 2023
@jagan-jaya jagan-jaya requested review from a team as code owners May 2, 2023 18:25
NishantJoshi00
NishantJoshi00 previously approved these changes May 4, 2023
crates/api_models/src/payments.rs Show resolved Hide resolved
};
if let storage_models::enums::PaymentMethod::Card = payment_method.payment_method {
let _ =
cards::get_lookup_key_from_locker(state, &token, &payment_method, &locker_id).await?;
Copy link
Member

Choose a reason for hiding this comment

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

@manoj-juspay Why is this call being made and then the value is dropped? Is this to get the card data from permanent locker to basilisk?

jarnura
jarnura previously approved these changes May 4, 2023
Copy link
Member

@jarnura jarnura left a comment

Choose a reason for hiding this comment

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

Other than @Narayanbhat166 changes, looks good to me.

@jarnura jarnura added 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 May 4, 2023
@jagan-jaya jagan-jaya dismissed stale reviews from jarnura and NishantJoshi00 via 79439b5 May 5, 2023 09:22
@jagan-jaya jagan-jaya added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels May 6, 2023
@jarnura jarnura added S-ready-for-merge and removed S-waiting-on-review Status: This PR has been implemented and needs to be reviewed R-waiting-on-L1 Review: Waiting on L1 reviewer labels May 8, 2023
@jarnura jarnura added this pull request to the merge queue May 8, 2023
Merged via the queue into main with commit 9cb3fa2 May 8, 2023
@SanchithHegde SanchithHegde deleted the fix-1011 branch May 8, 2023 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core flows A-payment-methods Area: Payment Methods
Projects
No open projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

[FEATURE] Making CVV optional field for payments flow
6 participants