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 bulk module actions #12205

Merged
merged 5 commits into from Jan 29, 2019

Conversation

@jolelievre
Copy link
Contributor

jolelievre commented Jan 17, 2019

Questions Answers
Branch? 1.7.5.x
Description? Bulk actions didn't work any more because cache was clear during parallel calls. So a request parameter was added to allow perform actions on a module without clearing cache. Then javascript was updated for bulk actions, so that all actions are performed without clearing the cache except for the last one which clears the cache at the end. The same bug happened when using the button "Upgrade all", it now uses the same process.
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #12011 & #12044
How to test? Try to perform bulk actions on modules (one or more). Also try to update all possible modules with the "Update all" button.

This change is Reviewable

@@ -222,7 +222,7 @@ export default class ModuleCard {
return (event.result !== false); // explicit false must be set from handlers to stop propagation of the click event.
};

_requestToController(action, element, forceDeletion) {
_requestToController(action, element, forceDeletion, disableCacheClear, requestCallback) {

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Jan 21, 2019

Member

callback is commonly used.

Show resolved Hide resolved admin-dev/themes/new-theme/js/pages/module/controller.js
@mbadrani

This comment has been minimized.

Copy link
Contributor

mbadrani commented Jan 23, 2019

@jolelievre tricky scenario, if you make an action (bulk actions) twice, you enter into an infinite loop
image
for my example I've disabled 3 modules on bulk actions (best suppliers, best vouchers and best-selling products) and disable another time (it appears on the list of bulk actions), best suppliers and best vouchers return an error pop-up (normal behavior), but the third one stay with his spinner for infinite time

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Jan 23, 2019

@mbadrani Is it QA approved? You said there is an infinite loop 😮

@jolelievre

This comment has been minimized.

Copy link
Contributor Author

jolelievre commented Jan 23, 2019

No it's not approved there is a bug when some of the bulk actions fail, the last one remains pending and the spinner is still visible.

@mbadrani

This comment has been minimized.

Copy link
Contributor

mbadrani commented Jan 23, 2019

If we consider only the aim of the PR it's fine for me but my last comment was just an observation in a particular tricky scenarii, tell me if this PR is good enough and if we will create in a second time something to fix this behavior

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Jan 23, 2019

I remove the "QA ✔️" since there is still a bug.

@jolelievre

This comment has been minimized.

Copy link
Contributor Author

jolelievre commented Jan 23, 2019

It shouldn't be long to fix so we'd better fix it now

@mbadrani

This comment has been minimized.

Copy link
Contributor

mbadrani commented Jan 29, 2019

well done @jolelievre

@mbadrani mbadrani added QA ✔️ and removed waiting for QA labels Jan 29, 2019

@Quetzacoalt91 Quetzacoalt91 merged commit d06115c into PrestaShop:1.7.5.x Jan 29, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eternoendless eternoendless changed the title Bulk module action Fix bulk module actions Feb 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment