Avoid exception when config.xml nodes exist for not-installed payment methods

[fredden]

Description

When a payment configuration node exists in XML but there is no <model> defined, getList() will throw an UnexpectedValueException. One use case for creating such a set-up is a module that provides default configuration overrides for payment methods which are not installed on the current website. For example:

<config>
  <default>
    <payment>
      <checkmo>
        <active>0</active>
      </checkmo>
      <not_installed_here>
        <debug>0</debug>
        <environment>production</environment>
      </not_installed_here>
    </payment>
  </default>
</config>

In this case, the payment method not_installed_here does not have a <model> node defined, as the payment method module is not installed on the website.

Related Pull Requests

None known

Fixed Issues

None known

Manual testing scenarios (*)

1. Add supplied XML or similar to a local module's config.xml
2. Browse to checkout with an item in basket
3. Observe (lack of) exception thrown during checkout page load

Questions or comments

I have not yet written/changed any unit/functional tests for this change. I suspect some existing tests may fail which I will review at a later date.

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)
• All automated tests passed successfully (all builds are green)

Resolved issues:

1. resolves [Issue] Avoid exception when config.xml nodes exist for not-installed payment methods #29555: Avoid exception when config.xml nodes exist for not-installed payment methods

[m2-assistant]

Hi @fredden. Thank you for your contribution
Here is 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

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

[ghost]

With this code change applied, there are issues reloading checkout payments. An example from internal testing:

Expected Result:

• Zero Subtotal Checkout is enabled
• Apply reward points so that order grand total reaches 0
• The free checkout method should be displayed on the frontend
• The existing payment methods should be removed from the frontend

Actual Result:

• Zero Subtotal Checkout is enabled
• Apply reward points so that order grand total reaches 0
• The free checkout method is not displayed at all
• The existing payment methods remain
• An error is displayed on the page stating the current payment method is no longer available

This is perhaps due to the second parameter of the getStoreMethods function being null instead of a Quote model, but cannot be sure.

[sidolov]

@magento create issue

[sidolov]

@fredden please, take a look at comment, looks like provided implementation broke the functionality.

[engcom-Charlie]

Hi @fredden, can you please look through at comment #27940 (comment) ?

[gabrieldagama]

The risk was set tomedium due to: the suggested changes shouldn't affect other areas but it affects a critical part of e-commerce.

[engcom-Charlie]

@fredden can you please fix static tests and cover changes by autotest?
Thank you.

[engcom-Charlie]

@fredden can you please cover changes by autotest? Otherwise, we can't proceed with your PR.
Thank you.

[fredden]

@engcom-Charlie, how would you suggest writing a test for this? I'm considering adding some XML nodes to app/code/Magento/Payment/etc/config.xml, but don't really want these to end up in a production release. Is there a way to inject/set configuration specifically for the test-suite?

[engcom-Charlie]

@fredden, you can do this, for example like here dev/tests/integration/_files/Magento/TestModuleFakePaymentMethod/etc/config.xml.
Thank you.

[engcom-Charlie]

@sidolov I checked test \Magento\Checkout\Model\ShippingInformationManagementTest::testQuoteApiWithOnlyVirtualProducts
without changes from this PR in Magento/Payment/Model/PaymentMethodList it falls with exception
UnexpectedValueException : Payment model name is not provided in config!

With changes from this PR test passed.
What do you think it will be enough for test coverage?

[sidolov]

@engcom-Charlie sounds reasonable, I see additional config was added for the exceptional case, so, it's enough to proceed with PR. Thank you!

[magento-engcom-team]

Hi @sidolov, thank you for the review.
ENGCOM-8283 has been created to process this Pull Request
✳️ @sidolov, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[engcom-Delta]

✔️ QA passed
Result according Manual testing scenarios:
❌ Before:
Exception error Payment model name is not provided in config! {"exception":"[object] (UnexpectedValueException(code: 0): Payment model name is not provided in config! in logs

✔️ After:
No exception errors

Result according comment with coupon code:

[engcom-Delta]

Note: Automation tests are passed

[m2-assistant]

Hi @fredden, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.