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

Finish Migrating "Catalog > Attributes & Features > Add new / edit attribute Group" page #10508

Closed
5 of 7 tasks
matks opened this issue Sep 19, 2018 · 3 comments · Fixed by #35270
Closed
5 of 7 tasks
Assignees
Labels
Attributes Label: Which BO under menu is concerned BO Category: Back Office Feature Type: New Feature migration symfony migration project PR available Solution: issue is being addressed

Comments

@matks
Copy link
Contributor

matks commented Sep 19, 2018

Part of Symfony migration project

Status

In order to finish the migration of this back-office page, we need to:

Todo List:

  • Continue the PR Migrate attribute group edit #31502 (see below)
  • The PR above misses one behavior: the ability to export the data presented by the Grid on the backoffice page. Please implement it.
  • Fix broken tests
  • Fix attribute group edition
  • The Multistore header must also be added to the new Symfony page (if it is not already implemented). You can look at other migrated Symfony pages to see how it is built.
  • The bug Uploading a texture to an attribute with a color code saved returns 404 #10596 was detected on the legacy page. We want to fix it in the Symfony page that will replace the legacy page.
  • Finally, we need a last Pull Request to remove the legacy backoffice page and enable the Symfony page (see below)

About PR #31502

The PR #31502 aimed to create a new Symfony backoffice page to replace the legacy page "Catalog > Attributes & Features > Add new / edit attribute Group". The new Symfony page should follow the coding standards we have for all Symfony pages while providing the same behavior that the legacy page. It aims to create an equivalent page, but powered by Symfony and Twig :) .

Just like other Symfony pages, the application layer is Symfony + Twig, but it uses Commands and Queries to fetch and modify data. Command and Query Handlers are adapters that do rely on legacy.

What you must do for PR #31502 is to fetch it

  • Verify it is correct (for you), if not please fix wrong items you have found
  • Have it reviewed (and approved) by committers
  • The QA will not test it: they will test the last PR (see below)

About the last PR

First Todo List items will be done by multiple PRs merged into develop. So the work of "create a new Symfony page that provides the same behavior/features as the legacy page" will be carried out BUT it is done on a Symfony page not accessible yet in the backoffice. You can browse it if you know the URL. It's like it's hidden.

In order to "finish" the job, a last PR is needed: this is the PR which removes legacy files and enables the LegacyLinks that will redirect old URLs to the new page. So the new Symfony page replaces the legacy page in the back-office. When this is done, the migration is complete.

It is possible QA will find problems when testing the last PR: behaviors that used to work with the legacy page and are not working / implemented in the Symfony page. Then you must fix them.

Please ask @MatShir if you have questions about the behavior or @matks if you have questions about the code

@matks matks added the migration symfony migration project label Sep 19, 2018
@matks matks added this to To do in Symfony Migration via automation Sep 19, 2018
@marionf
Copy link
Contributor

marionf commented Feb 8, 2019

@matks Do you think we can solve this issue during the migration of this page ?

@colinegin colinegin moved this from Backlog to To do in Symfony Migration May 20, 2019
@matks matks moved this from To do to In progress in Symfony Migration Jun 3, 2019
@matks matks moved this from In progress to Needs review in Symfony Migration Jul 5, 2019
@colinegin colinegin added this to Backlog in PrestaShop 1.7.7.3 via automation Jul 12, 2019
@colinegin colinegin moved this from Backlog to To be reviewed in PrestaShop 1.7.7.3 Jul 12, 2019
@colinegin colinegin moved this from To be reviewed to In progress in PrestaShop 1.7.7.3 Jul 12, 2019
@marionf marionf removed this from In progress in PrestaShop 1.7.7.3 Sep 11, 2019
@matks matks moved this from Needs review to In progress in Symfony Migration Sep 16, 2019
@colinegin colinegin added the Improvement Type: Improvement label Oct 22, 2019
@colinegin colinegin moved this from In progress to To do in Symfony Migration Dec 16, 2019
@hibatallahAouadni hibatallahAouadni added Attributes Label: Which BO under menu is concerned BO Category: Back Office Ready Status: Issue is ready to be worked on labels May 26, 2021
@marionf marionf added Feature Type: New Feature and removed Improvement Type: Improvement labels Dec 28, 2021
@prestonBot prestonBot added the PR available Solution: issue is being addressed label Feb 23, 2023
@MatShir MatShir changed the title Migrate "Catalog > Attributes & Features > Add new / edit attribute" page Migrate "Catalog > Attributes & Features > Add new / edit attribute Group" page Jul 6, 2023
@matks matks changed the title Migrate "Catalog > Attributes & Features > Add new / edit attribute Group" page Finish Migrating "Catalog > Attributes & Features > Add new / edit attribute Group" page Jul 26, 2023
@matks
Copy link
Contributor Author

matks commented Jul 26, 2023

Check-List (copied from #13989) to help verify new Symfony page is ready

Controller / template

  • All controller actions are migrated and protected with @AdminSecurity annotations when eligible and @DemoRestricted annotations when eligible
  • All links targeting this page are updated: either using _legacy_link or updating the code
  • The new SF routes have _legacy_link and _legacy_parameters provided and explained
  • Controller catches Domain Exception and translate them nicely
  • Twig templates must contain relevant blocks to allow partial extension by modules
  • JS being used on the page can be used as enabled extensions and use a PageMap for selectors
Conventions
  • The new controller actions comply with our naming convention
  • The new SF routes names and paths comply with our naming convention
  • The new Twig template names comply actions with our naming convention
  • Created files (controller, JS assets, CSS assets and twig views) must be in the right folder following the convention
  • CSS being added must follow theme conventions

Core

  • Commands and Queries only use native types but internally use VOs
  • All Handlers must have an Interface
Conventions
  • The created SF service names comply with our naming convention
  • The created files must follow the folder convention (Domain, Core, Adapter...)

Polishing

  • The legacy controller and templates files are gone
  • If PR includes a new concept/mechanism, documentation must be written
  • JS and CSS assets must be compiled and up-to-date
  • License headers must be up-to-date
  • New hooks have been added to the hooks list and the SQL updates
  • Performance must be correct: page loading time, filtering/sorting/searching response time, actions response time

Testing

  • Commands and Queries are tested with Behat tests
  • Eligible Core classes are covered with unit tests*
  • Controller is covered with survival test
  • (soon) Controller is covered with E2E tests using puppeeter
  • If we break E2E tests because IDs change, we must update the E2E tests

*Eligible = easy to isolate and unit tests provide values (for example testing getters/setters is useless)

@matks
Copy link
Contributor Author

matks commented Jul 27, 2023

Note: if I am not wrong when the 3 issues
#10510
#10508
#33126
will be completed, the whole Tab "Catalog > Attributes & Features > Attribute Group" will have been migrated 👍

@matthieu-rolland matthieu-rolland self-assigned this Jul 27, 2023
@matthieu-rolland matthieu-rolland removed their assignment Aug 21, 2023
@matthieu-rolland matthieu-rolland self-assigned this Sep 11, 2023
@matthieu-rolland matthieu-rolland moved this from To do to In progress in Symfony Migration Sep 11, 2023
@matthieu-rolland matthieu-rolland moved this from In progress to Needs review in Symfony Migration Sep 14, 2023
Symfony Migration automation moved this from Needs review to Done Apr 17, 2024
@prestashop-issue-bot prestashop-issue-bot bot removed the Ready Status: Issue is ready to be worked on label Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Attributes Label: Which BO under menu is concerned BO Category: Back Office Feature Type: New Feature migration symfony migration project PR available Solution: issue is being addressed
Projects
Development

Successfully merging a pull request may close this issue.

9 participants