Change CRUD operation commands to add/edit a Product in the Admin Panel

[pdt256]

Per the comment from Daniel in DDDinPHP

This is so bad, commands should be find-grained with business naming. You don't create your products (unless your business is related to factories). You don't update products, but rename titles, changing prices, etc.

We may need to change these commands to support the Admin Panel HTML form to add and edit new products to the store:

• Action/Product/CreateProductCommand
• Action/Product/UpdateProductCommand

The Admin Panel form:

The main application controller code (non open-source) to update a product:

    public function action_edit()
    {
        $productDTO = $this->getProductDTO();
        $post = $this->request->post();

        if (! empty($post)) {
            $this->updateProductDTOFromPost($productDTO, $post);

            try {
                $this->dispatch(new UpdateProductCommand($productDTO));

                Message::add('success', 'Product has been saved.');
                $this->redirect($productDTO->adminUrl);
            } catch (EntityValidatorException $e) {
                Message::add('error', 'Unable to save product!');
                $this->data['formErrors'] = $e->errors;
            }
        }

        $this->data['product'] = $productDTO;
        $this->data['action'] = $productDTO->adminUrl;
    }

    protected function getProductDTO()
    {
        try {
            $request = new GetProductRequest($id);
            $response = new GetProductResponse($this->pricing);
            $this->dispatchQuery(new GetProductQuery($request, $response));
            return $response->getProductDTOWithAllData();
        } catch (EntityNotFoundException $e) {
            Message::add('error', 'Product ' . HTML::chars($id) . ' not found!');
            $this->redirect('admin/product');
        }
    }

    private function updateProductDTOFromPost(ProductDTO & $productDTO, array $post)
    {
        $productValues = Arr::get($post, 'product');

        $unitPrice = (int) floor(Arr::get($productValues, 'unitPrice') * 100);

        $productDTO->name = trim(Arr::get($productValues, 'name'));
        $productDTO->sku = trim(Arr::get($productValues, 'sku'));
        $productDTO->description = trim(Arr::get($productValues, 'description'));
        $productDTO->unitPrice = $unitPrice;
        $productDTO->quantity = Arr::get($productValues, 'quantity');
        $productDTO->shippingWeight = Arr::get($productValues, 'shippingWeight');
        $productDTO->isInventoryRequired = Arr::get($productValues, 'isInventoryRequired', false);
        $productDTO->isPriceVisible = Arr::get($productValues, 'isPriceVisible', false);
        $productDTO->isActive = Arr::get($productValues, 'isActive', false);
        $productDTO->isVisible = Arr::get($productValues, 'isVisible', false);
        $productDTO->isTaxable = Arr::get($productValues, 'isTaxable', false);
        $productDTO->isShippable = Arr::get($productValues, 'isShippable', false);
        $productDTO->isShippable = Arr::get($productValues, 'isShippable', false);
        $productDTO->areAttachmentsEnabled = Arr::get($productValues, 'areAttachmentsEnabled', false);
    }

[pdt256]

Per Daniel:

Your form can produce the following commands (just for example):

• RenameProduct { id: 42, newName: "..." }
• ChangeProducePrice { ... }
• ChangeQuantity
• MakeProductVisible
• MakeProductShippable.
• MakeProductInvisible; And so on.

The commands above would produce fine-grained events:

• ProductWasRenamed { id: 42, newName: "..." }
• ProductPriceWasChanged; etc.

The thing is form is just UI detail. What if you would have buttons "Hide", "Unhide" (MakeProductInvisible, MakeProductVisible)? It doesn't change anything from the domain perspective, it's pure UI concern. Command (and events) should reflect domain terms.

However, it requires you to make mapping from the form on the commands.

[pdt256]

I'm not sure how pragmatic it would be to have a separate Command for each data point in the application. I do like the idea of knowing exactly what values change on the product. This would work very nice with Event Sourcing. This is also a great way to express the various ways in which you can manipulate the domain Entities.

The drawback is you basically need a separate Command, Handler, and unit test for each Entity member variable. For the Product Entity above, you would need 13 commands. That is 39 files vs the current 3. I understand that files are cheap; however, this may present a maintenance problem with so much boilerplate code. There will also be a performance penalty if you have to write to the DB on each Command execution, assuming you are not using Event Sourcing.

[pdt256]

From the Software Craftsmanship Slack channel #_craftsmanship

jamie [10:27 AM] How fine-grained are your commands? Lets say you have an admin form editing a product. You then modify the name, quantity, price, set active, and set taxable.

Do you have 1 command to ModifyProduct, or do you break it up into individual commands to RenameProduct, ChangeQuantity, MakeProductActive, etc.

Would using Event Sourcing change the way you approach the decision to isolate changes in commands?

lilobase [10:34 AM] I would make the RenameProduct, ChangeQuantity … commands and no, I can use an event store without changing anything

in fact it’s usually a mix, we have a « classical » datastore but we also store every command in a specific place so we can replay some action and get an audit log for free (edited)

and if one day we want to switch to an event store, we will be able to do so just by replaying all the events

jamie [10:42 AM] I like the idea of recording everything sent to the command bus even if you are not using Event Sourcing.

lilobase [10:44 AM] yes, we started this model before it was cool :slightly_smiling_face:, I worked in for clients in the public administration

jamie [10:44 AM] would you ever pass a value-object to a command? Example: add/update a mailing address. Would you have one command to ModifyShippingAddress, or a separate one for ModifyShippingCity, ModifyShippingPostalCode, etc.

lilobase [10:44 AM] and the need for an audit log was a must have for them

jamie [10:44 AM] command audit log = point in time database recovery :slightly_smiling_face:

lilobase [10:45 AM] command are just POJO, so in fact the command is the value object

jamie [10:45 AM] for free™ ah, I see. That makes sense.

lilobase [10:45 AM] and the data inside it is flat

jamie [10:45 AM] just scalars: strings, int, float, bool

lilobase [10:46 AM] yep, they are pure DTO

jamie [10:46 AM] thanks. That clears it up perfectly for me.

lilobase [10:46 AM] it’s to avoid any leak from the domain inside the infrastructure (edited) the infrastructure is dumb pipes (edited) and for the granularity it’s depend of the domain

if we have a domain centered around shipping, so yes we will have a fine granularity on the shipping’s events

jamie [10:48 AM] my only concern with fine-grained commands is the pragmatism of creating so much code duplication and boilerplate. I’m not scared of lots of files, but that may present a maintenance concern.

lilobase [10:49 AM] that why the fine granularity is for your core domain and it fact its allow you to make also very fine grained tests so basically your domaine is often more robust because you have less chances to forgot a logical path in the tests if the command is really fine grained (edited)

jamie [10:51 AM] I can see that.

lilobase [10:51 AM] and there is not really a lot of duplication you have to create an handler for each command

jamie [10:51 AM] for unit testing the commands, would you just spy on the dependency calls, or would you send a tracer test through the whole stack? assuming you are using an in-memory db.

lilobase [10:52 AM] usually I spy the dependency call

jamie [10:52 AM] yeah. me too. that is faster if you can trust the dependencies.

lilobase [10:52 AM] yea

jamie [10:52 AM] for core logic that touches money, I’ll run that code through the whole stack. I’ll pay the 100ms cost to know for sure that part works :slightly_smiling_face: Thanks for the insights @lilobase, and sorry to everyone if we hijacked this channel for our own benefit.

lilobase [10:54 AM] thanks for the questions :slightly_smiling_face:

[unkind]

I'm not sure how pragmatic it would be to have a separate Command for each data point in the application.

I'd say "each business intention". It's perfectly fine to have several values in a single command. Sometimes it's OK to have something like ChangeUserProfile { id, name, dateOfBirth, aboutMe } if business doesn't interested in highly level of graining in this specific action.

[lilobase]

Hello, two little things:

There will also be a performance penalty if you have to write to the DB on each Command execution, assuming you are not using Event Sourcing.

For performance you can dispatch multiple actions in the same request, and using a database transaction to make a global commit for all the updates involved

Also for the UI point of view, maybe you can take some insight from "task based UI" principles: https://cqrs.wordpress.com/documents/task-based-ui/ which try to avoid long form (and CRUD interface) to instead display more "business intent oriented" user actions.