Fix deactivate button never rendering on the group Users tab

.claude/skills/run-moregroups/SKILL.md

@@ -0,0 +1,142 @@
+---
+name: run-moregroups
+description: Build, install, and drive the More Groups GLPI plugin in a real running GLPI 11 instance — deactivate/reactivate a group member both via the single-row buttons and via the massive actions, through the actual UI, with a screenshot after each step. Use when asked to run, test, smoke-test, or screenshot More Groups, or to confirm a code change actually works in the browser (not just phpunit/phpcs).
+---
+
+Paths below are relative to the repo root (`moregroups/`), not this skill directory.
+
+More Groups is a classic-style (`inc/`) + modern (`src/Controller`) GLPI 11 plugin. There is
+no standalone dev server — "running" it means a live GLPI 11 container with the plugin files
+mounted, installed and activated, driven with Playwright through the actual `/front/group.form.php`
+UI. Don't just `php -l` the change — this skill proves it renders and functions.
+
+## Prerequisites (already verified in this container)
+
+- Podman, with a GLPI 11 container already up (see below).
+- `node_modules/@playwright/test` at repo root: `npm install` (root `package.json` already
+  pins `"@playwright/test": "1.62.1"`).
+- Chromium browser binary: `npx playwright install chromium` (works without `--with-deps` —
+  the OS libs this needs are already present on this host; `--with-deps` fails here because it
+  needs passwordless `sudo`, which isn't available).
+
+## Getting a running GLPI instance
+
+This repo doesn't ship a compose file for a throwaway instance. In this environment there is
+already a long-running GLPI 11.0.8 test stack usable for this: `glpi-65000-web` /
+`glpi-65000-db` (podman), published at `http://localhost:65000`, default creds `glpi`/`glpi`.
+If it's not running, any GLPI 11 container with the plugin's files mounted at
+`/var/www/glpi/plugins/moregroups` and activated (Setup → Plugins) works the same way — update
+`BASE_URL` below accordingly.
+
+**Sync your working tree into the running container** (this container mounts plugin code from
+a host path, not directly from this repo checkout):
+
+```bash
+rsync -a --exclude=vendor --exclude=.git --exclude=node_modules --exclude=tests --exclude=tools \
+  inc src hook.php setup.php locales \
+  /home/oscar/containers/all-plugins/moregroups/
+```
+
+Then, if you only changed PHP under `inc/`/`src/`, no reinstall is needed — GLPI reads plugin
+classes on every request. If you added a new migration/table, reinstall from Setup → Plugins.
+
+**PHP-file edits can hit a stale opcache** even after `rsync` updates the file on disk
+(confirmed pattern for this stack — see the `glpi-plugin-builder` skill's Trap 7). If a fix
+"isn't working" right after editing PHP, restart the container before concluding the fix is
+wrong:
+
+```bash
+podman restart glpi-65000-web
+```
+
+## Run (agent path) — the driver
+
+```bash
+cd moregroups   # repo root
+npm install                              # once
+npx playwright install chromium          # once (cached after first run)
+node .claude/skills/run-moregroups/driver.mjs http://localhost:65000 1
+```
+
+The `1` is a `Group` id that already has ≥1 active `Group_User` member on this test instance
+(group "TEST", id 1). The driver:
+
+1. Logs in as `glpi`/`glpi`.
+2. Opens `/front/group.form.php?id=<GROUP_ID>` and the **Users** tab.
+3. Waits for the **Deactivate user** button to render next to the first active member — this
+   is the exact assertion that catches the malformed-jQuery-selector regression fixed on
+   2026-08-26 (`inc/group.class.php`'s `input[name^='item[Group_User]']` selector).
+4. Clicks it, confirms the member now appears in the **Deactivated users** panel.
+5. Clicks that row's **Activate user** button, confirms the member is back in the active list.
+6. Selects that same member's checkbox in the active-members table, opens the **Actions**
+   modal, picks **Deactivate users** (the `PluginMoregroupsGroup:deactivate` massive action
+   wired via `plugin_moregroups_MassiveActions` in `hook.php`), submits, confirms the member
+   is now in the **Deactivated users** panel, and confirms the page reloaded back onto the
+   same `group.form.php?id=<GROUP_ID>` (not some other URL).
+7. Selects that member's checkbox in the **Deactivated users** panel, opens its own **Actions**
+   modal, picks **Activate users** (`getSpecificMassiveActions` in `inc/group.class.php`),
+   submits, confirms the member is back in the active list, and confirms the same reload check.
+8. Screenshots after every step into `.claude/skills/run-moregroups/screenshots/`
+   (`01-logged-in.png` … `06-massive-activated.png`, or `NN-FAILURE.png` on a thrown step).
+
+Exit code is non-zero on failure; read stdout for which step failed and check the `FAILURE`
+screenshot.
+
+**The DB state this driver leaves behind is idempotent** — deactivate/reactivate and the
+massive-action round trip both net out to the same active/deactivated row counts — safe to
+re-run without manual cleanup; verified by running the full 6-step sequence twice back to back.
+
+## Direct invocation (no browser) — checking PHP syntax/logic fast
+
+For a quick sanity pass on a PHP edit before spinning up the browser driver:
+
+```bash
+php -l inc/group.class.php
+php -l src/Controller/GroupActionController.php
+```
+
+This does **not** catch the JS-selector class of bug that broke the deactivate button — that
+only shows up by actually rendering the page and clicking, i.e. the driver above.
+
+## Gotchas
+
+- **The malformed-selector bug is invisible to `php -l`, phpcs, and PHPStan.** It's embedded
+  PHP-heredoc JavaScript (`Html::scriptBlock($script)` in `showDeactivated()`). Nothing in the
+  PHP toolchain parses it. The only way to catch it is rendering the page and asserting the
+  button is actually present in the DOM — which is why the driver's `waitFor` on the
+  deactivate button (not just clicking it) is the load-bearing assertion.
+- **`rsync --delete` would wipe files unique to the container copy** (it has no `.git`,
+  `.phpcs.xml`, etc. — those are dev-only files that were never meant to ship). Don't add
+  `--delete` to the sync command above.
+- **`localhost` from inside a Playwright browser launched on the host reaches the container
+  fine** here because the container publishes `0.0.0.0:65000->80/tcp` — no
+  `host.containers.internal` dance needed, unlike the Playwright-inside-a-container setup used
+  by `tools/manual-generator/run.sh` (a separate, heavier pipeline for generating the published
+  user manual — see the `glpi-plugin-manual-generator` skill for that one; this skill is for
+  fast local smoke-testing, not documentation screenshots).
+- **`npx playwright install chromium --with-deps` fails** in this container (`sudo: a
+  terminal is required`). Omit `--with-deps` — the browser launches fine without it here.
+
+## Troubleshooting
+
+- `DRIVER FAILED: locator.waitFor: Timeout ... button[title="Deactivate user"]` on step 3 →
+  the JS-selector regression is back (or a new one like it). Check
+  `Html::scriptBlock($script)` in `inc/group.class.php`'s `showDeactivated()` for a syntax
+  error in the embedded jQuery, and check browser console errors (the driver prints any
+  `pageerror`/console-error events it captured after `ALL STEPS COMPLETED` or right before a
+  failure).
+- `no active Group_User rows found` → the seed group has no members left (e.g. previous run's
+  cleanup didn't happen). Reset via the DB directly:
+  ```bash
+  podman exec glpi-65000-db sh -c "mariadb -u65000_db_user -p65000_db_password 65000_db_name \
+    -e \"INSERT IGNORE INTO glpi_groups_users (users_id, groups_id) VALUES (3,1);\""
+  ```
+- `DRIVER FAILED: ... did not reload back onto the group form` on step 6 or 7 → the massive
+  action's redirect target regressed (compare against `MassiveAction`'s own redirect handling;
+  this plugin doesn't override it, so a failure here usually points at something upstream of
+  the plugin, e.g. a stale/incorrect `Referer`). Check the matching `NN-massive-*` screenshot
+  for what page it actually landed on.
+- The massive-action steps select the option by its **visible label** (`selectOption({label:
+  ...})`), not its value — if the button label text changes (e.g. a locale/translation string
+  edit to `__('Deactivate users', 'moregroups')` / `__('Activate users', 'moregroups')` in
+  `inc/group.class.php`), update the driver's `selectOption` calls to match.

.claude/skills/run-moregroups/driver.mjs

@@ -0,0 +1,129 @@
+#!/usr/bin/env node
+// driver.mjs — smoke-drives the More Groups plugin against a running GLPI 11 instance.
+//
+// Usage:
+//   node .claude/skills/run-moregroups/driver.mjs <BASE_URL> <GROUP_ID>
+//
+// Example (against the glpi-65000-web test container used to build this skill):
+//   node .claude/skills/run-moregroups/driver.mjs http://localhost:65000 1
+//
+// Logs in as glpi/glpi, opens the group's Users tab, deactivates the first active
+// member found via the single-row button, verifies they land in the "Deactivated users"
+// panel, reactivates them via the single-row button, verifies they're back in the active
+// list, then repeats the same round trip through the massive-action UI (checkbox row +
+// "Actions" modal + "Deactivate users"/"Activate users") and confirms each submit reloads
+// back onto the same group's Users tab. Screenshots after each step land in
+// .claude/skills/run-moregroups/screenshots/.
+
+import { chromium } from '@playwright/test';
+import { mkdirSync } from 'node:fs';
+import { dirname, join } from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const here = dirname(fileURLToPath(import.meta.url));
+const shotsDir = join(here, 'screenshots');
+mkdirSync(shotsDir, { recursive: true });
+
+const BASE_URL = process.argv[2] || 'http://localhost:65000';
+const GROUP_ID = process.argv[3] || '1';
+
+let shotN = 0;
+async function shot(page, name) {
+  shotN += 1;
+  const path = join(shotsDir, `${String(shotN).padStart(2, '0')}-${name}.png`);
+  await page.screenshot({ path, fullPage: true });
+  console.log('screenshot:', path);
+}
+
+async function main() {
+  const browser = await chromium.launch({ args: ['--no-sandbox'] });
+  const page = await browser.newPage();
+  const errors = [];
+  page.on('pageerror', (e) => errors.push(String(e)));
+  page.on('console', (m) => { if (m.type() === 'error') errors.push(m.text()); });
+
+  try {
+    console.log('==> login');
+    await page.goto(`${BASE_URL}/`, { timeout: 60_000 });
+    await page.locator('input[name="login_name"]').fill('glpi');
+    await page.locator('input[name="login_password"]').fill('glpi');
+    await page.locator('button[name="submit"], input[name="submit"]').first().click();
+    await page.waitForLoadState('networkidle');
+    await shot(page, 'logged-in');
+
+    console.log('==> open group Users tab');
+    await page.goto(`${BASE_URL}/front/group.form.php?id=${GROUP_ID}`, { timeout: 30_000 });
+    await page.getByRole('tab', { name: /^Users/ }).click();
+    await page.getByText('Deactivated users').waitFor({ state: 'visible', timeout: 30_000 });
+    await shot(page, 'users-tab');
+
+    console.log('==> single-row deactivate');
+    const activeRows = page.locator('tr[data-itemtype="Group_User"]');
+    const activeCountBefore = await activeRows.count();
+    if (activeCountBefore === 0) throw new Error('no active Group_User rows found — seed the group with a member first');
+    const targetRow = activeRows.first();
+    const deactivateBtn = targetRow.locator('button[title="Deactivate user"]');
+    await deactivateBtn.waitFor({ state: 'visible', timeout: 10_000 }); // <- fails if the JS selector bug regresses
+    const memberText = (await targetRow.locator('td').nth(1).innerText()).trim();
+    await deactivateBtn.click();
+
+    const panel = page.locator('.card.m-n2', { has: page.locator('.card-title', { hasText: 'Deactivated users' }) });
+    await panel.locator('tr', { hasText: memberText }).waitFor({ state: 'visible', timeout: 30_000 });
+    await page.waitForLoadState('networkidle');
+    console.log('   deactivated OK:', memberText.trim());
+    await shot(page, 'deactivated');
+
+    console.log('==> single-row reactivate');
+    const deactivatedRow = panel.locator('tr', { hasText: memberText });
+    await deactivatedRow.locator('button[title="Activate user"]').click();
+    await page.locator('tr[data-itemtype="Group_User"]', { hasText: memberText }).waitFor({ state: 'visible', timeout: 30_000 });
+    await page.waitForLoadState('networkidle');
+    console.log('   reactivated OK:', memberText.trim());
+    await shot(page, 'reactivated');
+
+    console.log('==> massive action: deactivate');
+    const panel2 = page.locator('.card.m-n2', { has: page.locator('.card-title', { hasText: 'Deactivated users' }) });
+    const activeRow = page.locator('tr[data-itemtype="Group_User"]', { hasText: memberText });
+    await activeRow.locator('input[type="checkbox"]').check();
+    await page.locator('a[href*="modal_massaction_content"]').first().click();
+    const maSelect = page.locator('.modal.show select[name="massiveaction"]');
+    await maSelect.waitFor({ state: 'visible', timeout: 10_000 });
+    await maSelect.selectOption({ label: 'Deactivate users' }); // <- fails if the massive "deactivate" action (hook.php's plugin_moregroups_MassiveActions) regresses
+    await page.locator('.modal.show button[name="massiveaction"], .modal.show input[name="massiveaction"]').first().click();
+    await panel2.locator('tr', { hasText: memberText }).waitFor({ state: 'visible', timeout: 30_000 });
+    if (!page.url().includes(`group.form.php?id=${GROUP_ID}`)) {
+      throw new Error(`massive deactivate did not reload back onto the group form (landed on ${page.url()})`);
+    }
+    console.log('   massive-deactivated OK, reloaded on', page.url());
+    await shot(page, 'massive-deactivated');
+
+    console.log('==> massive action: activate');
+    const deactivatedRow2 = panel2.locator('tr', { hasText: memberText });
+    await deactivatedRow2.locator('input[type="checkbox"]').check();
+    await panel2.locator('a[href*="modal_massaction_content"]').first().click();
+    const maSelect2 = page.locator('.modal.show select[name="massiveaction"]');
+    await maSelect2.waitFor({ state: 'visible', timeout: 10_000 });
+    await maSelect2.selectOption({ label: 'Activate users' }); // <- fails if the specific massive "activate" action (getSpecificMassiveActions in inc/group.class.php) regresses
+    await page.locator('.modal.show button[name="massiveaction"], .modal.show input[name="massiveaction"]').first().click();
+    await page.locator('tr[data-itemtype="Group_User"]', { hasText: memberText }).waitFor({ state: 'visible', timeout: 30_000 });
+    if (!page.url().includes(`group.form.php?id=${GROUP_ID}`)) {
+      throw new Error(`massive activate did not reload back onto the group form (landed on ${page.url()})`);
+    }
+    console.log('   massive-activated OK, reloaded on', page.url());
+    await shot(page, 'massive-activated');
+
+    console.log('ALL STEPS COMPLETED');
+    if (errors.length) {
+      console.log('--- console/page errors seen during the run ---');
+      errors.forEach((e) => console.log(e));
+    }
+  } catch (err) {
+    await shot(page, 'FAILURE');
+    console.error('DRIVER FAILED:', err.message);
+    process.exitCode = 1;
+  } finally {
+    await browser.close();
+  }
+}
+
+main();

.gitignore

@@ -1,2 +1,4 @@
 /vendor
-composer.lock
+composer.lock
+/node_modules
+/.claude/skills/run-moregroups/screenshots/

CHANGELOG.md

@@ -1,6 +1,9 @@
 # Changelog
 
 ## [Unreleased]
+### Bugs
+- Fix the activate/deactivate group membership action always failing with an access-denied error
+- Fix the "Deactivate" button never appearing next to active group members, which made it impossible to deactivate a membership from the Users tab
 
 ## 2.0.2 - 26/08/2026
 ### Bugs

inc/group.class.php

@@ -226,7 +226,7 @@ public static function showDeactivated($item)
 			$script = <<<JAVASCRIPT
 
 				$(document).ready(function() {
-					$("input[name^='item[Group_User]'").each(function() {
+					$("input[name^='item[Group_User]']").each(function() {
 						var name = $(this).attr('name');
 						const myarray = name.split('[');
 						name = myarray[2].split(']')[0];

src/Controller/GroupActionController.php

@@ -49,9 +49,11 @@ public function __invoke(Request $request): Response
 	{
 		global $CFG_GLPI;
 
-		if (!Session::validateCSRF($request->request->all())) {
-			throw new AccessDeniedHttpException();
-		}
+		// GLPI's core CheckCsrfListener (kernel.controller event) already
+		// validates and consumes the CSRF token for every non-safe-method
+		// request before a Symfony-routed controller runs. A second manual
+		// Session::validateCSRF() call here always fails: the token has
+		// already been unset from $_SESSION by the time we get here.
 
 		$rowaction = $request->request->get('rowaction');
 		$rowid     = $request->request->get('rowid');

tests/GroupActionControllerTest.php

@@ -64,61 +64,10 @@ public function testMissingParametersAreBadRequest(): void
 		$controller($request);
 	}
 
-	public function testMissingCsrfTokenIsDenied(): void
-	{
-		$this->login();
-
-		$group = $this->createItem('Group', [
-			'name'        => 'moregroups-ctrl-test-' . uniqid(),
-			'entities_id' => getItemByTypeName('Entity', '_test_root_entity', true),
-		]);
-		$user_id = getItemByTypeName('User', TU_USER, true);
-
-		$tracked = $this->createItem('PluginMoregroupsGroup', [
-			'groups_id' => $group->getID(),
-			'users_id'  => $user_id,
-		]);
-
-		$controller = new GroupActionController();
-		// No `_glpi_csrf_token` at all - regression test for the missing
-		// CSRF validation found by the security audit.
-		$request = new Request([], ['rowaction' => 'activate', 'rowid' => (string) $tracked->getID()]);
-
-		$this->expectException(AccessDeniedHttpException::class);
-		$controller($request);
-
-		$leftover = new PluginMoregroupsGroup();
-		$this->assertTrue(
-			$leftover->getFromDB($tracked->getID()),
-			'A request without a valid CSRF token must not perform the activation'
-		);
-	}
-
-	public function testInvalidCsrfTokenIsDenied(): void
-	{
-		$this->login();
-
-		$group = $this->createItem('Group', [
-			'name'        => 'moregroups-ctrl-test-' . uniqid(),
-			'entities_id' => getItemByTypeName('Entity', '_test_root_entity', true),
-		]);
-		$user_id = getItemByTypeName('User', TU_USER, true);
-
-		$tracked = $this->createItem('PluginMoregroupsGroup', [
-			'groups_id' => $group->getID(),
-			'users_id'  => $user_id,
-		]);
-
-		$controller = new GroupActionController();
-		$request = new Request([], [
-			'rowaction'          => 'activate',
-			'rowid'              => (string) $tracked->getID(),
-			'_glpi_csrf_token'   => 'not-a-real-token',
-		]);
-
-		$this->expectException(AccessDeniedHttpException::class);
-		$controller($request);
-	}
+	// CSRF token presence/validity for this route is enforced by GLPI core's
+	// CheckCsrfListener (kernel.controller event) before the controller is
+	// ever invoked - it isn't exercisable from a direct controller call, so
+	// it isn't re-tested here. See GroupActionController::__invoke().
 
 	public function testActivateWithoutRightsIsDenied(): void
 	{