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

Docs: Api reference docs update for Payments - Create #4955

Merged
merged 25 commits into from
Jun 28, 2024

Conversation

gorakhnathy7
Copy link
Collaborator

@gorakhnathy7 gorakhnathy7 commented Jun 11, 2024

Type of Change

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

Description

This is regarding the API Reference Doc changes using OpenAPI.
This PR just the doc changes for Payments - Create (Including the Payments - Response)

Additional Changes

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

Motivation and Context

Partial resolution of #5014

How did you test it?

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

@gorakhnathy7 gorakhnathy7 requested a review from a team as a code owner June 11, 2024 19:46
@gorakhnathy7 gorakhnathy7 changed the title Api reference docs update for Payments - Create Docs: Api reference docs update for Payments - Create Jun 11, 2024
@hyperswitch-bot hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Jun 11, 2024
Copy link
Contributor

@inventvenkat inventvenkat left a comment

Choose a reason for hiding this comment

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

Adding the use case of the field will give more clarity to the reader. And please explain the field, instead of just converting to readable. For example: CustomerDetailsResponse should tell what details it is exactly, instead of just saying "Details of customer attached to this payment"

@@ -5221,6 +5221,7 @@
},
"AttemptStatus": {
"type": "string",
"description": "The status of the attempt",
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also include what each status means

Copy link
Collaborator Author

@gorakhnathy7 gorakhnathy7 Jun 12, 2024

Choose a reason for hiding this comment

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

Hey this has the following options in the enums -
Ref - hyperswitch/crates/common_enums/src/enums.rs Ln 39

pub enum AttemptStatus {
    Started,
    AuthenticationFailed,
    RouterDeclined,
    AuthenticationPending,
    AuthenticationSuccessful,
    Authorized,
    AuthorizationFailed,
    Charged,
    Authorizing,
    CodInitiated,
    Voided,
    VoidInitiated,
    CaptureInitiated,
    CaptureFailed,
    VoidFailed,
    AutoRefunded,
    PartialCharged,
    PartialChargedAndChargeable,
    Unresolved,
    #[default]
    Pending,
    Failure,
    PaymentMethodAwaited,
    ConfirmationAwaited,
    DeviceDataCollectionPending,
}

I'll add it here in this enum, the api-reference/openapi_spec.json file is autogenerated one.
Does that works?

Copy link
Member

Choose a reason for hiding this comment

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

Hey @gorakhnathy7, If the descriptions are added to the enum variants these are not included in the generated openapi file. This is something that the library might support or we have to manually add support for this. Currently you can have the description of the enum something like this

/// The status of the payment attempt
/// - Pending: The payment is in the processing state
/// - Succeeded: The payment has succeeded and the customer has been charged
enum AttemptStatus {
    Pending,
    Succeeded
}

cc: @SanchithHegde

@@ -7058,6 +7061,7 @@
},
"CaptureStatus": {
"type": "string",
"description": "The status of the capture",
Copy link
Contributor

Choose a reason for hiding this comment

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

This should include what each status means

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Similar to AttemptStatus, all status for CaptureMethod can also be described in a more elaborative way in their respective enum definition, that will work?

@@ -7550,6 +7554,7 @@
},
"ConnectorMetadata": {
"type": "object",
"description": "additional data related to some connectors",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should also include what data. If we cannot define the list, we should mention check with integration team to get the data.

Copy link
Collaborator Author

@gorakhnathy7 gorakhnathy7 Jun 12, 2024

Choose a reason for hiding this comment

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

Screenshot 2024-06-12 at 2 28 34 PM (2)
Ref - Link
There are child fields for this with relevant description, We can add a list of all the all the fields beforehand more like a preview before someone will open the child fields. Will that work?

@@ -8166,6 +8171,7 @@
},
"CustomerAcceptance": {
"type": "object",
"description": "Passing this object during payments confirm . The customer_acceptance sub object is usually passed by the SDK or client",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please mention what to pass here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Accepted, changing the description for this.

Copy link
Member

Choose a reason for hiding this comment

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

This seems to be a duplicate file, can you remove this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hey removed this, thanks

@gorakhnathy7 gorakhnathy7 self-assigned this Jun 13, 2024
@@ -307,6 +310,7 @@ pub enum BlocklistDataKind {
ExtendedCardBin,
}

/// Default value if not passed is set to 'automatic' which results in Auth and Capture in one single API request. Pass 'manual' or 'manual_multiple' in case you want do a separate Auth and Capture by first authorizing and placing a hold on your customer's funds so that you can use the Payments/Capture endpoint later to capture the authorized amount. Pass 'manual' if you want to only capture the amount later once or 'manual_multiple' if you want to capture the funds multiple times later. Both 'manual' and 'manual_multiple' are only supported by a specific list of processors
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 have a macro do this for us. If you want to do this manually then can you create this as a list so that the statuses are much more readable

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 remove this file?

inventvenkat
inventvenkat previously approved these changes Jun 18, 2024
@Narayanbhat166
Copy link
Member

List of fields to be removed from the api reference for payments request

  • merchant_id - remove in all three requests
  • capture_on
  • retry_action in PaymentsCreateRequest
  • feature_metadata
  • card_cvc in all three requests

Narayanbhat166
Narayanbhat166 previously approved these changes Jun 25, 2024
@@ -845,9 +845,11 @@ pub struct ToggleAllKVResponse {
#[schema(example = true)]
pub kv_enabled: bool,
}

/// Merchant connector details used to make payments.
Copy link
Member

Choose a reason for hiding this comment

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

This field can be removed in all the three requests from the api ref

@@ -390,7 +391,7 @@ pub struct PaymentsRequest {
/// Passing this object during payments creates a mandate. The mandate_type sub object is passed by the server.
pub mandate_data: Option<MandateData>,

/// Passing this object during payments confirm . The customer_acceptance sub object is usually passed by the SDK or client
/// We will be Passing this "CustomerAcceptance" object during Payments-Confirm. The customer_acceptance sub object is usually passed by the SDK or client
Copy link
Member

Choose a reason for hiding this comment

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

this tone can be changed

@@ -504,6 +504,7 @@ pub struct PaymentsRequest {
pub charges: Option<PaymentChargeRequest>,
}

/// Fee information to be charged on the payment being collected
Copy link
Member

Choose a reason for hiding this comment

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

cc: @kashif-m is this definition accurate?

@Narayanbhat166 Narayanbhat166 linked an issue Jun 25, 2024 that may be closed by this pull request
gorakhnathy7 and others added 3 commits June 26, 2024 01:14
Co-authored-by: Sanchith Hegde <22217505+SanchithHegde@users.noreply.github.com>
Co-authored-by: Sanchith Hegde <22217505+SanchithHegde@users.noreply.github.com>
Co-authored-by: Sanchith Hegde <22217505+SanchithHegde@users.noreply.github.com>
SanchithHegde
SanchithHegde previously approved these changes Jun 25, 2024
SanchithHegde
SanchithHegde previously approved these changes Jun 25, 2024
Narayanbhat166
Narayanbhat166 previously approved these changes Jun 26, 2024
@hyperswitch-bot hyperswitch-bot bot dismissed stale reviews from Narayanbhat166 and SanchithHegde via 5da110d June 27, 2024 07:34
@Gnanasundari24 Gnanasundari24 enabled auto-merge June 27, 2024 11:31
@SanchithHegde SanchithHegde disabled auto-merge June 27, 2024 18:06
SanchithHegde
SanchithHegde previously approved these changes Jun 27, 2024
Narayanbhat166
Narayanbhat166 previously approved these changes Jun 28, 2024
@Gnanasundari24 Gnanasundari24 enabled auto-merge June 28, 2024 10:42
@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Jun 28, 2024
Merged via the queue into main with commit f55cae2 Jun 28, 2024
11 checks passed
@Gnanasundari24 Gnanasundari24 deleted the Api-reference-docs-update-payment-create branch June 28, 2024 11:47
pixincreate added a commit that referenced this pull request Jun 28, 2024
…ror-handling-in-cypress

* 'main' of github.com:juspay/hyperswitch:
  Docs: Api reference docs update for Payments - Create (#4955)
  feat(cypress): add iatapay connector (#5093)
  chore: fix ui-test configs (#5152)
  chore(cards): add configuration option to change the decryption scheme locker (#5140)
  refactor(hyperswitch_constraint_graph): Removal of lifetime from the Constraint Graph framework (#5132)
  feat(core): customer_details storage in payment_intent (#5007)
  fix(connector): [ADYEN] send `browser_info` for all the card and googlepay payments (#5153)
  fix(users): clear cookie and alter parsing for sso (#5147)
  refactor(connector): added amount framework to paypal, payouts and routing (#4865)
  chore(version): 2024.06.28.0
  chore(postman): update Postman collection files
  chore: use generic phone numbers instead (#5142)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-api-contract-changes Metadata: This PR involves API contract changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating the API reference documentation
5 participants