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

[Feature] Different rules per locale #70

Merged
merged 6 commits into from
Apr 19, 2022

Conversation

Woeler
Copy link
Contributor

@Woeler Woeler commented Mar 15, 2022

Hello,

first time pull-requester here in the environment of Spatie. I'm hoping to get a feature in that I personally could use in one of my projects.

So, we have various translatable models, and of course our editors work in a Nova panel to edit and create them. The particular case we have is about news articles. The main language of our website is English, but we also publish in German and French.

In our workflow, an author will get to work writing and Article in English, and then publish it. After that is done he or she will begin translating the article to the other languages.

Currently we do not use any rules on our fields for news articles, because if we would set the title to required, it would require a title in English, German and French. But we only want the English title to be required, and the translations optional.

This pull request aims to solve this problem.

It contains six new public methods that can be called on the Translatable fields:

rules(array $rules)
The rules method will take a nested array of field names, which then in turn contain a key-value pair of a locale and its rules. For example:

$translatable->rules(['title' => ['en' => 'required']])

rulesFor(string $field, string $locale, $rules)
A more dynamic method to declare rules per field. A syntax I personally find a little more elegant and readable. It achieves the same behaviour like so:

$translatable->rulesFor('title', 'en', 'required');

In addition to those two, there are also methods for creation rules and update rules.

Of course, all methods work with all three types of Laravel rules declaration (string, array and Rule classes).

That's it from me. Any feedback is greatly appreciated.

Resolves: #64

@Woeler Woeler force-pushed the feature/different-rules-per-locale branch 4 times, most recently from 2eb4040 to 0743de5 Compare March 15, 2022 22:51
@Woeler Woeler marked this pull request as ready for review March 15, 2022 23:05
@Woeler Woeler force-pushed the feature/different-rules-per-locale branch from 038e263 to c4eedbc Compare March 15, 2022 23:38
@Woeler
Copy link
Contributor Author

Woeler commented Apr 15, 2022

Hello again, can I perhaps have a reply on this, if this is a feature with merge potential?

@freekmurze
Copy link
Member

Very nice, could you also update the readme with an example on how to use this?

@Woeler
Copy link
Contributor Author

Woeler commented Apr 18, 2022

@freekmurze Absolutely. I'll do it tomorrow, after Easter is over. Cheers!

@Woeler
Copy link
Contributor Author

Woeler commented Apr 19, 2022

@freekmurze I added it. I also added the composer test command which was referenced in the readme, but not available. Could you approve the workflow on this PR so the tests can run? Thanks.

@Woeler Woeler force-pushed the feature/different-rules-per-locale branch from fe0efa6 to 072875d Compare April 19, 2022 10:36
@Woeler
Copy link
Contributor Author

Woeler commented Apr 19, 2022

Rebased with main

@freekmurze
Copy link
Member

The tests fail because of Nova installation issues, but I'm willing to merge already if you already tested in manually and can confirm that it works as intended.

@Woeler
Copy link
Contributor Author

Woeler commented Apr 19, 2022

Hey, yes the tests run just fine locally. I have been using this functionality myself for quite some time already by extending the original Translatable class.
image

@freekmurze freekmurze merged commit 9474ceb into spatie:main Apr 19, 2022
@freekmurze
Copy link
Member

Thank you!

@Woeler Woeler deleted the feature/different-rules-per-locale branch October 8, 2022 14:41
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.

Different rules for each translation
2 participants