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.

Sign up for GitHub

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

Jump to bottom

Fix undefined property #11764

Merged
merged 2 commits into from
Feb 15, 2019
Merged

Fix undefined property #11764

merged 2 commits into from
Feb 15, 2019

Conversation

idnovate
Copy link
Contributor

@idnovate idnovate commented Dec 13, 2018 •
edited by PierreRambaud
Loading

Questions Answers
Branch? develop
Description? When you try to update cart from an Admin controller from a module it appears an error because customer doesn't exist in context
Type? bug fix
Category? CO
BC breaks? No
Deprecations? No

This change is Reviewable

@prestonBot prestonBot added the Bug Type: Bug label Dec 13, 2018
Quetzacoalt91
Quetzacoalt91 reviewed Dec 13, 2018
View reviewed changes
classes/Cart.php Outdated
@@ -1324,6 +1324,8 @@ public function updateQty(
// The $id_address_delivery must be linked with customer
$id_address_delivery = 0;
}
} else {
$id_address_delivery = 0;
Copy link
Member

@Quetzacoalt91 Quetzacoalt91 Dec 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Why is it necessary to reset the $id_address_delivery value? I thought you wanted to cover a case when the variable does not exist, but it exist from the method signature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just wanted to catch the warning when Context::getContext()->customer doesn't exist! $id_address_delivery = 0 is default value when customer doesn't has address.

My problem comes when I remove a cart rule with a free gift from the backoffice and I update cart.

@Quetzacoalt91 Quetzacoalt91 added the Waiting for QA Status: action required, waiting for test feedback label Jan 8, 2019
Quetzacoalt91
Quetzacoalt91 previously approved these changes Jan 8, 2019
View reviewed changes
@Quetzacoalt91
Copy link
Member

Sending this PR to QA review. In the meantime @idnovate, please take some time to run ./vendor/bin/php-cs-fixer fix --show-progress=dot on your folder and commit the modified files in order to pass the lint jobs of Travis.

@marionf
Copy link
Contributor

marionf commented Jan 8, 2019

Hello @idnovate

Can you send me the module ? It will alowed to reproduce the issue and check your PR

Thanks :)

@marionf marionf added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Jan 8, 2019
@idnovate
Copy link
Contributor Author

Sending this PR to QA review. In the meantime @idnovate, please take some time to run ./vendor/bin/php-cs-fixer fix --show-progress=dot on your folder and commit the modified files in order to pass the lint jobs of Travis.

I did a modification to the code and executed php-cs-fixer

@idnovate
Copy link
Contributor Author

Hello @idnovate

Can you send me the module ? It will alowed to reproduce the issue and check your PR

Thanks :)

I attach you a very simple module -> dummymodule.zip.

How to reproduce the issue:

  • Install module
  • Create a cart rule with a free gift as action. No conditions nor coupon is needed.
  • In a new tab with Incognito mode, no customer logged in, add a product to the cart. Cart rule should be applied automatically.
  • Open module configuration and the loop to remove the cart rules is executed. The error appears:
    image

@marionf marionf added QA ✔️ Status: check done, code approved and removed Waiting for author Status: action required, waiting for author feedback labels Feb 11, 2019
@prestonBot prestonBot added the develop Branch label Feb 15, 2019
@PierreRambaud PierreRambaud added this to the 1.7.6.0 milestone Feb 15, 2019
@PierreRambaud PierreRambaud merged commit f452df8 into PrestaShop:develop Feb 15, 2019
@PierreRambaud
Copy link
Contributor

Thanks @idnovate

@idnovate idnovate deleted the patch-7 branch March 15, 2019 12:00
@eternoendless eternoendless changed the title CO : Undefined variable Fix undefined property Apr 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PierreRambaud PierreRambaud PierreRambaud approved these changes

@Quetzacoalt91 Quetzacoalt91 Quetzacoalt91 left review comments

Assignees
No one assigned
Labels
Bug Type: Bug develop Branch QA ✔️ Status: check done, code approved
Projects
None yet
Milestone
1.7.6.0
Development

Successfully merging this pull request may close these issues.
5 participants
@idnovate @Quetzacoalt91 @marionf @PierreRambaud @prestonBot