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 override handling when PS_DISABLE_OVERRIDES is used #9234

Merged
merged 3 commits into from Jul 4, 2018

Conversation

@jocel1
Copy link
Contributor

commented Jun 29, 2018

Questions Answers
Branch? 1.7.4.x
Description? PS_DISABLE_OVERRIDES is not properly taken into account when index_cache file is automatically regenerated
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? Unittest added

Important guidelines


This change is Reviewable

@prestonBot prestonBot added the 1.7.4.x label Jun 29, 2018

@jocel1 jocel1 requested a review from Quetzacoalt91 Jun 29, 2018

@jocel1

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2018

Same as #7768 , on 1.7 branch

@jocel1 jocel1 added the Bug label Jun 29, 2018

$this->_include_override_path = false;
} else {
$this->_include_override_path = true;
}

This comment has been minimized.

Copy link
@Quetzacoalt91

Quetzacoalt91 Jun 29, 2018

Member

Is there a reason to add the else case, while the default value seems to be set before the __construct()?

This comment has been minimized.

Copy link
@jocel1

jocel1 Jun 29, 2018

Author Contributor

to be able to test it properly, otherwise in the same test it will never switch back to true. And it's more easier to understand that way

This comment has been minimized.

Copy link
@Quetzacoalt91

Quetzacoalt91 Jun 29, 2018

Member

We can probably set directly the if result in the $this->_include_override_path affectation, but I won't mind if you don't do it.

@Quetzacoalt91

This comment has been minimized.

Copy link
Member

commented Jun 29, 2018

This PR should be better on develop, expect if this issue appeared on 1.7.4.0.

@jocel1 jocel1 changed the base branch from 1.7.4.x to develop Jun 29, 2018

@jocel1 jocel1 force-pushed the jocel1:fix-generate-index branch from fb33f82 to 6277d10 Jun 29, 2018

@jocel1 jocel1 added develop and removed 1.7.4.x labels Jun 29, 2018

@jocel1 jocel1 added this to the 1.7.5.0 milestone Jun 29, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Jul 4, 2018

@Quetzacoalt91 Quetzacoalt91 merged commit 0c3c727 into PrestaShop:develop Jul 4, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Quetzacoalt91

This comment has been minimized.

Copy link
Member

commented Jul 4, 2018

Thank you @jocel1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.