Skip to content

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

Merged
merged 2 commits into from
May 28, 2025
Merged

Conversation

gcatanese
Copy link
Contributor

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

@gcatanese gcatanese requested a review from a team as a code owner May 23, 2025 11:20
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 as required in the OpenAPI spec are generated without the optional ? marker in the TypeScript method signatures.
  • Checkout API Updates: The deleteTokenForStoredPaymentDetails method in RecurringApi now requires shopperReference and merchantAccount as mandatory parameters. The getResultOfPaymentSession method in PaymentsApi now requires sessionResult as a mandatory parameter.
  • Test Adjustments: Tests in checkout.spec.ts have been updated to pass the newly mandatory query parameters (shopperReference, merchantAccount) to the deleteTokenForStoredPaymentDetails call and include them in the nock scope URL. A test case for getResultOfPaymentSession 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 and service.ts.

Changelog

Click here to see the changelog
  • src/tests/checkout.spec.ts
    • Updated the delete stored paymentMethod test to include shopperReference and merchantAccount query parameters in the nock URL (line 566).
    • Updated the delete stored paymentMethod test to pass shopperReference and merchantAccount as arguments to deleteTokenForStoredPaymentDetails (line 570).
    • Removed the test case Should handle request without query parameters for getResultOfPaymentSession (lines 575-585).
  • 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 and test descriptions (lines 184, 187, 195).
  • src/service.ts
    • Changed single quotes to double quotes in the string literal /possdk/v68 (line 54).
  • src/services/checkout/paymentsApi.ts
    • Changed the sessionResult parameter from optional (sessionResult?: string) to required (sessionResult: string) in the getResultOfPaymentSession method signature (line 65).
  • src/services/checkout/recurringApi.ts
    • Changed the shopperReference and merchantAccount parameters from optional (shopperReference?: string, merchantAccount?: string) to required (shopperReference: string, merchantAccount: string) in the deleteTokenForStoredPaymentDetails method signature (line 39).
  • 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).
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

  1. 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.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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.

@gcatanese gcatanese force-pushed the mandatory-query-param-fix branch from 636c9b1 to 60baf9c Compare May 28, 2025 08:00
Copy link

@ayodejidev ayodejidev removed the request for review from ayodeji-adyen May 28, 2025 08:16
@gcatanese gcatanese merged commit 5ee100e into main May 28, 2025
7 checks passed
@gcatanese gcatanese deleted the mandatory-query-param-fix branch May 28, 2025 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: getResultOfPaymentSession parameters are mandatory
3 participants