fix(checkin): section the check-in list by the grouping mode

lib/Service/CheckinService.php

@@ -130,11 +130,15 @@ public function getCheckinData(int $appointmentId): array {
 		$appointment = $this->appointmentMapper->find($appointmentId);
 		$responses = $this->responseMapper->findByAppointment($appointmentId);
 
-		// Get whitelisted groups for filtering
+		// The whitelist narrows the audience; the admin's grouping mode
+		// decides which groups section the list.
 		$whitelistedGroups = $this->configService->getWhitelistedGroups();
-
-		// Get the target attendees efficiently - filters by whitelisted groups when configured
 		$targetAttendees = $this->visibilityService->getTargetAttendees($appointment, $whitelistedGroups);
+		$sectionGroups = ConfigService::sectionGroups(
+			$this->configService->getResponseSummaryGroupsMode(),
+			$whitelistedGroups,
+			$this->visibilityService->getVisibilitySettings($appointment)['groups'],
+		);
 
 		// Create a map of user responses
 		$userResponseMap = [];
@@ -143,7 +147,7 @@ public function getCheckinData(int $appointmentId): array {
 		}
 
 		// Build group list for filtering UI
-		$userGroups = $this->buildGroupList($whitelistedGroups);
+		$userGroups = $sectionGroups ?? $this->allGroupsWithOthers();
 
 		// A place in line is not a place at the appointment. The list still
 		// carries everyone — an organizer has to be able to check in whoever
@@ -156,33 +160,30 @@ public function getCheckinData(int $appointmentId): array {
 		}
 
 		$users = array_map(
-			fn ($user) => $this->buildUserData($user, $userResponseMap, $whitelistedGroups, $confirmedIds),
+			fn ($user) => $this->buildUserData($user, $userResponseMap, $sectionGroups, $confirmedIds),
 			array_values($targetAttendees),
 		);
 
 		return [
 			'appointment' => $this->appointmentSerializer->serialize($appointment),
 			'users' => $users,
-			'userGroups' => array_values($userGroups),
+			'userGroups' => $userGroups,
 		];
 	}
 
 	/**
-	 * Build the list of groups for filtering.
+	 * Every group of the instance plus "Others", for the "all" mode filter.
+	 *
+	 * @return list<string>
 	 */
-	private function buildGroupList(array $whitelistedGroups): array {
-		if (empty($whitelistedGroups)) {
-			$groups = $this->visibilityService->getAllGroupIds();
-			// Hide the Guests app system group; guests fall under "Others"
-			// unless an admin opted them in explicitly via the whitelist.
-			$groups = array_values(array_filter(
-				$groups,
-				fn (string $g) => !GuestService::isGuestsSystemGroup($g),
-			));
-			$groups[] = 'Others';
-		} else {
-			$groups = $whitelistedGroups;
-		}
+	private function allGroupsWithOthers(): array {
+		// Hide the Guests app system group; guests fall under "Others"
+		// unless an admin opted them in explicitly via the whitelist.
+		$groups = array_values(array_filter(
+			$this->visibilityService->getAllGroupIds(),
+			fn (string $g) => !GuestService::isGuestsSystemGroup($g),
+		));
+		$groups[] = 'Others';
 
 		return $groups;
 	}
@@ -239,38 +240,29 @@ public function getCheckinSummary(Appointment $appointment, array $responses): a
 
 	/**
 	 * Build data structure for a single user.
+	 *
+	 * @param ?list<string> $sectionGroups null means every group
 	 */
-	private function buildUserData($user, array $userResponseMap, array $whitelistedGroups, array $confirmedIds = []): array {
+	private function buildUserData($user, array $userResponseMap, ?array $sectionGroups, array $confirmedIds = []): array {
 		$userId = $user->getUID();
 		$userGroupIds = $this->groupManager->getUserGroupIds($user);
 
-		// Hide the Guests app system group from the per-user group list
-		// unless the admin explicitly whitelisted it.
-		$guestAppWhitelisted = in_array(
-			GuestService::GUESTS_SYSTEM_GROUP,
-			array_map('strtolower', $whitelistedGroups),
-			true,
-		);
-		if (!$guestAppWhitelisted) {
-			$userGroupIds = array_values(array_filter(
+		// The sections this user belongs to, spelled as the section list
+		// spells them. Guests only get a section of their own when an admin
+		// opted the Guests app system group in.
+		if ($sectionGroups === null) {
+			$groups = array_values(array_filter(
 				$userGroupIds,
 				fn (string $g) => !GuestService::isGuestsSystemGroup($g),
 			));
+		} else {
+			$userGroupsLower = array_map('strtolower', $userGroupIds);
+			$groups = array_values(array_filter(
+				$sectionGroups,
+				fn (string $g) => in_array(strtolower($g), $userGroupsLower, true),
+			));
 		}
-
-		// Check if user belongs to any whitelisted group
-		$userInWhitelistedGroup = empty($whitelistedGroups);
-		if (!empty($whitelistedGroups)) {
-			foreach ($userGroupIds as $groupId) {
-				if (in_array($groupId, $whitelistedGroups)) {
-					$userInWhitelistedGroup = true;
-					break;
-				}
-			}
-		}
-
-		// Determine effective groups
-		$effectiveGroups = $userInWhitelistedGroup && !empty($userGroupIds) ? $userGroupIds : ['Others'];
+		$effectiveGroups = $groups !== [] ? $groups : ['Others'];
 
 		// Base user data
 		$userData = [

lib/Service/ConfigService.php

@@ -70,6 +70,30 @@ public function setResponseSummaryGroupsMode(string $mode): void {
 		$this->setStoredMode('response_summary_groups_mode', $mode, self::RESPONSE_SUMMARY_GROUP_MODES);
 	}
 
+	/**
+	 * Which groups a list sections by under $mode, or null when every group
+	 * does: the admin's list in "specific" mode, otherwise the groups a
+	 * restricted appointment names (issue #199), otherwise all or none.
+	 * Static so the services sharing it stay testable with a mocked instance.
+	 *
+	 * @param array<array-key, string> $whitelistedGroups
+	 * @param array<array-key, string|int> $restrictionGroups The appointment's own visibility groups
+	 * @return ?list<string>
+	 */
+	public static function sectionGroups(string $mode, array $whitelistedGroups, array $restrictionGroups): ?array {
+		if ($mode === self::RESPONSE_SUMMARY_MODE_SPECIFIC) {
+			return array_values($whitelistedGroups);
+		}
+
+		// A numerically-named group decodes to an int out of the JSON column.
+		$restrictionGroups = array_values(array_map('strval', $restrictionGroups));
+		if ($restrictionGroups !== []) {
+			return $restrictionGroups;
+		}
+
+		return $mode === self::RESPONSE_SUMMARY_MODE_ALL ? null : [];
+	}
+
 	/**
 	 * Check if a group is allowed based on whitelist configuration.
 	 * If no whitelist is configured, all groups are allowed.

lib/Service/ExportService.php

@@ -104,42 +104,24 @@ static function (array $a, array $b): int {
 
 	/**
 	 * Which groups the column may name, lowercased, or null when it may name
-	 * any of them.
-	 *
-	 * Mirrors the rule the response summary sections by
-	 * (ResponseSummaryService::isGroupAllowedCached), so the sheet says what
-	 * the overview shows. In order: the admin's group list decides when the
-	 * mode is "specific"; otherwise the groups a restricted appointment names
-	 * decide, because those are what let people in (issue #252); otherwise
-	 * every group counts in "all" mode and none in "none".
-	 *
-	 * A run covering several appointments takes the union of their
-	 * restrictions — one column cannot answer per appointment.
+	 * any of them — the groups the response summary sections by, so the sheet
+	 * says what the overview shows (issue #252). A run covering several
+	 * appointments takes the union of their restrictions — one column cannot
+	 * answer per appointment.
 	 *
 	 * @param list<Appointment> $appointments
 	 * @param array<array-key, string> $whitelistedGroups
 	 * @return ?list<string> null means "any group"
 	 */
 	private function nameableGroups(array $appointments, array $whitelistedGroups): ?array {
-		if ($this->configService->getResponseSummaryGroupsMode() === ConfigService::RESPONSE_SUMMARY_MODE_SPECIFIC) {
-			return array_values(array_map('strtolower', $whitelistedGroups));
-		}
-
 		$restrictionGroups = [];
 		foreach ($appointments as $appointment) {
-			// A numerically-named group decodes to an int out of the JSON column.
-			$groups = array_map('strval', $this->visibilityService->getVisibilitySettings($appointment)['groups']);
-			foreach ($groups as $groupId) {
-				$restrictionGroups[strtolower($groupId)] = true;
-			}
-		}
-		if ($restrictionGroups !== []) {
-			return array_keys($restrictionGroups);
+			$restrictionGroups = array_merge($restrictionGroups, $this->visibilityService->getVisibilitySettings($appointment)['groups']);
 		}
 
-		return $this->configService->getResponseSummaryGroupsMode() === ConfigService::RESPONSE_SUMMARY_MODE_ALL
-			? null
-			: [];
+		$groups = ConfigService::sectionGroups($this->configService->getResponseSummaryGroupsMode(), $whitelistedGroups, $restrictionGroups);
+
+		return $groups === null ? null : array_values(array_unique(array_map('strtolower', $groups)));
 	}
 
 	/**

lib/Service/ResponseSummaryService.php

@@ -178,24 +178,13 @@ private function buildCache(Appointment $appointment, array $responses): array {
 			}
 		}
 
-		// Pre-fetch group objects and their users for whichever groups can
-		// render as sections, mirroring isGroupAllowedCached()'s precedence:
-		// the whitelist in "specific" mode; a restricted appointment's own
-		// restriction groups regardless of mode (issue #199 — only those can
-		// ever pass, so fetching every instance group would be wasted work);
-		// every Nextcloud group otherwise, but only in "all" mode ("none"
-		// suppresses grouping entirely on a fully open appointment).
+		// Pre-fetch members only of the groups that can render as sections —
+		// scanning every instance group for a restricted appointment would be
+		// wasted work (issue #199).
 		/** @var array<array-key, list<\OCP\IUser>> $groupUsers */
 		$groupUsers = [];
-		if ($groupsMode === ConfigService::RESPONSE_SUMMARY_MODE_SPECIFIC) {
-			$sectionGroupIds = $sectionGroups;
-		} elseif (!empty($appointmentVisibleGroupsLower)) {
-			$sectionGroupIds = $visibilitySettings['groups'];
-		} elseif ($groupsMode === ConfigService::RESPONSE_SUMMARY_MODE_ALL) {
-			$sectionGroupIds = $this->visibilityService->getAllGroupIds();
-		} else {
-			$sectionGroupIds = [];
-		}
+		$sectionGroupIds = ConfigService::sectionGroups($groupsMode, $whitelistedGroups, $visibilitySettings['groups'])
+			?? $this->visibilityService->getAllGroupIds();
 		foreach ($sectionGroupIds as $groupId) {
 			$groupUsers[$groupId] = $this->visibilityService->getGroupMembers($groupId);
 		}

playwright.config.js

@@ -111,6 +111,8 @@ export default defineConfig({
 				'39-vacation-calendar.spec.js',
 				// Wipes every appointment and answers as two users.
 				'40-capacity-ui.spec.js',
+				// Switches the response summary grouping mode and the group list.
+				'41-checkin-group-filter.spec.js',
 			],
 			fullyParallel: false,
 			workers: 1,

psalm-baseline.xml

@@ -1620,7 +1620,6 @@
     <MixedArgument>
       <code><![CDATA[$user]]></code>
       <code><![CDATA[$userId]]></code>
-      <code><![CDATA[$whitelistedGroups]]></code>
     </MixedArgument>
     <MixedAssignment>
       <code><![CDATA[$userData['checkinAt']]]></code>

src/components/checkin/CheckinControls.vue

@@ -9,7 +9,7 @@
 				<MagnifyIcon :size="16" />
 			</NcTextField>
 		</div>
-		<div class="group-filter">
+		<div v-if="groupOptions.length" class="group-filter">
 			<NcSelect
 				:modelValue="selectedGroup"
 				:options="groupOptions"

tests/e2e/16-guests-integration.spec.js

@@ -219,6 +219,7 @@ test.describe('Attendance App - Guests integration', () => {
 
 		// Opt the system group in: now it should render like any other tracked group.
 		await saveAdminSettings(request, {
+			responseSummaryGroupsMode: 'specific',
 			whitelistedGroups: ['guest_app'],
 			whitelistedTeams: [],
 			permissions: {
@@ -263,6 +264,7 @@ test.describe('Attendance App - Guests integration', () => {
 		// Whitelist guest_app so the select shows it as a selected pill, then
 		// verify the friendly label replaces the raw id.
 		await saveAdminSettings(request, {
+			responseSummaryGroupsMode: 'specific',
 			whitelistedGroups: ['guest_app'],
 			whitelistedTeams: [],
 			permissions: {
@@ -295,6 +297,7 @@ test.describe('Attendance App - Guests integration', () => {
 			data: { email: GUEST_EMAIL_B, displayName: 'E2E Guest B' },
 		})
 		await saveAdminSettings(request, {
+			responseSummaryGroupsMode: 'specific',
 			whitelistedGroups: ['guest_app'],
 			whitelistedTeams: [],
 			permissions: {

tests/e2e/4-admin-settings.spec.js

@@ -290,6 +290,7 @@ test.describe('Attendance App - Admin Settings', () => {
 
 		test.beforeEach(async ({ request }) => {
 			await saveAdminSettings(request, {
+				responseSummaryGroupsMode: 'specific',
 				whitelistedGroups: [firstGroup, secondGroup],
 				whitelistedTeams: [],
 				permissions: { ...PERMISSIVE_PERMISSIONS },
@@ -336,7 +337,7 @@ test.describe('Attendance App - Admin Settings', () => {
 		})
 
 		test('should not offer the order list for fewer than two groups', async ({ page, loginAsUser, request }) => {
-			await saveAdminSettings(request, { whitelistedGroups: [firstGroup] })
+			await saveAdminSettings(request, { responseSummaryGroupsMode: 'specific', whitelistedGroups: [firstGroup] })
 
 			await loginAsUser('admin', 'admin')
 			await page.goto('/settings/admin/attendance')
@@ -425,7 +426,10 @@ test.describe('Attendance App - Admin Settings', () => {
 			await page.goto('/settings/admin/attendance')
 			await page.waitForLoadState('networkidle')
 
-			// Find the whitelisted groups selector
+			// The picker only appears once "Specific groups" mode is selected
+			const groupingRow = page.locator('[data-test="response-summary-groups"]')
+			await groupingRow.getByText('Specific groups', { exact: true }).click()
+
 			const groupsSelect = page.locator('[data-test="select-whitelisted-groups"]')
 			await expect(groupsSelect).toBeVisible()
 
@@ -468,6 +472,9 @@ test.describe('Attendance App - Admin Settings', () => {
 				await row.getByText('Specific groups', { exact: true }).click()
 			}
 
+			const groupingRow = page.locator('[data-test="response-summary-groups"]')
+			await groupingRow.getByText('Specific groups', { exact: true }).click()
+
 			const selectFields = [
 				'select-whitelisted-groups',
 				...permissionNames.map((permission) => `permission-${permission}-groups`),

tests/e2e/41-checkin-group-filter.spec.js

@@ -0,0 +1,48 @@
+import { createAppointmentViaAPI, deleteAllAppointments, expect, goToCheckin, reloadWebWorkers, resetAdminSettings, saveAdminSettings, test } from './fixtures/nextcloud.js'
+
+// The check-in list's group filter follows the response summary grouping
+// mode: nothing to pick from under "No grouping" (issue #212).
+test.describe('Check-in group filter follows the grouping mode', () => {
+	let appointmentId
+
+	test.beforeAll(async ({ request }) => {
+		const data = await createAppointmentViaAPI(request, {
+			name: 'Group Filter Test Meeting',
+			daysFromNow: 3,
+			durationHours: 1,
+		})
+		appointmentId = data.id
+	})
+
+	test.afterAll(async ({ request }) => {
+		await resetAdminSettings(request)
+		await reloadWebWorkers()
+		await deleteAllAppointments(request)
+	})
+
+	test.beforeEach(async ({ loginAsUser }) => {
+		await loginAsUser('admin', 'admin')
+	})
+
+	test('offers the group filter once the admin picked groups', async ({ page, request }) => {
+		await saveAdminSettings(request, {
+			responseSummaryGroupsMode: 'specific',
+			whitelistedGroups: ['admin'],
+		})
+		await reloadWebWorkers()
+
+		await goToCheckin(page, appointmentId)
+		await expect(page.locator('[data-test="select-group-filter"]')).toBeVisible()
+	})
+
+	test('hides the group filter again under "No grouping"', async ({ page, request }) => {
+		await saveAdminSettings(request, {
+			responseSummaryGroupsMode: 'none',
+			whitelistedGroups: [],
+		})
+		await reloadWebWorkers()
+
+		await goToCheckin(page, appointmentId)
+		await expect(page.locator('[data-test="select-group-filter"]')).toBeHidden()
+	})
+})