Оцените валидацию и обновление данных товара в контроллере?

Question

[Дмитрий]

Здравствуйте, в предыдущем моем вопросе я спрашивал как правильно делать валидацию формы. Были советы, переделал все. Дайте пожалуйста оценку. Что правильно? Что можно исправить и сделать по-другому?

app/Http/Controllers/Admin/ProductsController.php

use App\Http\Requests\ProductsUpdateRequest;

class ProductsController extends Controller
{
public function editRequestProduct(ProductsUpdateRequest $request, int $id)
{
        $validated = $request->validated();

        $objProducts = Products::find($id);
        if(!$objProducts){
            return abort('404');
        }

        $image = ImageDNK::save($request, 'image');
        $tab_bg = ImageDNK::save($request, 'tab_bg');
        $is_reccomended = $request->has('is_reccomended') ? true : false;

        $validated['image'] = $image;
        $validated['tab_bg'] = $tab_bg;
        $validated['is_reccomended'] = $is_reccomended;

        $objProducts->fill($validated);

        if(!$objProducts->save()){
            return back()->with('error', 'Товар не изменен. Попробуйте ещё раз');
        }

        $hasCategory = $request->input('product_category') != 0;

        if($hasCategory){
            $objCatsRels = new CategoriesRelationship();
            $objCatsRels = $objCatsRels->create([
                'object_id' => $id,
                'category_id' => $request->input('product_category'),
            ]);
        }

        if($objProducts && (!$hasCategory || $objCatsRels)){
            return back()->with('success', trans('messages.products.successUpdated'));
        }

        return back()->with('error', 'Товар не изменен. Попробуйте ещё раз');
    }
}

app/Http/Requests/ProductsUpdateRequest.php

<?php

namespace App\Http\Requests;

use Illuminate\Foundation\Http\FormRequest;
use Illuminate\Validation\Rule;

class ProductsUpdateRequest extends FormRequest
{
    public function rules()
    {
        return [
            'title' => 'required|string|min:4|max:25',
            'slug' => array(
                'required',
                'string',
                'min:4',
                'max:25',
                'regex:/^[a-z0-9а-яё-]+$/u',
                //'unique:products',
                Rule::unique('products')->ignore($this->id),

            ),
            'excerpt' => 'required|string|min:4|max:100',
            'content' => 'required|string|min:4|max:300',
            'descrtitle' => 'required|string|min:4|max:100',
            'descrtext' => 'required|string|min:4|max:300',
            'descr' => 'required|string|min:4|max:300',
            'regular_price' => array(
                'required',
                'regex:/\d+/',
            ),
            'discount' => array(
                'required',
                'min:0',
                'max:100',
                'regex:/^\d+(\.\d{1,2})?$/',
            ),
            'image' => 'mimes:jpeg,jpg,png,gif|max:10000',
            'tab_bg' => 'mimes:jpeg,jpg,png,gif|max:10000',
            'product_description_tab_content' => 'string|max:10000',
            'product_ingredients_tab_content' => 'string|max:10000',
            'product_usage_tab_content' => 'string|max:10000',
            'is_reccomended' => '',
        ];
    }
}

Спасибо

Comments

[NubasLol]

Уже намного лучше, но много логики в контроллере

[JhaoDa]

Пора уже перестать стесняться и начать форматировать код по PSR-1/2/12.

2019 год на дворе, хватит писать array().

В слове reccomended одна буква c, но две буквы m.

[Дмитрий]

NubasLol, что можно убрать из контроллера?

[NubasLol]

Дмитрий,

$image = ImageDNK::save($request, 'image');
        $tab_bg = ImageDNK::save($request, 'tab_bg');
        $is_reccomended = $request->has('is_reccomended') ? true : false;

        $validated['image'] = $image;
        $validated['tab_bg'] = $tab_bg;
        $validated['is_reccomended'] = $is_reccomended;

if($hasCategory){
            $objCatsRels = new CategoriesRelationship();
            $objCatsRels = $objCatsRels->create([
                'object_id' => $id,
                'category_id' => $request->input('product_category'),
            ]);
        }

[NubasLol]

Дмитрий, Проще говоря, все что не касается работы с http

[Дмитрий]

NubasLol, понял, куда его убрать правильно?

[NubasLol]

Дмитрий, зависит от проекта. В какой-то сервис класс.

Для начала, можешь хотя бы выделить это в отдельные функции в контроллере, потом увидешь, где собирается похожий функционал, и соберешь это в один класс.

[Дмитрий]

NubasLol, у меня есть папка

app/Helpers

где храню вспомогательные функции.
Жаль что это коммент - не могу отметить решением. Спасибо.

[NubasLol]

Дмитрий, плохая практика, хелперы делать

Ну если это не универсальные функции, которые в любом проекте могут пригодится

[Дмитрий]

Дмитрий, для этого

$objProducts = Products::find($id);
        if(!$objProducts){
            return abort('404');
        }

существуют Model Binding, раз ты все равно айдишник передаешь в метод.

[Дмитрий]

Нашел решение. Вместо:

public function addRequestProduct(ProductsRequest $request){
        $validated = $request->validated();
        $objProduct = new Products();

        $objProduct = $objProduct->create($validated, [
            'is_reccomended' => $request->has('is_reccomended') ? 1 : 0,
            'image' => ImageDNK::save($request, 'image'),
            'tab_bg' => ImageDNK::save($request, 'tab_bg'),
        ]);

        if(!$objProduct){
            return back()->with('error', 'Товар не создан. Попробуйте ещё раз');
        }

        $hasCategory = $request->input('product_category') != 0;

        if($hasCategory){
            $objCatsRels = new CategoriesRelationship();
            $objCatsRels = $objCatsRels->create([
                'object_id' => $objProduct->id,
                'category_id' => $request->input('product_category'),
            ]);
        }

        if($objProduct && (!$hasCategory || $objCatsRels)){
            return redirect(route('admin.products.edit', ['id' => $objProduct->id]))->with('success', trans('messages.products.successCreated'));
        }

        return back();
    }

Сделал:

public function addRequestProduct(ProductsRequest $request){
        $validated = $request->validated();
        $objProduct = new Products();

        $validated['is_reccomended'] = $request->has('is_reccomended') ? 1 : 0;
        $validated['image'] = ImageDNK::save($request, 'image');
        $validated['tab_bg'] = ImageDNK::save($request, 'tab_bg');

        $objProduct = $objProduct->create($validated);

        if(!$objProduct){
            return back()->with('error', 'Товар не создан. Попробуйте ещё раз');
        }

        $hasCategory = $request->input('product_category') != 0;

        if($hasCategory){
            $objCatsRels = new CategoriesRelationship();
            $objCatsRels = $objCatsRels->create([
                'object_id' => $objProduct->id,
                'category_id' => $request->input('product_category'),
            ]);
        }

        if($objProduct && (!$hasCategory || $objCatsRels)){
            return redirect(route('admin.products.edit', ['id' => $objProduct->id]))->with('success', trans('messages.products.successCreated'));
        }

        return back();
    }

При передаче вторым аргументом массива с boolean в метод create давало ошибку.

Answer 1

[NubasLol]

1)Правильно движешься, что валидация в реквестах.
2)Избыточный код

$objProducts = Products::find($id);
        if(!$objProducts){
            return abort('404');
        }

Используй findOrFail

3) $is_reccomended = $request->has('is_reccomended') ? true : false;

Метож has и так вернет тру или фолс, тернарный оператор лишний

4)(!$hasCategory || $objCatsRels)

Опять же, лишнее, ты же создаешь objCatsRels если не hasCategory

Ну и разнеи логику по сервисам.

В контроллере должен быть понятный читабельный код, что бы любой сразу понял, что в не происходит