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(pm_list): add required fields for sofort #3192

Merged
merged 27 commits into from
Jan 30, 2024
Merged

Conversation

AkshayaFoiger
Copy link
Contributor

@AkshayaFoiger AkshayaFoiger commented Dec 21, 2023

Type of Change

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

Description

Resolves #3704

Test Cases

  1. Create a sofort payment with mollie with empty payment method object. The payment should succeed.
{
  "amount": 1000,
  "currency": "EUR",
  "confirm": true,
  "capture_method": "automatic",
  "capture_on": "2022-09-10T10:11:12Z",
  "amount_to_capture": 1000,
  "customer_id": "StripeCustomer",
  "email": "[email protected]",
  "name": "John Doe",
  "phone": "999999999",
  "phone_country_code": "+65",
  "description": "Its my first payment request",
  "authentication_type": "three_ds",
  "return_url": "https://google.com",
  "statement_descriptor_name": "joseph",
  "statement_descriptor_suffix": "JS",
  "payment_method": "bank_redirect",
  "payment_method_type": "sofort",
  "payment_method_data": {
    "bank_redirect": {
      "sofort": {
      }
    }
  }
}

Must be tested for each of this connectors

  1. Adyen
None
  1. ACI
Required fields: 
     payment_method_data.bank_redirect.sofort.country
  1. GlobalPayments
Required fields: 
    billing.address.country
  1. Mollie
None
  1. Nuvie
Required fields: 
     email
     billing.address.first_name
     billing.address.last_name
     billing.address.country
  1. Paypal
Required fields: 
     payment_method_data.bank_redirect.sofort.billing_details.billing_name
     payment_method_data.bank_redirect.sofort.country
  1. Shift4
None
  1. Stripe
Required fields for mandates
     payment_method_data.bank_redirect.sofort.billing_details.billing_name
     payment_method_data.bank_redirect.sofort.billing_details.email
  1. Trustpay
Required fields: 
     billing.address.first_name
     billing.address.line1
     billing.address.city
     billing.address.zip
     billing.address.country
  1. Nexinets
None

-> Create a payment with confirm false

-> list payment methods, with client secret and publishable key

curl --location 'http://localhost:8080/account/payment_methods?client_secret={{client_secret}}' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'api-key:{{api_key}}' \
--data ''

Required fields must be populated, according to the connector

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
  • I added a CHANGELOG entry if applicable

@AkshayaFoiger AkshayaFoiger added A-connector-compatibility Area: Connector compatibility A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement M-api-contract-changes Metadata: This PR involves API contract changes labels Dec 21, 2023
@AkshayaFoiger AkshayaFoiger self-assigned this Dec 21, 2023
@AkshayaFoiger AkshayaFoiger requested review from a team as code owners December 21, 2023 13:54
@AkshayaFoiger AkshayaFoiger changed the title Dynamic field/sofort feat(pm_list): add required fields for sofort Dec 21, 2023
@github-actions github-actions bot added the S-conventions-not-followed Status: This PR does not follow contributing guidelines label Dec 21, 2023
@github-actions github-actions bot removed the S-conventions-not-followed Status: This PR does not follow contributing guidelines label Dec 21, 2023
crates/router/src/configs/defaults.rs Outdated Show resolved Hide resolved
@AkshayaFoiger AkshayaFoiger changed the base branch from main to dynamic_field/ideal January 9, 2024 07:11
@AkshayaFoiger AkshayaFoiger requested review from a team as code owners January 9, 2024 07:11
@AkshayaFoiger AkshayaFoiger changed the base branch from dynamic_field/ideal to main January 9, 2024 07:13
@AkshayaFoiger AkshayaFoiger changed the base branch from main to dynamic_field/ideal January 9, 2024 07:17
@AkshayaFoiger AkshayaFoiger changed the base branch from dynamic_field/ideal to main January 9, 2024 07:17
config/config.example.toml Show resolved Hide resolved
crates/router/src/connector/adyen/transformers.rs Outdated Show resolved Hide resolved
crates/router/src/connector/paypal/transformers.rs Outdated Show resolved Hide resolved
Comment on lines 293 to 295
preferred_language: Option<String>,
#[serde(rename = "payment_method_data[sofort][country]")]
country: api_enums::CountryAlpha2,
country: Option<api_enums::CountryAlpha2>,
Copy link
Contributor

Choose a reason for hiding this comment

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

is this not mandatory??

Copy link
Contributor Author

@AkshayaFoiger AkshayaFoiger Jan 12, 2024

Choose a reason for hiding this comment

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

Country is not mandatory for stripe sofort

Copy link
Contributor

Choose a reason for hiding this comment

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

as we checked , it is mandatory. make the necessary changes

Comment on lines +1118 to +1121
} => Ok(get_stripe_sepa_dd_mandate_billing_details(
billing_details,
is_customer_initiated_mandate_payment,
)?),
Copy link
Contributor

Choose a reason for hiding this comment

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

why are ideal changes done in sofort pr?

Copy link
Contributor Author

@AkshayaFoiger AkshayaFoiger Jan 12, 2024

Choose a reason for hiding this comment

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

Recurring pay for Ideal and Sofort in stripe happens through SEPA Direct Debit payments. Therefore both requires same request body resulting in repeated code, hence I removed the redundancy by moving it into a function get_stripe_sepa_dd_mandate_billing_details

Copy link
Contributor

Choose a reason for hiding this comment

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

can u please add the same comment in code to avoid confusion in future

Comment on lines 293 to 295
preferred_language: Option<String>,
#[serde(rename = "payment_method_data[sofort][country]")]
country: api_enums::CountryAlpha2,
country: Option<api_enums::CountryAlpha2>,
Copy link
Contributor

Choose a reason for hiding this comment

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

as we checked , it is mandatory. make the necessary changes

Comment on lines +1118 to +1121
} => Ok(get_stripe_sepa_dd_mandate_billing_details(
billing_details,
is_customer_initiated_mandate_payment,
)?),
Copy link
Contributor

Choose a reason for hiding this comment

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

can u please add the same comment in code to avoid confusion in future

@prasunna09 prasunna09 added the S-waiting-on-author Status: This PR is incomplete or needs to address review comments label Jan 18, 2024
crates/router/src/configs/defaults.rs Outdated Show resolved Hide resolved
prasunna09
prasunna09 previously approved these changes Jan 18, 2024
@prasunna09 prasunna09 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 Jan 18, 2024
ArjunKarthik
ArjunKarthik previously approved these changes Jan 24, 2024
@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Jan 30, 2024
Merged via the queue into main with commit 3d55e3b Jan 30, 2024
10 checks passed
@Gnanasundari24 Gnanasundari24 deleted the dynamic_field/sofort branch January 30, 2024 09:23
@pixincreate pixincreate removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-connector-compatibility Area: Connector compatibility A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement M-api-contract-changes Metadata: This PR involves API contract changes
Projects
No open projects
Status: Candidate
Development

Successfully merging this pull request may close these issues.

6 participants