Skip to content

Auto-generate Omeka site permissions from Teams role assignments - #199

Draft
alexdryden with Copilot wants to merge 54 commits into
developfrom
copilot/fix-issue-189
Draft

Auto-generate Omeka site permissions from Teams role assignments#199
alexdryden with Copilot wants to merge 54 commits into
developfrom
copilot/fix-issue-189

Conversation

Copilot AI commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

When a user's team role changes or sites are added/removed from a team, Omeka's native SitePermission records are not updated — leaving the two systems out of sync and making Teams non-idempotent if the module is disabled.

Changes

SitePermissionManager service (src/Service/)

New injectable service — following the AclRuleManager pattern — that owns all site-permission sync logic:

  • syncSitePermissionsForUser(userId, teamId) — assigns ROLE_ADMIN if can_add_site_pages, else ROLE_VIEWER, across all team sites
  • removeSitePermissionsForUser(userId, teamId, ?siteId) — removes or recalculates permissions on membership removal; recalculates from remaining team memberships before removing to avoid revoking access granted by a different team
  • syncSitePermissionsForTeamOnSiteAdded / removeSitePermissionsForTeamOnSiteRemoved — fan-out to all team users when the site list changes
  • updateUserDefaultSites / updateAllUserDefaultSites — absorbs Module::updateUserSites and updateAllUserSites

Registered via SitePermissionManagerFactory in module.config.php.

UpdateController — trigger 1: role changes

Constructor now receives SitePermissionManager. addTeamUser, removeTeamUser, and updateRole each call the service after their entity flush. Note: removeSitePermissionsForUser is called before $em->remove($team_user) so the row is still visible during recalculation — the $excludeTeamId parameter skips it explicitly.

UpdateController — triggers 2 & 3: site membership changes

The team-site add/remove block now tracks added/removed site IDs and calls syncSitePermissionsForTeamOnSiteAdded / removeSitePermissionsForTeamOnSiteRemoved after flush.

Module.php

updateUserSites and updateAllUserSites delegate to the service in one line each. siteCreate and siteUpdate call the appropriate sync/remove methods for teams that gained or lost the site.

Copilot AI and others added 2 commits August 10, 2026 20:19
- Create SitePermissionManager service with full site permission sync
  logic for all three trigger scenarios from issue #189
- Create SitePermissionManagerFactory, register in module.config.php
- Inject SitePermissionManager into UpdateController via constructor;
  wire addTeamUser, removeTeamUser, updateRole, and site add/remove
- Delegate Module::updateUserSites and updateAllUserSites to service
- Wire siteCreate and siteUpdate to sync/remove site permissions
- Add docs/service-layer-pattern.md with extended rationale

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Copilot AI changed the title Auto-generate site permissions (issue #189) + SitePermissionManager service layer Auto-generate Omeka site permissions from Teams role assignments Aug 10, 2026
Copilot AI requested a review from alexdryden August 10, 2026 20:22
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Copilot AI and others added 3 commits August 10, 2026 21:36
…ved via teamUpdateAction

- In teamUpdateAction, call syncSitePermissionsForUser after updating an
  existing team user's role (flush first so the new role is persisted) and
  after creating a new team user.
- In teamUpdateAction, call removeSitePermissionsForUser after deleting a
  team user, so the Omeka SitePermission row is cleaned up or recalculated
  from remaining team memberships.
- Restore ROLE_PRIORITY constant in SitePermissionManager (was accidentally
  removed in a previous edit; still needed by resolveHighestRole for the
  removal path).
- The addTeamUser/removeTeamUser/updateRole methods on UpdateController were
  dead code (never called); the sync logic they contained was never reachable
  from the team update form.

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…_VIEWER

The two team role → Omeka site permission mappings are:
- can_add_site_pages = true  → SitePermission::ROLE_MANAGER
- can_add_site_pages = false → SitePermission::ROLE_VIEWER

Updated all four references in SitePermissionManager (ROLE_PRIORITY constant,
docblock, sync path, and removal/recalculation path). Removed ROLE_EDITOR
from ROLE_PRIORITY since Teams only ever assigns viewer or manager.

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…o compare team roles not site roles

- SitePermissionManager: rewrite getHighestRoleFromOtherTeams to compare
  can_add_site_pages booleans across remaining team memberships (not Omeka
  site role strings). Any remaining team with can_add_site_pages=true yields
  ROLE_MANAGER; all false yields ROLE_VIEWER; no team-site relationship yields
  null (permission removed). Remove now-dead resolveHighestRole helper and
  ROLE_PRIORITY constant.

- Module.php: add siteUsersTeamsInfo event handler attached to
  view.edit.after on Omeka\Controller\SiteAdmin\Index. Uses Messenger
  (native Omeka warning system) to display the managed-by-Teams warning.
  Also renders the users-teams-info partial with team attribution data.

- view/teams/partial/site-admin/users-teams-info.phtml: new partial that
  injects team attribution labels into the #site-user-permissions table rows
  via JavaScript, annotating each row with the team(s) that granted the role.
  Warning is now handled by Messenger, not a custom div.

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…iation

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Copilot AI and others added 2 commits August 10, 2026 22:25
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…eflect syncing

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Copilot AI and others added 2 commits August 12, 2026 14:13
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
- Restore SecondaryResourcesForm to extend Form (reverts bad previous session change)
- Restore $secondaryResourcesForm->setData($postData) in UpdateController
- Convert team-update.phtml to thin wrapper delegating to edit-form.phtml partial,
  matching Omeka's edit.phtml pattern (pageTitle, trigger, partial, deleteConfirm, trigger)
- Refactor edit-form.phtml to Omeka form.phtml pattern: explicit formRow for o:name/
  o:description, page-actions at bottom inside form, CSRF rendered explicitly before closeTag,
  sectionNavEvent and action passed as variables
- Fix AddController: add TeamSitesAddRemoveForm, pass sitesForm to ViewModel, replace
  direct-entity site persist with API create call using teamSites[o:site] POST key,
  remove now-unused TeamSite import
- Update team-add.phtml to pass sectionNavEvent and action to shared partial

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…lidate TeamForm

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
This is a known issue with Laminas where certain conditions can
trigger a "Value is required and can't be empty" validation error.
The fix is to explicitly add an allow_empty to the validator.
…ation

- Rename teamSites → team_sites in TeamForm, TeamSitesAddRemoveForm, both
  controllers, and the edit-form view to match snake_case convention used by
  all other fields
- Add allow_empty + required=false for the nested o:site input filter so teams
  can be saved with no sites without a spurious validation error
- Fix swapped str_contains arguments in userAllowedToAssign (haystack/needle
  were reversed for resource-template and media checks)
- Remove history-referencing comments from TeamForm docblocks
- Update copilot instructions with comment/docblock style guidelines

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
… remove dead form classes

The fieldset wrapper caused a naming/nesting mismatch: the element name was
manually set to `team_sites[o:site]` while Laminas' input filter still
operated on the nested fieldset structure, silently dropping the submitted
values. Replacing it with a direct AllSiteSelect element named `team_sites`
makes POST data a flat array of site IDs, consistent with what the
controllers already expected.

Also removed 17 dead form/fieldset classes (TeamSitesFieldset,
TeamSitesAddRemoveForm, SecondaryResourcesForm, TeamResourcesForm,
TeamDetailsForm, TeamFieldset, TeamItemSetFieldset, TeamItemSetForm,
TeamItemsetAddRemoveForm, AddSiteToTeamFieldset, AddSitesToTeam,
AddMemberForm, TeamUserForm, TeamAddUserRole, TeamAddUserRoleFieldset)
and their service factories, and cleaned up module.config.php accordingly.

Fixed setOptions double-call in edit-form.phtml that silently dropped the
label option.

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…fined $entity in TeamResourceAdapter::delete

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Copilot AI and others added 7 commits August 12, 2026 17:19
…amAdapter

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…roller

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…Adapter

All site-permission syncing now fires through the API adapter layer:

- SitePermissionManager: add $flush param to all public sync methods;
  inner loops pass false and do a single flush at the end.
- TeamUserAdapter::create — sync after persist+flush.
- TeamUserAdapter::update — override to sync after parent::update (role changes).
- TeamSiteAdapter::create — sync after persist+flush.
- TeamSiteAdapter::delete — sync after remove+flush.
- TeamAdapter::update — snapshot pre/post user+site state and run targeted
  syncs after parent::update so the TeamAdapter hydrate path is covered.
- UpdateController::teamUpdateAction — remove redundant explicit sync diff block
  and pre-update snapshot (now handled by TeamAdapter::update).
- Module::siteCreate — route through api->create('team-site'); removes manual sync.
- Module::siteUpdate — route through api->create/delete('team-site'); removes manual sync.
- Module::userCreate — route through api->create('team-user'); removes need for
  separate sync.
- Module::userUpdate — route through api->create('team-user'); removes need for
  separate sync.

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…uses highest role, name display injected directly

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
…-side

Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants