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 to show the Modules tab in the product editor - global hook fix #19216

Merged
merged 9 commits into from
May 29, 2020
Merged

Fix to show the Modules tab in the product editor - global hook fix #19216

merged 9 commits into from
May 29, 2020

Conversation

Rolige
Copy link
Contributor

@Rolige Rolige commented May 19, 2020 •
edited by Progi1984
Loading

Questions Answers
Branch? develop
Description? Fix to show the Modules tab in the product editor in the Back Office.
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? #19246
How to test? Install a module using the hook "displayAdminProductsExtra" (@Matt75 has shared a sample module : https://github.com/Matt75/testhookdisplayadminproductsextra), edit any product and the Modules tab doesn't appear.
Example without fix:
Example without fix.
Example with fix:
Example with fix.

This change is Reviewable

Rolige eCommerce Solutions and others added 9 commits January 30, 2019 15:38
@Rolige Rolige requested a review from a team as a code owner May 19, 2020 18:36
@prestonBot prestonBot added Bug Type: Bug 1.7.7.x Branch labels May 19, 2020
@Progi1984
Copy link
Member

@Rolige Thank you for your contribution. 🤗

Could you explain in an issue your initial bug for this PR (or may be link to an issue) ?

Thanks 👍

@Progi1984 Progi1984 added the Waiting for author Status: action required, waiting for author feedback label May 20, 2020
@Rolige
Copy link
Contributor Author

Rolige commented May 20, 2020

@Rolige Thank you for your contribution. 🤗

Could you explain in an issue your initial bug for this PR (or may be link to an issue) ?

Thanks 👍

Really? Do I need to open an issue first before publish a PR?

@PierreRambaud
Copy link
Contributor

@Rolige Yes, it's easier to determine if the fix is relevant and need to be merge.
It's a request from @PrestaShop/prestashop-product-team 😉

@Rolige
Copy link
Contributor Author

Rolige commented May 20, 2020 •
edited
Loading

@Rolige Yes, it's easier to determine if the fix is relevant and need to be merge.
It's a request from @PrestaShop/prestashop-product-team 😉

Issue opened #19216, but this was a wasted time.

@Progi1984 Progi1984 linked an issue May 22, 2020 that may be closed by this pull request
The modules tab doesn't appear in the product editor #19246
Closed
@Progi1984 Progi1984 removed the Waiting for author Status: action required, waiting for author feedback label May 22, 2020
@prestonBot prestonBot added the develop Branch label May 22, 2020
@PierreRambaud PierreRambaud added Waiting for QA Status: action required, waiting for test feedback and removed 1.7.7.x Branch labels May 22, 2020
@PierreRambaud
Copy link
Contributor

@PrestaShop/prestashop-product-team I think this fix must be done in the 1.7.7.x wdyt?

@Progi1984 Progi1984 added the Waiting for PM Status: action required, waiting for product feedback label May 22, 2020
@colinegin
Copy link

Depends if it is a regression, i'm waiting for a better qualification of the issue

@colinegin colinegin added PM ✔️ Status: check done, behavior approved and removed Waiting for PM Status: action required, waiting for product feedback labels May 27, 2020
@colinegin
Copy link

I confirm this is not the correct branch, it should target 177x

@Matt75
Copy link
Contributor

Matt75 commented May 27, 2020

Can be tested with https://github.com/Matt75/testhookdisplayadminproductsextra

@Progi1984
Copy link
Member

Thanks @Matt75. I modify the PR ;)

@Progi1984 Progi1984 added this to the 1.7.7.0 milestone May 27, 2020
@matks
Copy link
Contributor

matks commented May 27, 2020

but this was a wasted time.

I agree

@matks
Copy link
Contributor

matks commented May 27, 2020

So the reason the feature was broken in 1.7.7 is because of PR #14249 which fixed issue #10412

TLDR: hooks are lowercased to ease matching, so any logic aiming to scan them, like hookCount() must look for lowercase hook names only

@matks matks mentioned this pull request May 28, 2020
Closed
@florine2623 florine2623 linked an issue May 28, 2020 that may be closed by this pull request
displayAdminOrderTabLink and displayAdminOrderTabContent not available #19439
Closed
@Robin-Fischer-PS Robin-Fischer-PS self-assigned this May 29, 2020
@Robin-Fischer-PS
Copy link
Contributor

It's OK on develop branch, thanks @Rolige !

@matks is going to make the 1.7.7.x PR 😉

@Robin-Fischer-PS Robin-Fischer-PS added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels May 29, 2020
@matks matks merged commit f80fc10 into PrestaShop:develop May 29, 2020
@Progi1984
Copy link
Member

Thanks @Rolige

@matks matks mentioned this pull request May 29, 2020
Merged
@matks
Copy link
Contributor

matks commented May 29, 2020

Here it is #19452

@eternoendless eternoendless modified the milestones: 1.7.7.0, 1.7.8.0 Jun 22, 2020
@lithiumbj lithiumbj mentioned this pull request Jun 25, 2020
Closed
@matks matks changed the title Fix to show the Modules tab in the product editor Fix to show the Modules tab in the product editor - global hook fix Jun 25, 2020
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

@Progi1984 Progi1984 Progi1984 approved these changes

@matks matks matks approved these changes

Assignees

@Robin-Fischer-PS Robin-Fischer-PS

Labels
Bug Type: Bug develop Branch PM ✔️ Status: check done, behavior approved QA ✔️ Status: check done, code approved
Projects
None yet
Milestone
1.7.8.0
10 participants
@Rolige @Progi1984 @PierreRambaud @colinegin @Matt75 @matks @Robin-Fischer-PS @eternoendless @prestonBot @SergioQuinonez