-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(router): Add payments get-intent API for v2 #6396
base: main
Are you sure you want to change the base?
Conversation
ee3e783
to
3fb466e
Compare
@@ -57,7 +57,7 @@ pub struct CalculateTax; | |||
pub struct SdkSessionUpdate; | |||
|
|||
#[derive(Debug, Clone)] | |||
pub struct CreateIntent; | |||
pub struct Intent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this OK or should this be PaymentsIntent
instead? Or we need separate CreateIntent
and GetIntent
?
params (("id" = String, Path, description = "The unique identifier for the Payment Intent")), | ||
responses( | ||
(status = 200, description = "Payment Intent", body = PaymentsIntentResponse), | ||
(status = 404, description = "Payment ID not found") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be 400
instead? What all status codes need to be mentioned here?
.find_payment_intent_by_id(key_manager_state, &request.id, key_store, storage_scheme) | ||
.await | ||
.change_context(errors::ApiErrorResponse::InternalServerError) | ||
.attach_printable("Payment Intent Not Found")?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels incomplete?
errors::StorageError, | ||
> { | ||
// validate customer_id if sent in the request | ||
if let Some(id) = payment_data.payment_intent.customer_id.clone() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be needed for GetIntent
. Should I remove this?
impl ApiEventMetric for PaymentsGetIntentRequest { | ||
fn get_api_event_type(&self) -> Option<ApiEventsType> { | ||
Some(ApiEventsType::Payment { | ||
payment_id: self.id.clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a reference instead?
Type of Change
Description
Added
get-intent
API for paymentsAdditional Changes
Motivation and Context
Closes #6395
How did you test it?
1a. Request
1b. Response
Checklist
cargo +nightly fmt --all
cargo clippy