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

UniqueValidator for multiple columns #797

Closed
lucianobaraglia opened this issue Aug 22, 2013 · 26 comments
Closed

UniqueValidator for multiple columns #797

lucianobaraglia opened this issue Aug 22, 2013 · 26 comments

Comments

@lucianobaraglia
Copy link
Contributor

@lucianobaraglia lucianobaraglia commented Aug 22, 2013

Is there any plan for this feature?

@samdark
Copy link
Member

@samdark samdark commented Aug 22, 2013

I think it's a common requirement that should be in the core framework.

@ghost ghost assigned samdark Aug 22, 2013
@qiangxue
Copy link
Member

@qiangxue qiangxue commented Aug 22, 2013

Can you show some examples? And what would the validator look like when being used in these examples?

@rawtaz
Copy link
Contributor

@rawtaz rawtaz commented Aug 22, 2013

Here are three of the current attempts at this for Yii1, for reference:

http://www.yiiframework.com/extension/composite-unique-key-validatable/
http://www.yiiframework.com/extension/unique-attributes-validator/

I completely agree that this should/must be a part of the core validators, as it is a common thing. For example when having multiple versions of entries in your database (i.e. having not just one column as the PK but two, where the latter is a version column).

@lucianobaraglia
Copy link
Contributor Author

@lucianobaraglia lucianobaraglia commented Aug 22, 2013

Simple example:

class State extends \yii\db\ActiveRecord
{

    public static function tableName()
    {
        return 'states';
    }

    /**
     * Rules
     */ 
    public function rules()
    {
        return array(
            array('country_id, name', 'required'),
            array('country_id, name', 'uniqueTogether'),
        );
    }

In django they use unique_together, so maybe a name like that could be used.

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Aug 22, 2013

The requirement is not well defined.

@lucianobaraglia In your example, do you mean country_id and name should be unique together? If the validation fails, which attribute should take the error? You are also introducing a new syntax where attributes for a validator should be validated together.

@lucianobaraglia
Copy link
Contributor Author

@lucianobaraglia lucianobaraglia commented Aug 22, 2013

@qiangxue I can't find a simpler way, but there are several posibilities talking about syntax.
And about wich attribute should take the error, maybe all of them could do it, because could be hidden inputs or even inputs not rendered at all.

In Symfony, for example, form validation is different from model validation, so there is the concept of "global form errors" and attribute errors.

And I agree, the requirement is poorly defined, sorry, but the problem is very common.

@lucianobaraglia
Copy link
Contributor Author

@lucianobaraglia lucianobaraglia commented Aug 24, 2013

Getting back to the multiple columns uniqueness validation, I've been thinking about how the rule should looks like and thought something like this:

    /**
     * Rules
     */ 
    public function rules()
    {
        return array(
            array('country_id, name', 'required'),
            array('country_id, code', 'uniqueTogether',             
                'attributeError' => 'code', 
                'message' => 'There is already a country using that code.'
            ),
            // Next rule, no attribute error specified, 
            // last listed takes it if validation fails
            array('country_id, name', 'uniqueTogether'),
        );
    }

attributeError refers to the attribute that takes the validation error.
As I said before, I am not stuck with the names used in the example, so uniqueTogether could be changed to anything else, same as attributeError.
I know the syntax is new, but also think it doesn't hurt having this special type of validator, because users will know the difference before using it, keeping in mind that is a validator that validates all the values together.

@lucianobaraglia
Copy link
Contributor Author

@lucianobaraglia lucianobaraglia commented Aug 26, 2013

@cebe that's a nice extension, I've tried it myself and it works great.
The only thing that doesn't convince me is that you can't see the attributes being validated at first view (you have to reach the end of the declaration).
Is just a minimal detail.
And it has the same problem that @qiangxue points out: you can't say wich attribute takes the error.

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Aug 26, 2013

I'm thinking to enhance the existing UniqueValidator to support this. The syntax would be like the following. Hope it covers most use cases:

// a1 needs to be unique
array('a1', 'unique')
// a1 needs to be unique, but its value will use a2 to check for the uniqueness
array('a1', 'unique', 'attributes' => 'a2')
// a1 and a2 need to unique together, and they both will receive error message
array('a1, a2', 'unique', 'attributes' => array('a1', 'a2'))
// a1 and a2 need to unique together, only a1 will receive error message
array('a1', 'unique', 'attributes' => array('a1', 'a2'))
// a1 and a2 need to unique together, a2 will take value 10, only a1 will receive error message
array('a1', 'unique', 'attributes' => array('a1', 'a2' => 10))
@lucianobaraglia
Copy link
Contributor Author

@lucianobaraglia lucianobaraglia commented Aug 26, 2013

@qiangxue I am so glad you are supporting this enhancement!!! 😃
It took me a little to get the new syntax (you had to see me focusing in the code with my nose in the screen).
Still it isn't that readable to me, it will work and that's enough.

The only comment I have to say is that being the same validator, it looks very similar to the current use of it.
Is the only difference the attributes array introduction?

EDIT: Is the only visible difference the attributes array introduction?

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Aug 26, 2013

Yes, it is current attributeName, and I am proposing to renaming it to be attributes and add support for accepting an array to support "unique together" feature.

@lucianobaraglia
Copy link
Contributor Author

@lucianobaraglia lucianobaraglia commented Aug 26, 2013

@qiangxue then it sounds great!!!

@Rupert-RR
Copy link

@Rupert-RR Rupert-RR commented Aug 26, 2013

I'm looking forward to this!

@rawtaz
Copy link
Contributor

@rawtaz rawtaz commented Aug 26, 2013

@qiangxue I think you did a good job here. I was a bit hesitant about using the regular syntax for attributes in the first element of the array, as it can be confusing at first, but as long as that isn't a problem for you technically to keep track of I like how dynamic it is with controlling which attributes get the error.

I think that to keep things simple, the same error message should be used for all the attributes (e.g. "The combination of A1, A2 and A3 already exists"), instead of there being a different sentence for each attribute and the error summary.

@marsuboss
Copy link
Contributor

@marsuboss marsuboss commented Dec 11, 2013

I have a table like this :
id
firstname
lastname

and (firstname, lastname) is unique key.

In model, i add : ['firstname', 'unique', 'attributes' => ['firstname', 'lastname']]

When i insert : firstname: a / lastname: a (record success)
Again insert : firstname: a / lastname: b and i have a message error because has already been taken...

@cebe
Copy link
Member

@cebe cebe commented Dec 11, 2013

@marsuboss yeah, what has been described in this issue is not implemented yet.

@lucianobaraglia
Copy link
Contributor Author

@lucianobaraglia lucianobaraglia commented Dec 11, 2013

I think it's not done yet...

@marsuboss
Copy link
Contributor

@marsuboss marsuboss commented Dec 11, 2013

Oh very sorry... 😅

@aiaiwuai
Copy link

@aiaiwuai aiaiwuai commented Dec 23, 2013

good

@lucianobaraglia
Copy link
Contributor Author

@lucianobaraglia lucianobaraglia commented Dec 23, 2013

Excellent, @qiangxue!!! 😄

Question: are there plans about GII supports adding unique validators out of the box?
I can't see unique keys information in yii table schema classes...

@aiaiwuai
Copy link

@aiaiwuai aiaiwuai commented Dec 23, 2013

the new existvalidatorClass have one statement
// a1 needs to be unique by checking the existence of both a2 and a3 (using a1 value)
['a1', 'unique', 'targetAttribute' => ['a2', 'a1' => 'a3']]
the ‘unique’ should be “exist” and when i test it the rules is
['username', 'exist','message'=>'用户名不存在','targetClass'=>'\common\models\User','targetAttribute'=>['username','username'=>'user_mobile_phone']],
The sql statement is SELECT 1 FROM user WHERE (username='wqs@suangangwang.com') AND (user_mobile_phone='wqs@suangangwang.com')
I think the logic should be ‘or’ instead of ”and“

@qiangxue
Copy link
Member

@qiangxue qiangxue commented Dec 23, 2013

@lucianobaraglia I don't think we will support this for now. It is a good-to-have feature, but not a must. You may help implement this though. ;)

@lucianobaraglia
Copy link
Contributor Author

@lucianobaraglia lucianobaraglia commented Dec 23, 2013

Nice to know!
I will do my best! 😃

@manishpatel1989
Copy link

@manishpatel1989 manishpatel1989 commented Aug 19, 2015

Hello ,

I want unique validation only in insert not in update.
Please help

@cebe
Copy link
Member

@cebe cebe commented Aug 21, 2015

use scenarios. also please use the forum for questions, github is for bug tracking.

@yiisoft yiisoft locked and limited conversation to collaborators Aug 21, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.