Skip to content

Conversation

@Ryouzanpaku
Copy link
Contributor

No description provided.

@Ryouzanpaku Ryouzanpaku requested a review from a team as a code owner August 18, 2025 09:25
@Ryouzanpaku Ryouzanpaku changed the title CC-2672/add-wero [CC-2672] add WERO Aug 18, 2025
@manas-unzer manas-unzer requested a review from Copilot September 8, 2025 07:31
Copy link

Copilot AI left a 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.

Copy link
Member

@sixer1182 sixer1182 left a 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.

Comment on lines +15 to +19
protected ?string $captureTrigger = null;
/** @see WeroAmountPaymentTypes */
protected ?string $amountPaymentType = null;
protected ?int $maxAuthToCaptureTime = null;
protected ?bool $multiCapturesAllowed = null;
Copy link
Member

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?

Copy link
Contributor Author

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);
Copy link
Member

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?

Copy link
Contributor Author

@Ryouzanpaku Ryouzanpaku Sep 19, 2025

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.

@Ryouzanpaku Ryouzanpaku requested a review from Copilot September 22, 2025 07:18
Copy link

Copilot AI left a 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.

@Ryouzanpaku Ryouzanpaku requested a review from Copilot September 22, 2025 07:59
Copy link

Copilot AI left a 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;

/*
Copy link

Copilot AI Sep 22, 2025

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.

Suggested change
/*
/**

Copilot uses AI. Check for mistakes.
Comment on lines 15 to +22
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;
Copy link

Copilot AI Sep 22, 2025

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.

Copilot uses AI. Check for mistakes.
@Ryouzanpaku Ryouzanpaku dismissed sixer1182’s stale review September 22, 2025 08:51

Fixed the CP comments.

@Ryouzanpaku Ryouzanpaku merged commit af41d62 into main Sep 22, 2025
8 of 9 checks passed
@Ryouzanpaku Ryouzanpaku deleted the CC-2672/add-wero branch September 22, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants