Separate form and input filters #20
Conversation
|
|
||
| // Add specific validation rules | ||
| $usernameValidators = $this->get('username')->getValidatorChain(); | ||
| $usernameValidators->addValidator($usernameUniqueValidator); |
There was a problem hiding this comment.
@bakura10 Why dont you just add inputs inside init() method? Then you'll have the updated ValidatorManager with UsernameUniqueValidator and EmailUniqueValidator.
There was a problem hiding this comment.
init() is not needed if you explicitly set dependencies like it is the case here. I don't know yet what to do.
There was a problem hiding this comment.
But all filters and validators are dependencies. It makes no sense you explicitly set dependencies.
There was a problem hiding this comment.
Make sense. I'll change that.
There was a problem hiding this comment.
Hmm in fact it's not so easy danizord, because the validator requires options… Not sure about how to do it. Plugin managers are always limited when it comes to option.. So I think the current solution is the best one for now :/.
|
@danizord : The problem is that the input is already added here, so I cannot use the "->add" factory method. And validator chain does not contain factory. So I either: let it as it is, or create the validators in the input filter itself rather than in the factory. |
|
I'd like to have this more separated – let's talk about this on IRC. |
|
I've tried to do some work about it today but..... I started to lost myself :/. I think I'll need to restart this PR. Sorry for the delay :(. |
|
@DASPRiD : I'm done. Now needs review :). One remark: I had too inject the NoObjectExsits validator insteaad of creating them inside the RegistrationInputFilter because it was a whole mess to mock (you cannot mock methods of existing object). NOTE: considering the rename from password_hash to passwordHash for instance, this is because otherwise hydrator won't work (well, maybe ClassMethods because it has supports to convert underscore_separated to camelCase, but most other hydrator don't do that). |
1 similar comment
| * @param object $object | ||
| * @return object | ||
| */ | ||
| public function hydrate(array $data, $object) | ||
| { | ||
| if (isset($data['password'])) { | ||
| $data['password_hash'] = $data['password']; | ||
| $data['passwordHash'] = $data['password']; |
There was a problem hiding this comment.
Why is this one happening?
There was a problem hiding this comment.
Mainly for hydrators, as most hydrators often do "set" . $key.
There was a problem hiding this comment.
I know, I don't like all of those naming confusions. Sometimes underscore_separated, sometimes camelCase... But the problem is complex and if we want something coherent we really should establish a standard for common components, so that each component by default can assume something about how data is named :/.
There was a problem hiding this comment.
We go with underscore_separated in all arrays so far, shouldn't be different here.
There was a problem hiding this comment.
I think it may be a problem for most hydrators that inflect the setter/getter based on the key. For instance DoctrineModule hydrator's assume that setter is "set" . ucfirst($key). So if we keep this underscore_separated people should define setFirst_Name method. I just went with the situation that will fit most situations without having to override each hydrator.
|
In RegistrationServiceFactory.php there is a line: Change to?: In RegistrationServiceFactoryTest.php there is a line: Change to?: I have no GitHubFu, so no idea on how to make the changes here for you. :( |
Corrected service identifier for RegistrationForm move to User path
This PR correctly uses field sets instead of forms. This allow much more easier extending as well as remove the need to rewrite the fields and validation rules.
Fieldsets also allow to enable relationships between entities.
One thing I'm not sure is the use of the ClassMethods hydrator by default. I think it should use DoctrineModule hydrator.
@mac_nibblet ping