2
\$\begingroup\$

I have some questions on how I can improve this "add" action method in a controller.

Using a single form, the user can upload a product to be displayed on the site. A Product is composed of three entities: Product, ProductFeatures and ProductMedias (stored in three database tables).

To prepare data for insertion and upload, I have created two helper classes with static methods to simplify the controllers. (This approach should help with rollback if necessary).

In one action (the add method of Products controller), I do multiple tasks:

  1. The setProductByForm method in ProductsTable creates an empty Product entity, populates it with form data, and saves it in the database.

    public function setProductByForm($form)
    {
         $product = $this->newEntity();
         $product = $this->patchEntity($product, $form);
         return $this->save($product);
    }
    
  2. The uploadFiles method in UploadFilesComponent stores the common images.

  3. A method in the ModelUtils class calls other methods that prepare and tranform the data in entities.

    public static function prepareProductsFeatures($formValues, $productId)
    {
        $productFeaturesArray = self::createProductsFeaturesArray($formValues);
        $productFeaturesEntities = self::createProductsFeaturesEntitiesArray(
            $productFeaturesArray, $productId
        );
        return $productFeaturesEntities;
    }
    
  4. I create the URL where thumbnail will stored (using a method in the UploadUtils class).

    public static function getOutputThumbUrl($mainImage, $productId)
    {
        return str_replace(PRODUCTS_IMAGES_FOLDER . $productId,
            PRODUCTS_THUMB_FOLDER, $mainImage);
    }
    
  5. The thumbnail is stored.

  6. Methods in ModelUtils transform data into entities and save them in MediasTable.

What is the best way to separate concepts and operations?

Note: I've tried to use afterSave but it's impossible to access request->data from it.

Controller action

public function add()
{
    $this->autoRender = false;

    if ($this->request->is('post'))
    {
        $productSaved = $this->Products->setProductByForm($this->request->data);

        if($productSaved)
        {
            $this->loadModel('ProductFeatures');
            $productFeaturesSaved = $this->ProductFeatures->setProductFeaturesEntities(
                ModelUtils::prepareProductsFeatures($this->request->data, $productSaved['id'])
            );

            $imagesUploaded = $this->UploadFile->uploadFiles(
                PRODUCTS_IMAGES_FOLDER . $productSaved['id'], $this->request->data['file']
            );

            $outputThumbUrl = UploadUtils::getOutputThumbUrl($imagesUploaded[0]['url'], $productSaved['id']);

            $thumbUploaded = $this->UploadFile->resizeImage([
                'input' => $imagesUploaded[0]['url'], 'output' => $outputThumbUrl,
                'width' => 250, 'height' => 250, 'mode' => 'stretch'
            ]);

            $this->loadModel('Medias');
            $thumbSaved = $this->Medias->setMediaEntity(
                ModelUtils::prepareMediaThumb($productSaved['id'], $outputThumbUrl)
            );
            $mediasSaved = $this->Medias->setMediasEntities(
                ModelUtils::prepareMedias($imagesUploaded, $productSaved['id'])
            );
        }
    }
}

Full source code

\$\endgroup\$
  • \$\begingroup\$ The short answer is that everything after if($productSaved) { should be in the afterSave method of the products table class book.cakephp.org/3.0/en/orm/table-objects.html#aftersave \$\endgroup\$ – AD7six Dec 4 '15 at 9:46
  • \$\begingroup\$ @AD7six I put all methods involved in together in ProductsTable file, but when I run this error was launched: pastebin.com/5Dxr83H9 (pasted it on pastebin), as ProductsTable got (temporary) pastebin.com/yJ0XqDjC \$\endgroup\$ – Ricardo Dec 5 '15 at 20:38
  • \$\begingroup\$ @AD7six other reasonable solution would be to use RequestAction within the Product Controller? \$\endgroup\$ – Ricardo Dec 5 '15 at 22:40
  • \$\begingroup\$ ReqeustAction is not a good solution \$\endgroup\$ – AD7six Dec 6 '15 at 18:04
  • \$\begingroup\$ @AD7six I try a new approach \$\endgroup\$ – Ricardo Dec 16 '15 at 11:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.