Cookiebot should showing for Administrators too

This should provide a solution. Sorry for the mostly bogus diff for the schema.yml, the original had dos newlines.

Wouldn't it be better to do the opposite of what you're trying to do with the patch?

- Create a checkbox "Don't display for users with the admin role"
- Remove the is User 1 check
- Wrap the code similar to like what you're doing with the current patch

Open to discussion of course 😇. I'm not a fan of having user 1 checks.

Me neither. I just wanted to create as little intrusive change as possible, to spare the need for warning users or creating update hooks. I don't really get the point of hardcoding uid 1 there, so if you think, as a maintainer, that refactoring that would be a better approach, I can probably take another look at it later.

I think it makes much more sense to replace the exclude_uid_1 with exclude_administrator_role or even more dynamic that you could select multiple roles. It will indeed take a bit more effort (updating the schema and an update hook to process it) but I think that's the more future proof way to go 😇.

How about this then? I decided to abandon the permission based approach completely and use role based one. This is maybe not an optimal way to do Drupal features, but I do think this is way better than before anyway (thanks for the suggestion!) and the permission based one just can't work for admins. This is clean and straightforward, and the admin can decide the roles which are affected, if any.

Also, the update path should work. Please review.

Fix has been tested and works as intended.

Fixing a missing import. Works even better now.

The last submitted patch, 9: 3396895-cookiebot-for-admins-9.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Patch addressing the very small coding standards errors the tests brought up.

I'm RTBC'ing the last change by @pontus.talvikarhu - maintainer consideration is ofc needed too as I wrote the original patch.

Apparently this is not so rare usecase. +1 for RTBC. Included a related issue that should be closed if this gets fixed.

Looks good to me. Will leave it a RTBC for a bit in case another maintainer wants to chime in. Closed the other issue as well as this one has the fix I suggested there as well.

Patch is rerolled in the merge request

Thanks @nicoschi, decided to merge this since none of the other maintainers objected to it since #15. Will tag a new release shortly.

Automatically closed - issue fixed for 2 weeks with no activity.