-
Notifications
You must be signed in to change notification settings - Fork 14
[CC-2672] add WERO #216
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
[CC-2672] add WERO #216
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.
Pull Request Overview
This PR adds support for the WERO payment method by implementing the necessary classes, constants, and transaction data handling. WERO is a new payment type that requires event-dependent payment configuration with capture triggers and amount payment types.
- Introduces WERO payment type with embedded transaction data resources
- Adds constants for WERO-specific capture triggers and payment types
- Implements additional transaction data handling for WERO payments
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Resources/PaymentTypes/Wero.php | Basic WERO payment type class |
| src/Resources/EmbeddedResources/WeroTransactionData.php | Transaction data container for WERO |
| src/Resources/EmbeddedResources/WeroEventDependentPayment.php | Event-dependent payment configuration |
| src/Constants/WeroCaptureTriggers.php | Capture trigger constants |
| src/Constants/WeroAmountPaymentTypes.php | Amount payment type constants |
| src/Constants/IdStrings.php | Added WERO ID string constant |
| src/Constants/AdditionalTransactionDataKeys.php | Added WERO key constant |
| src/Services/ResourceService.php | Added WERO type instantiation |
| src/Traits/HasAdditionalTransactionData.php | WERO transaction data getters/setters |
| src/Resources/TransactionTypes/AbstractTransactionType.php | WERO response handling |
| src/Resources/EmbeddedResources/Paypage/PaymentMethodConfig.php | WERO paypage config |
| test/unit/Resources/EmbeddedResources/WeroTransactionDataTest.php | Unit tests for WERO transaction data |
| test/unit/Traits/HasAdditionalTransactionDataTest.php | Unit tests for WERO trait methods |
| test/integration/PaymentTypes/WeroTest.php | Integration tests for WERO payments |
| test/integration/Resources/PaypageV2Test.php | Paypage tests with WERO config |
| test/unit/Services/ResourceServiceTest.php | Service tests including WERO URL mapping |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Please check my comments and copilot (he found actual errors).
In general: please resolve copilot comments before requesting reviews.
Always add a comment when resolving copilot comments.
| protected ?string $captureTrigger = null; | ||
| /** @see WeroAmountPaymentTypes */ | ||
| protected ?string $amountPaymentType = null; | ||
| protected ?int $maxAuthToCaptureTime = null; | ||
| protected ?bool $multiCapturesAllowed = null; |
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.
does it maybe make sense to set some default values?
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.
I would not for now. If we have default values we want to use, i can update that later
| ->setMaxAuthToCaptureTime(300) | ||
| ->setMultiCapturesAllowed(false) | ||
| ); | ||
| $authorization->setWeroTransactionData($weroData); |
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.
here there is just one test for each charge and authorize incl, ATD, but in java SDK you have a separate test for each without ATD. Which makes more sense?
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.
I think it doesnt make a big difference at.At some point i though it might not be worth adding an extra test, because its giving a proper reponse anyways.
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.
Pull Request Overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Pull Request Overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| use UnzerSDK\Adapter\HttpAdapterInterface; | ||
| use UnzerSDK\Resources\AbstractUnzerResource; | ||
|
|
||
| /* |
Copilot
AI
Sep 22, 2025
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.
Use proper PHPDoc format with /** instead of /* for class documentation comments.
| /* | |
| /** |
| use UnzerSDK\Constants\CustomerTypes; | ||
| use UnzerSDK\Constants\ShippingTypes; | ||
| use UnzerSDK\Exceptions\UnzerApiException; | ||
| use UnzerSDK\Resources\Customer; | ||
| use UnzerSDK\Resources\CustomerFactory; | ||
| use UnzerSDK\Resources\EmbeddedResources\Address; | ||
| use UnzerSDK\Resources\EmbeddedResources\Paylater\InstallmentPlansQuery; | ||
| use UnzerSDK\Resources\EmbeddedResources\Paylater\InstallmentPlan; | ||
| use UnzerSDK\Resources\EmbeddedResources\Paylater\InstallmentPlansQuery; |
Copilot
AI
Sep 22, 2025
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 import statements are not alphabetically ordered. InstallmentPlansQuery should come after InstallmentPlan.
No description provided.