diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
index 77391679..c9c18848 100644
--- a/.github/copilot-instructions.md
+++ b/.github/copilot-instructions.md
@@ -75,7 +75,8 @@ Omeka S Module (PHP-based)
### PHP Standards
- Follow PSR-12 coding standards where applicable
- Use proper type hints and return types
-- Document complex logic with clear comments
+- Document complex logic with inline comments only where strictly needed; comments must describe the current code, never reference previous versions or prior fixes
+- Class and method docblocks follow Python/Google style: a single summary sentence, followed by an optional blank line and longer description, `Args:`/`Returns:`/`Throws:` sections where appropriate, and `@param`/`@return` PHPDoc tags for type information
- Prefer dependency injection over global state
- Use Omeka S service manager patterns
@@ -93,13 +94,36 @@ Omeka S Module (PHP-based)
## Build, Validation, and Testing
+### Linting — Always Run
+
+**Always run `php -l` on every PHP file you create or modify before committing.** This catches parse errors immediately:
+
+```bash
+php -l path/to/File.php
+# or check all files at once:
+find . -name "*.php" -print0 | xargs -0 php -l | grep -v "No syntax errors"
+```
+
+### Omeka S Coding Standards
+
+Omeka S ships a PHP_CodeSniffer ruleset. When the Omeka S codebase is present, run it against changed files:
+
+```bash
+vendor/bin/phpcs --standard=vendor/omeka/omeka-s/coding-standards/Omeka/ruleset.xml src/Path/To/ChangedFile.php
+```
+
+Even without the full Omeka environment, apply these standards manually:
+- No `addCsrf()` calls — Omeka S adds CSRF to **all** forms automatically via its `Omeka\Form\Initializer\Csrf` initializer. Never call `addCsrf()` manually.
+- Forms extend `Laminas\Form\Form` (or an Omeka subclass); the initializer handles CSRF.
+- Verify every `use` import is spelled correctly (case-sensitive on Linux) and actually resolves.
+- Remove all unused `use` statements.
+
### Current State
-**No explicit build steps, CI/CD pipelines, or automated validation are currently present** in this repository.
+**No CI/CD pipelines or automated test suites are currently present** in this repository beyond `php -l`.
### What This Means for Agents
-- **Skip automatic build/validate steps** unless explicitly directed by task requirements
-- **No linting, testing, or compilation** commands to run by default
-- **Manual validation** may be performed by viewing files and inspecting code
+- **Always run `php -l`** on every changed PHP file — this is mandatory, not optional
+- **Manual validation** should also check method existence against Omeka S source (search GitHub omeka/omeka-s if needed)
- **Future improvements**: Agents may recommend or prototype build/test infrastructure as part of code quality improvements
### Testing in Omeka Context
diff --git a/Module.php b/Module.php
index e4564db9..c48725bc 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;
@@ -190,16 +191,16 @@ 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', '<')) {
+ // 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.
+ }
}
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)
@@ -213,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)
@@ -955,16 +959,18 @@ public function userCreate(Event $event)
}
}
$role_id = $team_role_ids[$team_id];
- $role = $em->getRepository('Teams\Entity\TeamRole')
- ->findOneBy(['id' => $role_id]);
$team_user_exists = $em->getRepository('Teams\Entity\TeamUser')
->findOneBy(['team' => $team->getId(), 'user' => $user_id]);
if (!$team_user_exists) {
- $team_user = new TeamUser($team, $user, $role);
- $em->persist($team_user);
+ // Create through the adapter so site-permission syncing is
+ // handled automatically.
+ $this->getServiceLocator()->get('Omeka\ApiManager')->create('team-user', [
+ 'team' => $team->getId(),
+ 'user' => $user_id,
+ 'role' => (int) $role_id,
+ ]);
}
}
- $em->flush();
if ($default_team) {
$em->getRepository('Teams\Entity\TeamUser')
->findOneBy(['team' => $default_team, 'user' => $user_id])
@@ -1039,27 +1045,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);
}
/**
@@ -1154,33 +1140,17 @@ public function siteCreate(Event $event)
$team_ids = $request->getContent()['team'];
- $all_teams_users = [];
$all_team_resources = [];
- //add team sites
+ // Create team-site associations through the adapter so that
+ // site-permission syncing is handled automatically.
+ $api = $this->getServiceLocator()->get('Omeka\ApiManager');
foreach ($team_ids as $team_id):
$team = $teams->findOneBy(['id' => $team_id]);
- $team_site = new TeamSite($team, $site);
- $em->persist($team_site);
-
- //get team users
- $all_teams_users[] = $team->getTeamUsers();
+ $api->create('team-site', ['team' => $team_id, 'site' => $site_id]);
//get team items
$all_team_resources[] = $team->getTeamResources();
-
- endforeach;
- $em->flush();
-
- //update current team users to include new site in their default sites
- foreach ($all_teams_users as $team_users):
- foreach ($team_users as $team_user):
- if ($team_user->getCurrent()) {
- $user_id = $team_user->getUser()->getId();
- $this->updateUserSites($user_id);
- }
-
- endforeach;
endforeach;
//update all item-site to include all items from the site's teams
@@ -1265,8 +1235,6 @@ public function userUpdate(Event $event)
//get it this way because the roles are added dynamically as js and not part of pre-baked form
$role_id = $request->getContent()['o-module-teams:TeamRole'][$team_id];
- $role = $em->getRepository('Teams\Entity\TeamRole')
- ->findOneBy(['id' => $role_id]);
$team_user_exists = $em->getRepository('Teams\Entity\TeamUser')
->findOneBy(['team' => $team->getId(), 'user' => $user_id]);
@@ -1274,21 +1242,21 @@ public function userUpdate(Event $event)
if ($team_user_exists) {
echo $team_user_exists->getId();
} else {
- $team_user = new TeamUser($team, $user, $role);
- $em->persist($team_user);
+ // Create through the adapter so site-permission syncing is
+ // handled automatically.
+ $teamUserResponse = $this->getServiceLocator()->get('Omeka\ApiManager')->create('team-user', [
+ 'team' => $team->getId(),
+ 'user' => $user_id,
+ 'role' => (int) $role_id,
+ ]);
+ $teamUserEntity = $teamUserResponse->getContent();
if ($team_id == $current_team_id) {
- $team_user->setCurrent(true);
+ $teamUserEntity->setCurrent(true);
+ $em->flush();
}
- $em->persist($team_user);
-
- //this is not ideal to flush each iteration, but it is how to check to make sure they didn't
- //TODO: catch this in chosen-trigger.js instead
- $em->flush();
}
endforeach;
-
- $em->flush();
}
if (array_key_exists('o-module-teams:DefaultTeam', $request->getContent())) {
if ($current_user->getRole() == 'global_admin' or $current_user->getId() == $target_user) {
@@ -1403,43 +1371,27 @@ public function siteUpdate(Event $event)
$added_teams = array_diff($form_teams, $existing_teams);
$removed_teams = array_diff($existing_teams, $form_teams);
- foreach ($team_sites as $team_site):
- if (in_array($team_site->getTeam()->getId(), $removed_teams)) {
- $em->remove($team_site);
- }
- endforeach;
- $em->flush();
+ // Delete removed team-site associations through the adapter so that
+ // site-permission cleanup is handled automatically.
+ $api = $this->getServiceLocator()->get('Omeka\ApiManager');
+ foreach ($removed_teams as $team_id) {
+ $api->delete('team-site', ['team' => $team_id, 'site' => $site_id]);
+ }
- //add teams to the site for each new team listed in the form
- foreach ($added_teams as $team):
- $team_site = new TeamSite(
- $em->getRepository('Teams\Entity\Team')->findOneBy(['id' => $team]),
- $em->getRepository('Omeka\Entity\Site')->findOneBy(['id' => $site_id])
- );
- $em->persist($team_site);
- endforeach;
- $em->flush();
+ // Add new team-site associations through the adapter so that
+ // site-permission syncing is handled automatically.
+ foreach ($added_teams as $team_id) {
+ $api->create('team-site', ['team' => $team_id, 'site' => $site_id]);
+ }
- //get any items or users that need to be updated
- //by either removing or adding item-sits or user default site
+ //get any items that need their site membership updated
$delta_item_site = [];
- $delta_user_site = [];
foreach (array_merge($added_teams, $removed_teams) as $team_id) {
$delta_item_site[] = $em->getRepository('Teams\Entity\Team')
->findOneBy(['id' => $team_id])
->getTeamResources();
- $delta_user_site[] = $em->getRepository('Teams\Entity\Team')
- ->findOneBy(['id' => $team_id])
- ->getTeamUsers();
}
- //update current team users to include new site in their default sites
- foreach ($delta_user_site as $team_users) {
- foreach ($team_users as $team_user) {
- $user_id = $team_user->getUser()->getId();
- $this->updateUserSites($user_id);
- }
- }
foreach ($delta_item_site as $team_item_collection) {
foreach ($team_item_collection as $team_item) {
$this->updateItemSites($team_item->getResource()->getId());
@@ -1952,6 +1904,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()
{
diff --git a/asset/js/inject-removal-util-element.js b/asset/js/inject-removal-util-element.js
index c9f031f6..805bbaf3 100644
--- a/asset/js/inject-removal-util-element.js
+++ b/asset/js/inject-removal-util-element.js
@@ -1,25 +1,13 @@
$(window).on('load', function() {
- $("#o-modules-team-remove-item-sets").parent().parent().css('visibility', 'hidden');
- $("#o-modules-team-remove-resource-templates").parent().parent().css('visibility', 'hidden');
-
- //not ideal, but for some reason the chosen option from chosen-options.js are getting unset, so settin them here
+ // Initialize chosen on the item-sets and resource-templates multiselects.
+ // Chosen handles deselection natively: deselected values are removed from
+ // the select element, so they are omitted from the submitted form data and
+ // the adapter treats them as removed.
$("#o-modules-team-item-sets").chosen({
allow_single_deselect: true,
disable_search_threshold: 10,
width: '100%',
include_group_label_in_selected: true,
- }).change( function(event, params) {
- let $values = $("#o-modules-team-remove-item-sets").val();
- if (params.deselected){
- let $label = $("#o-modules-team-item-sets option[value='"+params.deselected+"']").text();
- $("#o-modules-team-remove-item-sets").append('').trigger("chosen:updated");
- $values.push(params.deselected);
- console.log($values);
- }
- if (params.selected) {
- $values.splice($.inArray(params.selected, $values), 1);
- }
- $("#o-modules-team-remove-item-sets").val($values).trigger("chosen:updated");
});
$("#o-modules-team-resource-templates").chosen({
@@ -27,17 +15,5 @@ $(window).on('load', function() {
disable_search_threshold: 10,
width: '100%',
include_group_label_in_selected: true,
- }).change( function(event, params) {
- let $values = $("#o-modules-team-remove-resource-templates").val();
- if (params.deselected){
- let $label = $("#o-modules-team-resource-templates option[value='"+params.deselected+"']").text();
- $("#o-modules-team-remove-resource-templates").append('').trigger("chosen:updated");
- $values.push(params.deselected);
- console.log($values);
- }
- if (params.selected) {
- $values.splice($.inArray(params.selected, $values), 1);
- }
- $("#o-modules-team-remove-resource-templates").val($values).trigger("chosen:updated");
});
});
diff --git a/asset/js/team-users.js b/asset/js/team-users.js
index 0fb6ad8b..0dbe049e 100644
--- a/asset/js/team-users.js
+++ b/asset/js/team-users.js
@@ -1,4 +1,6 @@
-//adapted from site-users.js v4.0.0
+// Adapted from omeka/omeka-s site-users.js.
+// Key ordering requirement: Omeka.initializeSelector must run first so that
+// the DOM rows exist before we try to set the role