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

Feature/module manager categories #11070

Merged
merged 12 commits into from Nov 6, 2018

Conversation

Projects
None yet
6 participants
@PierreRambaud
Contributor

PierreRambaud commented Oct 18, 2018

Questions Answers
Branch? 1.7.5.x
Description? Categories need to be stored in a file.
Type? improvement
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket Fixes #10048
How to test? Check module manager categories order.

This change is Reviewable

@PierreRambaud PierreRambaud added this to the 1.7.5.0 milestone Oct 18, 2018

@PierreRambaud PierreRambaud requested a review from Quetzacoalt91 Oct 18, 2018

);
// Convert array to object to be consistent with current API call
$categories = json_decode(json_encode($categories));

This comment has been minimized.

@matks

matks Oct 18, 2018

Contributor

😄

This comment has been minimized.

@PierreRambaud

PierreRambaud Oct 18, 2018

Contributor

Don't want to introduce a BC break if I change the return of the API to an array :(

This comment has been minimized.

@PierreRambaud

PierreRambaud Oct 18, 2018

Contributor

https://media.giphy.com/media/c6DIpCp1922KQ/giphy.gif

@marionf marionf self-assigned this Oct 19, 2018

@marionf

This comment has been minimized.

Contributor

marionf commented Oct 23, 2018

@PierreRambaud

The order of categories isn't the good one

capture d ecran_499

Is should be the order defined here: #10048

@PierreRambaud

This comment has been minimized.

Contributor

PierreRambaud commented Oct 23, 2018

@marionf Sorry, it's ok now :)

@marionf marionf added QA ✔️ and removed waiting for QA labels Oct 23, 2018

@marionf marionf removed their assignment Oct 23, 2018

@marionf

This comment has been minimized.

Contributor

marionf commented Oct 23, 2018

@LouToma in #10048 the category defined is not the same in the first comment :

capture d ecran_506

and in your last comment :

capture d ecran_507

So, what is the good order ?

Also, the category "theme module" isn't displayed
This category should be displayed even if it's empty ?

@marionf

This comment has been minimized.

Contributor

marionf commented Oct 23, 2018

#10048 (comment)

So, the order isn't currently the good one

And "theme module" category isn't displayed even if I install a theme with its own modules

@Quetzacoalt91

This comment has been minimized.

Member

Quetzacoalt91 commented Oct 23, 2018

Dropping QA check as:

  • Translations must be included
  • "Theme modules" category must be checked
@PierreRambaud

This comment has been minimized.

Contributor

PierreRambaud commented Oct 23, 2018

@Quetzacoalt91 They said "Theme modules mustn't be displayed if it's empty..."

@Quetzacoalt91

This comment has been minimized.

Member

Quetzacoalt91 commented Oct 24, 2018

Well, as there are always some modules related to the current theme, I expected to see the category to be displayed. :)

@Quetzacoalt91 Quetzacoalt91 force-pushed the PierreRambaud:feature/module-manager-categories branch 2 times, most recently from 437713f to 81236c6 Oct 25, 2018

@Quetzacoalt91 Quetzacoalt91 force-pushed the PierreRambaud:feature/module-manager-categories branch from 81236c6 to 952fe10 Oct 25, 2018

@Quetzacoalt91 Quetzacoalt91 force-pushed the PierreRambaud:feature/module-manager-categories branch from 75c4330 to d37560f Nov 2, 2018

*/
public function build()
{
if (is_null(self::$moduleManager)) {
if (null === self::$moduleManager) {

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Nov 6, 2018

Member

The linter probably fails here

This comment has been minimized.

@PierreRambaud

PierreRambaud Nov 6, 2018

Contributor

It's not the linter, it's me :trollface:

@Quetzacoalt91 Quetzacoalt91 merged commit 5438153 into PrestaShop:1.7.5.x Nov 6, 2018

1 of 2 checks passed

Codacy/PR Quality Review Hang in there, Codacy is reviewing your Pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Quetzacoalt91

This comment has been minimized.

Member

Quetzacoalt91 commented Nov 6, 2018

Thank you @PierreRambaud

@PierreRambaud PierreRambaud deleted the PierreRambaud:feature/module-manager-categories branch Nov 6, 2018

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