search

localgovdrupal / localgov_geo

Entity for storing Geographic information, addresses, and areas; extensible types and plugable backends with defaults to start.
2 stars 1 forks source link

Default permissions and 1.x to 2.x upgrade permissions #95

Closed andybroomfield closed 10 months ago

andybroomfield commented 11 months ago

Following seen during update

[notice] The role Directory Editor has had the following non-existent permission(s) removed: access localgov_geo_library entity browser pages. [notice] The role Event Editor has had the following non-existent permission(s) removed: access localgov_geo_library entity browser pages. [notice] The role Author has had the following non-existent permission(s) removed: access localgov_geo_library entity browser pages. [notice] The role Contributor has had the following non-existent permission(s) removed: access localgov_geo_library entity browser pages. [notice] The role Editor has had the following non-existent permission(s) removed: access localgov_geo_library entity browser pages. [notice] The roles Directory Editor, Event Editor, Author, Contributor, Editor have had non-existent permissions removed. Check the logs for details.

This is easily resolved by setting the correct permissions for the geo_entity browser, someting to add to documentation as I don't think any permissions are set.

ekes commented 11 months ago

So it seems we weren't giving anyone permission to access the entity browser, which is confusing https://github.com/localgovdrupal/localgov_geo/blob/1.x/localgov_geo.module#L109 But we should. So I'd like to extend this a bit.

On a new install I assume we should be giving the permission access geo_entity_library entity browser pages to 'Editor' 'Author' roles. @andybroomfield does that make sense?

Then in addition for upgrading sites we should see if it's possible to load the permissions for all roles. See if access localgov_geo_library entity browser pages is assigned, remove it and replace it with access geo_entity_library entity browser pages.

Adnan-cds commented 11 months ago

Hi Ekes, this came up during manual testing of our site on last Friday. Editors were seeing a permission denied type message in the Geo browser while testing directories. Later I assigned the permissions manually. I thought this issue was specific to our site and so didn't report here!

andybroomfield commented 11 months ago

Yeah, I think a hook_roles on install is appropriate, checking which geo types are installed. And in the localgov_geo_update try to check and update permissions, or add as documentation.

Adnan-cds commented 11 months ago

Sorry, forgot to mention that I manually assigned the access geo_entity_library entity browser pages permission to all the roles that previously had the 'access localgov_geo_library entity browser pages' permission.

finnlewis commented 10 months ago

This is relates to https://www.drupal.org/node/3193348#comment-14969643

But our issue is perhaps more specific and we can fix with a more simple solution.

Tasks:

Write an update hook to find all roles with the permission to replace access localgov_geo_library entity browser pages with access geo_entity_library entity browser pages.

finnlewis commented 10 months ago

@alexburrows did you say you might have time to look at this?

finnlewis commented 10 months ago

Discussing in Merge Monday, we think we'll cover this with docs and release notes.

Release notes should read 👍

"There is a new permission that you will need to grant to your content editor roles: 'access localgov_geo_library entity browser pages'. Please review your permissions after upgrading.

finnlewis commented 10 months ago

We've done this with documentation, but @andybroomfield might look at an update hook or set some default config in localgov_geo.