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

#758: applied_taxes of empty cart causes 'Internal server error' message on PHP 7.2 #759

Merged
merged 3 commits into from Jul 24, 2019
Merged

#758: applied_taxes of empty cart causes 'Internal server error' message on PHP 7.2 #759

merged 3 commits into from Jul 24, 2019

Conversation

therealtbs
Copy link

@therealtbs therealtbs commented Jun 28, 2019 •
edited

Description (*)

From PHP 7.2 onward there is a warning generated when attempting to call count() on anything that is not an array or an object implementing Countable, when a cart is created, initially the appliedTaxes are null until a product is added.

Fixed Issues (if relevant)

  1. Cart applied_taxes of empty cart causes 'Internal server error' message on PHP 7.2 #758: Cart applied_taxes of empty cart causes 'Internal server error' message on PHP 7.2

Manual testing scenarios (*)

  1. See reproduction steps of Cart applied_taxes of empty cart causes 'Internal server error' message on PHP 7.2 #758

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)

@naydav
Copy link
Contributor

naydav commented Jun 28, 2019

Hi @therealtbs,

please cover this case (try to load cart taxes if taxes are not set) with API-functional tests
thanks

Examples:
https://github.com/magento/graphql-ce/tree/2.3-develop/dev/tests/api-functional/testsuite/Magento/GraphQl/Customer
https://github.com/magento/graphql-ce/tree/2.3-develop/dev/tests/api-functional/testsuite/Magento/GraphQl/Quote/Customer

Guide:
https://devdocs.magento.com/guides/v2.3/graphql/functional-testing.html

@therealtbs therealtbs marked this pull request as ready for review June 30, 2019 17:01
@okorshenko okorshenko added this to Work in Progress in Pull Request Progress Jul 10, 2019
@m2-community-project m2-community-project bot moved this from Work in Progress to Ready for Review in Pull Request Progress Jul 10, 2019
@therealtbs therealtbs changed the title #758: applied_taxes of empty cart causes 'Internal server error' message on PHP 7.2 - WIP #758: applied_taxes of empty cart causes 'Internal server error' message on PHP 7.2 Jul 12, 2019
@m2-community-project m2-community-project bot moved this from Ready for Review to Review in Progress in Pull Request Progress Jul 12, 2019
@naydav
Copy link
Contributor

naydav commented Jul 12, 2019

Hi @TomashKhamlai
Please verify the fix.
Thanks

@naydav naydav moved this from Review in Progress to Reviewer Approved in Pull Request Progress Jul 12, 2019
@naydav naydav moved this from Reviewer Approved to Acceptance Testing in Pull Request Progress Jul 12, 2019
@m2-community-project m2-community-project bot moved this from Acceptance Testing to Reviewer Approved in Pull Request Progress Jul 12, 2019
@magento-engcom-team
Copy link
Contributor

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

@magento-engcom-team
Copy link
Contributor

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

@TomashKhamlai TomashKhamlai added the QA in progress We are checking label Jul 13, 2019
@magento-engcom-team
Copy link
Contributor

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

1 similar comment
@magento-engcom-team
Copy link
Contributor

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

@TomashKhamlai TomashKhamlai added QA passed and removed QA in progress We are checking labels Jul 16, 2019
@magento-engcom-team magento-engcom-team merged commit 2f766b1 into magento:2.3-develop Jul 24, 2019
@ghost
Copy link

ghost commented Jul 24, 2019

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

magento-engcom-team pushed a commit that referenced this pull request Jul 24, 2019
@naydav
ENGCOM-5436: #758: applied_taxes of empty cart causes 'Internal serve…
e54b07d 
…r error' message on PHP 7.2 #759
@m2-community-project m2-community-project bot moved this from Reviewer Approved to Done in Pull Request Progress Jul 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lenaorobei lenaorobei lenaorobei left review comments

@VladimirZaets VladimirZaets VladimirZaets approved these changes

Assignees

@naydav naydav

@VladimirZaets VladimirZaets

@TomashKhamlai TomashKhamlai

Labels
Award: bug fix Award: test coverage Component: QuoteGraphQl QA passed Release Line: 2.3
Projects
Pull Request Progress
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@therealtbs @naydav @magento-engcom-team @VladimirZaets @lenaorobei @TomashKhamlai