add missing PaymentMethodInterface

[fooman]

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)

1. Fixes N/A

Manual testing scenarios (*)

1. Test checkout with different payment methods

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)

Resolved issues:

1. resolves [Issue] add missing PaymentMethodInterface #33008: add missing PaymentMethodInterface

[m2-assistant]

Hi @fooman. Thank you for your contribution
Here are some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[fooman]

@magento run all tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[gabrieldagama]

@magento create issue

[fooman]

I currently don't see how the B2B GrahpQl failure would be related:

Varnish cache server
'' contains 'The current customer does not have permission to manage negotiable quotes.'.

In regards to the semantic version failure I believe this to be backwards compatible and fixing an existing issue rather than introducing a new one.

[nuzil]

@magento run WebAPI Tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[nuzil]

Sorry but I do not see an issue with this, MethodInterface also has same methods defined, so there are no really issue exists.
Maybe somebody can convince me :-)

[fooman]

@nuzil

According to https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Checkout/Api/Data/PaymentDetailsInterface.php#L27

getPaymentMethods should return an array of
\Magento\Quote\Api\Data\PaymentMethodInterface
however some payment methods, for example Braintree only implement
\Magento\Payment\Model\MethodInterface
but not
\Magento\Quote\Api\Data\PaymentMethodInterface
as it is based on the Adapter class.

[orlangur]

There is already such methods duplication in

abstract class AbstractMethod extends \Magento\Framework\Model\AbstractExtensibleModel implements
    MethodInterface,
    PaymentMethodInterface

is there any real issue observable due to

would technically not be valid due to the missing interface

?

From a quick code review it looks like PaymentMethodInterface is what Quote needs to know about payment methods in its bounded context, there is even another similar interface in Payment module itself \Magento\Payment\Api\Data\PaymentMethodInterface.

My understanding is that Quote should be aware of Payment and not vice versa.

[fooman]

@orlangur in reply to

is there any real issue observable due to
would technically not be valid due to the missing interface

Yes. I wanted to plug into \Magento\Quote\Api\Data\PaymentMethodInterface to change the title of ALL payment methods however due to the difference between AbstractMethod and Adapter this was not possible.

As Adapter is being pushed as the successor to deprecated AbstractMethod it would be great if the two would fulfil the same interfaces. This PR does this.

[fooman]

My understanding is that Quote should be aware of Payment and not vice versa.

If you want to remove the dependency quote from payment there are a lot of bigger issues to work through. See https://github.com/magento/magento2/blob/2.4.3/app/code/Magento/Payment/Model/Method/Adapter.php#L22

[fooman]

And just to iterate the Interface I am adding

interface PaymentMethodInterface
{
    /**
     * Get payment method code
     *
     * @return string
     */
    public function getCode();

    /**
     * Get payment method title
     *
     * @return string
     */
    public function getTitle();
}

is already implemented here https://github.com/magento/magento2/blob/2.4.3/app/code/Magento/Payment/Model/Method/Adapter.php#L552-L566 and is part of the existing API for the class.