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(connector): [Nexixpay] add mandates flow for cards #6259

Merged
merged 10 commits into from
Dec 5, 2024

Conversation

mrudulvajpayee4935
Copy link
Contributor

@mrudulvajpayee4935 mrudulvajpayee4935 commented Oct 8, 2024

Type of Change

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

Description

Integrate mandates flow for card payments.
https://developer.nexi.it/en/funzionalita-/pagamenti-ricorrenti

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?

Cypress test:

1) Multiuse mandates
Screenshot 2024-11-29 at 12 50 53 PM

2) Singleuse mandates
Screenshot 2024-11-29 at 12 51 05 PM

3) Test cases in the headless mode
Screenshot 2024-12-04 at 11 58 30 AM

Note:

  1. Skipping no3ds test cases as it is not supported by nexixpay
  2. Remaining few test cases in headless mode is failing due to inconsistency in test environment from the connector - cc: @likhinbopanna @Gnanasundari24 **

Create Call

curl --location 'http://localhost:8080/payments' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'api-key: dev_VyjzlxA4qJnZUAalRRpB8MGDRzw6moojPrhbHThoGwxrZiyCvNnuIdVKoWvumalE' \
--data '{
    "amount": 3545,
    "currency": "EUR",
    "confirm": false,
    "customer_id": "tester799",
    "setup_future_usage": "off_session"
}'

Confirm Call

curl --location 'http://localhost:8080/payments/pay_sFK1K3juhPteyDXgEH8X/confirm' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'api-key: dev_VyjzlxA4qJnZUAalRRpB8MGDRzw6moojPrhbHThoGwxrZiyCvNnuIdVKoWvumalE' \
--data-raw '{
    "payment_method": "card",
    "return_url": "https://google.com",
    "payment_method_type": "credit",
    "payment_method_data": {
        "card": {
            "card_number": "4349940199004549",
            "card_exp_month": "12",
            "card_exp_year": "30",
            "card_holder_name": "Joseph Does",
            "card_cvc": "396"
        }
    },
    "billing": {
        "address": {
            "line1": "1467",
            "line2": "Harrison Street",
            "line3": "Harrison Street",
            "city": "San Fransico",
            "state": "California",
            "zip": "94122",
            "country": "IT",
            "first_name": "joseph",
            "last_name": "Doe"
        },
        "phone": {
            "number": "8056594427",
            "country_code": "+91"
        },
        "email": "[email protected]"
    },
    "shipping": {
        "address": {
            "line1": "1467",
            "line2": "Harrison Street",
            "line3": "Harrison Street",
            "city": "San Fransico",
            "state": "California",
            "zip": "94122",
            "country": "IT",
            "first_name": "joseph",
            "last_name": "Doe"
        },
        "phone": {
            "number": "8056594427",
            "country_code": "+91"
        },
        "email": "[email protected]"
    },
    "customer_acceptance": {
        "acceptance_type": "offline"
    }
}'

MIT Call

curl --location 'http://localhost:8080/payments' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'api-key: dev_VyjzlxA4qJnZUAalRRpB8MGDRzw6moojPrhbHThoGwxrZiyCvNnuIdVKoWvumalE' \
--data '{
    "amount": 3545,
    "currency": "EUR",
    "confirm": true,
    "customer_id": "tester799",
    "recurring_details": {
        "type": "payment_method_id",
        "data": "pm_WqDpelm1rZ4yvUkf579X"
    },
    "off_session": true
}'

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

@mrudulvajpayee4935 mrudulvajpayee4935 added A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement labels Oct 8, 2024
@mrudulvajpayee4935 mrudulvajpayee4935 self-assigned this Oct 8, 2024
@mrudulvajpayee4935 mrudulvajpayee4935 requested review from a team as code owners October 8, 2024 06:01
Copy link

semanticdiff-com bot commented Oct 8, 2024

data: data.clone(),
http_code: res.status_code,
})
if data.request.connector_mandate_id().is_none() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this check won't be needed if we use an enum for the PaymentsResponse
enum NexixpayResponse { PaymentsPresonse(NexixpayPaymentsResponse), MandateResponse(NexixpayMandateResponse), }
`

Copy link
Contributor

Choose a reason for hiding this comment

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

Take reference from Fiuu: Transformers

#[serde(untagged)]
pub enum FiuuPaymentsResponse {
    PaymentResponse(Box<PaymentsResponse>),
    QRPaymentResponse(Box<DuitNowQrCodeResponse>),
    Error(FiuuErrorResponse),
    RecurringResponse(Vec<Box<FiuuRecurringResponse>>),
}

awasthi21
awasthi21 previously approved these changes Nov 11, 2024
Copy link
Contributor

@srujanchikke srujanchikke left a comment

Choose a reason for hiding this comment

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

Please add cypress test cases as well.

connectors: &Connectors,
) -> CustomResult<String, errors::ConnectorError> {
Ok(format!("{}/orders/3steps/init", self.base_url(connectors)))
if req.request.connector_mandate_id().is_none() {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we check for off_session instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried this but it creates problem as normal payment and cit payment both use same endpoint.

Comment on lines 439 to 460
name: item.router_data.get_billing_full_name()?,
street: item.router_data.get_billing_line1()?,
city: item.router_data.get_billing_city()?,
post_code: item.router_data.get_billing_zip()?,
country: item.router_data.get_billing_country()?,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

All required fields should be added in payment_connector_required_fields.rs file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its already added

let customer_info = CustomerInfo {
card_holder_name: item.router_data.get_billing_full_name()?,
billing_address: billing_address.clone(),
shipping_address: Some(billing_address),
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 we hardcoding shipping address to billing_address ?

@mrudulvajpayee4935
Copy link
Contributor Author

Please add cypress test cases as well.
Could not run cypress tests since we don't have support for 3ds mandates cypress tests. cc: @likhinbopanna

srujanchikke
srujanchikke previously approved these changes Nov 13, 2024
awasthi21
awasthi21 previously approved these changes Nov 13, 2024
awasthi21
awasthi21 previously approved these changes Nov 15, 2024
srujanchikke
srujanchikke previously approved these changes Nov 15, 2024
@Gnanasundari24
Copy link
Contributor

@mrudulvajpayee4935 Please run the cypress testcases for nexixpay

@mrudulvajpayee4935
Copy link
Contributor Author

have support for 3ds mandates cypress tests.

do we have support for 3ds mandates cypress tests? @Gnanasundari24

customer_acceptance: null,
setup_future_usage: "on_session",
billing: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you declare billing as const and use it everywhere else the billing address are same

awasthi21
awasthi21 previously approved these changes Nov 29, 2024
card_exp_year: "50",
card_number: "4111111111111111",
card_exp_month: "08",
card_exp_year: "25",
Copy link
Member

Choose a reason for hiding this comment

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

august will come sooner than you might expect:

Suggested change
card_exp_year: "25",
card_exp_year: "50",

Comment on lines 20 to 23
customer_acceptance: {
acceptance_type: "offline",
accepted_at: "1963-05-03T04:07:52.723Z",
online: {
ip_address: "125.0.0.1",
user_agent: "amet irure esse",
},
},
Copy link
Member

Choose a reason for hiding this comment

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

In L53 you've declared customerAcceptance. Move it above, and re-use it here and below.

Comment on lines 179 to 184
status: 501,
body: {
error: {
type: "invalid_request",
message: "No threeds is not supported",
code: "IR_00",
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure about this?

status is expected to be 400. Unless the connector has specific error message, this line is not needed as it will be handled by Commons.

Same applies to else where below.

},
Response: {
status: 200,
trigger_skip: true,
Copy link
Member

Choose a reason for hiding this comment

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

You will have to change the implementation once #6588 gets merged here and else where for trigger_skip

Comment on lines 444 to 451
status: 501,
body: {
error: {
type: "invalid_request",
message: "No threeds is not supported",
code: "IR_00",
},
},
Copy link
Member

Choose a reason for hiding this comment

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

again, since the same object is used in multiple places, this can be re-used by creating a constant object.

Copy link
Member

@pixincreate pixincreate 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 that, looks good to me.

i also recommend you to run tests locally to see if everything works.

@@ -1,3 +1,13 @@
import { getCustomExchange } from "./Commons";
Copy link
Member

Choose a reason for hiding this comment

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

can you check the lints?

card_isin: "411111",
card_extended_bin: null,
card_exp_month: "08",
card_exp_year: "25",
Copy link
Member

Choose a reason for hiding this comment

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

increase this year to a later date. august 2025 will come sooner than you might expect.

@likhinbopanna likhinbopanna added this pull request to the merge queue Dec 5, 2024
Merged via the queue into main with commit 62521f3 Dec 5, 2024
16 of 18 checks passed
@likhinbopanna likhinbopanna deleted the nexixpay-card-mandates branch December 5, 2024 09:26
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 C-feature Category: Feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants