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

Add hooks displayAdminThemesListAfter and displayModuleConfigureExtraButtons for 8.0.0 #462

Merged
merged 3 commits into from Apr 26, 2022

Conversation

sowbiba
Copy link
Contributor

@sowbiba sowbiba commented Feb 22, 2022

Questions Answers
Description? Add hooks displayAdminThemesListAfter and displayModuleConfigureExtraButtons for autoupgrade of 8.0.0
Type? new feature
BC breaks? no
Deprecations? no
Fixed ticket? See PrestaShop/PrestaShop#27469 and PrestaShop/PrestaShop#27461
How to test? -
Possible impacts? -

jolelievre
jolelievre previously approved these changes Feb 23, 2022
Progi1984
Progi1984 previously approved these changes Feb 25, 2022
@Progi1984 Progi1984 added this to the 4.15.0 milestone Feb 25, 2022
upgrade/sql/8.0.0.sql Outdated Show resolved Hide resolved
Copy link
Contributor

@PierreRambaud PierreRambaud left a comment

Choose a reason for hiding this comment

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

The SQL is broken ^^

@AureRita AureRita self-assigned this Mar 9, 2022
@sowbiba sowbiba dismissed stale reviews from Progi1984 and jolelievre via 3951dac March 9, 2022 16:23
@sowbiba
Copy link
Contributor Author

sowbiba commented Mar 9, 2022

The SQL is broken ^^

oups. Fixed

Progi1984
Progi1984 previously approved these changes Mar 15, 2022
Copy link

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

Hi @sowbiba,

I tested it and hooks that you add for autoupgrade isn't show,

as you can see on this video

Can't change hooks of Autoupgrade

Untitled_.Mar.15.2022.11_31.AM.mp4

Moreover, with the module Quality assurance, we only find "displayModuleConfigureExtraButtons" and not "displayAdminThemesListAfter" as you can see :

the display module is here :

image

the display admin isn't here
image

atomiix
atomiix previously approved these changes Mar 21, 2022
PierreRambaud
PierreRambaud previously approved these changes Mar 21, 2022
PululuK
PululuK previously approved these changes Apr 11, 2022
upgrade/sql/8.0.0.sql Outdated Show resolved Hide resolved
@AureRita
Copy link

hi @sowbiba

I don't see any label, it is waiting for QA ? =)

@sowbiba
Copy link
Contributor Author

sowbiba commented Apr 20, 2022

We normally have to wait for the 2nd approval. But, as it's just a rebase, you can start your testing ;)

@AureRita
Copy link

Hi @sowbiba

I do an autoupdate from 1.7.8.5 to 8.0.0 and we have some exceptions :

On BO :
Modules > Modules Managers :
image

Modules > Modules Catalog
image

instead of :
image
(it's known issue)

Design > Theme Catalog
image

Design > Positions
image

Payment > Payment Methods :
image

Payment > Preferences
image

International > Translations
image

Marketing
image

Because of the exception on Design > Position, I can't check the hook on autoupgrade, I see that's a common issue between all these pages

Thank you for persisting on this pr

@sowbiba
Copy link
Contributor Author

sowbiba commented Apr 25, 2022

Hello @AureRita , you choosed a non working module to install (Gamiphy)

If you want to test a working module with configure page, you can use
prestashopexamplemodule.zip
or Buy Button Lite in the modules catalog page

@AureRita
Copy link

Hello @sowbiba

I tested you PR with an autoupgrade from 1.7.8.5 to 8.0.0, I deleted the module gamiphy (it break the design> position and other tab as you say it) but currently, the module autoupgrade didn't have other hook as you can see :

Untitled_.Apr.26.2022.9_34.AM.mp4

@sowbiba
Copy link
Contributor Author

sowbiba commented Apr 26, 2022

I understand now. My hook is not an autoupgrade hook, it's a BO hook. In the list of hooks Design > Positions, just filter with the names (displayAdminThemesListAfter and displayModuleCOnfigureExtraButtons) to have them

@sowbiba
Copy link
Contributor Author

sowbiba commented Apr 26, 2022

I understand now. My hook is not an autoupgrade hook, it's a BO hook. In the list of hooks Design > Positions, just filter with the names (displayAdminThemesListAfter and displayModuleCOnfigureExtraButtons) to have them

After installing a module which use them of course, like the example module I gave here PrestaShop/PrestaShop#27469 and here PrestaShop/PrestaShop#27461
Or you can use a MBO zip ;)

Copy link

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

That works ! I take your examplemodule and the two hooks are present as you can see on this video :

Untitled_.Apr.26.2022.2_41.PM.mp4

for the displayModuleConfigureExtraButtons hook :
image

for the displayAdminThemesListAfter hook
image

@sowbiba
Copy link
Contributor Author

sowbiba commented Apr 26, 2022

Thanks @AureRita

@sowbiba sowbiba merged commit 02ed599 into PrestaShop:dev Apr 26, 2022
@sowbiba sowbiba deleted the add-hooks-theme-modules branch April 26, 2022 23:02
atomiix pushed a commit to atomiix/autoupgrade that referenced this pull request Sep 16, 2022
Add hooks displayAdminThemesListAfter and displayModuleConfigureExtraButtons for 8.0.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants