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: PG Agnostic mandate using network_txns_id (Adyen, Authorizedotnet, Stripe) #855

Merged
merged 19 commits into from
May 3, 2023

Conversation

manoj-juspay
Copy link
Contributor

@manoj-juspay manoj-juspay commented Apr 11, 2023

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

Description

Implemented Adyen mandate using connector_mandate_id
https://juspay.atlassian.net/browse/HYP-37

Additional Changes

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

Motivation and Context

How did you test it?

Setup mandate followed by recurring payment

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed submitted code
  • I added unit tests for my changes where possible
  • I added a CHANGELOG entry if applicable

@manoj-juspay manoj-juspay added A-connector-integration Area: Connector integration A-core Area: Core flows S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels Apr 11, 2023
@manoj-juspay manoj-juspay self-assigned this Apr 11, 2023
@manoj-juspay manoj-juspay requested review from a team as code owners April 11, 2023 10:42
@jarnura jarnura added this to the April'23 release milestone Apr 14, 2023
@manoj-juspay manoj-juspay changed the title feat: Adyen mandate using connector_mandate_id feat: PG Agnostic mandate using network_txns_id (Adyen, Authorizedotnet, Stripe) Apr 14, 2023
@manojradhakrishnan manojradhakrishnan linked an issue Apr 17, 2023 that may be closed by this pull request
@@ -298,14 +298,20 @@ pub enum MandateTxnType {
#[derive(Default, Eq, PartialEq, Debug, serde::Deserialize, serde::Serialize, Clone)]
pub struct MandateIds {
pub mandate_id: String,
pub connector_mandate_id: Option<String>,
pub mandate_reference_id: Option<MandateReferenceId>,
Copy link
Member

Choose a reason for hiding this comment

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

Add comments for the ids

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

) {
let connector = new_mandate_data.connector.clone();
logger::debug!("{:?}", new_mandate_data);
resp.request
.set_mandate_id(api_models::payments::MandateIds {
mandate_id: new_mandate_data.mandate_id.clone(),
connector_mandate_id: new_mandate_data.connector_mandate_id.clone(),
mandate_reference_id: None, //This is not used further in case needed please handle
Copy link
Member

Choose a reason for hiding this comment

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

just pass the value based on match logic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

api::PaymentMethodData::Card(ref ccard) => {
let payment_details = PaymentDetails::CreditCard(CreditCardDetails {
card_number: ccard.card_number.clone(),
// expiration_date: format!("{expiry_year}-{expiry_month}").into(),
Copy link
Member

Choose a reason for hiding this comment

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

remove commented code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, it was there before maybe for some reference

@@ -4,7 +4,7 @@ use reqwest::Url;
use serde::{Deserialize, Serialize};

use crate::{
connector::utils::PaymentsAuthorizeRequestData,
connector::utils::{missing_field_err, PaymentsAuthorizeRequestData},
Copy link
Member

Choose a reason for hiding this comment

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

Remove missing_field_err in imports and use like utils::missing_field_err

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

expiry_month: card.card_exp_month.clone(),
expiry_year: card.card_exp_year.clone(),
cvc: None,
brand: Some(CardBrand::Visa), //Fixme
Copy link
Member

Choose a reason for hiding this comment

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

Explain the problem for FIXME. If there is a issue already linked add the issue here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

handled

expiry_month: card.card_exp_month.clone(),
expiry_year: card.card_exp_year.clone(),
cvc: None,
brand: Some(CardBrand::Visa), //Fixme
Copy link
Member

Choose a reason for hiding this comment

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

And we can't go with Hard coding the CardBrand.

Copy link
Contributor

@jagan-jaya jagan-jaya Apr 18, 2023

Choose a reason for hiding this comment

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

@manoj-juspay there is a method get_card_issuer in connector/utils check if that can solve this issue. if not add one in the existing utils to work for your case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure bro

@jarnura jarnura added the S-waiting-on-author Status: This PR is incomplete or needs to address review comments label Apr 17, 2023
@@ -246,6 +266,16 @@ pub struct AdyenCard {
expiry_month: Secret<String>,
expiry_year: Secret<String>,
cvc: Option<Secret<String>>,
brand: Option<CardBrand>, //Mandatory for mandate using network_txns_id
network_payment_reference: Option<String>, //
Copy link
Contributor

Choose a reason for hiding this comment

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

remove empty comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Comment on lines 493 to 496
let customer_id = item
.customer_id
.to_owned()
.ok_or_else(missing_field_err("customer_id"))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

move this to src/connector/utils.rs RouterData trait add a method get_customer_id which will throw error if it is not present

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

expiry_month: card.card_exp_month.clone(),
expiry_year: card.card_exp_year.clone(),
cvc: None,
brand: Some(CardBrand::Visa), //Fixme
Copy link
Contributor

@jagan-jaya jagan-jaya Apr 18, 2023

Choose a reason for hiding this comment

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

@manoj-juspay there is a method get_card_issuer in connector/utils check if that can solve this issue. if not add one in the existing utils to work for your case

@jarnura jarnura added S-needs-conflict-resolution Status: This PR needs conflicts to be resolved by the author and removed S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels Apr 19, 2023
jagan-jaya
jagan-jaya previously approved these changes Apr 26, 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.

Add required labels.

.connector_customer
.connector_list
.contains(&connector.connector_name);
if connector_customer_filter {
Copy link
Member

Choose a reason for hiding this comment

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

Add comments for None and else cases.

@manoj-juspay manoj-juspay added the M-database-changes Metadata: This PR involves database schema changes label May 2, 2023
@manoj-juspay manoj-juspay requested a review from jarnura May 2, 2023 11:54
@manoj-juspay manoj-juspay 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 2, 2023
jarnura
jarnura previously approved these changes May 2, 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 labels May 2, 2023
@jarnura jarnura added this pull request to the merge queue May 2, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks May 2, 2023
@jarnura jarnura dismissed stale reviews from jagan-jaya and themself via 3ef3233 May 3, 2023 08:22
@jarnura jarnura enabled auto-merge May 3, 2023 08:22
jarnura
jarnura previously approved these changes May 3, 2023
@jarnura jarnura added this pull request to the merge queue May 3, 2023
Merged via the queue into main with commit ed99655 May 3, 2023
@SanchithHegde SanchithHegde deleted the adyen_mandate branch May 3, 2023 11:48
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 A-core Area: Core flows M-database-changes Metadata: This PR involves database schema changes
Projects
No open projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

4 participants