Skip to content
This repository was archived by the owner on Dec 19, 2019. It is now read-only.

Remove PaymentMethodAdditionalDataInput From Schema #757

Merged
merged 1 commit into from
Jul 30, 2019

Conversation

pmclain
Copy link
Contributor

@pmclain pmclain commented Jun 27, 2019

Description (*)

The empty input PaymentMethodAdditionalDataInput generated an invalid schema. This commit updates the implementation for online payment methods and allows this element to be removed. Payment method specific inputs are moved out of PaymentMethodAdditionalDataInput and into PaymentMethodInput ie:

Before

mutation {
  setPaymentMethodOnCart(input:{
    cart_id:"blahblahblah"
    payment_method:{
      code:"authorizenet_acceptjs"
      additional_data:{
        authorizenet_acceptjs:{
          opaque_data_descriptor: "COMMON.ACCEPT.INAPP.PAYMENT"
          opaque_data_value: "fake-nonce"
          cc_last_4: 1111
        }
      }
    }
  }) {
    cart {
      selected_payment_method {
        code
      }
    }
  }
}

After

mutation {
  setPaymentMethodOnCart(input:{
    cart_id:"blahblahblah"
    payment_method:{
      code:"authorizenet_acceptjs"
      authorizenet_acceptjs:{
        opaque_data_descriptor: "COMMON.ACCEPT.INAPP.PAYMENT"
        opaque_data_value: "fake-nonce"
        cc_last_4: 1111
      }
    }
  }) {
    cart {
      selected_payment_method {
        code
      }
    }
  }
}

This PR also adds tests for the AuthorizenetAcceptjsGraphQl module and the test module for mocking the required response data.

Fixed Issues (if relevant)

  1. Empty PaymentMethodAdditionalDataInput causes schema invalidation #751

Manual testing scenarios (*)

  1. cd dev/tests/api-functional
  2. ../../../vendor/bin/phpunit testsuite/Magento/GraphQl/AuthorizenetAcceptjs/Customer/SetPaymentMethodTest.php
    3.../../../vendor/bin/phpunit testsuite/Magento/GraphQl/AuthorizenetAcceptjs/Guest/SetPaymentMethodTest.php

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)

@slavvka
Copy link
Member

slavvka commented Jun 27, 2019

Hey @pmclain. I have found the issue with WebAPI tests. It is the fatal error in fixtures which is currently not supported by the reports:

09:31:47 PHP Fatal error:  Magento\TestFramework\Annotation\ApiDataFixture::_applyOneFixture(): Failed opening required '/var/www/html/dev/tests/integration/testsuite/Magento/Graphql/AuthorizenetAcceptjs/_files/enable_authorizenetacceptjs.php' (include_path='/var/www/html/dev/tests/api-functional/./testsuite:/var/www/html/generated/code:/var/www/html/generated/code:/var/www/html/dev/tests/api-functional/./testsuite:/var/www/html/vendor/magento/zendframework1/library:.:/usr/share/pear:/usr/share/php') in /var/www/html/dev/tests/api-functional/framework/Magento/TestFramework/Annotation/ApiDataFixture.php on line 111

@magento-engcom-team
Copy link
Contributor

Hi @lenaorobei, thank you for the review.
ENGCOM-5369 has been created to process this Pull Request
✳️ @lenaorobei, 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

-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_TestModuleAuthorizenetAcceptjs" />
</config>
Copy link
Contributor

Choose a reason for hiding this comment

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

@pmclain , could you please add an empty line after </config> tag. Thanks

@pmclain
Copy link
Contributor Author

pmclain commented Jul 24, 2019

working on the integration test failures now. these were expected to fail once the CI was configured to run them #772

@ghost
Copy link

ghost commented Jul 24, 2019

Hi @pmclain, 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.

@ghost
Copy link

ghost commented Jul 30, 2019

Hi @pmclain, 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.

keharper pushed a commit to magento/devdocs that referenced this pull request Aug 10, 2019
This PR removes the PaymentMethodAdditionalDataInput and moves its children up
one level into the PaymentMethodInput.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants