[Issue] add missing PaymentMethodInterface

m2-assistant[bot] commented 3 years ago

This issue is automatically created based on existing pull request: magento/magento2#33003: add missing PaymentMethodInterface

Description (*)

During checkout payment information details can be requested via https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Checkout/Model/PaymentInformationManagement.php#L185

which uses

        $paymentDetails->setPaymentMethods($this->paymentMethodManagement->getList($cartId));

According to https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Checkout/Api/Data/PaymentDetailsInterface.php#L25 this should be an array of \Magento\Quote\Api\Data\PaymentMethodInterface

Any Payment Methods based on https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Payment/Model/Method/Adapter.php would technically not be valid due to the missing interface. The interface methods are already implemented. As the Adapter class already includes these methods this should not be a breaking change.

Related Pull Requests

Fixed Issues (if relevant)

Fixes N/A

Manual testing scenarios (*)

Test checkout with different payment methods

Additional Information to reproduce the issue

we have tried to debug the code. According to the interface app/code/Magento/Checkout/Api/Data/PaymentDetailsInterface.php:

https://github.com/magento/magento2/blob/84e7dfaf95401d0f431094d55ccb88c36526a6eb/app/code/Magento/Checkout/Api/Data/PaymentDetailsInterface.php#L24-L27

The overriding/implemented method should return the object of the class \Magento\Quote\Api\Data\PaymentMethodInterface[]. And we can see it is implemented in the below class:

https://github.com/magento/magento2/blob/84e7dfaf95401d0f431094d55ccb88c36526a6eb/app/code/Magento/Checkout/Model/PaymentDetails.php#L17-L20

Which internally calls the $this->getData(self::PAYMENT_METHODS);:

https://github.com/magento/magento2/blob/84e7dfaf95401d0f431094d55ccb88c36526a6eb/lib/internal/Magento/Framework/Model/AbstractExtensibleModel.php#L276-L305

Which is returning the mixed array as shown in the below screenshot:

I think you are talking about the same thing it should return the object of \Magento\Quote\Api\Data\PaymentMethodInterface[] but it is returning the mixed array.

Questions or comments

Contribution checklist (*)

[ ] Pull request has a meaningful description of its purpose
[ ] All commits are accompanied by meaningful commit messages
[ ] All new or changed code is covered with unit/integration tests (if applicable)
[ ] README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
[ ] All automated tests passed successfully (all builds are green)

engcom-Hotel commented 2 years ago

Hello @fooman,

Thanks for the report and collaboration!

We appreciate it if you provide the below information related to this issue:

Information on your environment
Steps to reproduce
Expected and actual results

Thanks

fooman commented 2 years ago

@engcom-Hotel thanks for checking in on this.

To answer your questions:

environment
all supported Magento versions

steps to reproduce
write a plugin to change the title of a payment method during checkout targeting \Magento\Quote\Api\Data\PaymentMethodInterface::getTitle()

expected results I can change the title of all payment methods that follow Magento best practices. According to https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Checkout/Api/Data/PaymentDetailsInterface.php#L25 this should be an array of \Magento\Quote\Api\Data\PaymentMethodInterface

actual results I can only change payment methods using the old approach (that extend \Magento\Payment\Model\Method\AbstractMethod) https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Payment/Model/Method/AbstractMethod.php#L27

however payment methods using \Magento\Payment\Model\Method\Adapter like Braintree do not work due to the missing interface so the documented return for PaymentDetailsInterface::getPaymentMethods() is not accurate.

m2-assistant[bot] commented 2 years ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
Details
If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.
[ ] 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
[ ] 4. Verify that the issue is reproducible on 2.4-develop branch
Details
- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 5. Add label Issue: Confirmed once verification is complete.
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

engcom-Hotel commented 1 year ago

Hello @fooman,

Thanks for the reply!

I request you please try to reproduce the issue in the latest Magento 2.4-develop branch and let us know if the issue is still reproducible for you.

Thanks

fooman commented 1 year ago

@engcom-Hotel the above code links point to 2.4-develop and the issue is still observable there.

engcom-Hotel commented 1 year ago

Hello @fooman,

I am trying to understand the issue. So as per your comment, we have tried to debug the code. According to the interface app/code/Magento/Checkout/Api/Data/PaymentDetailsInterface.php:

https://github.com/magento/magento2/blob/84e7dfaf95401d0f431094d55ccb88c36526a6eb/app/code/Magento/Checkout/Api/Data/PaymentDetailsInterface.php#L24-L27

The overriding/implemented method should return the object of the class \Magento\Quote\Api\Data\PaymentMethodInterface[]. And we can see it is implemented in the below class:

https://github.com/magento/magento2/blob/84e7dfaf95401d0f431094d55ccb88c36526a6eb/app/code/Magento/Checkout/Model/PaymentDetails.php#L17-L20

Which internall calls the $this->getData(self::PAYMENT_METHODS);:

https://github.com/magento/magento2/blob/84e7dfaf95401d0f431094d55ccb88c36526a6eb/lib/internal/Magento/Framework/Model/AbstractExtensibleModel.php#L276-L305

Which is returning the mixed array as shown in the below screenshot:

I think you are talking about the same thing it should return the object of \Magento\Quote\Api\Data\PaymentMethodInterface[] but it is returning the mixed array.

Please confirm.

Thanks

fooman commented 1 year ago

@engcom-Hotel thanks for taking a look. Yes this is indeed the issue. If you look at the individual classes Checkmo and Purchaseorder both extend \Magento\Payment\Model\Method\AbstractMethod which implements Magento\Quote\Api\Data\PaymentMethodInterface however the last one Magento\Payment\Model\Method\Adapter does not.

engcom-Hotel commented 1 year ago

Hello @fooman,

Thanks for the update!

I agree with your point. Hence updating the main description and confirming this issue.

Thanks

github-jira-sync-bot commented 1 year ago

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-7419 is successfully created for this GitHub issue.

m2-assistant[bot] commented 1 year ago

:white_check_mark: Confirmed by @engcom-Hotel. Thank you for verifying the issue.
Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

engcom-Hotel commented 1 month ago

Hello @fooman,

It seems the issue is not relevant as the app/code/Magento/Payment/Model/Method/AbstractMethod.php is marked as deprecated and we need to use the \Magento\Payment\Model\Method\Adapter class. Hence closing this issue.

Thanks

fooman commented 1 month ago

@engcom-Hotel it's the other way around \Magento\Payment\Model\Method\Adapter is missing the interface - see the PR here https://github.com/magento/magento2/pull/33003/files

engcom-Hotel commented 1 month ago

Hello @fooman,

I got your point, let me reopen it.

Thanks

github-jira-sync-bot commented 1 month ago

:x: Cannot export the issue. This GitHub issue is already linked to Jira issue(s): https://jira.corp.adobe.com/browse/AC-7419

magento / magento2