Как ограничить или оптимизировать доступ к валидации параметров на laravel в request?

Question

[lexstile]

Суть в том, что я бы хотел, чтобы изменять параметры (поля) заказа можно было только в следующих статусах:
table_id - NEW, WORK
user_id - NEW, WORK
total_person - NEW, WORK
status - NEW, WORK, PAID, CLOSE

Создал спец. ф-цию для этого в модели:

Model/canUpdate

public function canUpdate(string $type = 'order') {
        return match ($type) {
            'order' => in_array($this->status, [self::NEW, self::WORK]),
            'status' => in_array($this->status, [self::NEW, self::WORK, self::PAID, self::CLOSE]),
            'reverse' => in_array($this->status, [self::PAID, self::CLOSE]),
            default => false,
        };
    }

Но ничего не придумал, кроме как написать в authorize:

Часть кода из authorize

if (isset($this->table_id) || isset($this->user_id) || isset($this->total_person)) {
            return $this->order->canUpdate();
        }

        if (isset($this->status)) {
            return $this->order->canUpdate('status');
        }

OrderUpdateRequest

class OrderUpdateRequest extends FormRequest
{
    public function authorize()
    {
        if ($this->user()->isEmployee()) {
            if ($this->user()->id !== $this->order->user_id && $this->user()->id != $this->user_id) {
                return false;
            }
        }

        if (isset($this->table_id) || isset($this->user_id) || isset($this->total_person)) {
            return $this->order->canUpdate();
        }

        if (isset($this->status)) {
            return $this->order->canUpdate('status');
        }
        
        return false;
    }

    public function rules()
    {
        return [
            'table_id' => [
                'numeric',
                Rule::exists('tables', 'id')->where(function ($query) {
                    return $query->where('project_id', $this->project->id);
                }),
                Rule::unique('orders')->where(function ($query) {
                    return $query->where('table_id', $this->table_id)->whereIn('status', [Order::NEW, Order::WORK]);
                }),
            ],
            'user_id' => ['nullable', 'numeric', 'exists:users,id', new UserAssigneeToOrderRule($this->project)],
            'status' => ['string', new OrderStatusRule($this->order)],
            'total_person' => ['numeric', 'min:1', 'max:99'],
        ];
    }

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

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

    public function failedAuthorization() {
        throw new HttpResponseException(response()->json([
            'success' => false,
            'message' => ResponseHelper::error(__('orders.messages.update.denied')),
        ])->setStatusCode(403));
    }

    public function messages(): array
    {
        return [
            'table_id.unique' => __('orders.messages.update.table.unique'),
        ];
    }
}

Можно ли как-то без костылей это решить? (еще пробовал сделать кастомное правило и продублировать для каждого параметра в rules, но показалось, что это еще больший костыль)

Comments

[Дмитрий]

Зачем у вас canUpdate пытается проверять вообще все варианты? Этот метод к чему относится? К Policy? К модели?

[lexstile]

Дмитрий, модель

[Дмитрий]

lexstile, а что она там делает?

[lexstile]

Дмитрий, не очень понял, что и где?
Вызов метод модели в реквесте?

[Дмитрий]

lexstile, смотрите - у вас проверка могу ли я изменить модель валяется в нескольких местах - в реквесте, в модели. Зачем?

Answer 1

[Дмитрий]

php artisan make:policy OrderPolicy

class OrderPolicy
{
    use HandlesAuthorization;

    public function update(User $user, Order $order)
    {
         if (!$user->isEmployee()) {
                return false;
         }
         if ($user->id !== $order->user_id) { // остальной кусок ифа не понял - но он должен быть здесь.
                return false;
          }

          return in_array($order->status, [Order::NEW, Order::WORK]);
    }

}

class OrderUpdateRequest extends FormRequest
{
    public function authorize()
    {
          return $this->user()->can('update', $this->order);
    } 
}

Comments

[lexstile]

Очень интересно, благодарю.

if ($this->user()->isEmployee()) {
            # employee нельзя обновлять заказ другого пользователя
            # employee должен назначить заказ сначала на себя, чтобы потом обновлять
            if ($this->user()->id !== $this->order->user_id && $this->user()->id != $this->user_id) {
                return false;
            }
        }

[Дмитрий]

lexstile, все равно не понял - мне лень думать. Но должно быть желание сгруппировать функционал отвечающий за что-то одно - в одном месте.

З.Ы. - и нахрен эти елочки из ифов, это же мозг сломаешь читать. Не имплоер - иди нахрен. Не твой заказ - иди нахрен.

[lexstile]

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

Route::controller(\App\Http\Controllers\Admin\EmployeeController::class)->prefix('/employees')->group(function() {
                Route::get('/', 'getAll')->can('getEmployees', 'project');

                Route::get('/{user}', 'show')->can('getEmployee', 'project');
                Route::delete('/{user}', 'destroy')->can('deleteEmployee', 'project');
            });

Не твой заказ - иди нахрен.

Тут немного сложнее логика.
Если ты Админ, то изменять заказ возможно.
А для обычного сотрудника можно еще и изменять поле user_id - это назначить на себя.

[Дмитрий]

lexstile, ну значит

if (!$user->admin()) {
                return true;
         }
if (!$user->isEmployee()) {
                return false;
         }

[lexstile]

Дмитрий, этот кейс, получается, не обыграть в policy, к сожалению

$this->user()->id != $this->user_id // это входной параметр на изменение

[Дмитрий]

lexstile, внезапно.
первая ссылка:
https://arievisser.com/blog/passing-multiple-argum...

[lexstile]

Дмитрий, спасибо большое.
Главное - правильно составить вопрос, у меня не получилось, к сожалению.

[lexstile]

Дмитрий, в общем, получилось так (сложно):

public function authorize()
    {
        return $this->user()->can('update', [
            $this->order,
            [
                'user_id' => $this->user_id,
                'table_id' => $this->table_id,
                'total_person' => $this->total_person,
                'status' => $this->status,
            ]
        ]);
    }

    public function update(User $user, Order $order, array $params = [])
    {
        if ($user->isEmployee()) {
            # employee нельзя обновлять заказ другого пользователя
            # employee должен назначить заказ сначала на себя
            if ($user->id !== $order->user_id && $user->id != $params['user_id']) {
                return false;
            }
        }

        if ($user->isAdmin()) {
            # admin может изменять заказы только у себя или у employee
            if (!$order->user?->isEmployee() || $user->id !== $order->user_id) {
                return false;
            }
        }

        if (isset($params['table_id']) || isset($params['user_id']) || isset($params['total_person'])) {
            return $order->canUpdate();
        }

        if (isset($params['status'])) {
            return $order->canUpdate('status');
        }
    }

[Дмитрий]

lexstile, уфффф

$order->canUpdate('status'); - за чем это в модели? вот зачем вы это кладете туда?

[lexstile]

Дмитрий, чтобы пользоваться одним методом, где доступен заказ, чтобы изменять потом в одном месте, а не в нескольких.
Но я Вас услышал, возможно, вы и правы. Попробую переделать, если это возможно, т. к. не хочется писать каждый раз:

in_array($order->status, [Order::NEW, Order::WORK]);

И еще понял, параметры, похоже, не нужно передавать в полиси, можно же их вытащить из request()

[Дмитрий]

lexstile, у вас есть policy в котором вы определяете - можно редактировать этот заказ или нет. а так у вас уже два места где вы определяете можно редактировать заказ в модели и policy - на хрена? почему - вот это in_array($order->status, [Order::NEW, Order::WORK]); нельзя поставить в policy. В чем цимес?

[lexstile]

Дмитрий, есть несколько точек редактирования заказа (вот я и приуныл):
1. Непосредственно данные заказа (про которые говорим)
2. Далее, добавление блюд в заказ
3. И возврат денежных средств по заказу


Как я понимаю, нужно просто создать несколько методов в policy? (дополнительно на возврат, например)

[Дмитрий]

lexstile, конечно. У вас есть класс который решает что можно делать с заказом. дык и пихайте туда. в чем проблемы то?

[lexstile]

Дмитрий, все проблемы в голове, спасибо за разбор!
Готов выделить небольшую денежную компенсацию за уделенное время (вы каждый раз отвечаете на мои вопросы).

[Дмитрий]

lexstile, бросьте, чем больше толковых спецов вокруг - тем легче жизнь

[lexstile]

Дмитрий, хорошо, благодарю еще раз.