Unique option not working with templates (issue #291) #359
Conversation
| @@ -0,0 +1,6 @@ | |||
| \Nelmio\Alice\support\models\User: | |||
There was a problem hiding this comment.
I think we need a more complete test for that one, e.g. add another non unique field to the template. It would be also good to have another example with two level deep, i.e.
- template1: 2 fields with unique, 1 without
- template2: extends template 1, override 1 unique field to make it not unique, add another unique field
- 3: extends template 1
Don't hesitate to create dedicated entities for it if needed
|
Again thanks a lot for looking into it! It's definitely an issue that I don't think I would have had the time to look before a while! |
|
I will setup more tests and will change the implementation also so that it will be more flexible for other extensions. |
Don't over do it though, v3 is coming out ;) |
|
good advice for the tests because I found another bug within template inheritance. |
|
@reinfi what is the status of the PR? Is there any work left or is it ready for review? |
|
it is rady for review |
| return str_replace('extends ', '', $extension); | ||
| }, | ||
| $extensions | ||
| return array_reverse( |
There was a problem hiding this comment.
why the array_reverse? This will mess up the extending order which can be very problematic
There was a problem hiding this comment.
I removed it, I had no clue why I added it.
|
@reinfi looks like tests are failing. I guess that's what the |
|
You might want to rebase as well, no big change though :) |
PR fixes #291