refactor(connector): add amount conversion framework to cybersource #6335

gauravghodinde · 2024-10-16T07:06:40Z

Type of Change

Description

added StringMajorUnit for amount conversion

Additional Changes

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

Motivation and Context

fixes #5942

How did you test it?

Tested through postman:

Create a MCA for cybersource:
Create a Payment with Cybersource:

{
    "amount": 499,
    "currency": "USD",
    "confirm": true,
    "capture_method": "automatic",
    "customer_id": "test_rec7",
    "email": "[email protected]",
    "request_external_three_ds_authentication": true,
    "customer_acceptance": {
        "acceptance_type": "online"
    },
    "payment_method": "card",
    "payment_method_type": "credit",
    "payment_method_data": {
        "card": {
            "card_number": "4242424242424242",
            "card_exp_month": "03",
            "card_exp_year": "2030",
            "card_holder_name": "Test Holder",
            "card_cvc": "737"
        }
    },
    "billing": {
        "address": {
            "city": "test",
            "country": "US",
            "line1": "here",
            "line2": "there",
            "line3": "anywhere",
            "zip": "560095",
            "state": "Washington",
            "first_name": "One",
            "last_name": "Two"
        },
        "phone": {
            "number": "1234567890",
            "country_code": "+1"
        },
        "email": "[email protected]"
    },
    "authentication_type": "no_three_ds"
}

The payment should be succeeded

{
    "payment_id": "pay_LGt7Tbil8tLko2OpVHqJ",
    "merchant_id": "merchant_1730121122",
    "status": "succeeded",
    "amount": 499,
    "net_amount": 499,
    "shipping_cost": null,
    "amount_capturable": 0,
    "amount_received": 499,
    "connector": "cybersource",
    "client_secret": "pay_LGt7Tbil8tLko2OpVHqJ_secret_dycae051HVaAJYXSCdCH",
    "created": "2024-10-28T13:12:22.609Z",
    "currency": "USD",
    "customer_id": "test_rec7",
    "customer": {
        "id": "test_rec7",
        "name": null,
        "email": "[email protected]",
        "phone": null,
        "phone_country_code": null
    },
    "description": null,
    "refunds": null,
    "disputes": null,
    "mandate_id": null,
    "mandate_data": null,
    "setup_future_usage": null,
    "off_session": null,
    "capture_on": null,
    "capture_method": "automatic",
    "payment_method": "card",
    "payment_method_data": {
        "card": {
            "last4": "4242",
            "card_type": null,
            "card_network": "Visa",
            "card_issuer": null,
            "card_issuing_country": null,
            "card_isin": "424242",
            "card_extended_bin": null,
            "card_exp_month": "03",
            "card_exp_year": "2030",
            "card_holder_name": null,
            "payment_checks": {
                "avs_response": {
                    "code": "Y",
                    "codeRaw": "Y"
                },
                "card_verification": {
                    "resultCode": "M",
                    "resultCodeRaw": "M"
                }
            },
            "authentication_data": null
        },
        "billing": null
    },
    "payment_token": "token_LyziBYSQnkubuQbE91YF",
    "shipping": null,
    "billing": {
        "address": {
            "city": "test",
            "country": "US",
            "line1": "here",
            "line2": "there",
            "line3": "anywhere",
            "zip": "560095",
            "state": "Washington",
            "first_name": "One",
            "last_name": "Two"
        },
        "phone": {
            "number": "1234567890",
            "country_code": "+1"
        },
        "email": "[email protected]"
    },
    "order_details": null,
    "email": "[email protected]",
    "name": null,
    "phone": null,
    "return_url": null,
    "authentication_type": "no_three_ds",
    "statement_descriptor_name": null,
    "statement_descriptor_suffix": null,
    "next_action": null,
    "cancellation_reason": null,
    "error_code": null,
    "error_message": null,
    "unified_code": null,
    "unified_message": null,
    "payment_experience": null,
    "payment_method_type": "credit",
    "connector_label": null,
    "business_country": null,
    "business_label": "default",
    "business_sub_label": null,
    "allowed_payment_method_types": null,
    "ephemeral_key": {
        "customer_id": "test_rec7",
        "created_at": 1730121142,
        "expires": 1730124742,
        "secret": "epk_bb3a50134caa4214ad2ea7b534184d26"
    },
    "manual_retry_allowed": false,
    "connector_transaction_id": "7301211437366791304951",
    "frm_message": null,
    "metadata": null,
    "connector_metadata": null,
    "feature_metadata": null,
    "reference_id": "pay_LGt7Tbil8tLko2OpVHqJ_1",
    "payment_link": null,
    "profile_id": "pro_2pDEf41cwojHtrM9xItk",
    "surcharge_details": null,
    "attempt_count": 1,
    "merchant_decision": null,
    "merchant_connector_id": "mca_AFVVYXejTDoWa4IgaPnw",
    "incremental_authorization_allowed": false,
    "authorization_count": null,
    "incremental_authorizations": null,
    "external_authentication_details": null,
    "external_3ds_authentication_attempted": false,
    "expires_on": "2024-10-28T13:27:22.609Z",
    "fingerprint": null,
    "browser_info": null,
    "payment_method_id": null,
    "payment_method_status": null,
    "updated": "2024-10-28T13:12:24.692Z",
    "charges": null,
    "frm_metadata": null,
    "merchant_order_reference_id": null,
    "order_tax_amount": null,
    "connector_mandate_id": null
}

Check cybersource dashboard with respective connector_transaction_id to get the transaction data
the amount in dashboard should match with the amount passed in request.

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-10-16T07:06:42Z

Review changes with

Changed Files

File	Status
crates/router/src/types/api.rs	87% smaller
crates/router/tests/connectors/cybersource.rs	75% smaller
crates/router/src/connector/cybersource/transformers.rs	29% smaller
crates/router/src/connector/cybersource.rs	16% smaller
crates/common_utils/src/types.rs	0% smaller

deepanshu-iiitu

Please implement the suggested changes.

deepanshu-iiitu · 2024-10-17T05:37:30Z

crates/router/src/connector/cybersource.rs

+        let minor_amount =
+            req.request
+                .minor_amount
+                .ok_or(errors::ConnectorError::MissingRequiredField {
+                    field_name: "minor_amount",
+                })?;
+        let amount = convert_amount(self.amount_converter, minor_amount, req.request.currency)?;
+        let connector_router_data = cybersource::CybersourceRouterData::from((amount, req));
+
+        let connector_req =
+            cybersource::CybersourceZeroMandateRequest::try_from(&connector_router_data)?;


This change is not required because amount is always 0 for setup mandate flow. Please revert this change.

deepanshu-iiitu · 2024-10-17T05:39:22Z

crates/router/src/connector/cybersource.rs

+        // let connector_router_data = cybersource::CybersourceRouterData::try_from((
+        //     &self.get_currency_unit(),
+        //     req.request.currency,
+        //     req.request.amount_to_capture,
+        //     req,
+        // ));


Please remove these comments.

deepanshu-iiitu · 2024-10-17T05:41:46Z

crates/router/src/connector/cybersource/transformers.rs

+// impl<T> TryFrom<(&api::CurrencyUnit, enums::Currency, i64, T)> for CybersourceRouterData<T> {
+//     type Error = error_stack::Report<errors::ConnectorError>;
+//     fn try_from(
+//         (currency_unit, currency, amount, item): (&api::CurrencyUnit, enums::Currency, i64, T),
+//     ) -> Result<Self, Self::Error> {
+//         // This conversion function is used at different places in the file, if updating this, keep a check for those
+//         let amount = utils::get_amount_as_string(currency_unit, amount, currency)?;
+//         Ok(Self {
+//             amount,
+//             router_data: item,
+//         })
+//     }
+// }


Remove these comments

deepanshu-iiitu · 2024-10-17T05:43:22Z

crates/router/src/connector/cybersource/transformers.rs

+impl TryFrom<&CybersourceRouterData<&types::SetupMandateRouterData>>
+    for CybersourceZeroMandateRequest
+{
    type Error = error_stack::Report<errors::ConnectorError>;
-    fn try_from(item: &types::SetupMandateRouterData) -> Result<Self, Self::Error> {
+    fn try_from(
+        item_data: &CybersourceRouterData<&types::SetupMandateRouterData>,
+    ) -> Result<Self, Self::Error> {
+        let item = item_data.router_data.clone();
        let email = item.get_billing_email().or(item.request.get_email())?;
        let bill_to = build_bill_to(item.get_optional_billing(), email)?;

        let order_information = OrderInformationWithBill {
            amount_details: Amount {
-                total_amount: "0".to_string(),
+                total_amount: item_data.amount.clone(),
                currency: item.request.currency,
            },
            bill_to: Some(bill_to),


These changes are not required. Hard code the value to zero.
StringMajorUnit("0".to_string())

hey @deepanshu-iiitu,

when i make this change, cargo clippy gives this error

error[E0423]: cannot initialize a tuple struct which contains private fields --> crates/router/src/connector/cybersource/transformers.rs:97:31 | 97 | total_amount: StringMajorUnit("0".to_string()), | ^^^^^^^^^^^^^^^ | note: constructor is not visible here due to private fields --> /media/gaurav/hyperswitch/crates/common_utils/src/types.rs:552:28 | 552 | pub struct StringMajorUnit(String); | ^^^^^^ private field help: you might have meant to use the newassociated function | 97 | total_amount: StringMajorUnit::new("0".to_string()), | +++++

after using the StringMajorUnit::new("0".to_string()) gives this error

error[E0624]: associated function new is private --> crates/router/src/connector/cybersource/transformers.rs:97:48 | 97 | total_amount: StringMajorUnit::new("0".to_string()), | ^^^ private associated function | ::: /media/gaurav/hyperswitch/crates/common_utils/src/types.rs:556:5 | 556 | fn new(value: String) -> Self { | ----------------------------- private associated function defined here

there are two fixes to this problem that require to make changes in crates/common_utils/src/types.rs file

make the fn new public -> pub fn new for StringMajorUnit

pub fn new(value: String) -> Self { Self(value) }

make a public function zero and use it instead

pub fn zero() -> Self { Self("0".to_string()) }
&
amount_details: Amount { total_amount: StringMajorUnit::zero(), currency: item.request.currency, }, in crates/router/src/connector/cybersource/transformers.rs

what changes do u suggest?

You can make the function public

sure making the new function public

deepanshu-iiitu · 2024-10-17T05:46:54Z

crates/router/src/connector/cybersource/transformers.rs

+        item_data: &CybersourceRouterData<&types::RefundsRouterData<F>>,
    ) -> Result<Self, Self::Error> {
+        let item = item_data.router_data;
        Ok(Self {
            order_information: OrderInformation {
                amount_details: Amount {
-                    total_amount: item.amount.clone(),
-                    currency: item.router_data.request.currency,
+                    total_amount: item_data.amount.clone(),
+                    currency: item.request.currency,
                },
            },
            client_reference_information: ClientReferenceInformation {
-                code: Some(item.router_data.request.refund_id.clone()),
+                code: Some(item.request.refund_id.clone()),


These changes are not required.

deepanshu-iiitu · 2024-10-17T05:47:32Z

crates/router/src/connector/cybersource/transformers.rs

+        item_data: &CybersourceRouterData<&types::PaymentsPreProcessingRouterData>,
    ) -> Result<Self, Self::Error> {
+        let item = item_data.router_data.clone();
        let client_reference_information = ClientReferenceInformation {
-            code: Some(item.router_data.connector_request_reference_id.clone()),
+            code: Some(item.connector_request_reference_id.clone()),
        };
-        let payment_method_data = item.router_data.request.payment_method_data.clone().ok_or(
+        let payment_method_data = item.request.payment_method_data.clone().ok_or(


These changes are not required. please revert them

deepanshu-iiitu · 2024-10-17T05:47:39Z

crates/router/src/connector/cybersource/transformers.rs

+        let redirect_response = item.request.redirect_response.clone().ok_or(
            errors::ConnectorError::MissingRequiredField {
                field_name: "redirect_response",
            },
        )?;

        let amount_details = Amount {
-            total_amount: item.amount.clone(),
-            currency: item.router_data.request.currency.ok_or(
+            total_amount: item_data.amount.clone(),
+            currency: item.request.currency.ok_or(


These changes are not required. please revert them

deepanshu-iiitu · 2024-10-17T05:47:52Z

crates/router/src/connector/cybersource/transformers.rs

-                                    .router_data
-                                    .request
-                                    .get_complete_authorize_url()?,
+                                return_url: item.request.get_complete_authorize_url()?,


These changes are not required. please revert them

deepanshu-iiitu · 2024-10-17T05:51:46Z

@gauravghodinde Please implement the suggested changes

gauravghodinde · 2024-10-18T11:13:44Z

@deepanshu-iiitu please check this out, #6335 (comment)

gauravghodinde · 2024-10-22T13:12:36Z

hey @deepanshu-iiitu, i please review i have made the requested changes

dracarys18 · 2024-10-24T10:53:46Z

crates/common_utils/src/types.rs

+    /// forms a new StringMajorUnit default unit i.e zero
+    pub fn zero() -> Self {
+        Self("0".to_string())
+    }
+


Why do we need this? You can call StringMajorUnit::new(String::new("0"))

deepanshu-iiitu · 2024-10-24T11:37:30Z

I've tested the Payin flows, and everything is working as expected. We can proceed with the merge once @Sakilmostak completes the testing on the payouts side and provides his approval.

Gnanasundari24 · 2024-10-25T08:17:15Z

@deepanshu-iiitu Can you add the testcases for cybersource

gauravghodinde · 2024-10-29T12:23:39Z

hey @deepanshu-iiitu can you explain why the checks failed?

SanchithHegde · 2024-12-01T10:13:56Z

crates/router/src/connector/cybersource/transformers.rs

@@ -93,7 +88,7 @@ impl TryFrom<&types::SetupMandateRouterData> for CybersourceZeroMandateRequest {

        let order_information = OrderInformationWithBill {
            amount_details: Amount {
-                total_amount: "0".to_string(),
+                total_amount: StringMajorUnit::new("0".to_string()),


Suggested change

total_amount: StringMajorUnit::new("0".to_string()),

total_amount: StringMajorUnit::zero(),

And can you also make StringMajorUnit::new() private for now?

…ro instead

refactor(connector): add amount conversion framework to cybersource

9f4a297

gauravghodinde requested review from a team as code owners October 16, 2024 07:06

gorakhnathy7 added the hacktoberfest Issues that are up for grabs for Hacktoberfest participants label Oct 16, 2024

Merge branch 'main' into amount-conversion-framework-for-cybersource

6a411d4

deepanshu-iiitu requested changes Oct 17, 2024

View reviewed changes

gorakhnathy7 requested review from NishantJoshi00 and dracarys18 and removed request for NishantJoshi00 October 17, 2024 23:18

gorakhnathy7 requested a review from deepanshu-iiitu October 18, 2024 14:10

gorakhnathy7 assigned gauravghodinde Oct 21, 2024

refactor(connector): implement the requested changes

c4995fc

swangi-kumari added A-connector-integration Area: Connector integration C-refactor Category: Refactor labels Oct 24, 2024

deepanshu-iiitu approved these changes Oct 24, 2024

View reviewed changes

deepanshu-iiitu requested a review from Sakilmostak October 24, 2024 10:42

Merge branch 'main' into amount-conversion-framework-for-cybersource

ed2350a

dracarys18 reviewed Oct 24, 2024

View reviewed changes

deepanshu-iiitu self-requested a review October 24, 2024 10:57

deepanshu-iiitu previously approved these changes Oct 24, 2024

View reviewed changes

Sakilmostak previously approved these changes Oct 24, 2024

View reviewed changes

dracarys18 previously approved these changes Oct 25, 2024

View reviewed changes

likhinbopanna added this pull request to the merge queue Oct 29, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 29, 2024

likhinbopanna added this pull request to the merge queue Oct 30, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 30, 2024

gorakhnathy7 added the hacktoberfest-accepted Pull requests accepted as Hacktoberfest contributions label Oct 31, 2024

likhinbopanna added this pull request to the merge queue Nov 4, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 4, 2024

Gnanasundari24 added this pull request to the merge queue Nov 5, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 5, 2024

pixincreate added this pull request to the merge queue Nov 27, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 27, 2024

likhinbopanna added this pull request to the merge queue Nov 28, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 28, 2024

gauravghodinde and others added 2 commits December 1, 2024 07:25

Merge branch 'main' into amount-conversion-framework-for-cybersource

78c1294

refactor(connector): fix errors after merge

3464530

gauravghodinde dismissed stale reviews from deepanshu-iiitu, dracarys18, and Sakilmostak via 3464530 December 1, 2024 02:10

SanchithHegde reviewed Dec 1, 2024

View reviewed changes

refactor(connector): make new StringMajorUnit::new private and use ze…

147f525

…ro instead

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(connector): add amount conversion framework to cybersource #6335

refactor(connector): add amount conversion framework to cybersource #6335

gauravghodinde commented Oct 16, 2024 •

edited by Sakilmostak

Loading

semanticdiff-com bot commented Oct 16, 2024 •

edited

Loading

deepanshu-iiitu left a comment

deepanshu-iiitu Oct 17, 2024

deepanshu-iiitu Oct 17, 2024

deepanshu-iiitu Oct 17, 2024

deepanshu-iiitu Oct 17, 2024

gauravghodinde Oct 17, 2024

deepanshu-iiitu Oct 22, 2024

gauravghodinde Oct 22, 2024

deepanshu-iiitu Oct 17, 2024

deepanshu-iiitu Oct 17, 2024

deepanshu-iiitu Oct 17, 2024

deepanshu-iiitu Oct 17, 2024

deepanshu-iiitu commented Oct 17, 2024

gauravghodinde commented Oct 18, 2024

gauravghodinde commented Oct 22, 2024

dracarys18 Oct 24, 2024

deepanshu-iiitu commented Oct 24, 2024

Gnanasundari24 commented Oct 25, 2024

gauravghodinde commented Oct 29, 2024

SanchithHegde Dec 1, 2024

gauravghodinde Dec 2, 2024

	total_amount: StringMajorUnit::new("0".to_string()),
	total_amount: StringMajorUnit::zero(),

refactor(connector): add amount conversion framework to cybersource #6335

Are you sure you want to change the base?

refactor(connector): add amount conversion framework to cybersource #6335

Conversation

gauravghodinde commented Oct 16, 2024 • edited by Sakilmostak Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

semanticdiff-com bot commented Oct 16, 2024 • edited Loading

deepanshu-iiitu left a 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

deepanshu-iiitu commented Oct 17, 2024

gauravghodinde commented Oct 18, 2024

gauravghodinde commented Oct 22, 2024

Choose a reason for hiding this comment

deepanshu-iiitu commented Oct 24, 2024

Gnanasundari24 commented Oct 25, 2024

gauravghodinde commented Oct 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gauravghodinde commented Oct 16, 2024 •

edited by Sakilmostak

Loading

semanticdiff-com bot commented Oct 16, 2024 •

edited

Loading