Как в laravel правильно запретить редактирование сущности по условию?

Question

[lexstile]

Задача:
1. Cделать так, чтобы при валидации status срабатывала еще и ф-ция из модели canChangeStatus, которая говорит, в какой статус можно переводить заказ в текущий момент.
2. А также, чтобы срабатывала на обновление ф-ция из модели canUpdate, которая бы говорила, что заказ редактировать можно.

И хотелось бы это разрулить в OrderUpdateRequest.

Есть мысли/идеи, как это грамотно можно разрулить?

OrderUpdateRequest

class OrderUpdateRequest extends FormRequest
{

    public function rules()
    {
        switch ($this->getMethod())
        {
            case 'PATCH':
                return [
                    'table_id' => ['numeric', 'exists:tables,id'],
                    'user_id' => ['numeric', 'exists:users,id'],
                    'status' => ['string', 'in:'. Order::NEW.','.Order::WORK.','.Order::CLOSE.','.Order::PAID],
                ];
            case 'PUT':
                return [
                    'table_id' => ['required', 'numeric', 'exists:tables,id'],
                    'user_id' => ['required', 'numeric', 'exists:users,id'],
                    'status' => ['required', 'string', 'in:'. Order::NEW.','.Order::WORK.','.Order::CLOSE.','.Order::PAID],
                ];
        }
    }

    protected function prepareForValidation()
    {
        $this->merge([
            //
        ]);
    }

    public function failedValidation(Validator $validator)
    {
        throw new HttpResponseException(response()->json([
            'success' => false,
            'errors' => $validator->errors(),
        ])->setStatusCode(400));
    }
}

Order

class Order extends Model
{
    use HasFactory, SoftDeletes;

    const NEW = 'new';
    const WORK = 'work';
    const CLOSE = 'close';
    const CANCEL = 'cancel';
    const PAID = 'paid';

    protected $fillable = [
        'project_id',
        'table_id',
        'user_id',
        'status',
    ];

    public function dishes()
    {
        return $this->belongsToMany(Dishe::class)->withTimestamps();
    }

    public function canChangeStatus($status)
    {
        switch ($this->status) {
            case self::NEW: return $status === self::WORK || $status === self::CANCEL;
            case self::WORK: return $status === self::PAID || $status === self::CANCEL;
            case self::PAID: return $status === self::CLOSE || $status === self::CANCEL;
        }

        return false;
    }

    public function canUpdate() {
        if (in_array($this->status, [self::NEW, self::WORK, self::PAID])) {
            return true;
        }
    
        return false;
    }
}

OrderController

class OrderController extends BaseController
{
    public function update(OrderUpdateRequest $request, Project $project, Order $order)
    {
        $order->fill($request->all());
        $order->saveOrFail();

        return $this->baseJson(message: ResponseHelper::success(__('orders.messages.updated')));
    }
}

Answer 1

[Дмитрий]

class OrderUpdateRequest extends FormRequest
{
    /**
     * Determine if the user is authorized to make this request.
     *
     * @return bool
     */
    public function authorize(): bool
    {
        return $this->order->canChangeStatus($this->get('status'))&&$this->order->canUpdate();
    }
}

Не надо так:

switch ($this->getMethod())
        {
            case 'PATCH':
                return [
                    'table_id' => ['numeric', 'exists:tables,id'],
                    'user_id' => ['numeric', 'exists:users,id'],
                    'status' => ['string', 'in:'. Order::NEW.','.Order::WORK.','.Order::CLOSE.','.Order::PAID],
                ];
            case 'PUT':
                return [
                    'table_id' => ['required', 'numeric', 'exists:tables,id'],
                    'user_id' => ['required', 'numeric', 'exists:users,id'],
                    'status' => ['required', 'string', 'in:'. Order::NEW.','.Order::WORK.','.Order::CLOSE.','.Order::PAID],
                ];
        }

Comments

[lexstile]

Благодарю за совет, забыл про authorize.

А как нужно? (просто не очень понял, на что именно вы хотели обратить внимание, и что хотели донести)

Не надо так:

[lexstile]

$this->order->canChangeStatus($this->get('status'))

эта проверка не совсем корректна в authorize, так как нам нельзя редактировать именно статус заказа, остальное можно (п. 1 из тела вопроса).
То есть:
1. canChangeStatus - если false - нельзя менять статус на тот, который пришел с фронта (то есть, изменение статусов возможно только в определенной последовательности - вперед, но не назад)
2. canUpdate - если false - нельзя изменять заказ (для authorize эта проверка отлично подойдет, думаю)

[Дмитрий]

lexstile,
1. ну я так понимаю у вас два разных запроса - два разных реквеста. Не надо наверное в реквест вешать дополнительный функционал.

2. Тогда создайте своё правило для статуса

[lexstile]

1. Методы запроса разные, но разница лишь в том, что в одном случае поля все обязательные, для метода контроллера это особо не важно, что пришло, то и сохранил)
2. О, большое спасибо, интересно очень.

Route::controller(\App\Http\Controllers\OrderController::class)->prefix('/orders')->group(function() {
                Route::prefix('/{order}')->group(function() {
                    Route::patch('/', 'update')->can('view', 'project');
                    Route::put('/', 'update')->can('view', 'project');
                });
            });

[lexstile]

Посмотрел ваше предложение, но проблема в том, что нужно получать текущий статус заказа и отталкиваться от него, а как я понял, кастомные правила в основном используются для независимого сравнения?

[Дмитрий]

lexstile, что мешает? если на вскидку - то можно так.

class ValidateStatus implements Rule
{
    protected $order;
    protected $status;

    public function __construct(Order $order, $status)
    {
        $this->order = $order;
        $this->status = $status;
    }

    public function passes($attribute, $value)
    {
        // Access the extra param as $this->extraParam
        return $this->order->canChangeStatus($this->status);
    }
}

$request->validate([
    'name' => ['required', new ValidateStatus($this->order, $this->get('status'))],
]);

можете расскопать conditional rules - и посмотреть как сделано в самой ларке и забубенить так.

[Дмитрий]

lexstile, имхо у вас должно быть 2 метода, 2 урла, 2 метода контроллера и 2 реквеста. как бы зачем вам запихивать я так понимаю создание заказа и изменение в один метод и в один реквест? ну вы можете одним контроллером и одним реквестом обрабатывать все запросы, всего лишь пара 10 ифов - но смысл?

[lexstile]

Дмитрий, создание заказа - POST
объединены - PUT (полное изменение) и PATCH (частичное изменение)

Большое спасибо, прокинуть в конструктор не догадался :(

[Дмитрий]

lexstile, ну вообщем не готов говорить категорично - ибо не вижу картину целиком - но все же имхо стоит разделять такие вещи.

[lexstile]

Дмитрий, не спорю с вами, соглашусь, в будущем будет проще.
Спасибо ещё раз огромное.

Answer 2

[jazzus]

Грамотно - не заниматься проверкой прав в модели. Юзать политики авторизации и обрабатывать 403 на фронте. Убрать failedValidation т.к. Ларавел сам составит правильный ответ. Success не нужно, если есть ошибки значит не success. Убрать бессмысленные проверки в rules с одинаковым кодом. Использовать ларавелевский response в контроллере без оберток и велосипедов.

Comments

[lexstile]

Благодарю за советы.
Похоже, в политиках этим проверкам самое место.