From ed8cfe606c09e37ed3408ead3c8c14c9280af6fe Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 20:19:46 +0000
Subject: [PATCH 01/54] Add SitePermissionManager service and wire issue #189
triggers
- 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>
---
Module.php | 45 ++-
config/module.config.php | 1 +
docs/service-layer-pattern.md | 271 +++++++++++++++++
src/Controller/UpdateController.php | 30 +-
src/Service/SitePermissionManager.php | 288 +++++++++++++++++++
src/Service/SitePermissionManagerFactory.php | 16 ++
src/Service/UpdateControllerFactory.php | 6 +-
7 files changed, 627 insertions(+), 30 deletions(-)
create mode 100644 docs/service-layer-pattern.md
create mode 100644 src/Service/SitePermissionManager.php
create mode 100644 src/Service/SitePermissionManagerFactory.php
diff --git a/Module.php b/Module.php
index e4564db9..257bbda9 100644
--- a/Module.php
+++ b/Module.php
@@ -29,6 +29,7 @@
use Teams\Form\Element\BlankTeamSelect;
use Teams\Form\Element\RoleSelect;
use Teams\Form\Element\TeamSelect;
+use Teams\Service\SitePermissionManager;
use Omeka\Api\Adapter\ItemAdapter;
use Omeka\Api\Adapter\ItemSetAdapter;
use Omeka\Api\Adapter\MediaAdapter;
@@ -194,12 +195,7 @@ public function upgrade($oldVersion, $newVersion, ServiceLocatorInterface $servi
public function updateAllUserSites()
{
- $em = $this->getServiceLocator()->get('Omeka\EntityManager');
- $active_users = $em->getRepository('Teams\Entity\TeamUser')->findAllBy(['is_active' => true]);
-
- foreach ($active_users as $user) {
- $this->updateUserSites($user->getUser()->getId());
- }
+ $this->getServiceLocator()->get(SitePermissionManager::class)->updateAllUserDefaultSites();
}
public function handleConfigForm(AbstractController $controller)
@@ -1039,27 +1035,7 @@ public function updateItemSites($item_id)
public function updateUserSites($user_id)
{
- $em = $this->getServiceLocator()->get('Omeka\EntityManager');
-
- $userSettings = $this->getServiceLocator()->get('Omeka\Settings\User');
-
- $site_ids = [];
- $settingId = 'default_item_sites';
-
- $active_team = $em->getRepository('Teams\Entity\TeamUser')
- ->findOneBy(['user' => $user_id, 'is_current' => true]);
- if ($active_team) {
- $active_team = $active_team->getTeam();
-
- $team_sites = $active_team->getTeamSites();
-
- foreach ($team_sites as $team_site):
- $site_ids[] = $team_site->getSite()->getId();
- endforeach;
-
- //update default sites
- $userSettings->set($settingId, $site_ids, $user_id);
- }
+ $this->getServiceLocator()->get(SitePermissionManager::class)->updateUserDefaultSites($user_id);
}
/**
@@ -1183,6 +1159,12 @@ public function siteCreate(Event $event)
endforeach;
endforeach;
+ // Sync Omeka site permissions for all users in each new team-site relationship.
+ $sitePermissionManager = $this->getServiceLocator()->get(SitePermissionManager::class);
+ foreach ($team_ids as $team_id):
+ $sitePermissionManager->syncSitePermissionsForTeamOnSiteAdded((int)$team_id, $site_id);
+ endforeach;
+
//update all item-site to include all items from the site's teams
$siteAdapter = $this->getServiceLocator()->get('Omeka\ApiAdapterManager')->get('sites');
foreach ($all_team_resources as $team_resources):
@@ -1445,6 +1427,15 @@ public function siteUpdate(Event $event)
$this->updateItemSites($team_item->getResource()->getId());
}
}
+
+ // Sync Omeka site permissions for teams that gained or lost this site.
+ $sitePermissionManager = $this->getServiceLocator()->get(SitePermissionManager::class);
+ foreach ($added_teams as $team_id) {
+ $sitePermissionManager->syncSitePermissionsForTeamOnSiteAdded((int)$team_id, $site_id);
+ }
+ foreach ($removed_teams as $team_id) {
+ $sitePermissionManager->removeSitePermissionsForTeamOnSiteRemoved((int)$team_id, $site_id);
+ }
}
}
}
diff --git a/config/module.config.php b/config/module.config.php
index f091c5f1..9e9b0b43 100644
--- a/config/module.config.php
+++ b/config/module.config.php
@@ -78,6 +78,7 @@
'factories' => [
\Teams\Acl\TeamRolePermissionAssertion::class => \Teams\Service\Acl\TeamRolePermissionAssertionFactory::class,
\Teams\Service\AclRuleManager::class => \Teams\Service\AclRuleManagerFactory::class,
+ \Teams\Service\SitePermissionManager::class => \Teams\Service\SitePermissionManagerFactory::class,
],
],
'form_elements' => [
diff --git a/docs/service-layer-pattern.md b/docs/service-layer-pattern.md
new file mode 100644
index 00000000..ac18d0fc
--- /dev/null
+++ b/docs/service-layer-pattern.md
@@ -0,0 +1,271 @@
+# Service Layer Pattern — Rationale and Guide
+
+## The Problem: A 2,778-Line Monolith
+
+`Module.php` is the heart of the Teams module. Every Omeka S module has one, and for
+small modules that is fine. Teams is not a small module. At the time this document was
+written, `Module.php` contained approximately **60 public methods** and nearly
+**2,800 lines** in a single class. The methods covered:
+
+- ACL rule wiring
+- Query filtering
+- Event-driven entity sync (items, sites, users, assets, resource templates)
+- View helpers and form injection
+- Configuration handling
+- Admin UI display logic
+
+When everything lives in one place, the following problems compound over time:
+
+1. **No single concern is traceable.** To understand how site permissions work, a reader
+ must grep for scattered method calls and follow a chain through `Module.php`,
+ `UpdateController.php`, and form events — none of which have a shared namespace or
+ naming convention.
+
+2. **Logic cannot be reused without copy-paste.** Because business logic is embedded in
+ event-handler methods (which receive raw `Event` objects), it cannot be called from
+ another entry point (e.g., a CLI command, a background job, or a second controller)
+ without duplicating the code.
+
+3. **Testing is impossible in practice.** An event handler that calls
+ `$this->getServiceLocator()` is tightly coupled to the Laminas service container.
+ There is no way to instantiate just the relevant behaviour for a unit test without
+ bootstrapping the entire framework.
+
+4. **The class grows without bound.** Every new feature becomes a new method on
+ `Module.php` because there is no obvious alternative home for it. This is the
+ definition of the *Big Ball of Mud* anti-pattern.
+
+---
+
+## Why Service Layer Is the Right Pattern Here
+
+### What it is
+
+The **Service Layer** pattern (Fowler, *Patterns of Enterprise Application Architecture*,
+2002) draws a boundary between the application's *entry points* (HTTP controllers, event
+handlers, CLI commands) and its *domain logic* (business rules, entity manipulation,
+cross-cutting sync). Domain logic is placed in dedicated **service classes** that are
+injected into entry points as dependencies.
+
+In Laminas/Omeka S this is spelled out concretely:
+
+```
+Entry point Service Layer Domain / ORM
+───────────────── ──────────────────────── ──────────────────
+Module.php SitePermissionManager Doctrine EntityManager
+UpdateController AclRuleManager Omeka\Entity\*
+ (future) ItemSyncManager Teams\Entity\*
+```
+
+### Why not one of the alternatives?
+
+**Repository pattern alone** — Repositories handle query building, not business rules.
+They are the right home for "give me all TeamUsers for team X" but not for "given a
+TeamUser change, update the matching SitePermissions". Mixing rules into repositories
+produces the same coupling problem in a different class.
+
+**Fat controller** — Putting logic in controllers makes it unreachable from event
+handlers, and vice versa. The existing code already shows the pain: `updateUserSites`
+is defined on `Module.php` and called from both `siteCreate` and `siteUpdate` purely
+because controllers cannot directly call `Module` methods.
+
+**Traits on Module** — PHP traits are a copy-paste mechanism, not a boundary. A trait
+on `Module.php` still has access to `$this->getServiceLocator()`, still cannot be
+constructed independently, and still cannot be tested in isolation.
+
+**Doctrine lifecycle listeners** — These are appropriate for generic, entity-level
+concerns (timestamps, soft-delete flags). They are inappropriate for business logic
+that is specific to the Teams module, because they introduce a hidden dependency
+between the ORM layer and module-specific rules that would survive even if the module
+were disabled.
+
+**Service Layer** fits because:
+
+- Services receive their dependencies through the **constructor**, not through a global
+ service locator. This is standard Laminas Dependency Injection and makes every
+ dependency explicit and mockable.
+- A service class has a **single, nameable concern**. `SitePermissionManager` does
+ exactly one thing: keep Omeka site user permissions in sync with Teams role
+ assignments. A reader can open the file and understand its purpose in two minutes.
+- The same service can be **called from any entry point**: an event handler in
+ `Module.php`, a controller action, a background job, a future REST endpoint, or a
+ unit test.
+- Services are registered in the **Laminas service manager** via factory classes.
+ This is the established Omeka S convention for injectable objects — the same mechanism
+ used for API adapters, form elements, and authentication — so no new conventions need
+ to be introduced or learned.
+
+### Relationship to Omeka S conventions
+
+Omeka S itself follows this pattern. The core `application/Module.php` is thin; heavy
+logic lives in dedicated classes (`SiteAdapter`, `ItemAdapter`, `Acl`, etc.). Official
+Omeka S modules such as [CSVImport] and [BulkImport] extract complex processing into
+service classes registered through factories. Adopting the same pattern keeps the Teams
+module aligned with the ecosystem and makes it easier for Omeka-familiar developers to
+contribute.
+
+---
+
+## The Concrete Example: `SitePermissionManager`
+
+### Before
+
+`Module.php` contained `updateUserSites()`, a method that:
+- called `$this->getServiceLocator()` to fetch `Omeka\EntityManager` and
+ `Omeka\Settings\User` — two separate service-locator calls buried inside the method
+ body
+- was duplicated at three call sites inside `Module.php` and implicitly depended on
+ those call sites knowing to call `Module::updateUserSites()` instead of having
+ access to the logic directly
+- could not be called from `UpdateController` without going back through the module
+ event system
+
+The new feature (issue #189 — auto-generate site permissions) would have required
+adding *more* methods to `Module.php` with the same problems, and wiring them from
+`UpdateController` was impossible without adding a second copy of the same code.
+
+### After: `Teams\Service\SitePermissionManager`
+
+`SitePermissionManager` is a plain PHP class:
+
+```php
+class SitePermissionManager
+{
+ public function __construct(
+ EntityManager $entityManager,
+ UserSettings $userSettings
+ ) { … }
+
+ public function syncSitePermissionsForUser(int $userId, int $teamId): void { … }
+ public function removeSitePermissionsForUser(int $userId, int $teamId, ?int $siteId = null): void { … }
+ public function syncSitePermissionsForTeamOnSiteAdded(int $teamId, int $siteId): void { … }
+ public function removeSitePermissionsForTeamOnSiteRemoved(int $teamId, int $siteId): void { … }
+ public function updateUserDefaultSites(int $userId): void { … }
+ public function updateAllUserDefaultSites(): void { … }
+}
+```
+
+`SitePermissionManagerFactory` creates it:
+
+```php
+class SitePermissionManagerFactory implements FactoryInterface
+{
+ public function __invoke(ContainerInterface $container, …)
+ {
+ return new SitePermissionManager(
+ $container->get('Omeka\EntityManager'),
+ $container->get('Omeka\Settings\User')
+ );
+ }
+}
+```
+
+`config/module.config.php` registers it like any other Laminas service:
+
+```php
+'service_manager' => [
+ 'factories' => [
+ SitePermissionManager::class => SitePermissionManagerFactory::class,
+ ],
+],
+```
+
+**`Module.php`** now delegates in one line:
+
+```php
+public function updateUserSites($user_id)
+{
+ $this->getServiceLocator()->get(SitePermissionManager::class)
+ ->updateUserDefaultSites($user_id);
+}
+```
+
+**`UpdateController`** receives the service through its constructor:
+
+```php
+public function __construct(
+ EntityManager $entityManager,
+ SitePermissionManager $sitePermissionManager
+) { … }
+```
+
+And calls it directly after mutating team membership:
+
+```php
+$this->sitePermissionManager->syncSitePermissionsForUser($user_id, $team_id);
+```
+
+---
+
+## Design Decisions within `SitePermissionManager`
+
+### Multi-team safety
+
+A user can belong to multiple teams that share an Omeka site. Naively assigning the
+role dictated by one team could silently downgrade a privilege granted by another team.
+`SitePermissionManager` uses `resolveHighestRole()` to ensure that the role stored on
+the `SitePermission` entity is always the *highest* role granted by any of the user's
+current team memberships. When a membership is removed, `getHighestRoleFromOtherTeams()`
+recalculates from the remaining memberships before deciding whether to downgrade or
+remove the `SitePermission` entirely.
+
+### Role mapping
+
+Omeka S defines three site-level roles: `viewer`, `editor`, `admin`. The Teams module
+maps:
+
+| TeamRole condition | Omeka site role |
+|------------------------------|------------------------------|
+| `can_add_site_pages = true` | `SitePermission::ROLE_ADMIN` |
+| `can_add_site_pages = false` | `SitePermission::ROLE_VIEWER`|
+
+`ROLE_EDITOR` is not currently produced by the Teams module because TeamRole does not
+have a finer-grained "can edit but not administer" concept. If that is added later,
+only `SitePermissionManager` needs to be changed.
+
+### Flush discipline
+
+Each public method calls `$em->flush()` once at the end, after all entity mutations
+for that operation are complete. This avoids partial writes and reduces round trips.
+The exception is `updateAllUserDefaultSites`, which delegates to `updateUserDefaultSites`
+per user; the settings API (`UserSettings::set`) has its own persistence and does not
+require explicit flushes.
+
+---
+
+## The Broader Roadmap
+
+`SitePermissionManager` and `AclRuleManager` are examples of the pattern. They are
+not the finish line. The same extraction should be applied, incrementally, to other
+cohesive groups of methods currently living in `Module.php`:
+
+| Candidate service | Methods to extract |
+|------------------------------|------------------------------------------------------------------------------|
+| `ItemSyncManager` | `updateItemSites`, `itemCreate`, `itemUpdate`, `itemDelete`, `itemBatchCreate`|
+| `TeamQueryFilter` | `filterByTeam`, `getTeamContext`, `getOrphans` |
+| `ResourceTemplateSyncManager`| `resourceTemplateCreate`, `resourceTemplateUpdate` |
+| `AssetSyncManager` | `assetCreate`, `assetUpdate` |
+
+Each extraction follows the same four-step recipe:
+
+1. Create a service class in `src/Service/` with constructor-injected dependencies.
+2. Create a factory in `src/Service/` that pulls those dependencies from the container.
+3. Register the factory in `config/module.config.php`.
+4. Replace the body of the `Module.php` method(s) with a single-line delegation call.
+
+No existing public API or event wiring needs to change. `Module.php` retains its
+methods as thin delegators, so callers (including third-party modules that may be
+listening to Teams events) are unaffected.
+
+---
+
+## Summary
+
+| Concern | Before | After |
+|----------------------|--------------------------------|--------------------------------|
+| Where logic lives | `Module.php` method bodies | Named service class |
+| Dependencies | `$this->getServiceLocator()` | Constructor injection |
+| Reuse across entry points | Copy-paste or impossible | Call the service directly |
+| Testability | Requires full framework boot | Plain PHP instantiation |
+| Discoverability | Grep for scattered methods | Open the service class |
+| Aligned with Omeka S | Partially | Fully (same factory pattern) |
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index b4248ad6..0df81366 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -14,6 +14,7 @@
use Teams\Form\TeamResourcesForm;
use Teams\Form\TeamSitesAddRemoveForm;
use Teams\Form\TeamDetailsForm;
+use Teams\Service\SitePermissionManager;
use Laminas\EventManager\Event;
use Laminas\Mvc\Controller\AbstractActionController;
use Laminas\Stdlib\ArrayObject;
@@ -27,12 +28,19 @@ class UpdateController extends AbstractActionController
*/
protected $entityManager;
+ /**
+ * @var SitePermissionManager
+ */
+ protected SitePermissionManager $sitePermissionManager;
+
/**
* @param EntityManager $entityManager
+ * @param SitePermissionManager $sitePermissionManager
*/
- public function __construct(EntityManager $entityManager)
+ public function __construct(EntityManager $entityManager, SitePermissionManager $sitePermissionManager)
{
$this->entityManager = $entityManager;
+ $this->sitePermissionManager = $sitePermissionManager;
}
public function createNamedParameter(
@@ -62,6 +70,9 @@ public function addTeamUser(int $team_id, int $user_id, int $role_id)
//flushing here because this is a mini-form and we want to see the name pop up
//more efficient solution would be to have JS handle the popping and batch update
$this->entityManager->flush();
+
+ $this->sitePermissionManager->syncSitePermissionsForUser($user_id, $team_id);
+
return $team_user;
}
}
@@ -74,6 +85,9 @@ public function removeTeamUser(int $team_id, int $user)
$this->messenger()->addError("removed user");
$em = $this->entityManager;
+
+ $this->sitePermissionManager->removeSitePermissionsForUser($user, $team_id);
+
$team_user = $em->find('Teams\Entity\TeamUser', ['team' => $team_id, 'user' => $user]);
$em->remove($team_user);
@@ -93,6 +107,8 @@ public function updateRole(int $team_id, int $user_id, int $role_id)
$user_role = $em->find('Teams\Entity\TeamRole', $role_id);
$team_user->setRole($user_role);
$em->flush();
+
+ $this->sitePermissionManager->syncSitePermissionsForUser($user_id, $team_id);
}
}
@@ -382,8 +398,10 @@ public function teamUpdateAction()
$em = $this->entityManager;
//handle new sites
+ $added_site_ids = [];
foreach ($post_data['teamSites']['o:site'] as $site) {
if (!in_array($site, $current_sites)) {
+ $added_site_ids[] = (int)$site;
$site = $em->getRepository('Omeka\Entity\Site')->findOneBy(['id'=>$site]);
$ts = new TeamSite($team, $site);
$request = new Request('create', 'team_site');
@@ -398,8 +416,10 @@ public function teamUpdateAction()
}
//handle removed sites
+ $removed_site_ids = [];
foreach ($current_sites as $site) {
if (!in_array($site, $post_data['teamSites']['o:site'])) {
+ $removed_site_ids[] = (int)$site;
$ts = $em->getRepository('Teams\Entity\TeamSite')->findOneBy(['team'=>$team_id, 'site'=>$site]);
$request = new Request('delete', 'team_site');
$event = new Event('api.hydrate.pre', $this, [
@@ -411,6 +431,14 @@ public function teamUpdateAction()
}
}
$em->flush();
+
+ // Sync Omeka site permissions for all team users when sites are added or removed.
+ foreach ($added_site_ids as $added_site_id) {
+ $this->sitePermissionManager->syncSitePermissionsForTeamOnSiteAdded($team_id, $added_site_id);
+ }
+ foreach ($removed_site_ids as $removed_site_id) {
+ $this->sitePermissionManager->removeSitePermissionsForTeamOnSiteRemoved($team_id, $removed_site_id);
+ }
}
$successMessage = sprintf("Successfully updated the %s team", $team->getName());
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
new file mode 100644
index 00000000..8d086e23
--- /dev/null
+++ b/src/Service/SitePermissionManager.php
@@ -0,0 +1,288 @@
+ 2,
+ SitePermission::ROLE_EDITOR => 1,
+ SitePermission::ROLE_VIEWER => 0,
+ ];
+
+ /**
+ * @var EntityManager
+ */
+ private EntityManager $entityManager;
+
+ /**
+ * @var UserSettings
+ */
+ private UserSettings $userSettings;
+
+ public function __construct(EntityManager $entityManager, UserSettings $userSettings)
+ {
+ $this->entityManager = $entityManager;
+ $this->userSettings = $userSettings;
+ }
+
+ /**
+ * Sync Omeka site permissions for a user in a specific team.
+ *
+ * Assigns ROLE_ADMIN if the team role has `can_add_site_pages`, otherwise ROLE_VIEWER,
+ * for every site associated with the team.
+ *
+ * When the user already has a permission on a site (e.g. from another team), the
+ * higher of the two roles is kept, so no privilege is silently downgraded.
+ *
+ * @param int $userId
+ * @param int $teamId
+ */
+ public function syncSitePermissionsForUser(int $userId, int $teamId): void
+ {
+ $em = $this->entityManager;
+
+ $teamUser = $em->find('Teams\Entity\TeamUser', ['team' => $teamId, 'user' => $userId]);
+ if (!$teamUser) {
+ return;
+ }
+
+ $user = $teamUser->getUser();
+ $omekaRole = $teamUser->getRole()->getCanAddSitePages()
+ ? SitePermission::ROLE_ADMIN
+ : SitePermission::ROLE_VIEWER;
+
+ $teamSites = $em->getRepository('Teams\Entity\TeamSite')->findBy(['team' => $teamId]);
+
+ foreach ($teamSites as $teamSite) {
+ $site = $teamSite->getSite();
+ $sitePermissions = $site->getSitePermissions();
+
+ $criteria = Criteria::create()->where(Criteria::expr()->eq('user', $user));
+ $existingPermission = $sitePermissions->matching($criteria)->first();
+
+ if ($existingPermission) {
+ // Keep the higher of the existing and new role (multi-team safety).
+ $existingPermission->setRole(
+ $this->resolveHighestRole($existingPermission->getRole(), $omekaRole)
+ );
+ } else {
+ $sitePermission = new SitePermission();
+ $sitePermission->setSite($site);
+ $sitePermission->setUser($user);
+ $sitePermission->setRole($omekaRole);
+ $em->persist($sitePermission);
+ $sitePermissions->add($sitePermission);
+ }
+ }
+
+ $em->flush();
+ }
+
+ /**
+ * Remove or recalculate Omeka site permissions for a user leaving a team.
+ *
+ * If the user still belongs to other teams that share a given site, their
+ * permission is recalculated from those remaining memberships rather than
+ * simply removed, so a shared-site privilege is never accidentally revoked.
+ *
+ * @param int $userId
+ * @param int $teamId The team the user is being removed from.
+ * @param int|null $siteId If set, only process this one site (used when a
+ * single site is removed from a team).
+ */
+ public function removeSitePermissionsForUser(int $userId, int $teamId, ?int $siteId = null): void
+ {
+ $em = $this->entityManager;
+
+ $user = $em->find('Omeka\Entity\User', $userId);
+ if (!$user) {
+ return;
+ }
+
+ $criteria = ['team' => $teamId];
+ if ($siteId !== null) {
+ $criteria['site'] = $siteId;
+ }
+ $teamSites = $em->getRepository('Teams\Entity\TeamSite')->findBy($criteria);
+
+ foreach ($teamSites as $teamSite) {
+ $site = $teamSite->getSite();
+ $sitePermissions = $site->getSitePermissions();
+
+ $userCriteria = Criteria::create()->where(Criteria::expr()->eq('user', $user));
+ $existingPermission = $sitePermissions->matching($userCriteria)->first();
+
+ if (!$existingPermission) {
+ continue;
+ }
+
+ // Check whether any other team still grants this user access to this site.
+ $remainingRole = $this->getHighestRoleFromOtherTeams($userId, $teamId, $site->getId());
+
+ if ($remainingRole !== null) {
+ // User retains access via another team — update the role accordingly.
+ $existingPermission->setRole($remainingRole);
+ } else {
+ // No other team grants access; remove the permission entirely.
+ $sitePermissions->removeElement($existingPermission);
+ $em->remove($existingPermission);
+ }
+ }
+
+ $em->flush();
+ }
+
+ /**
+ * Sync Omeka site permissions for all users in a team when a site is added.
+ *
+ * @param int $teamId
+ * @param int $siteId
+ */
+ public function syncSitePermissionsForTeamOnSiteAdded(int $teamId, int $siteId): void
+ {
+ $teamUsers = $this->entityManager
+ ->getRepository('Teams\Entity\TeamUser')
+ ->findBy(['team' => $teamId]);
+
+ foreach ($teamUsers as $teamUser) {
+ $this->syncSitePermissionsForUser($teamUser->getUser()->getId(), $teamId);
+ }
+ }
+
+ /**
+ * Remove Omeka site permissions for all users in a team when a site is removed.
+ *
+ * @param int $teamId
+ * @param int $siteId
+ */
+ public function removeSitePermissionsForTeamOnSiteRemoved(int $teamId, int $siteId): void
+ {
+ $teamUsers = $this->entityManager
+ ->getRepository('Teams\Entity\TeamUser')
+ ->findBy(['team' => $teamId]);
+
+ foreach ($teamUsers as $teamUser) {
+ $this->removeSitePermissionsForUser($teamUser->getUser()->getId(), $teamId, $siteId);
+ }
+ }
+
+ /**
+ * Update the `default_item_sites` user setting so the user's item-creation form
+ * defaults to the sites of their current active team.
+ *
+ * Extracted from Module::updateUserSites() to follow the service-layer pattern.
+ *
+ * @param int $userId
+ */
+ public function updateUserDefaultSites(int $userId): void
+ {
+ $activeTeam = $this->entityManager
+ ->getRepository('Teams\Entity\TeamUser')
+ ->findOneBy(['user' => $userId, 'is_current' => true]);
+
+ if (!$activeTeam) {
+ return;
+ }
+
+ $siteIds = [];
+ foreach ($activeTeam->getTeam()->getTeamSites() as $teamSite) {
+ $siteIds[] = $teamSite->getSite()->getId();
+ }
+
+ $this->userSettings->set('default_item_sites', $siteIds, $userId);
+ }
+
+ /**
+ * Update `default_item_sites` for all users who have an active team.
+ *
+ * Extracted from Module::updateAllUserSites().
+ */
+ public function updateAllUserDefaultSites(): void
+ {
+ $activeTeamUsers = $this->entityManager
+ ->getRepository('Teams\Entity\TeamUser')
+ ->findBy(['is_current' => true]);
+
+ foreach ($activeTeamUsers as $teamUser) {
+ $this->updateUserDefaultSites($teamUser->getUser()->getId());
+ }
+ }
+
+ /**
+ * Determine the highest Omeka site role granted to a user by any team *other than*
+ * the one being removed. Returns null if no other team grants access to that site.
+ *
+ * @param int $userId
+ * @param int $excludeTeamId The team being removed, which should be ignored.
+ * @param int $siteId
+ * @return string|null
+ */
+ private function getHighestRoleFromOtherTeams(int $userId, int $excludeTeamId, int $siteId): ?string
+ {
+ $em = $this->entityManager;
+ $teamUsers = $em->getRepository('Teams\Entity\TeamUser')->findBy(['user' => $userId]);
+
+ $highestRole = null;
+ foreach ($teamUsers as $teamUser) {
+ if ($teamUser->getTeam()->getId() === $excludeTeamId) {
+ continue;
+ }
+
+ $teamSite = $em->getRepository('Teams\Entity\TeamSite')
+ ->findOneBy(['team' => $teamUser->getTeam()->getId(), 'site' => $siteId]);
+
+ if (!$teamSite) {
+ continue;
+ }
+
+ $role = $teamUser->getRole()->getCanAddSitePages()
+ ? SitePermission::ROLE_ADMIN
+ : SitePermission::ROLE_VIEWER;
+
+ $highestRole = $this->resolveHighestRole($highestRole, $role);
+ }
+
+ return $highestRole;
+ }
+
+ /**
+ * Return whichever of two Omeka site permission roles conveys higher privilege.
+ *
+ * @param string|null $current
+ * @param string $new
+ * @return string
+ */
+ private function resolveHighestRole(?string $current, string $new): string
+ {
+ if ($current === null) {
+ return $new;
+ }
+
+ $currentPriority = self::ROLE_PRIORITY[$current] ?? -1;
+ $newPriority = self::ROLE_PRIORITY[$new] ?? -1;
+
+ return $newPriority > $currentPriority ? $new : $current;
+ }
+}
diff --git a/src/Service/SitePermissionManagerFactory.php b/src/Service/SitePermissionManagerFactory.php
new file mode 100644
index 00000000..8aeff254
--- /dev/null
+++ b/src/Service/SitePermissionManagerFactory.php
@@ -0,0 +1,16 @@
+get('Omeka\EntityManager'),
+ $container->get('Omeka\Settings\User')
+ );
+ }
+}
diff --git a/src/Service/UpdateControllerFactory.php b/src/Service/UpdateControllerFactory.php
index 25e1febb..3df1ad5a 100644
--- a/src/Service/UpdateControllerFactory.php
+++ b/src/Service/UpdateControllerFactory.php
@@ -10,8 +10,10 @@ class UpdateControllerFactory implements FactoryInterface
{
public function __invoke(ContainerInterface $services, $requestedName, array $options = null)
{
- $addController = new UpdateController($services->get('Omeka\EntityManager'));
- return $addController;
+ return new UpdateController(
+ $services->get('Omeka\EntityManager'),
+ $services->get(SitePermissionManager::class)
+ );
}
}
From 38b8fa522a1aa9f378602aff345c21e9d88163f8 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 20:20:01 +0000
Subject: [PATCH 02/54] Remove docs/service-layer-pattern.md (rationale kept in
chat only)
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
docs/service-layer-pattern.md | 271 ----------------------------------
1 file changed, 271 deletions(-)
delete mode 100644 docs/service-layer-pattern.md
diff --git a/docs/service-layer-pattern.md b/docs/service-layer-pattern.md
deleted file mode 100644
index ac18d0fc..00000000
--- a/docs/service-layer-pattern.md
+++ /dev/null
@@ -1,271 +0,0 @@
-# Service Layer Pattern — Rationale and Guide
-
-## The Problem: A 2,778-Line Monolith
-
-`Module.php` is the heart of the Teams module. Every Omeka S module has one, and for
-small modules that is fine. Teams is not a small module. At the time this document was
-written, `Module.php` contained approximately **60 public methods** and nearly
-**2,800 lines** in a single class. The methods covered:
-
-- ACL rule wiring
-- Query filtering
-- Event-driven entity sync (items, sites, users, assets, resource templates)
-- View helpers and form injection
-- Configuration handling
-- Admin UI display logic
-
-When everything lives in one place, the following problems compound over time:
-
-1. **No single concern is traceable.** To understand how site permissions work, a reader
- must grep for scattered method calls and follow a chain through `Module.php`,
- `UpdateController.php`, and form events — none of which have a shared namespace or
- naming convention.
-
-2. **Logic cannot be reused without copy-paste.** Because business logic is embedded in
- event-handler methods (which receive raw `Event` objects), it cannot be called from
- another entry point (e.g., a CLI command, a background job, or a second controller)
- without duplicating the code.
-
-3. **Testing is impossible in practice.** An event handler that calls
- `$this->getServiceLocator()` is tightly coupled to the Laminas service container.
- There is no way to instantiate just the relevant behaviour for a unit test without
- bootstrapping the entire framework.
-
-4. **The class grows without bound.** Every new feature becomes a new method on
- `Module.php` because there is no obvious alternative home for it. This is the
- definition of the *Big Ball of Mud* anti-pattern.
-
----
-
-## Why Service Layer Is the Right Pattern Here
-
-### What it is
-
-The **Service Layer** pattern (Fowler, *Patterns of Enterprise Application Architecture*,
-2002) draws a boundary between the application's *entry points* (HTTP controllers, event
-handlers, CLI commands) and its *domain logic* (business rules, entity manipulation,
-cross-cutting sync). Domain logic is placed in dedicated **service classes** that are
-injected into entry points as dependencies.
-
-In Laminas/Omeka S this is spelled out concretely:
-
-```
-Entry point Service Layer Domain / ORM
-───────────────── ──────────────────────── ──────────────────
-Module.php SitePermissionManager Doctrine EntityManager
-UpdateController AclRuleManager Omeka\Entity\*
- (future) ItemSyncManager Teams\Entity\*
-```
-
-### Why not one of the alternatives?
-
-**Repository pattern alone** — Repositories handle query building, not business rules.
-They are the right home for "give me all TeamUsers for team X" but not for "given a
-TeamUser change, update the matching SitePermissions". Mixing rules into repositories
-produces the same coupling problem in a different class.
-
-**Fat controller** — Putting logic in controllers makes it unreachable from event
-handlers, and vice versa. The existing code already shows the pain: `updateUserSites`
-is defined on `Module.php` and called from both `siteCreate` and `siteUpdate` purely
-because controllers cannot directly call `Module` methods.
-
-**Traits on Module** — PHP traits are a copy-paste mechanism, not a boundary. A trait
-on `Module.php` still has access to `$this->getServiceLocator()`, still cannot be
-constructed independently, and still cannot be tested in isolation.
-
-**Doctrine lifecycle listeners** — These are appropriate for generic, entity-level
-concerns (timestamps, soft-delete flags). They are inappropriate for business logic
-that is specific to the Teams module, because they introduce a hidden dependency
-between the ORM layer and module-specific rules that would survive even if the module
-were disabled.
-
-**Service Layer** fits because:
-
-- Services receive their dependencies through the **constructor**, not through a global
- service locator. This is standard Laminas Dependency Injection and makes every
- dependency explicit and mockable.
-- A service class has a **single, nameable concern**. `SitePermissionManager` does
- exactly one thing: keep Omeka site user permissions in sync with Teams role
- assignments. A reader can open the file and understand its purpose in two minutes.
-- The same service can be **called from any entry point**: an event handler in
- `Module.php`, a controller action, a background job, a future REST endpoint, or a
- unit test.
-- Services are registered in the **Laminas service manager** via factory classes.
- This is the established Omeka S convention for injectable objects — the same mechanism
- used for API adapters, form elements, and authentication — so no new conventions need
- to be introduced or learned.
-
-### Relationship to Omeka S conventions
-
-Omeka S itself follows this pattern. The core `application/Module.php` is thin; heavy
-logic lives in dedicated classes (`SiteAdapter`, `ItemAdapter`, `Acl`, etc.). Official
-Omeka S modules such as [CSVImport] and [BulkImport] extract complex processing into
-service classes registered through factories. Adopting the same pattern keeps the Teams
-module aligned with the ecosystem and makes it easier for Omeka-familiar developers to
-contribute.
-
----
-
-## The Concrete Example: `SitePermissionManager`
-
-### Before
-
-`Module.php` contained `updateUserSites()`, a method that:
-- called `$this->getServiceLocator()` to fetch `Omeka\EntityManager` and
- `Omeka\Settings\User` — two separate service-locator calls buried inside the method
- body
-- was duplicated at three call sites inside `Module.php` and implicitly depended on
- those call sites knowing to call `Module::updateUserSites()` instead of having
- access to the logic directly
-- could not be called from `UpdateController` without going back through the module
- event system
-
-The new feature (issue #189 — auto-generate site permissions) would have required
-adding *more* methods to `Module.php` with the same problems, and wiring them from
-`UpdateController` was impossible without adding a second copy of the same code.
-
-### After: `Teams\Service\SitePermissionManager`
-
-`SitePermissionManager` is a plain PHP class:
-
-```php
-class SitePermissionManager
-{
- public function __construct(
- EntityManager $entityManager,
- UserSettings $userSettings
- ) { … }
-
- public function syncSitePermissionsForUser(int $userId, int $teamId): void { … }
- public function removeSitePermissionsForUser(int $userId, int $teamId, ?int $siteId = null): void { … }
- public function syncSitePermissionsForTeamOnSiteAdded(int $teamId, int $siteId): void { … }
- public function removeSitePermissionsForTeamOnSiteRemoved(int $teamId, int $siteId): void { … }
- public function updateUserDefaultSites(int $userId): void { … }
- public function updateAllUserDefaultSites(): void { … }
-}
-```
-
-`SitePermissionManagerFactory` creates it:
-
-```php
-class SitePermissionManagerFactory implements FactoryInterface
-{
- public function __invoke(ContainerInterface $container, …)
- {
- return new SitePermissionManager(
- $container->get('Omeka\EntityManager'),
- $container->get('Omeka\Settings\User')
- );
- }
-}
-```
-
-`config/module.config.php` registers it like any other Laminas service:
-
-```php
-'service_manager' => [
- 'factories' => [
- SitePermissionManager::class => SitePermissionManagerFactory::class,
- ],
-],
-```
-
-**`Module.php`** now delegates in one line:
-
-```php
-public function updateUserSites($user_id)
-{
- $this->getServiceLocator()->get(SitePermissionManager::class)
- ->updateUserDefaultSites($user_id);
-}
-```
-
-**`UpdateController`** receives the service through its constructor:
-
-```php
-public function __construct(
- EntityManager $entityManager,
- SitePermissionManager $sitePermissionManager
-) { … }
-```
-
-And calls it directly after mutating team membership:
-
-```php
-$this->sitePermissionManager->syncSitePermissionsForUser($user_id, $team_id);
-```
-
----
-
-## Design Decisions within `SitePermissionManager`
-
-### Multi-team safety
-
-A user can belong to multiple teams that share an Omeka site. Naively assigning the
-role dictated by one team could silently downgrade a privilege granted by another team.
-`SitePermissionManager` uses `resolveHighestRole()` to ensure that the role stored on
-the `SitePermission` entity is always the *highest* role granted by any of the user's
-current team memberships. When a membership is removed, `getHighestRoleFromOtherTeams()`
-recalculates from the remaining memberships before deciding whether to downgrade or
-remove the `SitePermission` entirely.
-
-### Role mapping
-
-Omeka S defines three site-level roles: `viewer`, `editor`, `admin`. The Teams module
-maps:
-
-| TeamRole condition | Omeka site role |
-|------------------------------|------------------------------|
-| `can_add_site_pages = true` | `SitePermission::ROLE_ADMIN` |
-| `can_add_site_pages = false` | `SitePermission::ROLE_VIEWER`|
-
-`ROLE_EDITOR` is not currently produced by the Teams module because TeamRole does not
-have a finer-grained "can edit but not administer" concept. If that is added later,
-only `SitePermissionManager` needs to be changed.
-
-### Flush discipline
-
-Each public method calls `$em->flush()` once at the end, after all entity mutations
-for that operation are complete. This avoids partial writes and reduces round trips.
-The exception is `updateAllUserDefaultSites`, which delegates to `updateUserDefaultSites`
-per user; the settings API (`UserSettings::set`) has its own persistence and does not
-require explicit flushes.
-
----
-
-## The Broader Roadmap
-
-`SitePermissionManager` and `AclRuleManager` are examples of the pattern. They are
-not the finish line. The same extraction should be applied, incrementally, to other
-cohesive groups of methods currently living in `Module.php`:
-
-| Candidate service | Methods to extract |
-|------------------------------|------------------------------------------------------------------------------|
-| `ItemSyncManager` | `updateItemSites`, `itemCreate`, `itemUpdate`, `itemDelete`, `itemBatchCreate`|
-| `TeamQueryFilter` | `filterByTeam`, `getTeamContext`, `getOrphans` |
-| `ResourceTemplateSyncManager`| `resourceTemplateCreate`, `resourceTemplateUpdate` |
-| `AssetSyncManager` | `assetCreate`, `assetUpdate` |
-
-Each extraction follows the same four-step recipe:
-
-1. Create a service class in `src/Service/` with constructor-injected dependencies.
-2. Create a factory in `src/Service/` that pulls those dependencies from the container.
-3. Register the factory in `config/module.config.php`.
-4. Replace the body of the `Module.php` method(s) with a single-line delegation call.
-
-No existing public API or event wiring needs to change. `Module.php` retains its
-methods as thin delegators, so callers (including third-party modules that may be
-listening to Teams events) are unaffected.
-
----
-
-## Summary
-
-| Concern | Before | After |
-|----------------------|--------------------------------|--------------------------------|
-| Where logic lives | `Module.php` method bodies | Named service class |
-| Dependencies | `$this->getServiceLocator()` | Constructor injection |
-| Reuse across entry points | Copy-paste or impossible | Call the service directly |
-| Testability | Requires full framework boot | Plain PHP instantiation |
-| Discoverability | Grep for scattered methods | Open the service class |
-| Aligned with Omeka S | Partially | Fully (same factory pattern) |
From 105169b0026afb0a7e4df94711adf6eb5c6663cd Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 20:39:00 +0000
Subject: [PATCH 03/54] Apply remaining changes
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Service/SitePermissionManager.php | 20 +++-----------------
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index 8d086e23..6d4f284f 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -21,15 +21,6 @@
*/
class SitePermissionManager
{
- /**
- * Role priority for resolving conflicts when a user belongs to multiple teams
- * that share a site. Higher number = higher privilege.
- */
- private const ROLE_PRIORITY = [
- SitePermission::ROLE_ADMIN => 2,
- SitePermission::ROLE_EDITOR => 1,
- SitePermission::ROLE_VIEWER => 0,
- ];
/**
* @var EntityManager
@@ -51,10 +42,8 @@ public function __construct(EntityManager $entityManager, UserSettings $userSett
* Sync Omeka site permissions for a user in a specific team.
*
* Assigns ROLE_ADMIN if the team role has `can_add_site_pages`, otherwise ROLE_VIEWER,
- * for every site associated with the team.
- *
- * When the user already has a permission on a site (e.g. from another team), the
- * higher of the two roles is kept, so no privilege is silently downgraded.
+ * for every site associated with the team. The role is set unconditionally so that
+ * downgrading a team role is immediately reflected in the site permission.
*
* @param int $userId
* @param int $teamId
@@ -83,10 +72,7 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$existingPermission = $sitePermissions->matching($criteria)->first();
if ($existingPermission) {
- // Keep the higher of the existing and new role (multi-team safety).
- $existingPermission->setRole(
- $this->resolveHighestRole($existingPermission->getRole(), $omekaRole)
- );
+ $existingPermission->setRole($omekaRole);
} else {
$sitePermission = new SitePermission();
$sitePermission->setSite($site);
From 2c0ed29fe13033d24fe7dac0b3e29f1765f94275 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 21:36:28 +0000
Subject: [PATCH 04/54] fix: sync site permissions when team member roles are
changed or removed 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>
---
src/Controller/UpdateController.php | 4 ++++
src/Service/SitePermissionManager.php | 9 +++++++++
2 files changed, 13 insertions(+)
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index 0df81366..ff91fd25 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -315,6 +315,7 @@ public function teamUpdateAction()
foreach ($oldTeamUsers as $oldTeamUser) {
if (!in_array($oldTeamUser,$formTeamUsers)) {
$this->api()->delete('team-user',['team'=>$team_id, 'user'=>$oldTeamUser]);
+ $this->sitePermissionManager->removeSitePermissionsForUser((int)$oldTeamUser, (int)$team_id);
}
}
//add team users or update permissions
@@ -325,6 +326,8 @@ public function teamUpdateAction()
if ($teamUserExists){
$role = $this->api()->read('team-role',['id'=>$teamUser['o:team_role']['o:id']])->getContent();
$teamUserExists[0]->getEntity()->setRole($role->getEntity());
+ $this->entityManager->flush();
+ $this->sitePermissionManager->syncSitePermissionsForUser((int)$teamUser['o:user']['o:id'], (int)$team_id);
} else {
$this->api()
->create('team-user',
@@ -333,6 +336,7 @@ public function teamUpdateAction()
'user'=>$teamUser['o:user']['o:id'],
'role'=>$teamUser['o:team_role']['o:id']
]);
+ $this->sitePermissionManager->syncSitePermissionsForUser((int)$teamUser['o:user']['o:id'], (int)$team_id);
}
}
}
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index 6d4f284f..7724f3cd 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -21,6 +21,15 @@
*/
class SitePermissionManager
{
+ /**
+ * Role priority for resolving conflicts when a user belongs to multiple teams
+ * that share a site. Higher number = higher privilege.
+ */
+ private const ROLE_PRIORITY = [
+ SitePermission::ROLE_ADMIN => 2,
+ SitePermission::ROLE_EDITOR => 1,
+ SitePermission::ROLE_VIEWER => 0,
+ ];
/**
* @var EntityManager
From 26bde25da608173f9616b8e16d0c6293c5f57478 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 21:40:41 +0000
Subject: [PATCH 05/54] fix: map can_add_site_pages to ROLE_MANAGER (not
ROLE_ADMIN) and ROLE_VIEWER
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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>
---
Module.php | 7 ++++++
config/module.ini | 2 +-
src/Service/SitePermissionManager.php | 31 ++++++++++++++++++++++-----
3 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/Module.php b/Module.php
index 257bbda9..16bbc7a2 100644
--- a/Module.php
+++ b/Module.php
@@ -191,6 +191,13 @@ public function upgrade($oldVersion, $newVersion, ServiceLocatorInterface $servi
$globalSettings = $serviceLocator->get('Omeka\Settings');
$globalSettings->set('teams_filter_bypass_roles', ["global_admin"]);
}
+ if (version_compare($oldVersion, '4.2.0', '<')) {
+ // Sync Omeka site permissions for all existing team members so that
+ // every user has the correct SitePermission row for every site their
+ // team is associated with. This backfills installations that were
+ // running before site-permission sync was introduced.
+ $serviceLocator->get(SitePermissionManager::class)->syncAllSitePermissions();
+ }
}
public function updateAllUserSites()
diff --git a/config/module.ini b/config/module.ini
index 59f8d97d..bc400db7 100644
--- a/config/module.ini
+++ b/config/module.ini
@@ -1,6 +1,6 @@
[info]
name = "Teams"
-version = "4.1.0"
+version = "4.2.0"
author = "Alex Dryden"
configurable = true
description = "Assigns items and itemsets to teams so that only approved team mates see them"
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index 7724f3cd..41d6eb1d 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -26,8 +26,7 @@ class SitePermissionManager
* that share a site. Higher number = higher privilege.
*/
private const ROLE_PRIORITY = [
- SitePermission::ROLE_ADMIN => 2,
- SitePermission::ROLE_EDITOR => 1,
+ SitePermission::ROLE_MANAGER => 1,
SitePermission::ROLE_VIEWER => 0,
];
@@ -50,7 +49,7 @@ public function __construct(EntityManager $entityManager, UserSettings $userSett
/**
* Sync Omeka site permissions for a user in a specific team.
*
- * Assigns ROLE_ADMIN if the team role has `can_add_site_pages`, otherwise ROLE_VIEWER,
+ * Assigns ROLE_MANAGER if the team role has `can_add_site_pages`, otherwise ROLE_VIEWER,
* for every site associated with the team. The role is set unconditionally so that
* downgrading a team role is immediately reflected in the site permission.
*
@@ -68,7 +67,7 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$user = $teamUser->getUser();
$omekaRole = $teamUser->getRole()->getCanAddSitePages()
- ? SitePermission::ROLE_ADMIN
+ ? SitePermission::ROLE_MANAGER
: SitePermission::ROLE_VIEWER;
$teamSites = $em->getRepository('Teams\Entity\TeamSite')->findBy(['team' => $teamId]);
@@ -225,6 +224,28 @@ public function updateAllUserDefaultSites(): void
}
}
+ /**
+ * Sync Omeka site permissions for every team-user-site combination.
+ *
+ * Iterates all TeamUser records and calls syncSitePermissionsForUser for
+ * each, ensuring every user has the correct SitePermission row for every
+ * site their team is associated with. Used by the module upgrade routine
+ * to bring existing installations into sync.
+ */
+ public function syncAllSitePermissions(): void
+ {
+ $teamUsers = $this->entityManager
+ ->getRepository('Teams\Entity\TeamUser')
+ ->findAll();
+
+ foreach ($teamUsers as $teamUser) {
+ $this->syncSitePermissionsForUser(
+ $teamUser->getUser()->getId(),
+ $teamUser->getTeam()->getId()
+ );
+ }
+ }
+
/**
* Determine the highest Omeka site role granted to a user by any team *other than*
* the one being removed. Returns null if no other team grants access to that site.
@@ -253,7 +274,7 @@ private function getHighestRoleFromOtherTeams(int $userId, int $excludeTeamId, i
}
$role = $teamUser->getRole()->getCanAddSitePages()
- ? SitePermission::ROLE_ADMIN
+ ? SitePermission::ROLE_MANAGER
: SitePermission::ROLE_VIEWER;
$highestRole = $this->resolveHighestRole($highestRole, $role);
From 2ec5bbc760201391bbb05df90418cf9141b5c76f Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 21:43:48 +0000
Subject: [PATCH 06/54] feat: site admin UI warning and team attribution; fix
removal logic to 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>
---
Module.php | 43 +++++++++++
src/Service/SitePermissionManager.php | 55 ++++++--------
.../partial/site-admin/users-teams-info.phtml | 71 +++++++++++++++++++
3 files changed, 134 insertions(+), 35 deletions(-)
create mode 100644 view/teams/partial/site-admin/users-teams-info.phtml
diff --git a/Module.php b/Module.php
index 16bbc7a2..2678e28f 100644
--- a/Module.php
+++ b/Module.php
@@ -1950,6 +1950,43 @@ public function siteEdit(Event $event)
echo $view->partial('teams/partial/site-admin/edit', ['site_teams' => $site_teams, 'team_ids' => $team_ids]);
}
+ /**
+ * Warns that site user roles are managed by Teams, and annotates each user
+ * row in the Omeka site-admin permissions table with the team(s) responsible.
+ *
+ * @param Event $event
+ */
+ public function siteUsersTeamsInfo(Event $event)
+ {
+ $view = $event->getTarget();
+ $site = $view->vars()->site;
+ if (!$site) {
+ return;
+ }
+
+ $messenger = new Messenger();
+ $messenger->addWarning(
+ 'User roles on this site are managed by the Teams module. '
+ . 'Manual changes made here may be overwritten the next time team memberships or roles are updated.'
+ );
+
+ $em = $this->getServiceLocator()->get('Omeka\EntityManager');
+ $teamSites = $em->getRepository('Teams\Entity\TeamSite')->findBy(['site' => $site->id()]);
+
+ // Build userId => [teamName, ...] for every user who has a team-managed
+ // permission on this site.
+ $teamManagedUsers = [];
+ foreach ($teamSites as $teamSite) {
+ $team = $teamSite->getTeam();
+ $teamUsers = $em->getRepository('Teams\Entity\TeamUser')->findBy(['team' => $team->getId()]);
+ foreach ($teamUsers as $teamUser) {
+ $userId = $teamUser->getUser()->getId();
+ $teamManagedUsers[$userId][] = $team->getName();
+ }
+ }
+
+ echo $view->partial('teams/partial/site-admin/users-teams-info', ['teamManagedUsers' => $teamManagedUsers]);
+ }
public function getModules()
{
@@ -2540,6 +2577,12 @@ public function attachListeners(SharedEventManagerInterface $sharedEventManager)
[$this, 'siteEdit']
);
+ $sharedEventManager->attach(
+ 'Omeka\Controller\SiteAdmin\Index',
+ 'view.edit.after',
+ [$this, 'siteUsersTeamsInfo']
+ );
+
//put the roles data in the user page
$sharedEventManager->attach(
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index 41d6eb1d..ae13be23 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -21,15 +21,6 @@
*/
class SitePermissionManager
{
- /**
- * Role priority for resolving conflicts when a user belongs to multiple teams
- * that share a site. Higher number = higher privilege.
- */
- private const ROLE_PRIORITY = [
- SitePermission::ROLE_MANAGER => 1,
- SitePermission::ROLE_VIEWER => 0,
- ];
-
/**
* @var EntityManager
*/
@@ -247,8 +238,15 @@ public function syncAllSitePermissions(): void
}
/**
- * Determine the highest Omeka site role granted to a user by any team *other than*
- * the one being removed. Returns null if no other team grants access to that site.
+ * Determine the Omeka site role a user should hold on a site based on their
+ * remaining team memberships, excluding one team (the one being removed).
+ *
+ * Logic:
+ * - If the user has no other team that includes this site, returns null
+ * (the site permission should be removed entirely).
+ * - If any remaining team with this site has can_add_site_pages = true,
+ * returns ROLE_MANAGER.
+ * - Otherwise returns ROLE_VIEWER.
*
* @param int $userId
* @param int $excludeTeamId The team being removed, which should be ignored.
@@ -260,7 +258,9 @@ private function getHighestRoleFromOtherTeams(int $userId, int $excludeTeamId, i
$em = $this->entityManager;
$teamUsers = $em->getRepository('Teams\Entity\TeamUser')->findBy(['user' => $userId]);
- $highestRole = null;
+ $hasAnySite = false;
+ $canManage = false;
+
foreach ($teamUsers as $teamUser) {
if ($teamUser->getTeam()->getId() === $excludeTeamId) {
continue;
@@ -273,32 +273,17 @@ private function getHighestRoleFromOtherTeams(int $userId, int $excludeTeamId, i
continue;
}
- $role = $teamUser->getRole()->getCanAddSitePages()
- ? SitePermission::ROLE_MANAGER
- : SitePermission::ROLE_VIEWER;
-
- $highestRole = $this->resolveHighestRole($highestRole, $role);
+ $hasAnySite = true;
+ if ($teamUser->getRole()->getCanAddSitePages()) {
+ $canManage = true;
+ break; // No need to check further — manager wins.
+ }
}
- return $highestRole;
- }
-
- /**
- * Return whichever of two Omeka site permission roles conveys higher privilege.
- *
- * @param string|null $current
- * @param string $new
- * @return string
- */
- private function resolveHighestRole(?string $current, string $new): string
- {
- if ($current === null) {
- return $new;
+ if (!$hasAnySite) {
+ return null;
}
- $currentPriority = self::ROLE_PRIORITY[$current] ?? -1;
- $newPriority = self::ROLE_PRIORITY[$new] ?? -1;
-
- return $newPriority > $currentPriority ? $new : $current;
+ return $canManage ? SitePermission::ROLE_MANAGER : SitePermission::ROLE_VIEWER;
}
}
diff --git a/view/teams/partial/site-admin/users-teams-info.phtml b/view/teams/partial/site-admin/users-teams-info.phtml
new file mode 100644
index 00000000..e5c23eff
--- /dev/null
+++ b/view/teams/partial/site-admin/users-teams-info.phtml
@@ -0,0 +1,71 @@
+teamManagedUsers array userId (int) => array of team names (string[])
+ */
+$teamManagedUsers = $this->teamManagedUsers;
+$translate = $this->plugin('translate');
+?>
+
From ee7d715779985e5b9eebd077cd5816c28b3791fe Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 22:03:36 +0000
Subject: [PATCH 07/54] Fix upgrade: require_once SitePermissionManager before
direct instantiation
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
Module.php | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/Module.php b/Module.php
index 2678e28f..0b5714ac 100644
--- a/Module.php
+++ b/Module.php
@@ -196,7 +196,15 @@ public function upgrade($oldVersion, $newVersion, ServiceLocatorInterface $servi
// every user has the correct SitePermission row for every site their
// team is associated with. This backfills installations that were
// running before site-permission sync was introduced.
- $serviceLocator->get(SitePermissionManager::class)->syncAllSitePermissions();
+ // NOTE: The service locator does not have module services available
+ // during upgrade, and the module autoloader is not yet registered,
+ // so we require the class file and instantiate it directly.
+ require_once __DIR__ . '/src/Service/SitePermissionManager.php';
+ $sitePermissionManager = new SitePermissionManager(
+ $serviceLocator->get('Omeka\EntityManager'),
+ $serviceLocator->get('Omeka\Settings\User')
+ );
+ $sitePermissionManager->syncAllSitePermissions();
}
}
From f90a766690965a2586e984e05de56710ac75ae67 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 22:10:33 +0000
Subject: [PATCH 08/54] Remove syncAllSitePermissions from upgrade; expose as
config form button
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
Module.php | 19 ++++++-------------
src/Form/ConfigForm.php | 9 +++++++++
2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/Module.php b/Module.php
index 0b5714ac..758a2520 100644
--- a/Module.php
+++ b/Module.php
@@ -192,19 +192,9 @@ public function upgrade($oldVersion, $newVersion, ServiceLocatorInterface $servi
$globalSettings->set('teams_filter_bypass_roles', ["global_admin"]);
}
if (version_compare($oldVersion, '4.2.0', '<')) {
- // Sync Omeka site permissions for all existing team members so that
- // every user has the correct SitePermission row for every site their
- // team is associated with. This backfills installations that were
- // running before site-permission sync was introduced.
- // NOTE: The service locator does not have module services available
- // during upgrade, and the module autoloader is not yet registered,
- // so we require the class file and instantiate it directly.
- require_once __DIR__ . '/src/Service/SitePermissionManager.php';
- $sitePermissionManager = new SitePermissionManager(
- $serviceLocator->get('Omeka\EntityManager'),
- $serviceLocator->get('Omeka\Settings\User')
- );
- $sitePermissionManager->syncAllSitePermissions();
+ // Site permission sync cannot run here: module entities are not
+ // registered with Doctrine during the upgrade step. Use the
+ // "Sync Site Permissions" button on the module config page instead.
}
}
@@ -224,6 +214,9 @@ public function handleConfigForm(AbstractController $controller)
$globalSettings->set('teams_site_admin_make_user', $params['teams_site_admin_make_user']);
$globalSettings->set('teams_filter_bypass_roles', $params['teams_filter_bypass_roles']);
+ if (!empty($params['teams_sync_site_permissions'])) {
+ $this->getServiceLocator()->get(SitePermissionManager::class)->syncAllSitePermissions();
+ }
}
public function getConfigForm(PhpRenderer $renderer)
diff --git a/src/Form/ConfigForm.php b/src/Form/ConfigForm.php
index ebb72a95..1429bffe 100644
--- a/src/Form/ConfigForm.php
+++ b/src/Form/ConfigForm.php
@@ -70,6 +70,15 @@ public function init()
],
]);
+ $this->add([
+ 'name' => 'teams_sync_site_permissions',
+ 'type' => 'submit',
+ 'attributes' => [
+ 'value' => 'Sync Site Permissions', // @translate
+ 'id' => 'teams_sync_site_permissions',
+ ],
+ ]);
+
}
public function setGlobalSettings($globalSettings)
{
From c322631ea46c01ff67309379500ecb7fbe307e7b Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 22:25:39 +0000
Subject: [PATCH 09/54] Fix ROLE_ADMIN constant, remove exclusion logic, update
sync language
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Service/SitePermissionManager.php | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index ae13be23..cf6a861e 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -123,8 +123,8 @@ public function removeSitePermissionsForUser(int $userId, int $teamId, ?int $sit
continue;
}
- // Check whether any other team still grants this user access to this site.
- $remainingRole = $this->getHighestRoleFromOtherTeams($userId, $teamId, $site->getId());
+ // Check whether any team still grants this user access to this site.
+ $remainingRole = $this->getHighestRoleFromOtherTeams($userId, $site->getId());
if ($remainingRole !== null) {
// User retains access via another team — update the role accordingly.
From 93ec134f9a6d5223a211b4f0de0c6d3a79e0ba07 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 22:26:25 +0000
Subject: [PATCH 10/54] Fix ROLE_ADMIN constant, remove exclusion logic, update
language to reflect syncing
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Service/SitePermissionManager.php | 46 ++++++++++++---------------
1 file changed, 21 insertions(+), 25 deletions(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index cf6a861e..67e37a75 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -40,9 +40,9 @@ public function __construct(EntityManager $entityManager, UserSettings $userSett
/**
* Sync Omeka site permissions for a user in a specific team.
*
- * Assigns ROLE_MANAGER if the team role has `can_add_site_pages`, otherwise ROLE_VIEWER,
+ * Assigns ROLE_ADMIN if the team role has `can_add_site_pages`, otherwise ROLE_VIEWER,
* for every site associated with the team. The role is set unconditionally so that
- * downgrading a team role is immediately reflected in the site permission.
+ * any change to a team role is immediately reflected in the site permission.
*
* @param int $userId
* @param int $teamId
@@ -58,7 +58,7 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$user = $teamUser->getUser();
$omekaRole = $teamUser->getRole()->getCanAddSitePages()
- ? SitePermission::ROLE_MANAGER
+ ? SitePermission::ROLE_ADMIN
: SitePermission::ROLE_VIEWER;
$teamSites = $em->getRepository('Teams\Entity\TeamSite')->findBy(['team' => $teamId]);
@@ -86,11 +86,12 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
}
/**
- * Remove or recalculate Omeka site permissions for a user leaving a team.
+ * Sync or remove Omeka site permissions for a user leaving a team.
*
* If the user still belongs to other teams that share a given site, their
- * permission is recalculated from those remaining memberships rather than
- * simply removed, so a shared-site privilege is never accidentally revoked.
+ * permission is recalculated across all team memberships so that the role
+ * reflects the current state — which may be unchanged, lowered, or elevated.
+ * If no team grants access to a site, the permission is removed entirely.
*
* @param int $userId
* @param int $teamId The team the user is being removed from.
@@ -123,14 +124,14 @@ public function removeSitePermissionsForUser(int $userId, int $teamId, ?int $sit
continue;
}
- // Check whether any team still grants this user access to this site.
- $remainingRole = $this->getHighestRoleFromOtherTeams($userId, $site->getId());
+ // Recalculate the role across all team memberships after the change.
+ $syncedRole = $this->getHighestRoleFromTeams($userId, $site->getId());
- if ($remainingRole !== null) {
- // User retains access via another team — update the role accordingly.
- $existingPermission->setRole($remainingRole);
+ if ($syncedRole !== null) {
+ // User still has access — sync to the recalculated role.
+ $existingPermission->setRole($syncedRole);
} else {
- // No other team grants access; remove the permission entirely.
+ // No team grants access to this site; remove the permission.
$sitePermissions->removeElement($existingPermission);
$em->remove($existingPermission);
}
@@ -238,22 +239,21 @@ public function syncAllSitePermissions(): void
}
/**
- * Determine the Omeka site role a user should hold on a site based on their
- * remaining team memberships, excluding one team (the one being removed).
+ * Determine the Omeka site role a user should hold on a site based on all
+ * of their current team memberships.
*
* Logic:
- * - If the user has no other team that includes this site, returns null
+ * - If no team the user belongs to includes this site, returns null
* (the site permission should be removed entirely).
- * - If any remaining team with this site has can_add_site_pages = true,
- * returns ROLE_MANAGER.
+ * - If any team with this site has can_add_site_pages = true,
+ * returns ROLE_ADMIN.
* - Otherwise returns ROLE_VIEWER.
*
* @param int $userId
- * @param int $excludeTeamId The team being removed, which should be ignored.
* @param int $siteId
* @return string|null
*/
- private function getHighestRoleFromOtherTeams(int $userId, int $excludeTeamId, int $siteId): ?string
+ private function getHighestRoleFromTeams(int $userId, int $siteId): ?string
{
$em = $this->entityManager;
$teamUsers = $em->getRepository('Teams\Entity\TeamUser')->findBy(['user' => $userId]);
@@ -262,10 +262,6 @@ private function getHighestRoleFromOtherTeams(int $userId, int $excludeTeamId, i
$canManage = false;
foreach ($teamUsers as $teamUser) {
- if ($teamUser->getTeam()->getId() === $excludeTeamId) {
- continue;
- }
-
$teamSite = $em->getRepository('Teams\Entity\TeamSite')
->findOneBy(['team' => $teamUser->getTeam()->getId(), 'site' => $siteId]);
@@ -276,7 +272,7 @@ private function getHighestRoleFromOtherTeams(int $userId, int $excludeTeamId, i
$hasAnySite = true;
if ($teamUser->getRole()->getCanAddSitePages()) {
$canManage = true;
- break; // No need to check further — manager wins.
+ break; // No need to check further — admin wins.
}
}
@@ -284,6 +280,6 @@ private function getHighestRoleFromOtherTeams(int $userId, int $excludeTeamId, i
return null;
}
- return $canManage ? SitePermission::ROLE_MANAGER : SitePermission::ROLE_VIEWER;
+ return $canManage ? SitePermission::ROLE_ADMIN : SitePermission::ROLE_VIEWER;
}
}
From 565999cc702c16224333cc54957be46c69f2287d Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 22:33:45 +0000
Subject: [PATCH 11/54] Refresh TeamUser entity in syncSitePermissionsForUser
to fix stale role on elevation
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Service/SitePermissionManager.php | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index 67e37a75..f957b4a6 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -56,6 +56,10 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
return;
}
+ // Refresh to ensure the role reflects the latest persisted state,
+ // not a potentially stale identity-map proxy.
+ $em->refresh($teamUser);
+
$user = $teamUser->getUser();
$omekaRole = $teamUser->getRole()->getCanAddSitePages()
? SitePermission::ROLE_ADMIN
From cb06e0a6e8e0825ea6dac91314c0c783ff11a9da Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 10 Aug 2026 22:41:43 +0000
Subject: [PATCH 12/54] Use syncAllSitePermissions at end of team edit,
matching sync button behavior exactly
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Controller/UpdateController.php | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index ff91fd25..00a0a0d7 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -327,7 +327,6 @@ public function teamUpdateAction()
$role = $this->api()->read('team-role',['id'=>$teamUser['o:team_role']['o:id']])->getContent();
$teamUserExists[0]->getEntity()->setRole($role->getEntity());
$this->entityManager->flush();
- $this->sitePermissionManager->syncSitePermissionsForUser((int)$teamUser['o:user']['o:id'], (int)$team_id);
} else {
$this->api()
->create('team-user',
@@ -336,7 +335,6 @@ public function teamUpdateAction()
'user'=>$teamUser['o:user']['o:id'],
'role'=>$teamUser['o:team_role']['o:id']
]);
- $this->sitePermissionManager->syncSitePermissionsForUser((int)$teamUser['o:user']['o:id'], (int)$team_id);
}
}
}
@@ -445,6 +443,10 @@ public function teamUpdateAction()
}
}
+ // Sync all site permissions now that all role and site changes are persisted,
+ // using the same logic as the sync button to guarantee correctness.
+ $this->sitePermissionManager->syncAllSitePermissions();
+
$successMessage = sprintf("Successfully updated the %s team", $team->getName());
$this->messenger()->addSuccess($successMessage);
From b699f7c90afbd79c19cdd61382ce82c365073421 Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 18:54:54 -0400
Subject: [PATCH 13/54] fix: only use the sync method
---
src/Controller/UpdateController.php | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index 00a0a0d7..4f9829d6 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -86,7 +86,7 @@ public function removeTeamUser(int $team_id, int $user)
$em = $this->entityManager;
- $this->sitePermissionManager->removeSitePermissionsForUser($user, $team_id);
+ $this->sitePermissionManager->syncSitePermissionsForUser($user, $team_id);
$team_user = $em->find('Teams\Entity\TeamUser', ['team' => $team_id, 'user' => $user]);
$em->remove($team_user);
@@ -315,7 +315,7 @@ public function teamUpdateAction()
foreach ($oldTeamUsers as $oldTeamUser) {
if (!in_array($oldTeamUser,$formTeamUsers)) {
$this->api()->delete('team-user',['team'=>$team_id, 'user'=>$oldTeamUser]);
- $this->sitePermissionManager->removeSitePermissionsForUser((int)$oldTeamUser, (int)$team_id);
+ $this->sitePermissionManager->syncSitePermissionsForUser((int)$oldTeamUser, (int)$team_id);
}
}
//add team users or update permissions
From 90793c062f916263b5f6160b8f75aa6ad394c6b4 Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 18:57:27 -0400
Subject: [PATCH 14/54] fix: return the devolop version and just add a single
sync all
---
src/Controller/UpdateController.php | 387 +++++++++++++++++-----------
1 file changed, 237 insertions(+), 150 deletions(-)
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index 4f9829d6..e2e20baa 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -2,25 +2,22 @@
namespace Teams\Controller;
use Doctrine\ORM\EntityManager;
-use Doctrine\ORM\NonUniqueResultException;
-use Doctrine\ORM\OptimisticLockException;
-use Doctrine\ORM\ORMException;
use Doctrine\ORM\QueryBuilder;
+use Omeka\Api\Exception\InvalidArgumentException;
use Omeka\Api\Request;
+use Teams\Entity\TeamAsset;
+use Teams\Entity\TeamResource;
+use Teams\Entity\TeamResourceTemplate;
use Teams\Entity\TeamSite;
use Teams\Entity\TeamUser;
-use Teams\Form\SecondaryResourcesForm;
use Teams\Form\TeamItemsetAddRemoveForm;
-use Teams\Form\TeamResourcesForm;
use Teams\Form\TeamSitesAddRemoveForm;
-use Teams\Form\TeamDetailsForm;
-use Teams\Service\SitePermissionManager;
+use Teams\Form\TeamUpdateForm;
use Laminas\EventManager\Event;
use Laminas\Mvc\Controller\AbstractActionController;
use Laminas\Stdlib\ArrayObject;
use Laminas\View\Model\ViewModel;
-
class UpdateController extends AbstractActionController
{
/**
@@ -28,19 +25,12 @@ class UpdateController extends AbstractActionController
*/
protected $entityManager;
- /**
- * @var SitePermissionManager
- */
- protected SitePermissionManager $sitePermissionManager;
-
/**
* @param EntityManager $entityManager
- * @param SitePermissionManager $sitePermissionManager
*/
- public function __construct(EntityManager $entityManager, SitePermissionManager $sitePermissionManager)
+ public function __construct(EntityManager $entityManager)
{
$this->entityManager = $entityManager;
- $this->sitePermissionManager = $sitePermissionManager;
}
public function createNamedParameter(
@@ -70,9 +60,6 @@ public function addTeamUser(int $team_id, int $user_id, int $role_id)
//flushing here because this is a mini-form and we want to see the name pop up
//more efficient solution would be to have JS handle the popping and batch update
$this->entityManager->flush();
-
- $this->sitePermissionManager->syncSitePermissionsForUser($user_id, $team_id);
-
return $team_user;
}
}
@@ -85,9 +72,6 @@ public function removeTeamUser(int $team_id, int $user)
$this->messenger()->addError("removed user");
$em = $this->entityManager;
-
- $this->sitePermissionManager->syncSitePermissionsForUser($user, $team_id);
-
$team_user = $em->find('Teams\Entity\TeamUser', ['team' => $team_id, 'user' => $user]);
$em->remove($team_user);
@@ -107,8 +91,6 @@ public function updateRole(int $team_id, int $user_id, int $role_id)
$user_role = $em->find('Teams\Entity\TeamRole', $role_id);
$team_user->setRole($user_role);
$em->flush();
-
- $this->sitePermissionManager->syncSitePermissionsForUser($user_id, $team_id);
}
}
@@ -136,14 +118,179 @@ public function processItemSets(int $item_set_id)
return $resource_array;
}
- /**
- * @throws ORMException
- * @throws OptimisticLockException
- * @throws NonUniqueResultException
- */
- public function teamUpdateAction()
+ public function processResources($request, $team, $existing_resources, $existing_resources_templates, $existing_assets, bool $delete = false)
{
+ $resource_array = array();
+ $resource_template_array = array();
+ $asset_array = array();
+
+
+ if ($delete == false) {
+ $collection = 'addCollections';
+ } else {
+ $collection = 'rmCollections';
+ }
+
+ //get ids of itemsets and their descendents
+ if (isset($request->getPost($collection)['o:itemset'])) {
+ foreach ($request->getPost($collection)['o:itemset'] as $item_set_id):
+ $resource_array += $this->processItemSets($item_set_id);
+ endforeach;
+ }
+
+ //get ids of things the user owns
+ if (isset($request->getPost($collection)['o:user'])) {
+ foreach ($request->getPost($collection)['o:user'] as $user_id):
+ if ((int)$user_id > 0) {
+ $user_id = (int)$user_id;
+ foreach ($this->api()->search('items', ['owner_id' => $user_id, 'bypass_team_filter'=>true])->getContent() as $item):
+
+ $resource_array += [$item->id() => true];
+
+ foreach ($this->api()->search('media', ['item_id'=>$item->id(), 'bypass_team_filter' => true])->getContent() as $media):
+ $resource_array += [$media->id() => true];
+ endforeach;
+
+ endforeach;
+
+ //also get the users itemsets
+ foreach ($this->api()->search('item_sets', ['owner_id' => $user_id, 'bypass_team_filter'=>true])->getContent() as $itemSet):
+ $resource_array += $this->processItemSets($itemSet->id());
+ endforeach;
+
+ //also ge the user's resource templates
+ $rts = $this->entityManager->getRepository('Omeka\Entity\ResourceTemplate')->findBy(['owner'=>$user_id]);
+ foreach ($rts as $rt):
+ $resource_template_array[$rt->getId()] = true;
+ endforeach;
+
+ //also get the user's assets
+ $assets = $this->entityManager->getRepository('Omeka\Entity\Asset')->findBy(['owner' => $user_id]);
+ foreach ($assets as $asset):
+ $asset_array[$asset->getId()] = true;
+ endforeach;
+ }
+ endforeach;
+ }
+
+ if ($delete == false) {
+ //remove elements that are already part of the team to prevent integrity constraint violation
+
+ //resources remove existing from the add list
+ foreach ($existing_resources as $resource):
+ $rid = $resource->getResource()->getId();
+ if (array_key_exists($rid, $resource_array)) {
+ unset($resource_array[$rid]);
+ }
+ endforeach;
+
+ //resource templates remove existing from the add list
+ foreach ($existing_resources_templates as $resource):
+ $rid = $resource->getResourceTemplate()->getId();
+ if (array_key_exists($rid, $resource_template_array)) {
+ unset($resource_template_array[$rid]);
+ }
+ endforeach;
+
+ //assets remove existing from the add list
+ foreach ($existing_assets as $asset):
+ $asset_id = $asset->getAsset()->getId();
+ if (array_key_exists($asset_id, $asset_array)) {
+ unset($asset_array[$asset_id]);
+ }
+ endforeach;
+
+ //add the resources to the team
+ foreach ($resource_array as $resource_id => $value):
+ $resource = $this->entityManager->getRepository('Omeka\Entity\Resource')
+ ->findOneBy(['id'=>$resource_id]);
+ if ($resource) {
+ $team_resource = new TeamResource($team, $resource);
+ $this->entityManager->persist($team_resource);
+ } else {
+ $this->logger()->err('The team resource could not be generated for this request because the resource could not be found');
+ }
+
+ endforeach;
+
+ //add the resource templates to the team
+ foreach ($resource_template_array as $resource_id => $value):
+ $resource = $this->entityManager->getRepository('Omeka\Entity\ResourceTemplate')
+ ->findOneBy(['id'=>$resource_id]);
+ if ($resource) {
+ $team_resource = new TeamResourceTemplate($team, $resource);
+ $this->entityManager->persist($team_resource);
+ } else {
+ $this->logger()->err('The team resource template could not be generated for this request because the resource template could not be found');
+
+ }
+
+ endforeach;
+ //add assets to the team
+ foreach ($asset_array as $asset_id => $value):
+ $asset = $this->entityManager->getRepository('Omeka\Entity\Asset')
+ ->findOneBy(['id' => $asset_id]);
+ if ($asset) {
+ $team_asset = new TeamAsset($team, $asset);
+ $this->entityManager->persist($team_asset);
+ } else {
+ $this->logger()->err('The team asset could not be generated for this request because the asset could not be found');
+ }
+
+
+ endforeach;
+
+ $this->entityManager->flush();
+ } else {
+ //remove resources
+ foreach (array_keys($resource_array) as $resource_id):
+ $team_resource = $this->entityManager->getRepository('Teams\Entity\TeamResource')
+ ->findOneBy(['resource'=>$resource_id, 'team'=>$team]);
+ if ($team_resource) {
+ $this->entityManager->remove($team_resource);
+ }
+ endforeach;
+
+ //remove resource templates
+ foreach (array_keys($resource_template_array) as $resource_id):
+ $team_resource_template = $this->entityManager->getRepository('Teams\Entity\TeamResourceTemplate')
+ ->findOneBy(['resource_template'=>$resource_id, 'team'=>$team]);
+ if ($team_resource_template) {
+ $this->entityManager->remove($team_resource_template);
+ }
+ endforeach;
+ $this->entityManager->flush();
+
+ //remove assets
+ foreach (array_keys($asset_array) as $asset_id):
+ $team_asset = $this->entityManager->getRepository('Teams\Entity\TeamAsset')
+ ->findOneBy(['asset'=>$asset_id, 'team'=>$team]);
+ if ($team_asset) {
+ $this->entityManager->remove($team_asset);
+ }
+ endforeach;
+ $this->entityManager->flush();
+
+ }
+ }
+
+ public function processAssets($request, $team, $existing_resources, $existing_resources_templates, bool $delete = false)
+ {
+
+ }
+ public function tempteamUpdateAction()
+ {
+ $id = $this->params()->fromRoute('id');
+ $team_id = $this->params()->fromRoute('id');
+
+ $resource_form = $this->getForm(AvailableResourcesForm::class);
+ $sites_form = $this->getForm(AvailableSitesForm::class);
+ $user_form = $this->getForm(AvailableSitesForm::class);
+ }
+
+ public function teamUpdateAction()
+ {
$team_id = $this->params()->fromRoute('id');
$team = $this->entityManager->getRepository('Teams\Entity\Team')->findOneBy(['id' => $team_id]);
$team_sites = $this->entityManager
@@ -187,12 +334,11 @@ public function teamUpdateAction()
$sites->setEmptyOption('None');
$sites->setValueOptions($valueOptions);
-
//set up the item set form
$itemsetForm = $this->getForm(TeamItemsetAddRemoveForm::class);
- $user = $this->identity();
+ $userId = $this->identity()->getId();
//TODO rename this to TeamDetail form or find a way to string these all together
- $teamDetailsForm = $this->getForm(TeamDetailsForm::class);
+ $form = $this->getForm(TeamUpdateForm::class);
//TODO: get team with a one line entity manager call
$criteria = ['id' => $team_id];
@@ -249,32 +395,21 @@ public function teamUpdateAction()
$fill = new ArrayObject;
$fill['o:name'] = $data->getJsonLd()['o:name'];
$fill['o:description'] = $data->getJsonLd()['o:description'];
- $teamDetailsForm->bind($fill);
+ $form->bind($fill);
//is it a post request?
//TODO (refactor) clean up this, only send what is needed
$request = $this->getRequest();
-
- $resourceForm = $this->getForm(TeamResourcesForm::class)->setAttribute('id', 'team-resources-form');
- $secondaryResourcesForm = $this->getForm(SecondaryResourcesForm::class,
- ['team_id'=>$team_id]
- );
-
- $bypass_team_filter_roles = $this->settings()->get('teams_filter_bypass_roles');
- $view = new ViewModel([
- 'team'=>$team,
- 'form' => $teamDetailsForm,
- 'resourceForm' => $resourceForm,
- 'secondaryResourcesForm' => $secondaryResourcesForm,
- 'bypassTeamFilterRoles' => $bypass_team_filter_roles,
- 'id' => $team_id,
+ $view = new ViewModel(['team'=>$team,
+ 'form' => $form,
+ 'id'=>$team_id,
'roles'=> $roles,
'roles_array' => $roles_array,
'all_u_collection' => $all_u_collection,
'team_u_collection' => $team_u_collection,
'team_u_array'=>$team_u_array,
'available_u_array'=>$available_u_array,
- 'user' => $user,
+ 'ident' => $userId,
'itemsetForm' => $itemsetForm,
'sitesForm' => $sitesForm,
]);
@@ -282,6 +417,29 @@ public function teamUpdateAction()
return $view;
}
+ $em = $this->entityManager;
+ $qb = $em->createQueryBuilder();
+ $existing_resources = $qb->select('tr')
+ ->from('Teams\Entity\TeamResource', 'tr')
+ ->where('tr.team = :team_id')
+ ->setParameter('team_id', $team_id)
+ ->getQuery()
+ ->getResult();
+
+ $existing_resource_templates = $qb->select('trt')
+ ->from('Teams\Entity\TeamResourceTemplate', 'trt')
+ ->where('trt.team = :team_id')
+ ->setParameter('team_id', $team_id)
+ ->getQuery()
+ ->getResult();
+
+ $existing_assets = $qb->select('ta')
+ ->from('Teams\Entity\TeamAsset', 'ta')
+ ->where('ta.team = :team_id')
+ ->setParameter('team_id', $team_id)
+ ->getQuery()
+ ->getResult();
+
$post_data = $request->getPost();
if (!$this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team_user', $team_id)) {
@@ -300,110 +458,45 @@ public function teamUpdateAction()
->getQuery()
->execute();
}
- if (!$this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team_user', $team_id)) {
- $this->messenger()->addError("You aren't authorized to change team members");
- return $view;
- } else {
- $teamUsers = $request->getPost('o:team_users');
- //remove team users not in the form
- $formTeamUsers = array();
- foreach ($teamUsers as $teamUser){
- $formTeamUsers[] = $teamUser['o:user']['o:id'];
- }
- $oldTeamUsers= $this->api()->search('team-user', ['team'=>$team_id], ['returnScalar'=>'user'])->getContent();
- foreach ($oldTeamUsers as $oldTeamUser) {
- if (!in_array($oldTeamUser,$formTeamUsers)) {
- $this->api()->delete('team-user',['team'=>$team_id, 'user'=>$oldTeamUser]);
- $this->sitePermissionManager->syncSitePermissionsForUser((int)$oldTeamUser, (int)$team_id);
- }
- }
- //add team users or update permissions
- foreach ($teamUsers as $teamUser) {
- //using search instead of read because read will throw a not found error instead of returning empty
- $teamUserExists = $this->api()->search('team-user', ['team'=>$team_id, 'user'=>$teamUser['o:user']['o:id']])->getContent();
-
- if ($teamUserExists){
- $role = $this->api()->read('team-role',['id'=>$teamUser['o:team_role']['o:id']])->getContent();
- $teamUserExists[0]->getEntity()->setRole($role->getEntity());
- $this->entityManager->flush();
- } else {
- $this->api()
- ->create('team-user',
- [
- 'team'=>$team_id,
- 'user'=>$teamUser['o:user']['o:id'],
- 'role'=>$teamUser['o:team_role']['o:id']
- ]);
+
+ if (!$this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team_user', $team_id)) {
+ $this->messenger()->addError("You aren't authorized to change team members");
+ return $view;
+ } else {
+ $current_users = $this->entityManager->getRepository('Teams\Entity\TeamUser')->findBy(['team' => $team_id]);
+
+ foreach ($current_users as $team_user) {
+ $this->entityManager->remove($team_user);
}
+ $this->entityManager->flush();
+
+ foreach ($request->getPost('o:team_users') as $team_user):
+ $user = $this->entityManager->getRepository('Omeka\Entity\User')
+ ->findOneBy(['id' => (int)$team_user['o:user']['o:id']]);
+ $role = $this->entityManager->getRepository('Teams\Entity\TeamRole')
+ ->findOneBy(['id' => (int)$team_user['o:team_role']['o:id']]);
+
+ $teamUser = new TeamUser($team, $user, $role);
+ $teamUser->setCurrent(null);
+ $this->entityManager->persist($teamUser);
+ endforeach;
+ $this->entityManager->flush();
+
}
- }
- //TODO:need to update this for users who have the update and bypass_team_filter
if (! $this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team', $team_id)){
$this->messenger()->addError("You aren't authorized to change this team");
return $view;
} else {
- //process items
- $formData = $this->params()->fromPost();
- $resourceForm->setData($formData);
- parse_str($formData['item_pool'], $itemPool);
- if ($formData['item_assignment_action'] && $formData['item_assignment_action'] !== 'no_action') {
- $this->jobDispatcher()->dispatch('Teams\Job\UpdateTeamResources', [
- 'teams' => [$team_id => $itemPool],
- 'action' => $formData['item_assignment_action'],
- ]);
- $this->messenger()->addSuccess('Item assignment in progress. To see the new item count, refresh the page.'); // @translate
- }
-
- //process item sets and resource templates
-
- //remove item sets
- $secondaryResourcesForm->setData($formData);
-
- $recursive = $formData['recursive_item_sets'] ?? false;
- if (isset($formData['remove_item_sets'])){
- $remove_item_sets = $formData['remove_item_sets'];
- foreach ($remove_item_sets as $item_set_id) {
- //todo: delete expects the id to be in the second parameter, for now just leaving empty because team resource uses a composite key
- $this->api()->delete('team-resource', [], ['team' => $team_id, 'resource' => $item_set_id],['recursive'=>$recursive, 'syncSites' => true]);
- }
- }
- //add item sets
- if (isset($formData['item_sets'])) {
- foreach ($formData['item_sets'] as $item_set_id) {
- //todo: implement the read operation
- $exists = $this->api()->search('team-resource', ['team' => $team_id, 'resource' => $item_set_id]);
- if (count($exists->getContent()) < 1) {
- $this->api()->create('team-resource', ['team' => $team_id, 'resource' => $item_set_id], [], ['recursive' => $recursive, 'syncSites' => true]);
- }
- }
- }
+ //first delete then add resources to team
+ $this->processResources($request, $team, $existing_resources, $existing_resource_templates, $existing_assets, true);
+ $this->processResources($request, $team, $existing_resources, $existing_resource_templates, $existing_assets, false);
- //remove resource templates
- $secondaryResourcesForm->setData($formData);
- if (isset($formData['remove_resource_templates'])){
- foreach ($formData['remove_resource_templates'] as $resource_template_id) {
- //todo: delete expects the id to be in the second parameter, for now just leaving empty because team resource uses a composite key
- $this->api()->delete('team-resource-template', [], ['team' => $team_id, 'resource-template' => $resource_template_id]);
- }
- }
- //add resource templates
- if (isset($formData['resource_templates'])){
- foreach ($formData['resource_templates'] as $resource_template_id) {
- if (count($this->api()->search('team-resource-template', ['team'=>$team_id, 'resource-template'=>$resource_template_id])->getContent())<1){
- $this->api()->create('team-resource-template', ['team'=>$team_id, 'resource-template'=>$resource_template_id]);
- }
- }
- }
-
- $em = $this->entityManager;
//handle new sites
- $added_site_ids = [];
foreach ($post_data['teamSites']['o:site'] as $site) {
if (!in_array($site, $current_sites)) {
- $added_site_ids[] = (int)$site;
$site = $em->getRepository('Omeka\Entity\Site')->findOneBy(['id'=>$site]);
$ts = new TeamSite($team, $site);
$request = new Request('create', 'team_site');
@@ -418,10 +511,8 @@ public function teamUpdateAction()
}
//handle removed sites
- $removed_site_ids = [];
foreach ($current_sites as $site) {
if (!in_array($site, $post_data['teamSites']['o:site'])) {
- $removed_site_ids[] = (int)$site;
$ts = $em->getRepository('Teams\Entity\TeamSite')->findOneBy(['team'=>$team_id, 'site'=>$site]);
$request = new Request('delete', 'team_site');
$event = new Event('api.hydrate.pre', $this, [
@@ -433,14 +524,6 @@ public function teamUpdateAction()
}
}
$em->flush();
-
- // Sync Omeka site permissions for all team users when sites are added or removed.
- foreach ($added_site_ids as $added_site_id) {
- $this->sitePermissionManager->syncSitePermissionsForTeamOnSiteAdded($team_id, $added_site_id);
- }
- foreach ($removed_site_ids as $removed_site_id) {
- $this->sitePermissionManager->removeSitePermissionsForTeamOnSiteRemoved($team_id, $removed_site_id);
- }
}
// Sync all site permissions now that all role and site changes are persisted,
@@ -453,6 +536,10 @@ public function teamUpdateAction()
return $this->redirect()->toRoute('admin/teams/detail',['id'=>$team_id]);
}
+ public function roleUpdateAction()
+ {
+ }
+
public function userAction()
{
$request = $this->getRequest();
From a5b07652b361601a32bbbdf9a35f431f2674c35b Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 19:21:45 -0400
Subject: [PATCH 15/54] fix: single sync at the end
---
src/Controller/UpdateController.php | 365 ++++++++++------------------
1 file changed, 128 insertions(+), 237 deletions(-)
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index e2e20baa..57bac863 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -2,22 +2,25 @@
namespace Teams\Controller;
use Doctrine\ORM\EntityManager;
+use Doctrine\ORM\NonUniqueResultException;
+use Doctrine\ORM\OptimisticLockException;
+use Doctrine\ORM\ORMException;
use Doctrine\ORM\QueryBuilder;
-use Omeka\Api\Exception\InvalidArgumentException;
use Omeka\Api\Request;
-use Teams\Entity\TeamAsset;
-use Teams\Entity\TeamResource;
-use Teams\Entity\TeamResourceTemplate;
use Teams\Entity\TeamSite;
use Teams\Entity\TeamUser;
+use Teams\Form\SecondaryResourcesForm;
use Teams\Form\TeamItemsetAddRemoveForm;
+use Teams\Form\TeamResourcesForm;
use Teams\Form\TeamSitesAddRemoveForm;
-use Teams\Form\TeamUpdateForm;
+use Teams\Form\TeamDetailsForm;
+use Teams\Service\SitePermissionManager;
use Laminas\EventManager\Event;
use Laminas\Mvc\Controller\AbstractActionController;
use Laminas\Stdlib\ArrayObject;
use Laminas\View\Model\ViewModel;
+
class UpdateController extends AbstractActionController
{
/**
@@ -25,12 +28,19 @@ class UpdateController extends AbstractActionController
*/
protected $entityManager;
+ /**
+ * @var SitePermissionManager
+ */
+ protected SitePermissionManager $sitePermissionManager;
+
/**
* @param EntityManager $entityManager
+ * @param SitePermissionManager $sitePermissionManager
*/
- public function __construct(EntityManager $entityManager)
+ public function __construct(EntityManager $entityManager, SitePermissionManager $sitePermissionManager)
{
$this->entityManager = $entityManager;
+ $this->sitePermissionManager = $sitePermissionManager;
}
public function createNamedParameter(
@@ -118,179 +128,14 @@ public function processItemSets(int $item_set_id)
return $resource_array;
}
- public function processResources($request, $team, $existing_resources, $existing_resources_templates, $existing_assets, bool $delete = false)
- {
- $resource_array = array();
- $resource_template_array = array();
- $asset_array = array();
-
-
- if ($delete == false) {
- $collection = 'addCollections';
- } else {
- $collection = 'rmCollections';
- }
-
- //get ids of itemsets and their descendents
- if (isset($request->getPost($collection)['o:itemset'])) {
- foreach ($request->getPost($collection)['o:itemset'] as $item_set_id):
- $resource_array += $this->processItemSets($item_set_id);
- endforeach;
- }
-
- //get ids of things the user owns
- if (isset($request->getPost($collection)['o:user'])) {
- foreach ($request->getPost($collection)['o:user'] as $user_id):
- if ((int)$user_id > 0) {
- $user_id = (int)$user_id;
- foreach ($this->api()->search('items', ['owner_id' => $user_id, 'bypass_team_filter'=>true])->getContent() as $item):
-
- $resource_array += [$item->id() => true];
-
- foreach ($this->api()->search('media', ['item_id'=>$item->id(), 'bypass_team_filter' => true])->getContent() as $media):
- $resource_array += [$media->id() => true];
- endforeach;
-
- endforeach;
-
- //also get the users itemsets
- foreach ($this->api()->search('item_sets', ['owner_id' => $user_id, 'bypass_team_filter'=>true])->getContent() as $itemSet):
- $resource_array += $this->processItemSets($itemSet->id());
- endforeach;
-
- //also ge the user's resource templates
- $rts = $this->entityManager->getRepository('Omeka\Entity\ResourceTemplate')->findBy(['owner'=>$user_id]);
- foreach ($rts as $rt):
- $resource_template_array[$rt->getId()] = true;
- endforeach;
-
- //also get the user's assets
- $assets = $this->entityManager->getRepository('Omeka\Entity\Asset')->findBy(['owner' => $user_id]);
- foreach ($assets as $asset):
- $asset_array[$asset->getId()] = true;
- endforeach;
- }
- endforeach;
- }
-
- if ($delete == false) {
- //remove elements that are already part of the team to prevent integrity constraint violation
-
- //resources remove existing from the add list
- foreach ($existing_resources as $resource):
- $rid = $resource->getResource()->getId();
- if (array_key_exists($rid, $resource_array)) {
- unset($resource_array[$rid]);
- }
- endforeach;
-
- //resource templates remove existing from the add list
- foreach ($existing_resources_templates as $resource):
- $rid = $resource->getResourceTemplate()->getId();
- if (array_key_exists($rid, $resource_template_array)) {
- unset($resource_template_array[$rid]);
- }
- endforeach;
-
- //assets remove existing from the add list
- foreach ($existing_assets as $asset):
- $asset_id = $asset->getAsset()->getId();
- if (array_key_exists($asset_id, $asset_array)) {
- unset($asset_array[$asset_id]);
- }
- endforeach;
-
- //add the resources to the team
- foreach ($resource_array as $resource_id => $value):
- $resource = $this->entityManager->getRepository('Omeka\Entity\Resource')
- ->findOneBy(['id'=>$resource_id]);
- if ($resource) {
- $team_resource = new TeamResource($team, $resource);
- $this->entityManager->persist($team_resource);
- } else {
- $this->logger()->err('The team resource could not be generated for this request because the resource could not be found');
- }
-
- endforeach;
-
- //add the resource templates to the team
- foreach ($resource_template_array as $resource_id => $value):
- $resource = $this->entityManager->getRepository('Omeka\Entity\ResourceTemplate')
- ->findOneBy(['id'=>$resource_id]);
- if ($resource) {
- $team_resource = new TeamResourceTemplate($team, $resource);
- $this->entityManager->persist($team_resource);
- } else {
- $this->logger()->err('The team resource template could not be generated for this request because the resource template could not be found');
-
- }
-
- endforeach;
-
- //add assets to the team
- foreach ($asset_array as $asset_id => $value):
- $asset = $this->entityManager->getRepository('Omeka\Entity\Asset')
- ->findOneBy(['id' => $asset_id]);
- if ($asset) {
- $team_asset = new TeamAsset($team, $asset);
- $this->entityManager->persist($team_asset);
- } else {
- $this->logger()->err('The team asset could not be generated for this request because the asset could not be found');
- }
-
-
- endforeach;
-
- $this->entityManager->flush();
- } else {
- //remove resources
- foreach (array_keys($resource_array) as $resource_id):
- $team_resource = $this->entityManager->getRepository('Teams\Entity\TeamResource')
- ->findOneBy(['resource'=>$resource_id, 'team'=>$team]);
- if ($team_resource) {
- $this->entityManager->remove($team_resource);
- }
- endforeach;
-
- //remove resource templates
- foreach (array_keys($resource_template_array) as $resource_id):
- $team_resource_template = $this->entityManager->getRepository('Teams\Entity\TeamResourceTemplate')
- ->findOneBy(['resource_template'=>$resource_id, 'team'=>$team]);
- if ($team_resource_template) {
- $this->entityManager->remove($team_resource_template);
- }
- endforeach;
- $this->entityManager->flush();
-
- //remove assets
- foreach (array_keys($asset_array) as $asset_id):
- $team_asset = $this->entityManager->getRepository('Teams\Entity\TeamAsset')
- ->findOneBy(['asset'=>$asset_id, 'team'=>$team]);
- if ($team_asset) {
- $this->entityManager->remove($team_asset);
- }
- endforeach;
- $this->entityManager->flush();
-
- }
- }
-
- public function processAssets($request, $team, $existing_resources, $existing_resources_templates, bool $delete = false)
- {
-
- }
- public function tempteamUpdateAction()
- {
- $id = $this->params()->fromRoute('id');
- $team_id = $this->params()->fromRoute('id');
-
- $resource_form = $this->getForm(AvailableResourcesForm::class);
- $sites_form = $this->getForm(AvailableSitesForm::class);
- $user_form = $this->getForm(AvailableSitesForm::class);
- }
-
+ /**
+ * @throws ORMException
+ * @throws OptimisticLockException
+ * @throws NonUniqueResultException
+ */
public function teamUpdateAction()
{
+
$team_id = $this->params()->fromRoute('id');
$team = $this->entityManager->getRepository('Teams\Entity\Team')->findOneBy(['id' => $team_id]);
$team_sites = $this->entityManager
@@ -334,11 +179,12 @@ public function teamUpdateAction()
$sites->setEmptyOption('None');
$sites->setValueOptions($valueOptions);
+
//set up the item set form
$itemsetForm = $this->getForm(TeamItemsetAddRemoveForm::class);
- $userId = $this->identity()->getId();
+ $user = $this->identity();
//TODO rename this to TeamDetail form or find a way to string these all together
- $form = $this->getForm(TeamUpdateForm::class);
+ $teamDetailsForm = $this->getForm(TeamDetailsForm::class);
//TODO: get team with a one line entity manager call
$criteria = ['id' => $team_id];
@@ -395,21 +241,32 @@ public function teamUpdateAction()
$fill = new ArrayObject;
$fill['o:name'] = $data->getJsonLd()['o:name'];
$fill['o:description'] = $data->getJsonLd()['o:description'];
- $form->bind($fill);
+ $teamDetailsForm->bind($fill);
//is it a post request?
//TODO (refactor) clean up this, only send what is needed
$request = $this->getRequest();
- $view = new ViewModel(['team'=>$team,
- 'form' => $form,
- 'id'=>$team_id,
+
+ $resourceForm = $this->getForm(TeamResourcesForm::class)->setAttribute('id', 'team-resources-form');
+ $secondaryResourcesForm = $this->getForm(SecondaryResourcesForm::class,
+ ['team_id'=>$team_id]
+ );
+
+ $bypass_team_filter_roles = $this->settings()->get('teams_filter_bypass_roles');
+ $view = new ViewModel([
+ 'team'=>$team,
+ 'form' => $teamDetailsForm,
+ 'resourceForm' => $resourceForm,
+ 'secondaryResourcesForm' => $secondaryResourcesForm,
+ 'bypassTeamFilterRoles' => $bypass_team_filter_roles,
+ 'id' => $team_id,
'roles'=> $roles,
'roles_array' => $roles_array,
'all_u_collection' => $all_u_collection,
'team_u_collection' => $team_u_collection,
'team_u_array'=>$team_u_array,
'available_u_array'=>$available_u_array,
- 'ident' => $userId,
+ 'user' => $user,
'itemsetForm' => $itemsetForm,
'sitesForm' => $sitesForm,
]);
@@ -417,29 +274,6 @@ public function teamUpdateAction()
return $view;
}
- $em = $this->entityManager;
- $qb = $em->createQueryBuilder();
- $existing_resources = $qb->select('tr')
- ->from('Teams\Entity\TeamResource', 'tr')
- ->where('tr.team = :team_id')
- ->setParameter('team_id', $team_id)
- ->getQuery()
- ->getResult();
-
- $existing_resource_templates = $qb->select('trt')
- ->from('Teams\Entity\TeamResourceTemplate', 'trt')
- ->where('trt.team = :team_id')
- ->setParameter('team_id', $team_id)
- ->getQuery()
- ->getResult();
-
- $existing_assets = $qb->select('ta')
- ->from('Teams\Entity\TeamAsset', 'ta')
- ->where('ta.team = :team_id')
- ->setParameter('team_id', $team_id)
- ->getQuery()
- ->getResult();
-
$post_data = $request->getPost();
if (!$this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team_user', $team_id)) {
@@ -458,42 +292,103 @@ public function teamUpdateAction()
->getQuery()
->execute();
}
+ if (!$this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team_user', $team_id)) {
+ $this->messenger()->addError("You aren't authorized to change team members");
+ return $view;
+ } else {
+ $teamUsers = $request->getPost('o:team_users');
+ //remove team users not in the form
+ $formTeamUsers = array();
+ foreach ($teamUsers as $teamUser){
+ $formTeamUsers[] = $teamUser['o:user']['o:id'];
+ }
-
- if (!$this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team_user', $team_id)) {
- $this->messenger()->addError("You aren't authorized to change team members");
- return $view;
- } else {
- $current_users = $this->entityManager->getRepository('Teams\Entity\TeamUser')->findBy(['team' => $team_id]);
-
- foreach ($current_users as $team_user) {
- $this->entityManager->remove($team_user);
+ $oldTeamUsers= $this->api()->search('team-user', ['team'=>$team_id], ['returnScalar'=>'user'])->getContent();
+ foreach ($oldTeamUsers as $oldTeamUser) {
+ if (!in_array($oldTeamUser,$formTeamUsers)) {
+ $this->api()->delete('team-user',['team'=>$team_id, 'user'=>$oldTeamUser]);
}
- $this->entityManager->flush();
-
- foreach ($request->getPost('o:team_users') as $team_user):
- $user = $this->entityManager->getRepository('Omeka\Entity\User')
- ->findOneBy(['id' => (int)$team_user['o:user']['o:id']]);
- $role = $this->entityManager->getRepository('Teams\Entity\TeamRole')
- ->findOneBy(['id' => (int)$team_user['o:team_role']['o:id']]);
-
- $teamUser = new TeamUser($team, $user, $role);
- $teamUser->setCurrent(null);
- $this->entityManager->persist($teamUser);
- endforeach;
- $this->entityManager->flush();
-
}
+ //add team users or update permissions
+ foreach ($teamUsers as $teamUser) {
+ //using search instead of read because read will throw a not found error instead of returning empty
+ $teamUserExists = $this->api()->search('team-user', ['team'=>$team_id, 'user'=>$teamUser['o:user']['o:id']])->getContent();
+
+ if ($teamUserExists){
+ $role = $this->api()->read('team-role',['id'=>$teamUser['o:team_role']['o:id']])->getContent();
+ $teamUserExists[0]->getEntity()->setRole($role->getEntity());
+ } else {
+ $this->api()
+ ->create('team-user',
+ [
+ 'team'=>$team_id,
+ 'user'=>$teamUser['o:user']['o:id'],
+ 'role'=>$teamUser['o:team_role']['o:id']
+ ]);
+ }
+ }
+ }
+ //TODO:need to update this for users who have the update and bypass_team_filter
if (! $this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team', $team_id)){
$this->messenger()->addError("You aren't authorized to change this team");
return $view;
} else {
- //first delete then add resources to team
- $this->processResources($request, $team, $existing_resources, $existing_resource_templates, $existing_assets, true);
- $this->processResources($request, $team, $existing_resources, $existing_resource_templates, $existing_assets, false);
+ //process items
+ $formData = $this->params()->fromPost();
+ $resourceForm->setData($formData);
+ parse_str($formData['item_pool'], $itemPool);
+ if ($formData['item_assignment_action'] && $formData['item_assignment_action'] !== 'no_action') {
+ $this->jobDispatcher()->dispatch('Teams\Job\UpdateTeamResources', [
+ 'teams' => [$team_id => $itemPool],
+ 'action' => $formData['item_assignment_action'],
+ ]);
+ $this->messenger()->addSuccess('Item assignment in progress. To see the new item count, refresh the page.'); // @translate
+ }
+
+ //process item sets and resource templates
+ //remove item sets
+ $secondaryResourcesForm->setData($formData);
+
+ $recursive = $formData['recursive_item_sets'] ?? false;
+ if (isset($formData['remove_item_sets'])){
+ $remove_item_sets = $formData['remove_item_sets'];
+ foreach ($remove_item_sets as $item_set_id) {
+ //todo: delete expects the id to be in the second parameter, for now just leaving empty because team resource uses a composite key
+ $this->api()->delete('team-resource', [], ['team' => $team_id, 'resource' => $item_set_id],['recursive'=>$recursive, 'syncSites' => true]);
+ }
+ }
+ //add item sets
+ if (isset($formData['item_sets'])) {
+ foreach ($formData['item_sets'] as $item_set_id) {
+ //todo: implement the read operation
+ $exists = $this->api()->search('team-resource', ['team' => $team_id, 'resource' => $item_set_id]);
+ if (count($exists->getContent()) < 1) {
+ $this->api()->create('team-resource', ['team' => $team_id, 'resource' => $item_set_id], [], ['recursive' => $recursive, 'syncSites' => true]);
+ }
+ }
+ }
+
+ //remove resource templates
+ $secondaryResourcesForm->setData($formData);
+ if (isset($formData['remove_resource_templates'])){
+ foreach ($formData['remove_resource_templates'] as $resource_template_id) {
+ //todo: delete expects the id to be in the second parameter, for now just leaving empty because team resource uses a composite key
+ $this->api()->delete('team-resource-template', [], ['team' => $team_id, 'resource-template' => $resource_template_id]);
+ }
+ }
+ //add resource templates
+ if (isset($formData['resource_templates'])){
+ foreach ($formData['resource_templates'] as $resource_template_id) {
+ if (count($this->api()->search('team-resource-template', ['team'=>$team_id, 'resource-template'=>$resource_template_id])->getContent())<1){
+ $this->api()->create('team-resource-template', ['team'=>$team_id, 'resource-template'=>$resource_template_id]);
+ }
+ }
+ }
+
+ $em = $this->entityManager;
//handle new sites
foreach ($post_data['teamSites']['o:site'] as $site) {
if (!in_array($site, $current_sites)) {
@@ -536,10 +431,6 @@ public function teamUpdateAction()
return $this->redirect()->toRoute('admin/teams/detail',['id'=>$team_id]);
}
- public function roleUpdateAction()
- {
- }
-
public function userAction()
{
$request = $this->getRequest();
From eb4c007a8d99b348480790fb5201f810a6abbf30 Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 19:24:21 -0400
Subject: [PATCH 16/54] fix: restore to pre feat state
---
src/Controller/UpdateController.php | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index 57bac863..b4248ad6 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -14,7 +14,6 @@
use Teams\Form\TeamResourcesForm;
use Teams\Form\TeamSitesAddRemoveForm;
use Teams\Form\TeamDetailsForm;
-use Teams\Service\SitePermissionManager;
use Laminas\EventManager\Event;
use Laminas\Mvc\Controller\AbstractActionController;
use Laminas\Stdlib\ArrayObject;
@@ -28,19 +27,12 @@ class UpdateController extends AbstractActionController
*/
protected $entityManager;
- /**
- * @var SitePermissionManager
- */
- protected SitePermissionManager $sitePermissionManager;
-
/**
* @param EntityManager $entityManager
- * @param SitePermissionManager $sitePermissionManager
*/
- public function __construct(EntityManager $entityManager, SitePermissionManager $sitePermissionManager)
+ public function __construct(EntityManager $entityManager)
{
$this->entityManager = $entityManager;
- $this->sitePermissionManager = $sitePermissionManager;
}
public function createNamedParameter(
@@ -421,10 +413,6 @@ public function teamUpdateAction()
$em->flush();
}
- // Sync all site permissions now that all role and site changes are persisted,
- // using the same logic as the sync button to guarantee correctness.
- $this->sitePermissionManager->syncAllSitePermissions();
-
$successMessage = sprintf("Successfully updated the %s team", $team->getName());
$this->messenger()->addSuccess($successMessage);
From 2c4bea1e4a8a504cff91a675f8e29cada2fc9fe1 Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 19:33:41 -0400
Subject: [PATCH 17/54] fix: single sync at the end
---
src/Controller/UpdateController.php | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index b4248ad6..5375ed5e 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -18,6 +18,7 @@
use Laminas\Mvc\Controller\AbstractActionController;
use Laminas\Stdlib\ArrayObject;
use Laminas\View\Model\ViewModel;
+use Teams\Service\SitePermissionManager;
class UpdateController extends AbstractActionController
@@ -27,12 +28,19 @@ class UpdateController extends AbstractActionController
*/
protected $entityManager;
+ /**
+ * @var SitePermissionManager
+ */
+ protected SitePermissionManager $sitePermissionManager;
+
/**
* @param EntityManager $entityManager
+ * @param SitePermissionManager $sitePermissionManager
*/
- public function __construct(EntityManager $entityManager)
+ public function __construct(EntityManager $entityManager, SitePermissionManager $sitePermissionManager)
{
$this->entityManager = $entityManager;
+ $this->sitePermissionManager = $sitePermissionManager;
}
public function createNamedParameter(
@@ -412,7 +420,7 @@ public function teamUpdateAction()
}
$em->flush();
}
-
+ $this->sitePermissionManager->syncAllSitePermissions();
$successMessage = sprintf("Successfully updated the %s team", $team->getName());
$this->messenger()->addSuccess($successMessage);
From ebe86eb184a14e6ed0459d75f99994f589401630 Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 20:29:47 -0400
Subject: [PATCH 18/54] fix: add logging
---
src/Service/SitePermissionManager.php | 25 +++++++++++++++++---
src/Service/SitePermissionManagerFactory.php | 12 +++++++++-
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index f957b4a6..3eaec264 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -4,6 +4,7 @@
use Doctrine\Common\Collections\Criteria;
use Doctrine\ORM\EntityManager;
use Omeka\Entity\SitePermission;
+use Omeka\Mvc\Controller\Plugin\Logger;
use Omeka\Settings\UserSettings;
/**
@@ -31,10 +32,22 @@ class SitePermissionManager
*/
private UserSettings $userSettings;
- public function __construct(EntityManager $entityManager, UserSettings $userSettings)
+ /**
+ * @var Logger
+ */
+ protected $logger;
+
+ /**
+ * @param EntityManager $entityManager
+ * @param UserSettings $userSettings
+ * @param Logger $logger
+ */
+
+ public function __construct(EntityManager $entityManager, UserSettings $userSettings, Logger $logger)
{
$this->entityManager = $entityManager;
$this->userSettings = $userSettings;
+ $this->logger = $logger;
}
/**
@@ -61,9 +74,15 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$em->refresh($teamUser);
$user = $teamUser->getUser();
- $omekaRole = $teamUser->getRole()->getCanAddSitePages()
+ $omekaSiteRole = $teamUser->getRole()->getCanAddSitePages()
? SitePermission::ROLE_ADMIN
: SitePermission::ROLE_VIEWER;
+ $this->logger->info(sprintf(
+ 'Syncing Omeka site permissions for user %d in team %d: setting role %s for all team sites.',
+ $userId,
+ $teamId,
+ $omekaSiteRole
+ ));
$teamSites = $em->getRepository('Teams\Entity\TeamSite')->findBy(['team' => $teamId]);
@@ -75,7 +94,7 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$existingPermission = $sitePermissions->matching($criteria)->first();
if ($existingPermission) {
- $existingPermission->setRole($omekaRole);
+ $existingPermission->setRole($omekaSiteRole);
} else {
$sitePermission = new SitePermission();
$sitePermission->setSite($site);
diff --git a/src/Service/SitePermissionManagerFactory.php b/src/Service/SitePermissionManagerFactory.php
index 8aeff254..704bf316 100644
--- a/src/Service/SitePermissionManagerFactory.php
+++ b/src/Service/SitePermissionManagerFactory.php
@@ -6,11 +6,21 @@
class SitePermissionManagerFactory implements FactoryInterface
{
+ /**
+ *
+ * @param ContainerInterface $container
+ * @param $requestedName
+ * @param array|null $options
+ * @return SitePermissionManager
+ */
public function __invoke(ContainerInterface $container, $requestedName, array $options = null)
{
+ $logger = $container->get('Omeka\Logger');
+
return new SitePermissionManager(
$container->get('Omeka\EntityManager'),
- $container->get('Omeka\Settings\User')
+ $container->get('Omeka\Settings\User'),
+ $container->get('Omeka\Logger')
);
}
}
From 29682df05e2a9f386eb248f8de90a0b0c6b27310 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 11 Aug 2026 00:50:02 +0000
Subject: [PATCH 19/54] Add detailed debug logging and fix $omekaRole typo in
SitePermissionManager
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Service/SitePermissionManager.php | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index 3eaec264..6cb37528 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -74,13 +74,18 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$em->refresh($teamUser);
$user = $teamUser->getUser();
- $omekaSiteRole = $teamUser->getRole()->getCanAddSitePages()
+ $teamRole = $teamUser->getRole();
+ $canAddSitePages = $teamRole->getCanAddSitePages();
+ $omekaSiteRole = $canAddSitePages
? SitePermission::ROLE_ADMIN
: SitePermission::ROLE_VIEWER;
$this->logger->info(sprintf(
- 'Syncing Omeka site permissions for user %d in team %d: setting role %s for all team sites.',
+ '[SitePermissionManager] syncSitePermissionsForUser: userId=%d, teamId=%d, teamRoleId=%d, teamRoleName="%s", canAddSitePages=%s => omekaSiteRole="%s"',
$userId,
$teamId,
+ $teamRole->getId(),
+ $teamRole->getName(),
+ $canAddSitePages ? 'true' : 'false',
$omekaSiteRole
));
@@ -94,12 +99,25 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$existingPermission = $sitePermissions->matching($criteria)->first();
if ($existingPermission) {
+ $this->logger->info(sprintf(
+ '[SitePermissionManager] siteId=%d: updating existing SitePermission from "%s" to "%s" for userId=%d',
+ $site->getId(),
+ $existingPermission->getRole(),
+ $omekaSiteRole,
+ $userId
+ ));
$existingPermission->setRole($omekaSiteRole);
} else {
+ $this->logger->info(sprintf(
+ '[SitePermissionManager] siteId=%d: creating new SitePermission with role "%s" for userId=%d',
+ $site->getId(),
+ $omekaSiteRole,
+ $userId
+ ));
$sitePermission = new SitePermission();
$sitePermission->setSite($site);
$sitePermission->setUser($user);
- $sitePermission->setRole($omekaRole);
+ $sitePermission->setRole($omekaSiteRole);
$em->persist($sitePermission);
$sitePermissions->add($sitePermission);
}
From a060432b3b0b832de938c788b6d671e215fa89f5 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 11 Aug 2026 00:50:53 +0000
Subject: [PATCH 20/54] Fix Logger type hint: use Laminas\Log\Logger instead of
controller plugin
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Service/SitePermissionManager.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index 6cb37528..65de0e3c 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -4,7 +4,7 @@
use Doctrine\Common\Collections\Criteria;
use Doctrine\ORM\EntityManager;
use Omeka\Entity\SitePermission;
-use Omeka\Mvc\Controller\Plugin\Logger;
+use Laminas\Log\Logger;
use Omeka\Settings\UserSettings;
/**
From 759efcc21ad9f74594589be2344073d534e78ba4 Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 21:00:00 -0400
Subject: [PATCH 21/54] chore: detailed logging
---
src/Service/SitePermissionManager.php | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index 65de0e3c..1979803a 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -76,17 +76,19 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$user = $teamUser->getUser();
$teamRole = $teamUser->getRole();
$canAddSitePages = $teamRole->getCanAddSitePages();
+ $rawCanAddSitePages = $teamRole->getRawCanAddSitePages();
$omekaSiteRole = $canAddSitePages
? SitePermission::ROLE_ADMIN
: SitePermission::ROLE_VIEWER;
$this->logger->info(sprintf(
- '[SitePermissionManager] syncSitePermissionsForUser: userId=%d, teamId=%d, teamRoleId=%d, teamRoleName="%s", canAddSitePages=%s => omekaSiteRole="%s"',
+ '[SitePermissionManager] syncSitePermissionsForUser: userId=%d, teamId=%d, teamRoleId=%d, teamRoleName="%s", canAddSitePages=%s => omekaSiteRole="%s", rawCanAddSitePages=%s',
$userId,
$teamId,
$teamRole->getId(),
$teamRole->getName(),
$canAddSitePages ? 'true' : 'false',
- $omekaSiteRole
+ $omekaSiteRole,
+ $rawCanAddSitePages
));
$teamSites = $em->getRepository('Teams\Entity\TeamSite')->findBy(['team' => $teamId]);
From 9a0a4e8009813ac664bf570e5a41ea4af4f5612a Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 21:02:18 -0400
Subject: [PATCH 22/54] fix: typo
---
src/Service/SitePermissionManager.php | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index 1979803a..fd1cb150 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -76,7 +76,6 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$user = $teamUser->getUser();
$teamRole = $teamUser->getRole();
$canAddSitePages = $teamRole->getCanAddSitePages();
- $rawCanAddSitePages = $teamRole->getRawCanAddSitePages();
$omekaSiteRole = $canAddSitePages
? SitePermission::ROLE_ADMIN
: SitePermission::ROLE_VIEWER;
@@ -88,7 +87,7 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$teamRole->getName(),
$canAddSitePages ? 'true' : 'false',
$omekaSiteRole,
- $rawCanAddSitePages
+ $canAddSitePages
));
$teamSites = $em->getRepository('Teams\Entity\TeamSite')->findBy(['team' => $teamId]);
From ff455a5c3bc6ebc8291e10fb6c005ac6762392be Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 21:03:02 -0400
Subject: [PATCH 23/54] fix: typo
---
src/Service/SitePermissionManager.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index fd1cb150..c5b7405b 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -79,7 +79,7 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$omekaSiteRole = $canAddSitePages
? SitePermission::ROLE_ADMIN
: SitePermission::ROLE_VIEWER;
- $this->logger->info(sprintf(
+ $this->logger->err(sprintf(
'[SitePermissionManager] syncSitePermissionsForUser: userId=%d, teamId=%d, teamRoleId=%d, teamRoleName="%s", canAddSitePages=%s => omekaSiteRole="%s", rawCanAddSitePages=%s',
$userId,
$teamId,
From 5fcd49ad7ca2551247396755d0622371826679d4 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 11 Aug 2026 01:22:25 +0000
Subject: [PATCH 24/54] Fix partial DQL polluting TeamRole identity map with
incomplete entities
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Controller/UpdateController.php | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index 5375ed5e..fc32c9e3 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -232,9 +232,14 @@ public function teamUpdateAction()
$available_u_array = array_diff($all_u_array, $team_u_array);
//TODO (refactor) was trying to see if there was an easier way to get these objects into an array but consistency is more important
- $role_query = $this->entityManager->createQuery('select partial r.{id, name} from Teams\Entity\TeamRole r');
- $roles = $role_query->getResult();
- $roles_array = $role_query->getResult(\Doctrine\ORM\Query::HYDRATE_ARRAY);
+ // Use HYDRATE_ARRAY so that TeamRole entities are not partially loaded
+ // into the Doctrine identity map. A partial select (partial r.{id, name})
+ // caches TeamRole objects with only id/name populated; any later access
+ // via lazy-loading (e.g. getCanAddSitePages()) then returns the empty
+ // default instead of the real database value.
+ $role_query = $this->entityManager->createQuery('select r from Teams\Entity\TeamRole r');
+ $roles_array = $role_query->getResult(\Doctrine\ORM\Query::HYDRATE_ARRAY);
+ $roles = $roles_array;
//create an array object to hold the contents to pre-fill the form with
//TODO (emulate) this is the procedure to use to populate forms. Copy this.
From 0980830d73f2cfd5f7e21a873b3246a73d2ea3ab Mon Sep 17 00:00:00 2001
From: Alex Dryden
Date: Mon, 10 Aug 2026 21:58:11 -0400
Subject: [PATCH 25/54] refactor: old, weird code with api calls mostly
---
src/Controller/UpdateController.php | 120 +---------------------------
1 file changed, 3 insertions(+), 117 deletions(-)
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index fc32c9e3..a58665c4 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -185,32 +185,7 @@ public function teamUpdateAction()
$user = $this->identity();
//TODO rename this to TeamDetail form or find a way to string these all together
$teamDetailsForm = $this->getForm(TeamDetailsForm::class);
-
- //TODO: get team with a one line entity manager call
- $criteria = ['id' => $team_id];
- $qb = $this->entityManager->createQueryBuilder();
- $entityClass = 'Teams\Entity\Team';
-
- $qb->select('omeka_root')->from($entityClass, 'omeka_root');
- foreach ($criteria as $field => $value) {
- $qb->andWhere($qb->expr()->eq(
- "omeka_root.$field",
- $this->createNamedParameter($qb, $value)
- ));
- }
- $qb->setMaxResults(1);
-
- $entity = $qb->getQuery()->getOneOrNullResult();
-
-
$data = $this->api()->read('team', ['id'=>$team_id])->getContent();
- $request = new Request('update', 'team');
- $event = new Event('api.hydrate.pre', $this, [
- 'entity' => $entity,
- 'request' => $request,
- ]);
- $this->getEventManager()->triggerEvent($event);
-
//TODO (refactor) this is probably a stupid way to do this
@@ -231,15 +206,7 @@ public function teamUpdateAction()
//get the users available to be added to the team
$available_u_array = array_diff($all_u_array, $team_u_array);
- //TODO (refactor) was trying to see if there was an easier way to get these objects into an array but consistency is more important
- // Use HYDRATE_ARRAY so that TeamRole entities are not partially loaded
- // into the Doctrine identity map. A partial select (partial r.{id, name})
- // caches TeamRole objects with only id/name populated; any later access
- // via lazy-loading (e.g. getCanAddSitePages()) then returns the empty
- // default instead of the real database value.
- $role_query = $this->entityManager->createQuery('select r from Teams\Entity\TeamRole r');
- $roles_array = $role_query->getResult(\Doctrine\ORM\Query::HYDRATE_ARRAY);
- $roles = $roles_array;
+ $roles = $this->api()->search('team-role')->getContent();
//create an array object to hold the contents to pre-fill the form with
//TODO (emulate) this is the procedure to use to populate forms. Copy this.
@@ -285,17 +252,7 @@ public function teamUpdateAction()
$this->messenger()->addError("You aren't authorized to change the team details");
return $view;
} else {
- //first update the team name and description
- $qb = $this->entityManager->createQueryBuilder();
- $qb->update('Teams\Entity\Team', 'team')
- ->set('team.name', '?1')
- ->set('team.description', '?2')
- ->where('team.id = ?3')
- ->setParameter(1, $post_data['o:name'])
- ->setParameter(2, $post_data['o:description'])
- ->setParameter(3, $team_id)
- ->getQuery()
- ->execute();
+ $this->api()->update('team', $team_id, ['o:name' => $post_data['o:name'], 'o:description' => $post_data['o:description']]);
}
if (!$this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team_user', $team_id)) {
$this->messenger()->addError("You aren't authorized to change team members");
@@ -320,8 +277,7 @@ public function teamUpdateAction()
$teamUserExists = $this->api()->search('team-user', ['team'=>$team_id, 'user'=>$teamUser['o:user']['o:id']])->getContent();
if ($teamUserExists){
- $role = $this->api()->read('team-role',['id'=>$teamUser['o:team_role']['o:id']])->getContent();
- $teamUserExists[0]->getEntity()->setRole($role->getEntity());
+ $this->api()->update('team-user', ['team' => $team_id, 'user' => $teamUser['o:user']['o:id']], ['role' => $teamUser['o:team_role']['o:id']]);
} else {
$this->api()
->create('team-user',
@@ -431,74 +387,4 @@ public function teamUpdateAction()
return $this->redirect()->toRoute('admin/teams/detail',['id'=>$team_id]);
}
-
- public function userAction()
- {
- $request = $this->getRequest();
- if ($request->isPost()) {
- $data = $request->getPost();
- $user_teams = $data['user-information']['o-module-teams:Team'];
- //wrong!! not really able to get from the param, would need to extract from the return url
- $user_id = $this->params('id');
- $em = $this->entityManager;
-
- foreach ($user_teams as $team_id):
- $team = $em->getRepository('Teams\Entity\Team')->findOneBy(['id' => $team_id]);
- $user = $em->getRepository('Omeka\Entity\User')->findOneBy(['id'=>$user_id]);
- $role = $em->getRepository('Teams\Entity\TeamRole')->findOneBy(['id' => 1]);
- $team_user = new TeamUser($team, $user, $role);
- $team_user->setCurrent(null);
- $em->persist($team_user);
- endforeach;
- $em->flush();
- $request = $this->getRequest();
- $return = $request->getHeader('referer');
-// return $this->redirect()->toUrl($data['return_url']);
- return $this->redirect()->toUrl($return);
- }
- }
-
- public function currentTeamAction()
- {
- $user_id = $this->identity()->getId();
- $request = $this->getRequest();
-
- if ($request->isPost()) {
- $data = $request->getPost();
-
- $em = $this->entityManager;
- $team_user = $em->getRepository('Teams\Entity\TeamUser');
- $old_current = $team_user->findOneBy(['user' => $user_id, 'is_current' => 1]);
- $new_current = $team_user->findOneBy(['user'=> $user_id, 'team'=>$data['team_id']]);
-
- if ($old_current) {
- $old_current->setCurrent(null);
- $em->flush();
- }
- if ($new_current) {
- $new_current->setCurrent(true);
- $em->flush();
- $team = $new_current->getTeam();
-
- //the sites for the team the user just switched to
-
- $team_sites = $team->getTeamSites();
- $site_ids = [];
- foreach ($team_sites as $team_site):
- $site_ids[] = strval($team_site->getSite()->getId());
- endforeach;
-
- //update so those are the user's default sites for items
- $settingId = 'default_item_sites';
- $settingValue = $site_ids;
- $this->userSettings()->set($settingId, $settingValue, $user_id);
- } else {
- $this->messenger()->addError("Team not found");
- }
-
-
-
- return $this->redirect()->toUrl($data['return_url']);
- }
- }
}
From 75e6cd9c0b3e9696eba93b207faa9eb93dab0e09 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 11 Aug 2026 02:16:59 +0000
Subject: [PATCH 26/54] Refactor controllers to use API; clean up dead imports
and dead code
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Api/Adapter/AbstractTeamEntityAdapter.php | 2 +-
src/Api/Adapter/TeamAdapter.php | 2 +-
src/Api/Adapter/TeamRoleAdapter.php | 4 +-
src/Api/Adapter/TeamSiteAdapter.php | 2 +-
src/Api/Adapter/TeamUserAdapter.php | 19 ++-
src/Controller/AddController.php | 19 +--
src/Controller/DeleteController.php | 31 +---
src/Controller/IndexController.php | 58 +------
src/Controller/UpdateController.php | 157 ++++++++----------
9 files changed, 110 insertions(+), 184 deletions(-)
diff --git a/src/Api/Adapter/AbstractTeamEntityAdapter.php b/src/Api/Adapter/AbstractTeamEntityAdapter.php
index 5974b328..1daa6d34 100644
--- a/src/Api/Adapter/AbstractTeamEntityAdapter.php
+++ b/src/Api/Adapter/AbstractTeamEntityAdapter.php
@@ -301,7 +301,7 @@ public function batchDelete(Request $request)
public function update(Request $request)
{
- AbstractAdapter::update($request);
+ return parent::update($request);
}
public function batchUpdate(Request $request)
diff --git a/src/Api/Adapter/TeamAdapter.php b/src/Api/Adapter/TeamAdapter.php
index 9918de6d..5715f295 100644
--- a/src/Api/Adapter/TeamAdapter.php
+++ b/src/Api/Adapter/TeamAdapter.php
@@ -183,7 +183,7 @@ public function batchCreate(Request $request)
public function update(Request $request)
{
- AbstractAdapter::batchCreate($request);
+ return parent::update($request);
}
public function batchUpdate(Request $request)
diff --git a/src/Api/Adapter/TeamRoleAdapter.php b/src/Api/Adapter/TeamRoleAdapter.php
index 6702a03d..69c12f4d 100644
--- a/src/Api/Adapter/TeamRoleAdapter.php
+++ b/src/Api/Adapter/TeamRoleAdapter.php
@@ -224,7 +224,7 @@ public function batchCreate(Request $request)
public function update(Request $request)
{
- AbstractAdapter::batchCreate($request);
+ return parent::update($request);
}
public function batchUpdate(Request $request)
@@ -234,7 +234,7 @@ public function batchUpdate(Request $request)
public function delete(Request $request)
{
- AbstractAdapter::delete($request);
+ return parent::delete($request);
}
public function batchDelete(Request $request)
diff --git a/src/Api/Adapter/TeamSiteAdapter.php b/src/Api/Adapter/TeamSiteAdapter.php
index 1417087e..cfbbd26a 100644
--- a/src/Api/Adapter/TeamSiteAdapter.php
+++ b/src/Api/Adapter/TeamSiteAdapter.php
@@ -120,7 +120,7 @@ public function batchCreate(Request $request)
public function update(Request $request)
{
- AbstractTeamEntityAdapter::batchCreate($request);
+ return parent::update($request);
}
public function batchUpdate(Request $request)
diff --git a/src/Api/Adapter/TeamUserAdapter.php b/src/Api/Adapter/TeamUserAdapter.php
index afb0f491..6bf1a8d0 100644
--- a/src/Api/Adapter/TeamUserAdapter.php
+++ b/src/Api/Adapter/TeamUserAdapter.php
@@ -68,11 +68,18 @@ public function hydrate(
}
}
- if ($this->shouldHydrate($request, 'o:role')) {
- $role = $request->getValue('o:role');
- if (!is_null($role)) {
- $role = trim($role);
- $entity->setRole($role);
+ // Accept 'role' (plain key used by update) or 'o:role' (JSON-LD key).
+ // Both must resolve to a TeamRole entity before setting.
+ $roleId = null;
+ if ($this->shouldHydrate($request, 'role')) {
+ $roleId = $request->getValue('role');
+ } elseif ($this->shouldHydrate($request, 'o:role')) {
+ $roleId = $request->getValue('o:role');
+ }
+ if (!is_null($roleId)) {
+ $roleEntity = $this->getEntityManager()->find('Teams\Entity\TeamRole', (int) $roleId);
+ if ($roleEntity) {
+ $entity->setRole($roleEntity);
}
}
@@ -184,7 +191,7 @@ public function batchCreate(Request $request)
public function update(Request $request)
{
- AbstractAdapter::batchCreate($request);
+ return parent::update($request);
}
public function batchUpdate(Request $request)
diff --git a/src/Controller/AddController.php b/src/Controller/AddController.php
index 6c7bc523..e7dc122a 100644
--- a/src/Controller/AddController.php
+++ b/src/Controller/AddController.php
@@ -12,7 +12,6 @@
use Teams\Entity\TeamResource;
use Teams\Entity\TeamResourceTemplate;
use Teams\Entity\TeamSite;
-use Teams\Entity\TeamUser;
use Teams\Form\SecondaryResourcesForm;
use Teams\Form\TeamItemSetForm;
use Teams\Form\TeamResourcesForm;
@@ -85,17 +84,13 @@ public function teamAddAction()
$teamEntity = $this->entityManager->getRepository('Teams\Entity\Team')
->findOneBy(['id' => (int)$newTeam->getContent()->id()]);
if ($request->getPost('o:team_users')) {
- foreach ($request->getPost('o:team_users') as $team_user):
- $user = $this->entityManager->getRepository('Omeka\Entity\User')
- ->findOneBy(['id' => (int)$team_user['o:user']['o:id']]);
- $role = $this->entityManager->getRepository('Teams\Entity\TeamRole')
- ->findOneBy(['id' => (int)$team_user['o:team_role']['o:id']]);
-
- $teamUser = new TeamUser($teamEntity, $user, $role);
- $teamUser->setCurrent(null);
- $this->entityManager->persist($teamUser);
- endforeach;
- $this->entityManager->flush();
+ foreach ($request->getPost('o:team_users') as $team_user) {
+ $this->api()->create('team-user', [
+ 'team' => $newTeam->getContent()->id(),
+ 'user' => (int) $team_user['o:user']['o:id'],
+ 'role' => (int) $team_user['o:team_role']['o:id'],
+ ]);
+ }
}
//persist the sites (no possibility of duplicates, so don't need to save to associative array)
diff --git a/src/Controller/DeleteController.php b/src/Controller/DeleteController.php
index dd7040d5..b933dbb6 100644
--- a/src/Controller/DeleteController.php
+++ b/src/Controller/DeleteController.php
@@ -2,10 +2,7 @@
namespace Teams\Controller;
use Doctrine\ORM\EntityManager;
-use Doctrine\ORM\QueryBuilder;
use Omeka\Api\Exception\InvalidArgumentException;
-use Omeka\Api\Request;
-use Laminas\EventManager\Event;
use Laminas\Mvc\Controller\AbstractActionController;
use Laminas\View\Model\ViewModel;
@@ -23,17 +20,6 @@ public function __construct(EntityManager $entityManager)
{
$this->entityManager = $entityManager;
}
- public function createNamedParameter(
- QueryBuilder $qb,
- $value,
- $prefix = 'omeka_'
- ) {
- $index = 0;
- $placeholder = $prefix . $index;
- $index++;
- $qb->setParameter($placeholder, $value);
- return ":$placeholder";
- }
public function teamDeleteAction()
{
//is there an id?
@@ -75,13 +61,10 @@ public function roleDeleteAction()
{
$user = $this->identity()->getRole();
$id = $this->params()->fromRoute('id');
- $role = $this->entityManager->getRepository('Teams\Entity\TeamRole')
- ->findOneBy(['id'=> $id]);
$request = $this->getRequest();
- //test to see if anyone has this role. If they do, don't delete it.
$role_users = $this->entityManager->getRepository('Teams\Entity\TeamUser')
- ->findBy(['role'=>$id]);
+ ->findBy(['role' => $id]);
$view = new ViewModel(
[
'role_users' => $role_users,
@@ -96,17 +79,17 @@ public function roleDeleteAction()
$this->messenger()->addError('You are not authorized to delete roles');
return $view;
}
- if ($role_users){
+ if ($role_users) {
$this->messenger()->addError('This role can not be deleted while users are assigned to it');
return $view;
}
if ($request->getPost('confirm') == 'Delete') {
- $this->entityManager->remove($role);
- $this->entityManager->flush();
- $this->messenger()->addSuccess(sprintf('Successfully deleted role "%s"', $role->getName()));
+ $role = $this->entityManager->getRepository('Teams\Entity\TeamRole')
+ ->findOneBy(['id' => $id]);
+ $roleName = $role ? $role->getName() : $id;
+ $this->api()->delete('team-role', $id);
+ $this->messenger()->addSuccess(sprintf('Successfully deleted role "%s"', $roleName));
}
return $this->redirect()->toRoute('admin/teams/roles');
-
-
}
}
diff --git a/src/Controller/IndexController.php b/src/Controller/IndexController.php
index ac3174b4..181d4b5a 100644
--- a/src/Controller/IndexController.php
+++ b/src/Controller/IndexController.php
@@ -2,7 +2,6 @@
namespace Teams\Controller;
use Doctrine\ORM\EntityManager;
-use Omeka\Api\Request;
use Omeka\Form\ConfirmForm;
use Laminas\EventManager\Event;
use Laminas\Form\Form;
@@ -160,58 +159,19 @@ public function deleteAction()
$form = $this->getForm(ConfirmForm::class);
$form->setData($this->getRequest()->getPost());
if ($form->isValid()) {
- $entityManager = $this->entityManager;
$user_id = $this->identity()->getId();
- $team_id = $entityManager
+ $team_id = $this->entityManager
->getRepository('Teams\Entity\TeamUser')
- ->findOneBy(['is_current'=>true, 'user'=>$user_id])
+ ->findOneBy(['is_current' => true, 'user' => $user_id])
->getTeam()->getId();
- //array of media ids
- $media_ids = [];
-
- //date to update last modified
- $datetime = new \DateTime('now');
- foreach ($this->api()->read('items', $this->params('id'))->getContent()->media() as $media):
- $media_ids[] = $media->id();
- endforeach;
-
- $entity = $entityManager
- ->getRepository('Teams\Entity\TeamResource')
- ->findOneBy(['team'=>$team_id, 'resource'=> $this->params('id')]);
-
- $request = new Request('delete', 'team_resource');
- $event = new Event('api.hydrate.pre', $this, [
- 'entity' => $entity,
- 'request' => $request,
- ]);
- $this->getEventManager()->triggerEvent($event);
- if ($entity) {
- $entity->getResource()->setModified($datetime);
- $entityManager->remove($entity);
-
- //remove associated media from the team
- foreach ($media_ids as $media_id):
- $tr = $entityManager->getRepository('Teams\Entity\TeamResource')
- ->findOneBy(['team' => $team_id, 'resource' => $media_id]);
- if ($tr) {
- $entityManager->remove($tr);
- $this->messenger()->addSuccess('Associated Media successfully removed from your team.'); // @translate
- $entityManager->getRepository('Omeka\Entity\Resource')
- ->findOneBy(['id'=>$media_id])->setModified($datetime);
- }
- endforeach;
- $entityManager->flush();
- $this->messenger()->addSuccess('Item successfully removed from your team. Item remains available to other teams if they are linked to it.'); // @translate
- } else {
- $this->messenger()->addError('something went wrong'); // @translate
- }
- $event = new Event('api.execute.post', $this, [
- 'entity' => $entity,
- 'request' => $request,
- ]);
- $this->getEventManager()->triggerEvent($event);
-
+ $this->api()->delete(
+ 'team-resource',
+ [],
+ ['team' => $team_id, 'resource' => $this->params('id')],
+ ['recursive' => true]
+ );
+ $this->messenger()->addSuccess('Item successfully removed from your team. Item remains available to other teams if they are linked to it.'); // @translate
} else {
$this->messenger()->addFormErrors($form);
}
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index a58665c4..94d1a218 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -5,7 +5,6 @@
use Doctrine\ORM\NonUniqueResultException;
use Doctrine\ORM\OptimisticLockException;
use Doctrine\ORM\ORMException;
-use Doctrine\ORM\QueryBuilder;
use Omeka\Api\Request;
use Teams\Entity\TeamSite;
use Teams\Entity\TeamUser;
@@ -43,91 +42,6 @@ public function __construct(EntityManager $entityManager, SitePermissionManager
$this->sitePermissionManager = $sitePermissionManager;
}
- public function createNamedParameter(
- QueryBuilder $qb,
- $value,
- $prefix = 'omeka_'
- ) {
- $index = 0;
- $placeholder = $prefix . $index;
- $index++;
- $qb->setParameter($placeholder, $value);
- return ":$placeholder";
- }
-
- public function addTeamUser(int $team_id, int $user_id, int $role_id)
- {
- if (! $this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team', $team_id)){
- $this->messenger()->addError("You aren't authorized to change this team");
- return null;
- } else {
- $team = $this->entityManager->find('Teams\Entity\Team', $team_id);
- $user = $this->entityManager->find('Omeka\Entity\User', $user_id);
- $role = $this->entityManager->find('Teams\Entity\TeamRole', $role_id);
- $team_user = new TeamUser($team, $user, $role);
- $this->entityManager->persist($team_user);
-
- //flushing here because this is a mini-form and we want to see the name pop up
- //more efficient solution would be to have JS handle the popping and batch update
- $this->entityManager->flush();
- return $team_user;
- }
- }
-
- public function removeTeamUser(int $team_id, int $user)
- {
- if (! $this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team', $team_id)){
- $this->messenger()->addError("You aren't authorized to change this team");
- } else {
- $this->messenger()->addError("removed user");
-
- $em = $this->entityManager;
- $team_user = $em->find('Teams\Entity\TeamUser', ['team' => $team_id, 'user' => $user]);
- $em->remove($team_user);
-
- //flushing here because this is a mini-form and we want to see the name pop up
- //more efficient solution would be to have JS handle the popping and batch update
- $em->flush();
- }
- }
-
- public function updateRole(int $team_id, int $user_id, int $role_id)
- {
- if (! $this->teamAuth()->teamAuthorized($this->identity(), 'update', 'team', $team_id)){
- $this->messenger()->addError("You aren't authorized to change this team");
- } else {
- $em = $this->entityManager;
- $team_user = $em->find('Teams\Entity\TeamUser', ['team' => $team_id, 'user'=>$user_id]);
- $user_role = $em->find('Teams\Entity\TeamRole', $role_id);
- $team_user->setRole($user_role);
- $em->flush();
- }
-
- }
-
- public function processItemSets(int $item_set_id)
- {
- $resource_array = array();
- if ((int)$item_set_id>0) {
- $item_set_id = (int)$item_set_id;
-
- //TODO: why isn't this a list?
- //add all items belonging to itemset
- foreach ($this->api()->search('items', ['item_set_id'=>$item_set_id, 'bypass_team_filter' => true])->getContent() as $item):
- $resource_array += [$item->id() => true];
-
- //add all media belonging to to the item
- foreach ($this->api()->search('media', ['item_id'=>$item->id(), 'bypass_team_filter' => true])->getContent() as $media):
- $resource_array += [$media->id()=>true];
- endforeach;
- endforeach;
- }
- //add itemset itself
- $resource_array += [$item_set_id => true];
-
- return $resource_array;
- }
-
/**
* @throws ORMException
* @throws OptimisticLockException
@@ -206,7 +120,14 @@ public function teamUpdateAction()
//get the users available to be added to the team
$available_u_array = array_diff($all_u_array, $team_u_array);
- $roles = $this->api()->search('team-role')->getContent();
+ // Build a name => id map for the view's role selector.
+ // Using api()->search() avoids loading partial entities into the Doctrine
+ // identity map, which was the root cause of the canAddSitePages=false bug.
+ $roleRepresentations = $this->api()->search('team-role')->getContent();
+ $roles = [];
+ foreach ($roleRepresentations as $roleRep) {
+ $roles[$roleRep->name()] = $roleRep->id();
+ }
//create an array object to hold the contents to pre-fill the form with
//TODO (emulate) this is the procedure to use to populate forms. Copy this.
@@ -233,7 +154,6 @@ public function teamUpdateAction()
'bypassTeamFilterRoles' => $bypass_team_filter_roles,
'id' => $team_id,
'roles'=> $roles,
- 'roles_array' => $roles_array,
'all_u_collection' => $all_u_collection,
'team_u_collection' => $team_u_collection,
'team_u_array'=>$team_u_array,
@@ -387,4 +307,65 @@ public function teamUpdateAction()
return $this->redirect()->toRoute('admin/teams/detail',['id'=>$team_id]);
}
+
+ /**
+ * Adds a user to teams via the user-edit form.
+ *
+ * TODO: Role id=1 is hardcoded here. Should be made configurable or
+ * resolved to a named default role rather than relying on a magic integer.
+ */
+ public function userAction()
+ {
+ $request = $this->getRequest();
+ if ($request->isPost()) {
+ $data = $request->getPost();
+ $user_teams = $data['user-information']['o-module-teams:Team'];
+ $user_id = $this->params('id');
+ $em = $this->entityManager;
+
+ foreach ($user_teams as $team_id):
+ $team = $em->getRepository('Teams\Entity\Team')->findOneBy(['id' => $team_id]);
+ $user = $em->getRepository('Omeka\Entity\User')->findOneBy(['id' => $user_id]);
+ // TODO: resolve a proper default role instead of hardcoding id=1
+ $role = $em->getRepository('Teams\Entity\TeamRole')->findOneBy(['id' => 1]);
+ $team_user = new TeamUser($team, $user, $role);
+ $team_user->setCurrent(null);
+ $em->persist($team_user);
+ endforeach;
+ $em->flush();
+ return $this->redirect()->toUrl($request->getHeader('referer'));
+ }
+ }
+
+ /**
+ * Switches the user's active team and updates their default item sites.
+ */
+ public function currentTeamAction()
+ {
+ $user_id = $this->identity()->getId();
+ $request = $this->getRequest();
+
+ if ($request->isPost()) {
+ $data = $request->getPost();
+ $em = $this->entityManager;
+ $teamUserRepo = $em->getRepository('Teams\Entity\TeamUser');
+
+ $old_current = $teamUserRepo->findOneBy(['user' => $user_id, 'is_current' => 1]);
+ $new_current = $teamUserRepo->findOneBy(['user' => $user_id, 'team' => $data['team_id']]);
+
+ if ($old_current) {
+ $old_current->setCurrent(null);
+ $em->flush();
+ }
+ if ($new_current) {
+ $new_current->setCurrent(true);
+ $em->flush();
+ $this->sitePermissionManager->updateUserDefaultSites($user_id);
+ } else {
+ $this->messenger()->addError("Team not found");
+ }
+
+ return $this->redirect()->toUrl($data['return_url']);
+ }
+ }
}
From 84dfaa5121f4a3d8f84befc7a0075259f9808379 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 11 Aug 2026 02:18:57 +0000
Subject: [PATCH 27/54] Fix review issues: logging level, unused var, XSS
protection in inline scripts
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
src/Service/SitePermissionManager.php | 2 +-
src/Service/SitePermissionManagerFactory.php | 2 --
view/teams/partial/resource-template/edit.phtml | 2 +-
view/teams/partial/site-admin/edit.phtml | 2 +-
view/teams/partial/site-admin/users-teams-info.phtml | 2 +-
view/teams/partial/team/add-user-and-role.phtml | 4 ++--
view/teams/partial/user/edit.phtml | 4 ++--
7 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/Service/SitePermissionManager.php b/src/Service/SitePermissionManager.php
index c5b7405b..fd1cb150 100644
--- a/src/Service/SitePermissionManager.php
+++ b/src/Service/SitePermissionManager.php
@@ -79,7 +79,7 @@ public function syncSitePermissionsForUser(int $userId, int $teamId): void
$omekaSiteRole = $canAddSitePages
? SitePermission::ROLE_ADMIN
: SitePermission::ROLE_VIEWER;
- $this->logger->err(sprintf(
+ $this->logger->info(sprintf(
'[SitePermissionManager] syncSitePermissionsForUser: userId=%d, teamId=%d, teamRoleId=%d, teamRoleName="%s", canAddSitePages=%s => omekaSiteRole="%s", rawCanAddSitePages=%s',
$userId,
$teamId,
diff --git a/src/Service/SitePermissionManagerFactory.php b/src/Service/SitePermissionManagerFactory.php
index 704bf316..e22ddb5c 100644
--- a/src/Service/SitePermissionManagerFactory.php
+++ b/src/Service/SitePermissionManagerFactory.php
@@ -15,8 +15,6 @@ class SitePermissionManagerFactory implements FactoryInterface
*/
public function __invoke(ContainerInterface $container, $requestedName, array $options = null)
{
- $logger = $container->get('Omeka\Logger');
-
return new SitePermissionManager(
$container->get('Omeka\EntityManager'),
$container->get('Omeka\Settings\User'),
diff --git a/view/teams/partial/resource-template/edit.phtml b/view/teams/partial/resource-template/edit.phtml
index b62f88fb..34be274a 100644
--- a/view/teams/partial/resource-template/edit.phtml
+++ b/view/teams/partial/resource-template/edit.phtml
@@ -12,7 +12,7 @@ endforeach;
window.addEventListener("load", function () {
- let current_teams = ;
+ let current_teams = ;
let select = document.getElementById('team');
diff --git a/view/teams/partial/site-admin/edit.phtml b/view/teams/partial/site-admin/edit.phtml
index 8f4face4..a2bbcf60 100644
--- a/view/teams/partial/site-admin/edit.phtml
+++ b/view/teams/partial/site-admin/edit.phtml
@@ -3,7 +3,7 @@
window.addEventListener("load", function () {
- let current_teams = team_ids)?>;
+ let current_teams = team_ids, JSON_HEX_TAG | JSON_HEX_AMP)?>;
let select = document.getElementById('team_selected');
diff --git a/view/teams/partial/site-admin/users-teams-info.phtml b/view/teams/partial/site-admin/users-teams-info.phtml
index e5c23eff..43f5f756 100644
--- a/view/teams/partial/site-admin/users-teams-info.phtml
+++ b/view/teams/partial/site-admin/users-teams-info.phtml
@@ -11,7 +11,7 @@ $translate = $this->plugin('translate');
?>
+if (empty($teamManagedUsers)) {
+ return;
+}
+?>
+
+
+
+
+
+
+
+
+
+
+
+ $teams): ?>
+
+
+
escapeHtml(implode(', ', $teams)); ?>
+
+
+
+
+
From c7b7a24104f249028f0e3f2204ec4cc5789cf4cf Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 11 Aug 2026 19:35:21 +0000
Subject: [PATCH 30/54] fix: role edits, team-site domain error, null sites
crash, and users page team info
- manage-users.phtml: load Teams' own team-users.js instead of Omeka's
site-users.js, which hard-codes o:site_permission field names and never
updates the __index__ placeholder for o:team_users fields, causing role
changes to be silently discarded on save.
- TeamAuth.php: normalize 'team-site' and 'team_site' to the 'site' domain
before validation, matching the existing pattern for resource domains.
Without this, creating a team-site threw InvalidArgumentException.
- UpdateController.php: guard the teamSites post value with ?? [] so that
removing all sites from a team no longer crashes with a null-haystack
TypeError on line 241.
- view/omeka/site-admin/index/users.phtml: override the Omeka core template
(copied from commit e107b1c1d993ea93f10b587330b1f3e1b88aebcc) to inject a
'Managed by Team(s)' column into each user row and add a warning banner,
replacing the previous approach that attached to view.edit.after (the site
details page) instead of the user-permissions page.
- Module.php: remove the now-superseded view.edit.after / siteUsersTeamsInfo
event attachment that was incorrectly rendering team info on the site
details page rather than the user-permissions page.
Co-authored-by: alexdryden <47127862+alexdryden@users.noreply.github.com>
---
Module.php | 6 --
src/Controller/UpdateController.php | 2 +-
src/Mvc/Controller/Plugin/TeamAuth.php | 7 +-
view/omeka/site-admin/index/users.phtml | 123 ++++++++++++++++++++++++
view/teams/team/manage-users.phtml | 2 +-
5 files changed, 131 insertions(+), 9 deletions(-)
create mode 100644 view/omeka/site-admin/index/users.phtml
diff --git a/Module.php b/Module.php
index 758a2520..db99db96 100644
--- a/Module.php
+++ b/Module.php
@@ -2578,12 +2578,6 @@ public function attachListeners(SharedEventManagerInterface $sharedEventManager)
[$this, 'siteEdit']
);
- $sharedEventManager->attach(
- 'Omeka\Controller\SiteAdmin\Index',
- 'view.edit.after',
- [$this, 'siteUsersTeamsInfo']
- );
-
//put the roles data in the user page
$sharedEventManager->attach(
diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php
index 6968c659..788eafd3 100644
--- a/src/Controller/UpdateController.php
+++ b/src/Controller/UpdateController.php
@@ -229,7 +229,7 @@ public function teamUpdateAction()
}
//handle new sites
- $postSites = $post_data['teamSites']['o:site'];
+ $postSites = $post_data['teamSites']['o:site'] ?? [];
foreach ($postSites as $site_id) {
if (!in_array($site_id, $current_sites)) {
$this->api()->create('team-site', ['team' => $team_id, 'site' => $site_id]);
diff --git a/src/Mvc/Controller/Plugin/TeamAuth.php b/src/Mvc/Controller/Plugin/TeamAuth.php
index 8074575d..8de8775a 100644
--- a/src/Mvc/Controller/Plugin/TeamAuth.php
+++ b/src/Mvc/Controller/Plugin/TeamAuth.php
@@ -59,10 +59,15 @@ public function teamAuthorized(User $user, string $action, string $domain, int $
$action = 'add';
}
$resourceDomains = ['team_resource','team_resource_template','team_asset','team-resource','team-resource-template','team-asset'];
- if (in_array($domain,$resourceDomains)) {
+ if (in_array($domain, $resourceDomains)) {
$domain = 'resource';
}
+ $siteDomains = ['team_site', 'team-site'];
+ if (in_array($domain, $siteDomains)) {
+ $domain = 'site';
+ }
+
//validate inputs
if (!in_array($action, $this->actions)) {
throw new InvalidArgumentException(
diff --git a/view/omeka/site-admin/index/users.phtml b/view/omeka/site-admin/index/users.phtml
new file mode 100644
index 00000000..18ab0dad
--- /dev/null
+++ b/view/omeka/site-admin/index/users.phtml
@@ -0,0 +1,123 @@
+plugin('translate');
+$this->headScript()->appendFile($this->assetUrl('js/site-users.js', 'Omeka'));
+$this->htmlElement('body')->appendAttribute('class', 'sites users');
+$form->prepare();
+$escape = $this->plugin('escapeHtml');
+$delete = $translate('Delete');
+$restore = $translate('Restore');
+$roles = [
+ 'viewer' => $translate('Viewer'),
+ 'editor' => $translate('Creator'),
+ 'admin' => $translate('Manager'),
+];
+
+// Build userId => [teamName, ...] map for the teams column.
+$teamManagedUsers = [];
+try {
+ $services = $this->getHelperPluginManager()->getServiceLocator();
+ $entityManager = $services->get('Omeka\EntityManager');
+ $teamSites = $entityManager->getRepository('Teams\Entity\TeamSite')
+ ->findBy(['site' => $site->id()]);
+ foreach ($teamSites as $teamSite) {
+ $team = $teamSite->getTeam();
+ $teamUsers = $entityManager->getRepository('Teams\Entity\TeamUser')
+ ->findBy(['team' => $team->getId()]);
+ foreach ($teamUsers as $teamUser) {
+ $userId = $teamUser->getUser()->getId();
+ $teamManagedUsers[$userId][] = $team->getName();
+ }
+ }
+} catch (\Exception $e) {
+ // If Teams module data is unavailable, silently omit the column.
+ $teamManagedUsers = null;
+}
+
+$userRowTemplate = '
+
+
setLabel("Add and Remove Templates and Item Sets");
- $secondaryResourcesForm->get('recursive_item_sets')->setOptions(['label'=>'Recursively manage items from item sets?']);
- $secondaryResourcesForm->get('recursive_item_sets')->setOptions(['info'=>'Add/remove all of the items from the item sets when you add/remove them']);
-
+ $secondaryResourcesForm->setLabel($translate('Add and Remove Templates and Item Sets'));
+ $secondaryResourcesForm->get('recursive_item_sets')->setOptions(['label' => $translate('Recursively manage items from item sets?')]);
+ $secondaryResourcesForm->get('recursive_item_sets')->setOptions(['info' => $translate('Add/remove all of the items from the item sets when you add/remove them')]);
echo $this->formCollection($secondaryResourcesForm);
?>
+
+ setLabel($translate('Add and Remove Templates and Item Sets'));
+ if (!$isAdd) {
+ $secondaryResourcesForm->get('recursive_item_sets')
+ ->setOptions(['label' => $translate('Recursively manage items from item sets?')]);
+ $secondaryResourcesForm->get('recursive_item_sets')
+ ->setOptions(['info' => $translate('Add/remove all of the items from the item sets when you add/remove them')]);
+ }
+ echo $this->formCollection($secondaryResourcesForm);
+ ?>
+
-
- setLabel($translate('Add and Remove Templates and Item Sets'));
- $secondaryResourcesForm->get('recursive_item_sets')->setOptions(['label' => $translate('Recursively manage items from item sets?')]);
- $secondaryResourcesForm->get('recursive_item_sets')->setOptions(['info' => $translate('Add/remove all of the items from the item sets when you add/remove them')]);
- echo $this->formCollection($secondaryResourcesForm);
- ?>
-