PrestaShop
PrestaShop copied to clipboard
PrestaShop
avoid dashactivity, dashproduct - module can't be configured
| Questions | Answers |
|---|---|
| Branch? | develop & 8.1.x |
| Description? | Following lines 82, 85 and 89 of admin-dev/themes/default/template/controllers/dashboard/helpers/view/view.tpl ; and following line 227 of js/admin/dashboard.js ; and following lines 425 and 428 of controllers/admin/AdminDashboardController.php : DASHBOARD_ALLOWED_HOOKS must be with hook* |
| Type? | bug fix |
| Category? | BO |
| BC breaks? | no |
| Deprecations? | no |
| How to test? | Go to BO ; In Activity overview > Click on the configure button ; Edit any of the parameters ; See NO error This hook is not allowed here ; Refresh the page > See that the edition IS taken into account |
| UI Tests | https://github.com/florine2623/testing_pr/actions/runs/8047242351 |
| Fixed issue or discussion? | Fixes #34326 |
| Related PRs | No |
| Sponsor company | No |
Hi, thanks for this contribution!
I found some issues with the Pull Request description:
- Your pull request does not seem to fix any issue, consider creating one (see note below) and linking it by writing
Fixes #1234.
Would you mind having a look at it? This will help us understand how interesting your contribution is, thank you very much!
About linked issues
Please consider opening an issue before submitting a Pull Request:
- If it's a bug fix, it helps maintainers verify that the bug is effectively due to a defect in the code, and that it hasn't been fixed already.
- It can help trigger a discussion about the best implementation path before a single line of code is written.
- It may lead the Core Product team to mark that issue as a priority, further attracting the maintainers' attention.
(Note: this is an automated message, but answering it will reach a real human)
Hi @Hlavtox, today, there is that in admin-dev/themes/default/template/controllers/dashboard/helpers/view/view.tpl
and that in js/admin/dashboard.js
@Hlavtox It is a legit fix because in the payload, I see hookDashboardZoneOne, and hookDashboardZoneTwo not dashboardZoneOne. We could either change the array values or introduce bc breaks by changing the payload, so it removes hook prefix. I think proposed fix should be ok.
Hello @florine2623, thank you for the bug report of dashactivity parameters saving. I found the problem: the saving function is missing in dashactivity module. I proposed another PR: https://github.com/PrestaShop/dashactivity/pull/46 to solve the other problem. Ok, good to know that my proposition in this PR is OK, thanks!
Hello @NicolasCador,
For https://github.com/PrestaShop/dashactivity/pull/46 Some checks were not successful
Hello @paulnoelcholot, yes you're right. But there is a problem, the error is:
Line dashactivity.php
443 Parameter #1 $variable of method ModuleCore::getPermission() expects
array, string given.
It really expects a string, and not an array:
-
in PS 8.0.x and 8.1.x it has been corrected: @param string $variable (action)
-
in PS 1.7.7.x and 1.7.8, getPermission() just calls getPermissionStatic(), expecting a string: /**
- Check employee permission for module.
- @param array $variable (action)
- @param Employee $employee
- @return bool if module can be transplanted on hook */ public function getPermission($variable, $employee = null) { return Module::getPermissionStatic($this->id, $variable, $employee); }
/**
- Check employee permission for module (static method).
- @param int $id_module
- @param string $variable (action)
- @param Employee $employee
- @return bool if module can be transplanted on hook */ public static function getPermissionStatic($id_module, $variable, $employee = null) {...
So how to do?