-
Notifications
You must be signed in to change notification settings - Fork 75
Checkout: mark mandatory query parameters #1502
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
Conversation
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.
Hello @gcatanese, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello team, gemini-code-assist here with a summary of this pull request. The main goal of this PR is to address an issue where query parameters generated by the SDK's templating engine were incorrectly marked as optional, even when the OpenAPI specification defined them as required. This PR fixes the Mustache template responsible for generating API method signatures to correctly reflect the required
status of query parameters. Consequently, it updates specific Checkout API methods (deleteTokenForStoredPaymentDetails
and getResultOfPaymentSession
) and their corresponding tests to use the now-mandatory query parameters.
Highlights
- Code Generation Fix: The core change involves modifying the
api-single.mustache
template to ensure that query parameters marked asrequired
in the OpenAPI spec are generated without the optional?
marker in the TypeScript method signatures. - Checkout API Updates: The
deleteTokenForStoredPaymentDetails
method inRecurringApi
now requiresshopperReference
andmerchantAccount
as mandatory parameters. ThegetResultOfPaymentSession
method inPaymentsApi
now requiressessionResult
as a mandatory parameter. - Test Adjustments: Tests in
checkout.spec.ts
have been updated to pass the newly mandatory query parameters (shopperReference
,merchantAccount
) to thedeleteTokenForStoredPaymentDetails
call and include them in the nock scope URL. A test case forgetResultOfPaymentSession
that previously handled optional parameters was removed. - Minor Code Style Fixes: Some minor code style changes (like changing single quotes to double quotes) were made in
httpClient.spec.ts
andservice.ts
.
Changelog
Click here to see the changelog
- src/tests/checkout.spec.ts
- Updated the
delete stored paymentMethod
test to includeshopperReference
andmerchantAccount
query parameters in the nock URL (line 566). - Updated the
delete stored paymentMethod
test to passshopperReference
andmerchantAccount
as arguments todeleteTokenForStoredPaymentDetails
(line 570). - Removed the test case
Should handle request without query parameters for getResultOfPaymentSession
(lines 575-585).
- Updated the
- src/tests/httpClient.spec.ts
- Changed single quotes to double quotes for
reqheaders
keys in the first test (lines 119, 124, 130). - Added semicolons after
expect
calls in the first test (lines 127, 133). - Changed single quotes to double quotes for
reqheaders
keys in the second test (lines 156, 161, 167). - Added semicolons after
expect
calls in the second test (lines 164, 170). - Changed single quotes to double quotes for
describe
andtest
descriptions (lines 184, 187, 195).
- Changed single quotes to double quotes for
- src/service.ts
- Changed single quotes to double quotes in the string literal
/possdk/v68
(line 54).
- Changed single quotes to double quotes in the string literal
- src/services/checkout/paymentsApi.ts
- Changed the
sessionResult
parameter from optional (sessionResult?: string
) to required (sessionResult: string
) in thegetResultOfPaymentSession
method signature (line 65).
- Changed the
- src/services/checkout/recurringApi.ts
- Changed the
shopperReference
andmerchantAccount
parameters from optional (shopperReference?: string, merchantAccount?: string
) to required (shopperReference: string, merchantAccount: string
) in thedeleteTokenForStoredPaymentDetails
method signature (line 39).
- Changed the
- templates/typescript/api-single.mustache
- Modified the logic for generating query parameters in method signatures to include the optional marker
?
only if the parameter is not required ({{^required}}?{{/required}}
) (line 27).
- Modified the logic for generating query parameters in method signatures to include the optional marker
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Template made params wrong,
Optional, though required all along.
Code fixed with care,
Now mandatory there,
Making API calls strong.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request correctly addresses the issue of mandatory query parameters not being marked as such in the generated SDK code. The change in the Mustache template ensures that the ?
optional marker is only added to query parameters when they are not marked as required in the OpenAPI specification. The corresponding updates in the test files and generated service files (paymentsApi.ts
, recurringApi.ts
) correctly reflect this change by making the relevant parameters mandatory.
The core logic of the PR is sound and achieves the stated objective. The changes are well-contained and directly address the reported issues.
Summary of Findings
- Stylistic changes (quotes and semicolons): The PR includes changes from single quotes to double quotes and the addition of semicolons after
expect
calls in test files. These are minor stylistic changes. According to the review settings, I have not added specific comments for these low-severity issues.
Merge Readiness
Based on my review, the code changes correctly implement the intended functionality and address the reported issues. No critical or high-severity issues were found. The low-severity stylistic issues are minor and do not block merging. The pull request appears ready to be merged. Please note that I am unable to directly approve the pull request; another reviewer will need to do so.
636c9b1
to
60baf9c
Compare
|
Query parameters are always marked as Optional, even when the OpenAPI specs define them as required.
This PR updates the Mustache template that generates the method signatures and fixes the Checkout functions that include mandatory query parameters.
Note: other API will be corrected when the SDK automation Bot runs again.
Fix #1479 #1475