-
Notifications
You must be signed in to change notification settings - Fork 116
Generate Webhooks models #808
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.
Summary of Changes
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!
This pull request updates the webhook models for ACS, Report, Transfers, and Transaction services to reflect the latest OpenAPI specifications. Key changes include refining existing model properties, enhancing error handling for enum validations, and introducing new models and fields to support expanded data requirements and functionalities, such as detailed 3D Secure information and precise transfer scheduling. The updates ensure that the webhook integration remains current and robust.
Highlights
- Updated Webhook Models: The models for ACS, Report, Transfers, and Transaction webhooks have been regenerated to align with the latest OpenAPI specifications, ensuring up-to-date API interactions.
- Enhanced Error Handling for Enum Values: Error handling for invalid enum values has been improved across various models, changing from throwing
InvalidArgumentExceptionto logging errors usingerror_log. This prevents application crashes due to unexpected data while still providing valuable debugging information. - Deprecation of
SplFileObjectDeserialization: TheSplFileObjectdeserialization logic has been removed fromObjectSerializer.phpin multiple webhook modules, streamlining the serialization process and potentially simplifying file handling. - Enriched Relayed Authentication Data: The
RelayedAuthenticationRequestmodel in ACS Webhooks now includes new fields:environment,threeDSRequestorAppURL,timestamp, andtype, providing richer context for relayed authentication requests. - Introduction of
ThreeDSecureModel: A newThreeDSecuremodel has been introduced and integrated intoIssuedCardandTransferViewCategoryDatawithin Transaction and Transfer Webhooks, allowing for more detailed 3D Secure information to be processed. - New
ExecutionDateModel for Transfers: TheExecutionDatemodel has been added to Transfer Webhooks, includingdateandtimezonefields, and integrated intoTransferData. This enables more precise scheduling and tracking of transfers. - Expanded Party Identification Details: The
PartyIdentificationmodel in Transfer Webhooks now includesemailandurlfields, enhancing the information available for counterparty identification. - Extended International Bank Identification: Support for Australian BSB codes (
auBsbCode) and Canadian routing numbers (caRoutingNumber) has been added toAdditionalBankIdentificationin Transfer Webhooks, broadening international bank identification capabilities.
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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
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.
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 updates various webhook models based on the latest OpenAPI specifications. The changes include adding new models and properties, updating documentation, and modifying error handling for invalid enum values.
A major concern is the change in error handling for invalid enum values in setters across numerous model classes. The previous implementation threw an InvalidArgumentException, which is a fail-fast approach. The new implementation uses error_log and continues execution, which can lead to objects being in an invalid state. This can cause subtle bugs that are hard to trace. It is highly recommended to revert this change and throw an exception when an invalid enum value is provided to ensure data integrity and robust error handling.
| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'status', must be one of '%s'", | ||
| "status: unexpected enum value '%s' - Supported values are [%s]", | ||
| $status, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setStatus() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid status value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'status', must be one of '%s'",
$status,
implode("', '", $allowedValues)
)
);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.
@gemini-code-assist This is done by design: the goal is not to throw an exception when an unknown value is encountered, but rather just log a warning and use the value as-is (this is a valid value as it is defined by the API). Revise and close all your comments related to this issue.
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.
Thank you for clarifying the design decision. I understand that the intention is to log unexpected enum values as warnings and proceed, as these are considered valid by the API. I will revise and close all related comments accordingly.
| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'challengeIndicator', must be one of '%s'", | ||
| "challengeIndicator: unexpected enum value '%s' - Supported values are [%s]", | ||
| $challengeIndicator, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setChallengeIndicator() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid challengeIndicator value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'challengeIndicator', must be one of '%s'",
$challengeIndicator,
implode("', '", $allowedValues)
)
);| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'deviceChannel', must be one of '%s'", | ||
| "deviceChannel: unexpected enum value '%s' - Supported values are [%s]", | ||
| $deviceChannel, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setDeviceChannel() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid deviceChannel value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'deviceChannel', must be one of '%s'",
$deviceChannel,
implode("', '", $allowedValues)
)
);| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'exemptionIndicator', must be one of '%s'", | ||
| "exemptionIndicator: unexpected enum value '%s' - Supported values are [%s]", | ||
| $exemptionIndicator, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setExemptionIndicator() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid exemptionIndicator value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'exemptionIndicator', must be one of '%s'",
$exemptionIndicator,
implode("', '", $allowedValues)
)
);| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'messageCategory', must be one of '%s'", | ||
| "messageCategory: unexpected enum value '%s' - Supported values are [%s]", | ||
| $messageCategory, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setMessageCategory() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid messageCategory value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'messageCategory', must be one of '%s'",
$messageCategory,
implode("', '", $allowedValues)
)
);| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'type', must be one of '%s'", | ||
| "type: unexpected enum value '%s' - Supported values are [%s]", | ||
| $type, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setType() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid type value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'type', must be one of '%s'",
$type,
implode("', '", $allowedValues)
)
);| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'scaOnApproval', must be one of '%s'", | ||
| "scaOnApproval: unexpected enum value '%s' - Supported values are [%s]", | ||
| $scaOnApproval, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setScaOnApproval() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid scaOnApproval value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'scaOnApproval', must be one of '%s'",
$scaOnApproval,
implode("', '", $allowedValues)
)
);| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'type', must be one of '%s'", | ||
| "type: unexpected enum value '%s' - Supported values are [%s]", | ||
| $type, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setType() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid type value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'type', must be one of '%s'",
$type,
implode("', '", $allowedValues)
)
);| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'accountType', must be one of '%s'", | ||
| "accountType: unexpected enum value '%s' - Supported values are [%s]", | ||
| $accountType, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setAccountType() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid accountType value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'accountType', must be one of '%s'",
$accountType,
implode("', '", $allowedValues)
)
);| error_log( | ||
| sprintf( | ||
| "Invalid value '%s' for 'type', must be one of '%s'", | ||
| "type: unexpected enum value '%s' - Supported values are [%s]", | ||
| $type, | ||
| implode("', '", $allowedValues) | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); |
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.
The error handling for invalid enum values in setType() has been changed from throwing an InvalidArgumentException to logging an error with error_log. This is a significant change that can lead to issues. By not throwing an exception, the application will continue to run with an invalid type value, which can cause unpredictable behavior and hard-to-debug issues later on. It's better to fail fast. Please consider reverting this to throw an InvalidArgumentException.
throw new \InvalidArgumentException(
sprintf(
"Invalid value '%s' for 'type', must be one of '%s'",
$type,
implode("', '", $allowedValues)
)
);|
Warning Gemini encountered an error creating the review. You can try again by commenting |



Generate webhook models to reflect latest OpenAPI specs:
ACS Webhooks
balancePlatform.authentication.relayedeventRelayedAuthenticationRequestadd new attributesthreeDSRequestorAppURL,environment,timestamp,typeReport Webhooks
Transaction Webhooks
IssuedCardadd new attributethreeDSecureTransferViewCategoryDataadd new attributethreeDSecureTranfers Webhooks
IssuedCardadd new attributethreeDSecureTransferViewCategoryDataadd new attributethreeDSecureAdditionalBankIdentificationadd new enumsauBsbCodeandcaRoutingNumberexecutionDateinTransfer,TransferDataandTransferInfoemailandurltoPartyIdentificationandUltimatePartyIdentificationTransferDataadd new attributecreatedAtandupdatedAt, deprecatecreationDate(use insteadcreatedAt)approvalExpiredtoTransferDataandTransferEvent