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

UniqueObject Validators doesn't work if you give the column a different name, than the fields name. #377

Open
DennisDobslaf opened this issue Feb 10, 2014 · 12 comments
Labels

Comments

@DennisDobslaf
Copy link

UniqueObject Validators doesn't work if you give the column a different name, than the fields name.

So, if you have an user entity with id as identifier and name the column like 'user_id' (so, the webform also contains a field names 'user_id' the validator will fail at`

if (!isset($context[$columnName])) {

because, there is no 'id' (but 'user_id') in the form.

So, as renaming columns is a supported behaviour shouldn't the UniqueObject validator rely on the choosen/given columnName?

If have a working example here, but don't know, if it is correct in all ways...
Could make a pull request for that.

@Ocramius
Copy link
Member

This seems like a design flaw.

@UFOMelkor do you think we can make the UniqueObject validator rely solely on $value (as an array) and on $context only if the user wants that?

@DennisDobslaf
Copy link
Author

In my temporary solution, I grap the column name of the identifier fields.
It works so far ;-)

Have a look at:

https://github.com/DennisDobslaf/DoctrineModule/blob/master/src/DoctrineModule/Validator/UniqueObject.php

2014-02-10 Marco Pivetta [email protected]

This seems like a design flaw.

@UFOMelkor https://github.com/UFOMelkor do you think we can make
$context optional?

Reply to this email directly or view it on GitHubhttps://github.com//issues/377#issuecomment-34697962
.

@DennisDobslaf
Copy link
Author

I don't like the 'columnName' index, as it is kind of a magic constant. But
found no other (quick) solution for me :(

2014-02-11 Dennis Dobslaf [email protected]

In my temporary solution, I grap the column name of the identifier fields.
It works so far ;-)

Have a look at:

https://github.com/DennisDobslaf/DoctrineModule/blob/master/src/DoctrineModule/Validator/UniqueObject.php

2014-02-10 Marco Pivetta [email protected]

This seems like a design flaw.

@UFOMelkor https://github.com/UFOMelkor do you think we can make
$context optional?

Reply to this email directly or view it on GitHubhttps://github.com//issues/377#issuecomment-34697962
.

@Ocramius
Copy link
Member

@DennisDobslaf doctrine deals with field names - column names are a low level problem that you shouldn't deal with, so that's absolutely the wrong fix/approach.

@DennisDobslaf
Copy link
Author

Ok, understood this. I could change my 'user_id' into 'id', but that's not
a final solution either. ;-)

2014-02-11 Marco Pivetta [email protected]

@DennisDobslaf https://github.com/DennisDobslaf doctrine deals with
field names - column names are a low level problem that you shouldn't deal
with, so that's absolutely the wrong fix/approach.

Reply to this email directly or view it on GitHubhttps://github.com//issues/377#issuecomment-34703322
.

@UFOMelkor
Copy link
Contributor

Sorry for the long delay, I was on holidays.

@Ocramius
I used the $context parameter because it's also used by some zf2 classes (see Zend\InputFilter\Input, Zend\Validator\ValidatorChain and Zend\Validator\Identical).

@Ocramius
Copy link
Member

Ocramius commented Mar 5, 2014

I know where it is used, I just really dislike that approach :P

@UFOMelkor
Copy link
Contributor

:-D

I'm fine with using $context only optional, but I'm not sure whether this will change the reported issues. IMHO we need especially a little more documentation for the UniqueObjectValidator.

One questions because I'm doing to much java and to less zf2 at the moment: For supporting Zend\Form we need the $context, don't we?

@Ocramius
Copy link
Member

Ocramius commented Mar 5, 2014

@UFOMelkor the other way would be to have a fieldset for multi-field values...

But yes, $context seems to have this necessary evil. What is still broken is indeed documentation about the fact that it is opinionated.

@UFOMelkor
Copy link
Contributor

Ok, I will look on this and send a PR :-)

@DennisDobslaf
Copy link
Author

What about this approach(!)

https://github.com/DennisDobslaf/DoctrineModule/blob/master/src/DoctrineModule/Validator/UniqueObject.php

Usage is like

$uniqueEmailValidator = new UniqueObject(array(
'object_repository' => $userRepository,
'fields' => array('email'),
'object_manager' => $this->getObjectManager(),
'token' => array(
'id' => 'user_id'
),
));

@Bilge
Copy link
Contributor

Bilge commented May 1, 2015

I don't know why that PR is referencing this issue because not only does that PR not address this issue in any way, it also breaks BC as an added bonus.

I still receive the Expected context to contain id exception whether or not I elect to use_context in 0.8.1.

The fallacy seems to be assuming that the entity's identifier field will be provided by the form. I'm sure this is possible if you're editing an existing entity but if you're adding a new entity with a unique field that is different from the identifier field then the identifier will not be available since it is auto-generated by the DB.

@driehle driehle added the Bug label Dec 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants