Prestashop: Migrate "Catalog > Discounts > Catalog price rules > Add new / edit catalog price rule" page

[matks]

Part of Symfony migration project
Type: page
Functional specs of the page : https://github.com/PrestaShop/prestashop-specs/blob/master/back-office/catalog/catalog-price-rules.md

Todo:

• [x] Migrate page https://github.com/PrestaShop/PrestaShop/pull/13716
• [ ] Migrate condition groups
• [ ] Remove "recommended modules" button

Check-List (copied from #13989)

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)

[MatShir]

Don't forget to hide the "Recommended Modules" button

[colinegin]

Hello @matks , it seems that this issue has been done, can we close it ?

[matks]

@zuk3975 Can you check https://github.com/PrestaShop/PrestaShop/pull/13716 and confirm only condition group is missing for this page ?

[zuk3975]

@zuk3975 Can you check #13716 and confirm only condition group is missing for this page ?

Create/edit form is migrated and working as expected.
Condition group settings are missing.

[matks]

About catalog price rules conditions block

OK, I have a tough question for you guys 😬 @PrestaShop/prestashop-product-team

_So if you did not notice, the "condition groups" and "conditions" list below is not a real list._

If you add items in the list, they are not saved until you hit the "save" button.
You can delete any item, but cannot edit items.
You can delete items, but they are not really deleted until you hit the "save" button. If you refresh before hitting the save button, you lose your changes.

This "only save on submit" behavior, implemented in legacy page, make things a lot harder to do (and migrate) 😅

So I was considering ... shall we change this ? I'm asking if I can make "condition groups" and "conditions" lists below a real list.

By "real list" I mean:

• when adding a new item, it's saved immediately
• when deleting an item, it's saved immediately

So the 2 parts of this page (the "catalog price rule" form above and the "conditions" below) would be 100% separated.

The only downside of this change is that you cannot add conditions when you are creating a new catalog price rule. You must hit the SAVE button once, then you are moved into "edition" mode ; and in this mode you can start adding conditions.

This "real list" behavior is a lot easier to implement because, instead of "trying to remember all the changes user did until finally the SAVE button is hit so I can save them" logic, you have 3 small logics:

• one list that displays conditions (= database content)
• for each row, a "delete" button that deletes the row then refreshes the list
• below, a "add a condition" form that adds new items to databse then refreshes the list (modifié)

However please note that, from a data point of view, a HTML point of view and a technical behavior point of view, this would be a Breaking Change

Before, although overly complex and quite hard to decypher, everything in the page was a big form and every piece of logic related to "saving data into database" was happening on the "submit" button. So if a module wanted to customize this logic it had only 1 logic to modify.

If we do this change, the catalog price rule (the form above) will be one piece of logic, the conditions list will be another. Module wishing to modify the whole logic will have to modify multiple parts of the code instead of a single one. However (I hope) these multiple parts will be small and understandable parts (while, today, it's one big piece of code that I needed 24 hours to finally understand)

[colinegin]

I won't talk for the tech part, but for the interface and functional part, the legacy page is really not satisfying as it is currently. So I'm clearly in favor of introducing a breaking change if he can improve the global UX on this page.

My only worry is about the first save needed after completing the generic catalog price rule form. We need to make it quite clear that it's not a definitive save, that the merchant will have another action to perform afterwards to finish the configuration of his rule.
What happens if he stops configuring it after the first save ? It means that the rule will be applied on the whole catalog and not on specific groups of products ? It might be a bit dangerous.

[matks]

What happens if he stops configuring it after the first save ? It means that the rule will be applied on the whole catalog and not on specific groups of products ? It might be a bit dangerous.

You are right, this is the pain point.

It's exactly the same issue with Product page and combinations. In current Product page, it was tried to avoid this by creating "ghost products" (so you can start adding combinations without hitting "save") but it created the "ghosts products" issue.

[matks]

I can keep the current behavior which avoids this problem. On technical side, it's more complicated but not much. The real tech issue is that the logic used to implement this behavior is very unusual so it's hard to understand, both for us and module developers, and it's very hard to customize (for module developers). So writing the code is fine, making it easy to understand, use and maintain is the pain point.

[colinegin]

ping @matks let me know when you are available to discuss about it ;)