Skip to content

Rc_v2.2.16 - #120

Closed
bartoszkaluzny-solteq wants to merge 40 commits into
paytrail:masterfrom
Solteq:RC_v2.2.16
Closed

Rc_v2.2.16#120
bartoszkaluzny-solteq wants to merge 40 commits into
paytrail:masterfrom
Solteq:RC_v2.2.16

Conversation

@bartoszkaluzny-solteq

Copy link
Copy Markdown
Contributor

No description provided.

bartoszkaluzny-solteq and others added 30 commits August 29, 2025 13:54
Sync Solteq:staging <-> paytrail:master
SQMAGOPC-658: Apple screen reader issues with current module
* mftf-automation: Add GitHub Actions workflow to run MFTF tests for Paytrail module
* mftf-automation: Remove obsolete Paytrail configuration and currency reset steps from MFTF test files
SQMAGOPC-658: fix screen reader for selected elements
SQMAGOPC-668: Receipt controller improvement
Sync paytrail:master <-> Solteq:staging
SQMAGOPC-672: Partial refund issue investigation and fix
Sync v2.2.12 <-> Solteq:staging
SQMAGOPC-674: investigate issue 116 in github
SQMAGOPC-675:  add 'klarna' to the flag list for manual invoice
SQMAGOPC-646: Unable to add card
# Conflicts:
#	composer.json
#	etc/module.xml
bartoszkaluzny-solteq and others added 8 commits March 18, 2026 10:28
SQMAGOPC-678: Check issue with recurring_payment_schedule on admin panel
SQMAGOPC-677: Module needs support for cancelling Klarna payments that are using ManualInvoiceActivation
SQMAGOPC-677: Module needs support for cancelling Klarna payments that are using ManualInvoiceActivation

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Bumps the Paytrail_PaymentService module to v2.2.16 and introduces a new gateway command + observer flow to cancel Klarna invoices when an order is cancelled, along with an admin dropdown change for recurring payment selection.

Changes:

  • Bump module/composer version to 2.2.16 and document changes in the changelog.
  • Add an order_cancel_after observer to trigger a new invoice_cancellation gateway command for Klarna.
  • Change the “No recurring payment” admin option to an empty option in the recurring payment schedule attribute source.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
etc/module.xml Module setup_version bump to 2.2.16.
composer.json Package version bump to 2.2.16.
CHANGELOG.md Adds 2.2.16 release notes.
etc/events.xml Registers a new observer for order cancellation.
etc/di.xml Adds invoice_cancellation command wiring and additional virtual types.
Observer/OrderInvoiceCancellation.php New observer that executes invoice cancellation on order cancel.
Gateway/Command/InvoiceCancellation.php New command implementation for invoice cancellation.
Gateway/Http/Client/InvoiceCancellation.php New HTTP client wrapper calling the Paytrail SDK cancelInvoice.
Gateway/Request/InvoiceCancellationDataBuilder.php Builds the request payload for invoice cancellation.
Gateway/Response/InvoiceCancellationHandler.php Response handler for invoice cancellation command.
Model/Attribute/SelectData.php Adjusts recurring payment schedule select options.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}
}
$this->_options[] = ['label' => __('No recurring payment'), 'value' => self::NO_RECURRING_PAYMENT_VALUE];
$this->_options[] = ['label' => __(' '), 'value' => ''];

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added option uses an empty-string value ('') even though the attribute is defined as type => int (see the data patch) and the default is SelectData::NO_RECURRING_PAYMENT_VALUE (null). Saving an empty string for an int EAV attribute may coerce to 0 / not match the configured default and can break downstream logic. Consider keeping the value as null (or another consistent sentinel) and only adjusting the label/display text if you want the admin UI to appear blank.

Suggested change
$this->_options[] = ['label' => __(' '), 'value' => ''];
$this->_options[] = ['label' => __(' '), 'value' => self::NO_RECURRING_PAYMENT_VALUE];

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +41
$order = $observer->getEvent()->getOrder();
$paymentMethod = $order
->getPayment()
->getAdditionalInformation()[OrderPaymentMethodData::SELECTED_PAYMENT_METHOD_CODE];

if ($order->getState() === Order::STATE_CANCELED && $paymentMethod === 'klarna') {

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This observer runs on every order cancellation, but it immediately reads Paytrail-specific additionalInformation via array indexing. For non-Paytrail orders (or Paytrail orders missing this key), this can throw and flood logs, preventing the cancellation flow from running. Guard by first checking the payment method code (e.g., Paytrail config code) and fetch the selected method via getAdditionalInformation($key) with an empty/null fallback.

Copilot uses AI. Check for mistakes.
Comment on lines +56 to +57
'Invoice cancellation successfully with message: %message',
['message' => $this->getResponseMessage($response['data']->getHttpStatusCode())]

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The translated phrase uses a %message placeholder, but the rest of the module uses Magento's positional placeholders (e.g. %1, %2). As written, the output may literally include %message. Use the standard placeholder style so the rendered order comment shows the resolved message correctly.

Suggested change
'Invoice cancellation successfully with message: %message',
['message' => $this->getResponseMessage($response['data']->getHttpStatusCode())]
'Invoice cancellation successfully with message: %1',
$this->getResponseMessage($response['data']->getHttpStatusCode())

Copilot uses AI. Check for mistakes.
$order->getPayment()->getTransactionId(),
__(
'Invoice cancellation successfully with message: %message',
['message' => $this->getResponseMessage($response['data']->getHttpStatusCode())]

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$response['data'] is dereferenced (->getHttpStatusCode()) without verifying it is present/object. In the gateway client, there are execution paths where error can be null while data is still null (e.g., caught RequestException without a response), which would cause a fatal error here. Add a check that data is non-null and supports getHttpStatusCode() (or treat missing data as an error).

Copilot uses AI. Check for mistakes.
Comment thread etc/di.xml Outdated
Comment on lines +376 to +379
<virtualType name="PaytrailInvoiceCancellationCommandManager" type="Magento\Payment\Gateway\Command\CommandManager">
<arguments>
<argument name="commandPool" xsi:type="object">PaytrailInvoiceCancellationCommandPool</argument>
</arguments>

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PaytrailInvoiceCancellationCommandManager references PaytrailInvoiceCancellationCommandPool, but that pool is not defined anywhere in the module (search only finds this reference). Either define the missing virtualType/type for the pool or remove this unused CommandManager virtualType to avoid DI configuration that can break when compiled/instantiated.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bartoszkaluzny-solteq this is a really valid comment. we have only <virtualType name="PaytrailCommandPool" type="Magento\Payment\Gateway\Command\CommandPool"> defined, so it works but not using this command pool, please validate the code

@konrad-konieczny konrad-konieczny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check not used and non existing command pool

Comment thread etc/di.xml Outdated
Comment on lines +376 to +379
<virtualType name="PaytrailInvoiceCancellationCommandManager" type="Magento\Payment\Gateway\Command\CommandManager">
<arguments>
<argument name="commandPool" xsi:type="object">PaytrailInvoiceCancellationCommandPool</argument>
</arguments>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bartoszkaluzny-solteq this is a really valid comment. we have only <virtualType name="PaytrailCommandPool" type="Magento\Payment\Gateway\Command\CommandPool"> defined, so it works but not using this command pool, please validate the code

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.

3 participants