Problem/Motivation
Please refer the problem/motivation section of #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method
Proposed resolution
Write EntityResourceTestBase subclass for the ShortcutSet entity.
Unfortunately, ShortcutSet does not have an access control handler for 'view'
operation, so that needs to be added too. Fortunately, ShortcutSet and Shortcut entities are closely related. And Shortcut has 'access shortcuts' fot 'view' operation. So, the logic use it for ShortcutSet too!
Unfortunately, ShortcutSet has 'customize-form' link relation types, but drupal core hasn't info about this type, sot that needs to be added too. We can use RFC6861 for this, like edit form.
Remaining tasks
References
1. Follow-up of #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method
2. Subtask of #2824572: Write EntityResourceTestBase subclasses for every other entity type.
Comment | File | Size | Author |
---|---|---|---|
#13 | interdiff.txt | 862 bytes | Anonymous (not verified) |
#13 | entityresource_provide-2843779-13.patch | 8.82 KB | Anonymous (not verified) |
Comments
Comment #2
sumanthkumarc CreditAttribution: sumanthkumarc at Azri Solutions commentedComment #3
sumanthkumarc CreditAttribution: sumanthkumarc at Azri Solutions commentedComment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedThe patch contains two additional changes:
1.
Without this change we have problem here:
2.
Without this change we have problem here:
Comment #6
sumanthkumarc CreditAttribution: sumanthkumarc at Azri Solutions commented@vaplas , thanks for working on issue. You can now assign yourself.
Comment #7
Wim LeersSupernit: needs blank line between these.
ShortcutSet
If it was only the first point, I'd have RTBC'd. But with that second point, it can't go in as-is. So, marking NW just for that. Easy fix! :)
Comment #8
yogeshmpawarComment #9
yogeshmpawarUpdated patch as per comment #7 & also added interdiff.
Comment #10
Wim LeersThanks!
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commentedThank you @Wim Leers for review and @Yogesh Pawar for help! A small additional trivial cleanup. (I was so happy that the tests passed, that I sent the patch faster than necessary, sorry for this).
Comment #12
Wim LeersHah :)
Nice additional clean-up!
Comment #13
Anonymous (not verified) CreditAttribution: Anonymous commentedAdded @todo in
getNormalizedPostEntity
, by analogy with the otherconfig
cases. Please, point me this is right change or we need some other change? (sorry for the eternal flaws)Comment #14
Wim LeersEven better :)
Comment #16
Anonymous (not verified) CreditAttribution: Anonymous commentedRandom fail.
Comment #18
Wim LeersDrupalCI infra fail.
Comment #19
catchCommitted 3d292b3 and pushed to 8.4.x. Thanks!
Moving to 8.3.x for cherry-pick after the commit freeze.
Fixed this on commit:
s/IRI/URI/
Comment #21
alexpottIn order for this to be backported with need an issue summary update to clarify what's being fixed on this issue. Since it went beyond just providing tests and into the land of fixing access control.
Comment #22
Anonymous (not verified) CreditAttribution: Anonymous commented#19: I'm not sure about s/IRI/URI/. Looks like IRI - Internationalised Resource Identifier. But maybe URI makes sense too.
This is copy from
because it have similar behavior, which looks like in the scope of the RFC6861 specification.
#21: Tried to do it.
Comment #23
Wim LeersThanks, @vaplas!
Comment #25
Gábor HojtsyLooks good to me for a cherry-pick to 8.3.x as per #19. The view permission on shortcut sets was neutral before and is now tied to the access shortcuts permission which makes total sense.