Follow-up of #3017935: Media items should not be available at /media/{id} to users with 'view media' permission by default .
As pointed out in my comment, the update being a post update means that it is not possible for distributions/install profiles to provide an update function that will set it to FALSE by default.
The post update function should check that the value is still NULL before setting it, that allows to have a different update or post update that sets the value differently.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | interdiff-3036787-5-7.txt | 651 bytes | johnchque |
| #7 | media_post_update_true-3036787-7.patch | 693 bytes | johnchque |
| #5 | interdiff-3036787-2-5.txt | 568 bytes | johnchque |
| #5 | media_post_update_true-3036787-5.patch | 722 bytes | johnchque |
| #2 | media_post_update_true-3036787-2.patch | 721 bytes | johnchque |
Comments
Comment #2
johnchqueOuch! Maybe we should consider to changing to a normal update function. Anyway, update the post update already existing.
Comment #5
johnchqueMy bad, this should work instead.
Comment #6
berdirI would suggest we first get a $config object with getEditable() and then use that same object to check the value and set it, should be easier to read.
Comment #7
johnchqueThat's true, reads way better now. :)
Comment #8
berdirThanks. I think test coverage for this seems a bit overkill.
Still not sure why this is a post update function, changing plain config in a regular update is OK. If it would be then we wouldn't need this and could simply make sure our own update runs after that.
Comment #9
alexpottI think this is a bug and given it is update related I think we should get this in before 8.7.x. I agree that test coverage is OTT.
Committed and pushed be7966d8e3 to 8.8.x and d0aab49b3a to 8.7.x. Thanks!