Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #26083 - problem with unsAdditionalInformation in \Magento\Payment\Model\Info #26084

Merged

Conversation

marcoaacoliveira
Copy link
Member

@marcoaacoliveira marcoaacoliveira commented Dec 17, 2019

Description (*)

Fix problem with unsAdditionalInformation method in \Magento\Payment\Model\Info, it wasn't init data properly.

Fixed Issues (if relevant)

  1. Problem when trying to unset additional data in payment method. #26083: Problem when trying to unset additional data in payment method

Manual testing scenarios (*)

  1. Method used
  2. Quote saved
  3. Additional information was successful removed from database.

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)

@m2-assistant
Copy link

m2-assistant bot commented Dec 17, 2019

Hi @marcoaacoliveira. 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.

@aleron75 aleron75 added the Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests label Dec 18, 2019
@aleron75
Copy link
Contributor

Hello @marcoaacoliveira, thanks for your contribution.

Due to Magento Definition of Done all code must be covered by tests.

For this specific case, you should cover your fix by automated tests with the scenario which leads to the issue: tests should fail on the mainline and pass with your fix.

To answer the question "which kind of tests should I write", the answer is:
pick the most lightweight (execution time-wise) test type that will provide sufficient coverage.

Thanks again!

@aleron75 aleron75 self-assigned this Dec 18, 2019
@marcoaacoliveira
Copy link
Member Author

@aleron75 thanks for reviewing, I'll work on it.

@aleron75 aleron75 added Auto-Tests: Covered All changes in Pull Request is covered by auto-tests and removed Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests labels Dec 28, 2019
@magento-engcom-team
Copy link
Contributor

Hi @aleron75, thank you for the review.
ENGCOM-6524 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@marcoaacoliveira thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@engcom-Delta
Copy link
Contributor

✔️ QA passed

magento-engcom-team pushed a commit that referenced this pull request Jan 3, 2020
@magento-engcom-team magento-engcom-team merged commit 408b725 into magento:2.4-develop Jan 3, 2020
@m2-assistant
Copy link

m2-assistant bot commented Jan 3, 2020

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

@sdzhepa sdzhepa mentioned this pull request May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants