Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

правило валидации для Attachment, загруженных ранее полями Upload, Cr… #2745

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mavsan
Copy link
Contributor

@mavsan mavsan commented Nov 8, 2023

Часто необходимо выполнять валидацию файлов, загруженных через Upload. Можно через Observer, но лучше, лгично и читаемо - через контроллер или Form Request в стандартном валидаторе Laravel. Предлагаю такое правило для валидатора.

Как использовать:

use Orchid\Rule\AttachmentRule;

class SomeClassScreen  extends Screen {

    public function save(Request $request) {
        $request->validate([
            'file' => ['required', new AttachmentRule('bail|image|dimensions:max_width=100,max_height=100', true)]
        ]);
    }
}

или

namespace App\Http\Requests;

use Orchid\Rule\AttachmentRule;

class SomeRequest extends FormRequest
{
    public function rules(): array
    {
        return [
            'file' => ['required', new AttachmentRule('bail|image|dimensions:max_width=100,max_height=100')],
        ];
    }
}

Класс принимает 2 параметра в конструктор - правила валидации и надо ли удалять файлы, не прошедшие валидацию (по умолчанию - true).

Есть один нюанс: required должно быть вынесено, как в примере выше, т.к. если поле не пришло или пустое - валидатор просто пропускает это поле, даже если указать required в параметрах AttachmentRule.

Вот, что получаем в итоге:
image

После валидации:
image

Ну и исправить описание ошибок, чтобы было не "Поле ... должно быть картинкой", а - "Файл ... должен быть картинкой" ну или как-то так, если это критично, конечно.

*/
protected function removeFiles(\Illuminate\Support\Collection $errors): void
{
$errors->each(function ($message, $key) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Думаю, что можем в том же стили как в начале:

$errors->filter(fn($message, $key) => $this->attachment->has($key))
    ->each(fn($message, $key) => $this->attachment->get($key)->delete());

protected function setAttachment(mixed $ids): void
{
$this->attachment = is_array($ids)
? Attachment::whereIn('id', $ids)->get()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нужно использовать модель не напрямую, так как разработчик может её изменить, что бы поддерживать это, достаточно:

$model = Dashboard::model(Attachment::class);

if ($attachment->exists) {
/** @var Filesystem|Cloud $disk */
$disk = Storage::disk($attachment->disk);
return new FileSymfony($disk->path($attachment->physicalPath()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это будет работать только с файлами которые находятся на локальной машине, всякие ftp/s3 и т.п. уже вызовут ошибку. Возможно имеет смысл либо скачивать, либо к проверке добавить, что файл доступен.

@tabuna
Copy link
Member

tabuna commented Nov 8, 2023

@mavsan У меня мысль которая потенциально является решением, так же и валидации. Так как код непосредственной загрузки достаточно короткий и легко воспроизводимым, то возможно имеет смысл добавить методы которые указали бы route, например:

Upload::make('images')->route('my.route.to.upload.image')

и уже в собственном маршруте мы можем делать различные проверки и не сохранять файл:

public function upload(Request $request)
{
    // My any validation

    $file = new File($request->file('photo'));
    $attachment = $file->load();

    return response()->json($attachment);
}

…ния файла, указанного в Attachment, перед созданием экземпляра File; изменен метод удаления файлов, не прошедших валидацию
@mavsan
Copy link
Contributor Author

mavsan commented Nov 8, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants