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

Documentation for custom rules and closure rules #611

Merged
merged 4 commits into from
Feb 7, 2023

Conversation

MissaelAnda
Copy link
Contributor

Document Custom Rule Classes and Closure Rules

Add a description to Closure rule with a DocComent:

$request->validate([
    'complex_rule' => [
        /** This is a very specific validation therefore it needs a better description */
        function ($attribute, $value, $fail) { $fail('error'); }
    ],
]);

Add more info to a Custom Validation Rule:

class CustomRule implements Rule
{
    // ...

    public static function docs(): array
    {
        return [
            'description' => 'This rule makes something very specific',
            'example' => '52KB',
        ];
    }
}

This is a followup for #551

I'm missing the tests and documentation changes, if you could point me where and how should I do them it'd be nice.

@shalvah
Copy link
Contributor

shalvah commented Feb 4, 2023

The class that tests this is the ValidationRuleParsingTest. You should probably add two new test methods, something like can_parse_custom_closure_rules and can_parse_custom_rule_classes. You can follow the existing examples.

@shalvah
Copy link
Contributor

shalvah commented Feb 4, 2023

It feels a bit iffy to me to have an example defined on a rule. I guess it can work, but I'm uncertain if we want to go that path. For example, what happens if one parameter has this rule and others defined? The example will either be overwritten or hardcoded to this, which may or may not be unexpected behaviour. Maybe we should leave that out for now (after all, you can already specify examples for parameters in a FormRequest).

@MissaelAnda
Copy link
Contributor Author

The idea with the example is to define a default example in case none given like the string rules "setter"s, the problem maybe is that the example is always assigned. I'll change it to only set the example with the "setter" field and add the tests.

@shalvah shalvah merged commit 7f134a4 into knuckleswtf:master Feb 7, 2023
@shalvah
Copy link
Contributor

shalvah commented Feb 7, 2023

Can you send in a docs PR as well?

@MissaelAnda MissaelAnda deleted the document-custom-rules branch February 7, 2023 21:57
@MissaelAnda
Copy link
Contributor Author

I just relized that I'm missing InvokableRule, it only needs to change ParsesValidationRules to import it and change line 194 to:

if ($rule instanceof Rule || $rule instanceof InvokableRule) {

@shalvah
Copy link
Contributor

shalvah commented Feb 8, 2023

Added.

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

Successfully merging this pull request may close these issues.

2 participants