Skip to content

Fix for issue 25147 - Totals information management module - only set carrier/method code if set in address #25510

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

Merged

Conversation

joe-vortex
Copy link
Member

@joe-vortex joe-vortex commented Nov 7, 2019

  • Totals Information Management - setting quote shipping method when address method is not null

Description (*)

Fix for setting shipping carrier and method codes only when address contains it.

Fixed Issues (if relevant)

  1. Fixes Totals Information Management - setting quote shipping method when address method is null #25147: Totals Information Management - setting quote shipping method when address method is null - Totals Information Management - setting quote shipping method when address method is null #25147

Manual testing scenarios (*)

  1. Install Amasty Conditions module and go to checkout
  2. Without amasty module - create api call to /V1/guest-carts/:cartId/totals-information then refresh page. Shipping will show as null in console and 0.00 in sumary

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

- Totals Information Management - setting quote shipping method when address method is not null
@joe-vortex joe-vortex requested a review from paliarush as a code owner November 7, 2019 11:48
@m2-assistant
Copy link

m2-assistant bot commented Nov 7, 2019

Hi @joe-vortex. 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.3-develop instance - deploy vanilla Magento instance

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

@joe-vortex
Copy link
Member Author

@paliarush I've rerun the static tests several times and they keep failing, any ideas?

@sidolov sidolov changed the base branch from 2.3-develop to 2.4-develop December 5, 2019 17:16
@joe-vortex
Copy link
Member Author

@sidolov any action required from me?

@sidolov sidolov added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Aug 18, 2020
Copy link
Contributor

@sidolov sidolov left a comment

Choose a reason for hiding this comment

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

@joe-vortex could you please cover changes with automated test?

@engcom-Foxtrot
Copy link
Contributor

@magento run all tests

@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-8154 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-Foxtrot
Copy link
Contributor

@magento run all tests

@gabrieldagama
Copy link
Contributor

@magento run Unit Tests,Static Tests, Functional Tests CE

@engcom-Foxtrot
Copy link
Contributor

@magento run all tests

… method when address method is null - static tests fix.
@engcom-Foxtrot
Copy link
Contributor

@magento run all tests

@engcom-Foxtrot engcom-Foxtrot added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Sep 17, 2020
@engcom-Foxtrot
Copy link
Contributor

Manual testing is omitted #25147 (comment)

@m2-assistant
Copy link

m2-assistant bot commented Sep 18, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Component: Checkout Partner: AYKO Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: accept Release Line: 2.4 Severity: S2 Major restrictions or short-term circumventions are required until a fix is available.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Totals Information Management - setting quote shipping method when address method is null
6 participants