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

Add enum() method to Str class #75

Merged
merged 2 commits into from
Aug 23, 2023
Merged

Conversation

bertramakers
Copy link
Contributor

Hi,

I propose to add a new enum() method on the Str class. This would make validation of enum properties a lot easier, so we don't need to manually write something like this every time we have an enum property:

Str::make('example')
    ->validate(function (mixed $value, callable $fail) {
        if (!in_array($value, [... allowed values ...])) {
            $fail('must be one of: ' . implode(', ', [... allowed values ...]));
        }
    });

It would also tie in nicely with the enum keyword in JSON schema which is used in OpenAPI specs, so it would also be possible to document the possible enum values in the auto-generated OpenAPI documentation that you are working on in #62

See: https://json-schema.org/draft/2020-12/json-schema-validation.html#section-6.1

(Technically JSON schema allows this attribute on any type, but it seemed most useful to me on string fields for now. It could also be added to other fields later of course.)

@tobyzerner
Copy link
Owner

tobyzerner commented Aug 23, 2023

Great idea, thanks for the PR! I think I forgot about enums when I was implementing the data type features because they're on a separate page in the Swagger docs 😅

@tobyzerner tobyzerner merged commit 55a6482 into tobyzerner:main Aug 23, 2023
2 of 3 checks passed
@tobyzerner
Copy link
Owner

If you have a moment, could you please add this to the docs as well?

@bertramakers bertramakers deleted the str-enum branch August 23, 2023 12:10
@bertramakers
Copy link
Contributor Author

If you have a moment, could you please add this to the docs as well?

Done in #76

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