[Voter] [Normalizer] Turn off generated Voter and Normalizer by default, also fix failed test #416
Conversation
pluseg
changed the title
| // Replace with your own logic | ||
| // See https://symfony.com/doc/current/security/voters.html | ||
| // | ||
| // return in_array($attribute, ['POST_EDIT', 'POST_VIEW']) |
There was a problem hiding this comment.
Maybe replace POST by something else? It means "after" also, this can be confusing.
There was a problem hiding this comment.
Good catch. At the very least, we could change this to BLOG_POST_EDIT... though there is also no rule that says that things need to be upper case...
If we add a question about the entity/class, then we would derive this from the entity class name.
| // return in_array($attribute, ['POST_EDIT', 'POST_VIEW']) | ||
| // && $subject instanceof \App\Entity\YourEntity; | ||
|
|
||
| return false; |
There was a problem hiding this comment.
I'm not sure this is better: the now commented line were fine uncommented, no? Because there is this line to remove now, this might confuse some.
| // Replace with your own logic | ||
| // See https://symfony.com/doc/current/serializer/custom_normalizer.html | ||
| // | ||
| // return $data instanceof \App\Entity\YourEntity; |
There was a problem hiding this comment.
Same note as above, it'd make sense uncommenting this to me.
There was a problem hiding this comment.
Totally agree, That's what I said in the previous PR.
There was a problem hiding this comment.
Hmm. I agree. The current example code seems pretty clear, and it's better than needing to know to remove that return false. I think we should leave as-is or add a second question to each maker:
What class will this voter/normalizer handle? (e.g. BlogPost or leave blank)
(if a non-FQN were passed, we'd look for it in Entity). If a class is passed, we use that in the generated code. If none is passed, we leave the class-checking part off of the voter and, for the normalizer, probably put a big TODO in supportsNormalization().
WDYT?
This PR was merged into the 1.0-dev branch. Discussion ---------- #395 - guess entity name in normalizer template Hi, To fix #395, I propose to do like in #658. _This is only for Normalizer, because Voter was already fixed in https://github.com/symfony/maker-bundle/pull/658_ This intends to replace : #416 Commits ------- d2dc12a #395 - guess entity name in normalizer template
|
Feature merge in #672, this one should be closed |
jrushlow
added
Feature
|
Thank you @pluseg for the work on this! It looks like this fix has been merged in a separate PR. |
This PR was merged into the 1.0-dev branch. Discussion ---------- #395 - guess entity name in normalizer template Hi, To fix symfony/maker-bundle#395, I propose to do like in symfony/maker-bundle#658. _This is only for Normalizer, because Voter was already fixed in https://github.com/symfony/maker-bundle/pull/658_ This intends to replace : symfony/maker-bundle#416 Commits ------- d2dc12a #395 - guess entity name in normalizer template
6 participants
It fixes #395.
There are hardcoded non-existent classes for Voter and Normalizer.
Also, it fixes one failed test of Validator.
@lyrixx Sorry, I had to recreate the previous PR — #414.