[transfers] Code generation: update services and models#862
Conversation
Summary of ChangesHello @AdyenAutomationBot, 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 introduces automated code updates for the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request contains automated changes to update services and models for the transfers service. The changes primarily involve adding new properties, constants, and updating PHPDoc comments. My review focuses on improving the quality of the generated code. I've identified several inconsistencies in property nullability definitions versus their usage, which could lead to runtime errors. I've also suggested adding PHP type hints to new methods for better type safety and code clarity, improving validation logic to be more robust, and fixing minor formatting issues in documentation.
| if (!in_array($sourceOfFunds, $allowedValues, true)) { | ||
| error_log( | ||
| sprintf( | ||
| "sourceOfFunds: unexpected enum value '%s' - Supported values are [%s]", | ||
| $sourceOfFunds, | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Using error_log for invalid enum values can lead to silent failures and makes debugging harder. It's better to throw an InvalidArgumentException to enforce valid data at the time of setting the property. This ensures the object is always in a valid state.
if (!is_null($sourceOfFunds) && !in_array($sourceOfFunds, $allowedValues, true)) {
throw new \InvalidArgumentException(
sprintf(
"Invalid value for 'sourceOfFunds', must be one of '%s'",
implode("', '", $allowedValues)
)
);
}| 'accountHolder' => false, | ||
| 'accountIdentification' => false | ||
| 'accountIdentification' => false, | ||
| 'storedPaymentMethodId' => false |
There was a problem hiding this comment.
There's an inconsistency regarding the nullability of the storedPaymentMethodId property. It's marked as non-nullable here, but the corresponding getter/setter PHPDocs and its initialization in the constructor suggest it can be null. To ensure consistency and prevent potential issues, this should be marked as nullable.
'storedPaymentMethodId' => true| * Gets storedPaymentMethodId | ||
| * | ||
| * @return string|null | ||
| */ | ||
| public function getStoredPaymentMethodId() | ||
| { | ||
| return $this->container['storedPaymentMethodId']; | ||
| } | ||
|
|
||
| /** | ||
| * Sets storedPaymentMethodId | ||
| * | ||
| * @param string|null $storedPaymentMethodId The unique token that identifies the stored bank account details of the counterparty for a payout. | ||
| * | ||
| * @return self | ||
| */ | ||
| public function setStoredPaymentMethodId($storedPaymentMethodId) | ||
| { | ||
| $this->container['storedPaymentMethodId'] = $storedPaymentMethodId; | ||
|
|
||
| return $this; | ||
| } |
There was a problem hiding this comment.
For better code clarity and type safety, please add scalar type hints to the newly added methods getStoredPaymentMethodId and setStoredPaymentMethodId. This aligns with modern PHP best practices and improves consistency with other methods in the class that already use type hints.
/**
* Gets storedPaymentMethodId
*
* @return string|null
*/
public function getStoredPaymentMethodId(): ?string
{
return $this->container['storedPaymentMethodId'];
}
/**
* Sets storedPaymentMethodId
*
* @param string|null $storedPaymentMethodId The unique token that identifies the stored bank account details of the counterparty for a payout.
*
* @return self
*/
public function setStoredPaymentMethodId(?string $storedPaymentMethodId): self
{
$this->container['storedPaymentMethodId'] = $storedPaymentMethodId;
return $this;
}| * Sets status | ||
| * | ||
| * @param string $status The status of the transfer. Possible values: - **credited**: the funds are credited to your user's transfer instrument or bank account. | ||
| * @param string $status The status of the transfer. Possible values: - **credited**: the funds are credited to your user's transfer instrument or bank account.- **accepted**: the request is accepted by the integration. |
There was a problem hiding this comment.
There seems to be a formatting issue in the PHPDoc. A space is missing between the description for credited and the new accepted status, which makes it harder to read.
* @param string $status The status of the transfer. Possible values: - **credited**: the funds are credited to your user's transfer instrument or bank account. - **accepted**: the request is accepted by the integration.| protected static $openAPINullables = [ | ||
| 'cardIdentification' => false, | ||
| 'networkPaymentReference' => false, | ||
| 'reference' => false, | ||
| 'sourceOfFunds' => false | ||
| ]; |
There was a problem hiding this comment.
The properties in this model are marked as non-nullable, but their PHPDocs and usage in the constructor suggest they can be null. This inconsistency can lead to unexpected behavior. Please update the nullability settings to reflect that they are optional.
protected static $openAPINullables = [
'cardIdentification' => true,
'networkPaymentReference' => true,
'reference' => true,
'sourceOfFunds' => true
];| public function getCardIdentification() | ||
| { | ||
| return $this->container['cardIdentification']; | ||
| } | ||
|
|
||
| /** | ||
| * Sets cardIdentification | ||
| * | ||
| * @param \Adyen\Model\Transfers\CardIdentification|null $cardIdentification cardIdentification | ||
| * | ||
| * @return self | ||
| */ | ||
| public function setCardIdentification($cardIdentification) | ||
| { | ||
| $this->container['cardIdentification'] = $cardIdentification; | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * Gets networkPaymentReference | ||
| * | ||
| * @return string|null | ||
| */ | ||
| public function getNetworkPaymentReference() | ||
| { | ||
| return $this->container['networkPaymentReference']; | ||
| } | ||
|
|
||
| /** | ||
| * Sets networkPaymentReference | ||
| * | ||
| * @param string|null $networkPaymentReference The unique reference assigned by the card network for the pay-in transaction. | ||
| * | ||
| * @return self | ||
| */ | ||
| public function setNetworkPaymentReference($networkPaymentReference) | ||
| { | ||
| $this->container['networkPaymentReference'] = $networkPaymentReference; | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * Gets reference | ||
| * | ||
| * @return string|null | ||
| */ | ||
| public function getReference() | ||
| { | ||
| return $this->container['reference']; | ||
| } | ||
|
|
||
| /** | ||
| * Sets reference | ||
| * | ||
| * @param string|null $reference Your internal reference that identifies this funding instrument. Required if `sourceOfFunds` is **DEPOSIT_ACCOUNT**. | ||
| * | ||
| * @return self | ||
| */ | ||
| public function setReference($reference) | ||
| { | ||
| $this->container['reference'] = $reference; | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * Gets sourceOfFunds | ||
| * | ||
| * @return string|null | ||
| */ | ||
| public function getSourceOfFunds() | ||
| { | ||
| return $this->container['sourceOfFunds']; | ||
| } | ||
|
|
||
| /** | ||
| * Sets sourceOfFunds | ||
| * | ||
| * @param string|null $sourceOfFunds Indicates where the funds used for the transfer originated. Possible values are: - **DEBIT** for card-to-card transfers. - **DEPOSIT_ACCOUNT** for wallet-to-card transfers. | ||
| * | ||
| * @return self | ||
| */ | ||
| public function setSourceOfFunds($sourceOfFunds) | ||
| { | ||
| $allowedValues = $this->getSourceOfFundsAllowableValues(); | ||
| if (!in_array($sourceOfFunds, $allowedValues, true)) { | ||
| error_log( | ||
| sprintf( | ||
| "sourceOfFunds: unexpected enum value '%s' - Supported values are [%s]", | ||
| $sourceOfFunds, | ||
| implode(', ', $allowedValues) | ||
| ) | ||
| ); | ||
| } | ||
| $this->container['sourceOfFunds'] = $sourceOfFunds; | ||
|
|
||
| return $this; | ||
| } |
There was a problem hiding this comment.
To improve code quality and prevent type-related errors, please add scalar type hints to the method signatures for all getters and setters in this class. This aligns with modern PHP best practices.
For example:
public function getCardIdentification(): ?\Adyen\Model\Transfers\CardIdentification
{
// ...
}
public function setCardIdentification(?\Adyen\Model\Transfers\CardIdentification $cardIdentification): self
{
// ...
}This should be applied to all new getter/setter methods in this file.
| 'email' => false, | ||
| 'firstName' => false, | ||
| 'fullName' => false, | ||
| 'fundingInstrument' => false, |
There was a problem hiding this comment.
There's an inconsistency regarding the nullability of the fundingInstrument property. It's marked as non-nullable here, but the corresponding getter/setter PHPDocs and its initialization in the constructor suggest it can be null. To ensure consistency and prevent potential issues, this should be marked as nullable.
'fundingInstrument' => true,| * Gets fundingInstrument | ||
| * | ||
| * @return \Adyen\Model\Transfers\FundingInstrument|null | ||
| */ | ||
| public function getFundingInstrument() | ||
| { | ||
| return $this->container['fundingInstrument']; | ||
| } | ||
|
|
||
| /** | ||
| * Sets fundingInstrument | ||
| * | ||
| * @param \Adyen\Model\Transfers\FundingInstrument|null $fundingInstrument fundingInstrument | ||
| * | ||
| * @return self | ||
| */ | ||
| public function setFundingInstrument($fundingInstrument) | ||
| { | ||
| $this->container['fundingInstrument'] = $fundingInstrument; | ||
|
|
||
| return $this; | ||
| } |
There was a problem hiding this comment.
For better code clarity and type safety, please add scalar type hints to the newly added methods getFundingInstrument and setFundingInstrument. This aligns with modern PHP best practices and improves consistency with other methods in the class that already use type hints.
/**
* Gets fundingInstrument
*
* @return \Adyen\Model\Transfers\FundingInstrument|null
*/
public function getFundingInstrument(): ?\Adyen\Model\Transfers\FundingInstrument
{
return $this->container['fundingInstrument'];
}
/**
* Sets fundingInstrument
*
* @param \Adyen\Model\Transfers\FundingInstrument|null $fundingInstrument fundingInstrument
*
* @return self
*/
public function setFundingInstrument(?\Adyen\Model\Transfers\FundingInstrument $fundingInstrument): self
{
$this->container['fundingInstrument'] = $fundingInstrument;
return $this;
}
AdyenAutomationBot
force-pushed
the
sdk-automation/transfers
branch
4 times, most recently
from
ab558b8 to
487d97b
Compare
AdyenAutomationBot
force-pushed
the
sdk-automation/transfers
branch
from
487d97b to
4fe81ef
Compare
|
2 participants
This PR contains the automated changes for the
transfersservice.The commit history of this PR reflects the
adyen-openapicommits that have been applied.Transfers API
storedPaymentMethodIdinBankAccountV3STATUS_ACCEPTEDinConfirmationTrackingDatafundingInstrumentinUltimatePartyIdentificationPLATFORM_PAYMENT_TYPE_DCC_MARKUPinPlatformPaymentTransferData,TransferDataCategoryData,TransferEvent