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

fix(core): add ephemeral key to payment_create response when customer_id is mentioned #1133

Merged
merged 5 commits into from
May 19, 2023

Conversation

hrithikesh026
Copy link
Contributor

Type of Change

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

Description

Added ephemeral_key field to payment_create response(only payment create).

Additional Changes

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

Motivation and Context

Through this change, merchant need not do a separate api call for ephemeral key after payment creation.

How did you test it?

manual

This is how payment create response would look after this change is merged.
Note ephemeral_field at the end.

{
    "payment_id": "pay_DUrBhwXG03XovxSB6Tra",
    "merchant_id": "merchant_1682859385",
    "status": "requires_confirmation",
    "amount": 6969,
    "amount_capturable": null,
    "amount_received": null,
    "connector": null,
    "client_secret": "pay_DUrBhwXG03XovxSB6Tra_secret_4sq0ABEu4FpVvO2pxyc6",
    "created": "2023-05-11T14:39:11.331Z",
    "currency": "USD",
    "customer_id": "cus_7mb0kUzEAzmMPzpkWiFh",
    "description": "Its my first payment request",
    "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",
            "exp_month": "10",
            "exp_year": "25"
        }
    },
    "payment_token": "token_xhYBvwM9ULXYKm02dcDi",
    "shipping": {
        "address": {
            "city": "San Fransico",
            "country": "US",
            "line1": "1467",
            "line2": "Harrison Street",
            "line3": "Harrison Street",
            "zip": "94122",
            "state": "California",
            "first_name": "joseph",
            "last_name": "Doe"
        },
        "phone": {
            "number": "8056594427",
            "country_code": "+91"
        }
    },
    "billing": {
        "address": {
            "city": "San Fransico",
            "country": "US",
            "line1": "1467",
            "line2": "Harrison Street",
            "line3": "Harrison Street",
            "zip": "94122",
            "state": "California",
            "first_name": "joseph",
            "last_name": "Doe"
        },
        "phone": {
            "number": "8056594427",
            "country_code": "+91"
        }
    },
    "metadata": {
        "udf1": "value1",
        "payload": null,
        "login_date": "2019-09-10T10:11:12Z",
        "new_customer": "true",
        "order_details": null,
        "allowed_payment_method_types": null
    },
    "email": "guest@example.com",
    "name": "John Doe",
    "phone": "999999999",
    "return_url": "https://google.com/",
    "authentication_type": "no_three_ds",
    "statement_descriptor_name": "joseph",
    "statement_descriptor_suffix": "JS",
    "next_action": null,
    "cancellation_reason": null,
    "error_code": null,
    "error_message": null,
    "payment_experience": null,
    "payment_method_type": "credit",
    "connector_label": null,
    "business_country": "US",
    "business_label": "default",
    "business_sub_label": null,
    "allowed_payment_method_types": null,
    "ephemeral_key": {
        "created_at": 1683815951,
        "expires": 1683819551,
        "secret": "epk_4af4735ea3a54fc0bb4d8320f43feaf9"
    }
}

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

Comment on lines 3 to 8
#[derive(Debug, serde::Serialize, serde::Deserialize, Clone, Eq, PartialEq)]
pub struct EphemeralKeyCreateResponse {
pub created_at: i64,
pub expires: i64,
pub secret: String,
}
Copy link
Member

Choose a reason for hiding this comment

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

Ephemeral key is specific to one customer not I feel it's better to have it in the response json.

cc: @bernard-eugine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean, we should only send EphemeralKey as a string in the response? without created_at and expires fields.

Copy link
Member

Choose a reason for hiding this comment

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

@hrithikesh026 I meant we should add customerid in the response since it's only specific to that customer.

Copy link
Contributor Author

@hrithikesh026 hrithikesh026 May 15, 2023

Choose a reason for hiding this comment

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

EphemeralKeyCreateResponse is a field in PaymentsResponse, customer_id will be available in PaymentsResponse.
Will it be okay to provide customer id at two different places ?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah @hrithikesh026 since it's specific to one customer I think it must be in that block of response itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made the above mentioned change.

crates/router/src/core/payments.rs Outdated Show resolved Hide resolved
@hrithikesh026 hrithikesh026 changed the title fix(core): add ephemeral key to payment_create response fix(core): add ephemeral key to payment_create response when customer_id is mentioned May 16, 2023
dracarys18
dracarys18 previously approved these changes May 16, 2023
Copy link
Member

@dracarys18 dracarys18 left a comment

Choose a reason for hiding this comment

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

LGTM

@hrithikesh026 hrithikesh026 added C-bug Category: Bug A-core Area: Core flows R-waiting-on-L2 Review: Waiting on L2 reviewer labels May 17, 2023
@hrithikesh026 hrithikesh026 added this to the May 2023 Release milestone May 17, 2023
@hrithikesh026 hrithikesh026 added R-waiting-on-L1 Review: Waiting on L1 reviewer and removed R-waiting-on-L2 Review: Waiting on L2 reviewer labels May 17, 2023
jarnura
jarnura previously approved these changes May 18, 2023
@@ -0,0 +1,9 @@
use serde;

#[derive(Debug, serde::Serialize, serde::Deserialize, Clone, Eq, PartialEq)]
Copy link
Member

Choose a reason for hiding this comment

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

Add Toschema and comments explain the fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jarnura made the above mentioned changes

Comment on lines 52 to 68
let ephemeral_key = match request.customer_id.clone() {
Some(customer_id) => helpers::make_ephemeral_key(
state,
customer_id,
merchant_account.merchant_id.clone(),
)
.await
.ok()
.and_then(|ek| {
if let services::ApplicationResponse::Json(ek) = ek {
Some(ek)
} else {
None
}
}),
None => None,
};
Copy link
Member

Choose a reason for hiding this comment

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

We can use a separate function as get_ephemeral_key and inside the function also use and_then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jarnura made the above mentioned changes

@hrithikesh026 hrithikesh026 dismissed stale reviews from jarnura and dracarys18 via 2dffc00 May 18, 2023 09:11
jarnura
jarnura previously approved these changes May 18, 2023
@jarnura jarnura enabled auto-merge May 18, 2023 14:44
@Narayanbhat166 Narayanbhat166 added the M-api-contract-changes Metadata: This PR involves API contract changes label May 19, 2023
@jarnura jarnura added this pull request to the merge queue May 19, 2023
@jarnura jarnura added S-ready-for-merge and removed R-waiting-on-L1 Review: Waiting on L1 reviewer labels May 19, 2023
Merged via the queue into main with commit f394c4a May 19, 2023
6 checks passed
@jarnura jarnura deleted the add_ephemeral_key_to_payment_create_response branch May 19, 2023 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core flows C-bug Category: Bug M-api-contract-changes Metadata: This PR involves API contract changes
Projects
No open projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants