feat(connector): [Novalnet] Add zero auth mandate #6631

cookieg13 · 2024-11-21T11:30:40Z

Type of Change

Description

Added zero auth mandates for Novalnet
Made changes so that create_webhook_url fn can accept merchant_connector_account_id instead of connector_name

Additional Changes

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

Motivation and Context

How did you test it?

Zero auth mandate for novalnet
Note:
Skipping no3ds test cases as it is not supported by novalnet cc: @Likhin Bopanna

Tested manually
https://docs.google.com/document/d/1i4SSX837Dh9ipfOz24bDwJF8osR_UPmfPLWUbLP3bh0/edit?usp=sharing

Webhooks
Webhooks working as expected for /payments. Tested locally.

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

semanticdiff-com · 2024-11-21T11:30:43Z

Review changes with

Changed Files

File	Status
crates/router/src/core/payments.rs	54% smaller
crates/hyperswitch_connectors/src/connectors/novalnet/transformers.rs	4% smaller
crates/router/src/core/payments/transformers.rs	3% smaller
crates/router/src/core/utils.rs	2% smaller
crates/hyperswitch_connectors/src/connectors/novalnet.rs	1% smaller
crates/hyperswitch_connectors/src/utils.rs	0% smaller
crates/hyperswitch_domain_models/src/router_request_types.rs	0% smaller
crates/router/src/core/payments/helpers.rs	0% smaller
crates/router/src/core/relay/utils.rs	0% smaller
cypress-tests/cypress/e2e/PaymentUtils/Novalnet.js	0% smaller

SanchithHegde · 2024-11-25T08:22:59Z

crates/hyperswitch_connectors/src/connectors/novalnet/transformers.rs

+        let enforce_3d = match item.auth_type {
+            enums::AuthenticationType::ThreeDs => Some(1),
+            enums::AuthenticationType::NoThreeDs => None,
+        };
+        let test_mode = match item.test_mode {
+            Some(true) => 1,
+            Some(false) | None => 0,
+        };


Should we consider using constants or enums here?

SanchithHegde · 2024-11-25T08:23:12Z

crates/hyperswitch_connectors/src/connectors/novalnet/transformers.rs

+            mobile: item.get_optional_billing_phone_number(),
+            billing: Some(billing),
+            // no_nc is used to indicate if minimal customer data is passed or not
+            no_nc: 1,


We can use constants here.

SanchithHegde · 2024-11-25T08:24:24Z

crates/hyperswitch_connectors/src/connectors/novalnet/transformers.rs

+        let custom = NovalnetCustom { lang };
+        let hook_url = item.request.get_webhook_url()?;
+        let return_url = item.request.get_return_url()?;
+        let create_token = Some(1);


SanchithHegde · 2024-11-25T08:34:58Z

crates/router/src/core/payments/transformers.rs

+        let webhook_url = Some(helpers::create_webhook_url(
+            router_base_url,
+            &attempt.merchant_id,
+            connector_name,
+        ));


@Narayanbhat166 @sai-harsha-vardhan Should we consider passing MerchantConnectorAccountId here, instead of the connector name? Basically update the create_webhook_url() function as well?

Yes, it's always recommended to use MerchantConnectorAccountId in webhook url

@sai-harsha-vardhan @SanchithHegde I made a new fn create_webhook_url_using_mca_id ,but only used it in line 226 (crates/router/src/core/payments/transformers.rs:226) . In the other places where create_webhook_url() is currently used the MerchantConnectorAccountId value is Optional, so we cant replace connector name with simply MerchantConnectorAccountId.

we will always have the merchant_connector_id when constructing the router data. So if it is not found you can raise an internal server error.

I do not see any harm in updating the existing webhook function.

updated the existing create_webhook_url fn

crates/router/src/core/payments/helpers.rs

Narayanbhat166 · 2024-11-28T12:44:24Z

crates/router/src/core/payments/helpers.rs

 ) -> String {
    format!(
-        "{}/webhooks/{}/{}",
+        "/webhooks/{}/{}",


the base url should be in the beginning, and there are only two params in the format string, whereas there are three parameters

Narayanbhat166 · 2024-11-28T12:45:22Z

crates/router/src/core/payments/transformers.rs

@@ -2322,7 +2322,7 @@ impl<F: Clone> TryFrom<PaymentAdditionalData<'_, F>> for types::PaymentsAuthoriz
        let payment_data = additional_data.payment_data.clone();
        let router_base_url = &additional_data.router_base_url;
        let connector_name = &additional_data.connector_name;
-        let attempt = &payment_data.payment_attempt;
+        let attempt = &payment_data.payment_attempt.clone();


why are we cloning the attempt here? clone the fields instead

crates/router/src/core/payments/transformers.rs

Narayanbhat166 · 2025-01-02T17:10:53Z

crates/hyperswitch_connectors/src/connectors/novalnet/transformers.rs

+#[serde(untagged)]
+pub enum NovalNetAmount {
+    StringMinor(StringMinorUnit),
+    Int(i64),


you can use MinorUnit here right?

sure, I will take this up in a separate PR #6981

crates/router/src/core/payments.rs

Narayanbhat166 · 2025-01-03T11:04:23Z

crates/router/src/core/payments/transformers.rs

@@ -2745,11 +2745,17 @@ impl<F: Clone> TryFrom<PaymentAdditionalData<'_, F>> for types::PaymentsAuthoriz
            attempt,
            connector_name,
        ));
-
+        let merchant_connector_account_id = payment_data


@sahkal @sai-harsha-vardhan we will not have merchant_connector_id in case of tunnel payments right? I think we need to provide a fallback to connector_name if merchant connector id is not present.

yes, correct! but again putting a fallback to fetch with connector_name would also not work since tunnel merchant will never have a MCA in Hs

yes that is a valid point, we cannot make merchant_connector_id as mandatory is what my concern is, because it will not be present in case of creds_identifier

added connector_name as fallback

Narayanbhat166 · 2025-01-03T12:55:58Z

crates/router/src/core/payments/transformers.rs

@@ -3569,6 +3578,19 @@ impl<F: Clone> TryFrom<PaymentAdditionalData<'_, F>> for types::SetupMandateRequ
                    .map(|customer| customer.clone().into_inner())
            });
        let amount = payment_data.payment_attempt.get_total_amount();
+        let merchant_connector_account_id = payment_data


please add the fallback here as well

Narayanbhat166 · 2025-01-03T12:58:12Z

crates/router/src/core/payments/transformers.rs

+            .payment_attempt
+            .merchant_connector_id
+            .clone()
+            .get_required_value("merchant_connector_id")


provide the fallback here as well

cookieg13 requested review from a team as code owners November 21, 2024 11:30

cookieg13 self-assigned this Nov 21, 2024

cookieg13 changed the title ~~Add novalnet zero auth mandate~~ feat(connector): [Novalnet] Add zero auth mandate Nov 21, 2024

cookieg13 force-pushed the addNovalnetSetupMandate branch from df85bb8 to 4fdf8c1 Compare November 22, 2024 06:10

srujanchikke previously approved these changes Nov 25, 2024

View reviewed changes

SanchithHegde reviewed Nov 25, 2024

View reviewed changes

cookieg13 dismissed srujanchikke’s stale review via 0b20c37 November 26, 2024 07:28

cookieg13 force-pushed the addNovalnetSetupMandate branch from 4fdf8c1 to 0b20c37 Compare November 26, 2024 07:28

cookieg13 requested a review from ShankarSinghC November 26, 2024 07:29

cookieg13 force-pushed the addNovalnetSetupMandate branch from 0b20c37 to 73667aa Compare November 26, 2024 07:30

cookieg13 requested review from SanchithHegde and srujanchikke November 26, 2024 11:45

SanchithHegde previously approved these changes Nov 27, 2024

View reviewed changes

crates/router/src/core/payments/helpers.rs Outdated Show resolved Hide resolved

cookieg13 dismissed SanchithHegde’s stale review via 64d9202 November 27, 2024 11:35

cookieg13 force-pushed the addNovalnetSetupMandate branch from 0fd4b9a to 64d9202 Compare November 27, 2024 11:35

cookieg13 removed request for ShankarSinghC and srujanchikke November 27, 2024 11:37

SanchithHegde previously approved these changes Nov 27, 2024

View reviewed changes

srujanchikke previously approved these changes Nov 27, 2024

View reviewed changes

cookieg13 dismissed stale reviews from srujanchikke and SanchithHegde via 4c94106 November 28, 2024 08:18

cookieg13 force-pushed the addNovalnetSetupMandate branch 2 times, most recently from 4c94106 to 004a8b6 Compare November 28, 2024 08:22

Narayanbhat166 reviewed Nov 28, 2024

View reviewed changes

Narayanbhat166 requested changes Nov 28, 2024

View reviewed changes

cookieg13 force-pushed the addNovalnetSetupMandate branch from 004a8b6 to 9f96ab9 Compare December 2, 2024 09:01

cookieg13 dismissed likhinbopanna’s stale review via 6553eee December 9, 2024 10:27

cookieg13 force-pushed the addNovalnetSetupMandate branch from b94e6f7 to 6553eee Compare December 9, 2024 10:27

SanchithHegde mentioned this pull request Dec 27, 2024

refactor(router): refactor relay flow addressing comments of /relay feature pr #6954

Open

14 tasks

cookieg13 added 2 commits January 2, 2025 14:16

add zero auth mandates for novalnet

10562e6

modify create_webhook_url fn to take merchant_connector_id as input

39a645b

cookieg13 force-pushed the addNovalnetSetupMandate branch 3 times, most recently from 31be626 to 3053db8 Compare January 2, 2025 11:14

likhinbopanna previously approved these changes Jan 2, 2025

View reviewed changes

srujanchikke previously approved these changes Jan 2, 2025

View reviewed changes

Narayanbhat166 previously approved these changes Jan 2, 2025

View reviewed changes

SanchithHegde reviewed Jan 3, 2025

View reviewed changes

crates/router/src/core/payments.rs Outdated Show resolved Hide resolved

cookieg13 dismissed stale reviews from Narayanbhat166, srujanchikke, and likhinbopanna via a3cf7e0 January 3, 2025 09:30

cookieg13 force-pushed the addNovalnetSetupMandate branch from 3053db8 to a3cf7e0 Compare January 3, 2025 09:30

SanchithHegde previously approved these changes Jan 3, 2025

View reviewed changes

cookieg13 mentioned this pull request Jan 3, 2025

Use MinorUnit for Novalnet Amount #6981

Open

srujanchikke previously approved these changes Jan 3, 2025

View reviewed changes

likhinbopanna previously approved these changes Jan 3, 2025

View reviewed changes

Narayanbhat166 requested changes Jan 3, 2025

View reviewed changes

add cypress test trigger skip for no3ds zero auth mandate config

c595f10

cookieg13 dismissed stale reviews from likhinbopanna, srujanchikke, and SanchithHegde via c595f10 January 3, 2025 12:34

cookieg13 force-pushed the addNovalnetSetupMandate branch from a3cf7e0 to c595f10 Compare January 3, 2025 12:34

chore: run formatter

e742315

cookieg13 requested a review from Narayanbhat166 January 3, 2025 12:42

Narayanbhat166 reviewed Jan 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(connector): [Novalnet] Add zero auth mandate #6631

feat(connector): [Novalnet] Add zero auth mandate #6631

cookieg13 commented Nov 21, 2024 •

edited

Loading

semanticdiff-com bot commented Nov 21, 2024 •

edited

Loading

SanchithHegde Nov 25, 2024

cookieg13 Nov 26, 2024

SanchithHegde Nov 25, 2024

SanchithHegde Nov 25, 2024

SanchithHegde Nov 25, 2024

sai-harsha-vardhan Nov 26, 2024

cookieg13 Nov 26, 2024 •

edited

Loading

Narayanbhat166 Nov 27, 2024

cookieg13 Nov 28, 2024

Narayanbhat166 Nov 28, 2024

Narayanbhat166 Nov 28, 2024

Narayanbhat166 Jan 2, 2025

cookieg13 Jan 3, 2025

Narayanbhat166 Jan 3, 2025

sahkal Jan 3, 2025

Narayanbhat166 Jan 3, 2025

cookieg13 Jan 3, 2025

Narayanbhat166 Jan 3, 2025

Narayanbhat166 Jan 3, 2025

feat(connector): [Novalnet] Add zero auth mandate #6631

Are you sure you want to change the base?

feat(connector): [Novalnet] Add zero auth mandate #6631

Conversation

cookieg13 commented Nov 21, 2024 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

semanticdiff-com bot commented Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cookieg13 Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cookieg13 commented Nov 21, 2024 •

edited

Loading

semanticdiff-com bot commented Nov 21, 2024 •

edited

Loading

cookieg13 Nov 26, 2024 •

edited

Loading