laravel / prompts

Beautiful and user-friendly forms for your command-line PHP applications.
https://laravel.com/docs/prompts
MIT License
533 stars 94 forks source link

Support for custom validation and aliases #102

Closed cerbero90 closed 10 months ago

cerbero90 commented 10 months ago

Following up the improvement for checking the Prompts validation when testing, this PR introduces the ability to globally set a custom validation logic for Laravel Prompts.

The end goal is to be able to validate Laravel Prompts by leveraging the existing Laravel validation rules, while keeping Laravel Prompts framework agnostic.

To achieve this, the static method Prompt::validateUsing() can be called to determine the global logic to validate Laravel Prompts. If a prompt has its own validation logic set via the parameter validate, such logic has the precedence over the global validation logic.

If this PR gets merged, we can then set the global validation logic in the Laravel framework by updating the ConfiguresPrompts trait like this draft PR is currently doing:

protected function configurePrompts(InputInterface $input)
{
    // ...
    Prompt::validateUsing($this->validatePrompt(...));
    // ...
}

protected function validatePrompt(Prompt $prompt)
{
    if (! $rules = $prompt->validate) {
        return null;
    }

    [$alias, $value] = [$prompt->alias(), $prompt->value()];

    $validator = Validator::make([$alias => $value], [$alias => $rules], $this->messages(), $this->attributes());

    return $validator->errors()->first();
}

protected function messages()
{
    return [];
}

protected function attributes()
{
    return [];
}

The above logic would let the user pass the Laravel validation rules to the validate parameter like this:

text('What is your name?', validate: 'required|min:2');

and take full advantage of the validator features like setting custom error messages or custom attributes.

This PR also introduces aliases so that each prompt can be uniquely differentiated. Useful for both validation and any future use-case that needs to distinguish a prompt in a short and unique way.

The alias can be set via the as parameter, e.g.:

text('What is your name?', validate: 'required|min:2', as: 'name');

Otherwise the alias is automatically inferred by counting each prompt i.e. prompt_1, prompt_2, etc.

Ultimately this PR is designed to keep Laravel Prompts framework agnostic and let the frameworks decide their own preferred way to validate Laravel Prompts.

cerbero90 commented 10 months ago

@jessarcher please note that there is a PHPStan error on the Progress class that doesn't depend on the changes of this PR šŸ‘

jessarcher commented 10 months ago

Hey @cerbero90,

I dig this approach for adding Laravel's validation rules without increasing the dependencies.

What are your thoughts on leaving out the as stuff for now, to reduce the changes and complexity a little? I understand one of the challenges with using the validator is that validation messages often refer to the field name, but I think we could initially just set the field name as something like "answer" on the Validator instance so the messages still make sense. I think this would also be a nicer default than prompt_1, etc, and because we display validation errors immediately alongside the prompt, I don't see the need to make them unique like you would with a standard web form.

cerbero90 commented 10 months ago

Thanks for your quick reply @jessarcher :)

I'm always in favour of simplicity šŸ‘ the reason for the aliases is merely for being able to also define custom error messages and attributes for each prompt individually.

Having answer as the default field name would mean that:

An alternative to the current default aliasing may be converting the label into snake case. That would work well when we use something like Full name as a label (the alias would become full_name, which is then displayed as full name by the validator), but it would not work so well if we use labels like What is your full name?. In the latter case it would make sense to set a custom alias.

If you prefer, we can get rid of the aliases altogether but we would encounter the 2 problems listed above šŸ¤”

jessarcher commented 10 months ago

I think I'm okay with those problems, at least for the initial version of this feature. We can always add them in a follow-up. But just to make sure I fully understand, can you please elaborate a little more on the problems and specifically what type of customization you'd want to make that wouldn't be possible? Are you just wanting to be able to make the message say "The full name field is required" instead of "the answer field is required"?

I've also fixed the PHPStan error in main if you'd like to rebase.

cerbero90 commented 10 months ago

Sure, no worries :) and yes, the issue with dropping the aliases is only related to the possibility to customise the name of the field and the related error messages. If we want to have an error completely different from the default provided by the validator, we can't. Without aliases we lose the ability to completely being able to customise our output.

Another drawback is for all the validation rules that depend on other fields, like the following:

But if you are ok with that at this stage, I'll remove the aliases from this PR šŸ‘

jessarcher commented 10 months ago

Thanks! Would interdependent validation even make sense when each prompt is validated in isolation?

cerbero90 commented 10 months ago

Yes you are right, I didn't think about that :)

cerbero90 commented 10 months ago

Aliases removed šŸ‘

cerbero90 commented 10 months ago

@jessarcher if it's possible to bump the version of Prompts once this PR is merged, I'll make sure to require the new version in this PR :)

jessarcher commented 10 months ago

Sure thing - thanks!