-
Notifications
You must be signed in to change notification settings - Fork 76
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
Rewrote injectUniqueIdentifierToRules() to be more generic #118
Conversation
This removes all the additional functionality provided for the Would you be able to make this PR against |
Yeah it unfortunately does, but I was actually quite puzzled why it was there in the first place. Especially since I think that it doesn't work if you want to specify additonal where clauses for a unique rule like this:
So I am not really sure if it is really breaking BC or rather removing a "feature" that didn't work completely in the first place. Furthermore I don't think much of anything would break because most likely nobody expected that functionality to be there in the first place and just wrote out the complete But if you still want to make a new major release I can do a PR against EDIT: Hmm this is actually quite a big issue, because the custom rule i mentioned in #117 always expects the id to be ignored to be the last parameter. The only idea i can come up with to resolve this would be adding a placeholder. Something like |
Just tossing in some commentary on this issue. I think the fundamental problem is that the Validator service itself does not support this custom rule. The key is to add a custom Validator and then use logic within that class to handle the problem. Unique ID injection is a common enough use case that making it part of the model is a short cut. Uniqueness across multiple columns, while useful in some cases, is not really a great thing to shove into the model unless provided as a bolt on trait (which also just feels wrong). Maybe you should look into providing a placeholder replacement functionality within a custom Validator macro. http://laravel.com/docs/4.2/validation#custom-validation-rules <?php
use Illuminate\Validation\Factory;
use Illuminate\Validation\ValidationServiceProvider as ServiceProvider;
class ValidationServiceProvider extends ServiceProvider {
public function register()
{
$this->registerPresenceVerifier();
$this->app->bindShared('validator', function($app)
{
$validator = new Factory($app['translator'], $app);
// The validation presence verifier is responsible for determining the existence
// of values in a given data collection, typically a relational database or
// other persistent data stores. And it is used to check for uniqueness.
if (isset($app['validation.presence']))
{
$validator->setPresenceVerifier($app['validation.presence']);
}
// Add custom validation rule macros
$macros = [
'unique_with' => 'App\Validators\UniqueWithValidationMacro',
];
foreach( $macros as $macro => $class)
{
$validator->extend($macro, $class . '@validate' . ucfirst(studly_case($macro)));
$validator->replacer($macro, $class . '@replace' . ucfirst(studly_case($macro)));
}
return $validator;
});
}
} On the ValidationMacro classes you'll have to get clever and figure out how to inject the model ID dynamically. Fortunately Hope this helps you figure this use case out. |
This may be a non-issue now as a check in the @dwightwatson this PR could probably be closed. |
The latest release on the master branch now supports adding additional unique injectors and ships with support for the default Laravel |
A simple implementation of the problem mentioned in #117.