PrestaShop / prestafony-project

Some resources to help you migrate PrestaShop to Symfony 3
https://github.com/PrestaShop/prestafony-project/projects/1
11 stars 8 forks source link

How to create a form for Object Model creation/update page? #56

Closed mickaelandrieu closed 4 years ago

mickaelandrieu commented 6 years ago

We'd like to stick with CQRS for it and not rely directly with the old Object Models.

But this is not really easy, and this need to be the most reusable as possible as "all" creation/update pages are the same, only the Object models change.

As our Object models are dynamic (ie, every module can alter his structure), we need to take it in account and/or provide some hooks (as we did for Configuration form pages) to allow PrestaShop developers to alter easily their data.

Also, don't forget that columns available in Grids and the form for creation/update are not related: creation/update is about data, when the grids are only views (and can contains some columns not related at all to the main object models).

mickaelandrieu commented 6 years ago

This is a first POC I've done => https://github.com/PrestaShop/PrestaShop/pull/9314

eternoendless commented 6 years ago

After playing with the idea for a while and trying to write a closer-to-real-life implementation of the CQRS, here are my thoughts.

Even though we don't have a Model in the DDD sense of it, ObjectModels are the closest thing to it we currently do have. We cannot get rid of them (until next major version), and we don't want to build Yet Another System to circumvent them.

In fact, I think we need to embrace them.

Look at how AdminControllers currently handle the creation process. It's heavily based on the ObjectModel definition. It basically:

  1. Copies properties from POST into the Model's properties (using the Model definition)
  2. Invokes $model->validateFields()
  3. Invokes $model->add()

Ok not exactly as step 2 is actually run as a loop on the POST fields before step 1 (not sure why), but the idea is generally the same.

It's nothing too fancy. But this means that most of the domain validation we need is already being done by ObjectModel.

Now, how do we get to them without depending explicitly on them? Since we can't change the lower layers anyway, let's just follow the DDD spirit and design our system top-down.

Let's explore our Domain requirements using an example, the creation of a new Manufacturer (brand). Here's what it looks like in the BO now:

screen shot 2018-07-20 at 17 51 30

And it's related tables:

ps_manufacturer:
+-----------------+------------------+------+-----+---------+----------------+
| Field           | Type             | Null | Key | Default | Extra          |
+-----------------+------------------+------+-----+---------+----------------+
| id_manufacturer | int(10) unsigned | NO   | PRI | NULL    | auto_increment |
| name            | varchar(64)      | NO   |     | NULL    |                |
| date_add        | datetime         | NO   |     | NULL    |                |
| date_upd        | datetime         | NO   |     | NULL    |                |
| active          | tinyint(1)       | NO   |     | 0       |                |
+-----------------+------------------+------+-----+---------+----------------+

ps_manufacturer_lang:
+-------------------+------------------+------+-----+---------+-------+
| Field             | Type             | Null | Key | Default | Extra |
+-------------------+------------------+------+-----+---------+-------+
| id_manufacturer   | int(10) unsigned | NO   | PRI | NULL    |       |
| id_lang           | int(10) unsigned | NO   | PRI | NULL    |       |
| description       | text             | YES  |     | NULL    |       |
| short_description | text             | YES  |     | NULL    |       |
| meta_title        | varchar(128)     | YES  |     | NULL    |       |
| meta_keywords     | varchar(255)     | YES  |     | NULL    |       |
| meta_description  | varchar(255)     | YES  |     | NULL    |       |
+-------------------+------------------+------+-----+---------+-------+

We know what a creation form looks like and we know which are the fields in it. Let's create a Command that describes our business need:

class AddManufacturerCommand
{

    //... define all properties

    public function __construct(
        $name,
        $descriptions,
        array $shortDescriptions,
        array $metaTitles,
        array $metaKeywords,
        array $metaDescriptions,
        $active
    ) {
        // set all properties
    }
}

Commands describe an action. They don't fulfill that action. That's what CommandHandlers are for:

class AddManufacturerHandler
{
    public function handle(AddManufacturerCommand $command)
    {
        // create the manufacturer using the Manufacturer ObjectModel
        // since we don't have aggregate roots, we may also ensure context coherence here
    }
}

Since Commands describe our business while disregarding all the implementation details, they are future-proof. We can keep them in Core as they describe our domain.

CommandHandlers on the other hand do all the heavy lifting. Since they use ObjectModels they MUST be placed in the Adapter namespace. We MAY declare an interface for each one of them (they always have to declare a handle(TheCommandInCore $command)).

This means that when we get to work on replacing ObjectModels with something else, all we'll have to do is reimplement the CommandHandlers, and all the consumers will continue working as before, even if we change the whole stack underneath!

Of course, this change of paradigm will take some getting used to, as from an application perspective we'll have to stop thinking about model-entity mutations and transition to "state mutations" or "events" (if we ever get to do Event Sourcing). In reality, I think it will have more to do with naming things differently than drastically changing the way we do things (but I may be wrong).

Another advantage of this paradigm is this is that if we stick to it, we'll end up with an API describing everything PrestaShop is capable of doing and exactly what it needs to perform each and every task. Once we have such a thing, we'll be able to mark all the Core classes as private and make modules dependent on Commands (and Queries) only.

I still have to figure out what Queries will look like, but I think we can agree Commands are the more urgent matter.

I'll update your POC with my implementation soon.

eternoendless commented 6 years ago

I decided to make my own POC so we can compare: https://github.com/PrestaShop/PrestaShop/pull/9370

matks commented 6 years ago

I like this approach. This is heavy work but I like the fact it provides:

However how will you handle the fact that modules can add properties to ObjectModels ? In your AddManufacturerCommand the manufactrer properties are listed as private fields. Do you expect the module developer to extend your AddManufacturerCommand ?

eternoendless commented 6 years ago

I think custom ObjectModels are a mistake and should not be encouraged. Allowing them to be extended brings entropy and uncertainty as it relies on dynamic interfaces, not to mention hellish schema upgrades.

Modules should use their own entities for storage, and leave the Core alone :)

sarjon commented 6 years ago

@eternoendless regarding options configuration, shouldn't they also be handled (at some point) using CQRS?

PierreRambaud commented 6 years ago

Totally agreed with @eternoendless if we encourage custom ObjectModels we will not be able to prevent all case during migration (especially with autoupgrade module). But they have always the capability to create custom table to catch more information about a category, product, or anything else :)

eternoendless commented 6 years ago

regarding options configuration, shouldn't they also be handled (at some point) using CQRS?

Yes, as stated in my POC:

  1. Any alteration on the state of the application MUST be performed through a Command.