Merge pull request #41 from nextcloud/fix/noid/whos-off-leave-type-icons

fix(whos-off): stop showing sick colleagues as sunbathing

Author: karlitschek

SPECIFICATION.md

@@ -493,6 +493,21 @@ is what counts (§7).
 - **Team calendar / who's-off:** managers see their direct reports; HR sees the
   whole company; every employee sees their own team (peers who share the same
   `manager_uid`). A month/timeline view rendered from approved + pending requests.
+  - **Whose leave type is shown.** The type travels with an event when the admin set
+    the shared-calendar visibility to `reveal`, or when the viewer is somebody who
+    could open that request and read the type off it anyway — its owner, their line
+    manager, or HR (§2, `canView`). Everyone else gets `typeId: null` and the client
+    labels the absence generically. With no viewer the type is withheld from all
+    (fail-closed).
+    - The policy protects a *colleague's* privacy — a peer must not learn that
+      somebody is on sick leave. It is not a restriction on HR, who record sick
+      leave, nor on the line manager who approved the absence, and withholding it
+      from them only degraded their own view: with no type to label the absence,
+      the client fell back to a generic marker, so an HR timeline of sick
+      colleagues read as a row of holidays.
+    - The generic marker must be **neutral about the reason**. Withholding why
+      somebody is away and then implying a cheerful reason is worse than either
+      revealing it or saying nothing.
 - **Conflict warning:** when a manager reviews a request, compute the maximum number
   of concurrently-absent team members on any day in the requested range. If it meets
   or exceeds a configurable threshold (admin setting **max concurrent absences per

js/absence-main.mjs

@@ -52,11 +52,18 @@ public function resolveScopeUids(string $actorUid, string $scope): array {
 	 * Who's-off events + per-day concurrency for a set of employees in a range.
 	 *
 	 * The leave *type* of another employee is only revealed when the admin set the
-	 * shared-calendar visibility to "reveal"; under the default "neutral" policy the
-	 * viewer sees that a colleague is absent but not the category (e.g. sick leave),
-	 * mirroring {@see CalendarService::sharedTitle}. The viewer always sees their own
-	 * types. When no viewer is given, types are neutralised for everyone under the
-	 * neutral policy (fail-closed).
+	 * shared-calendar visibility to "reveal", or when the viewer is somebody who may
+	 * open that request and read the type off it anyway — its owner, their line
+	 * manager, or HR ({@see PermissionService::canView()}). Under the default
+	 * "neutral" policy everyone else sees that a colleague is absent but not the
+	 * category, mirroring {@see CalendarService::sharedTitle}. When no viewer is
+	 * given, types are neutralised for everyone (fail-closed).
+	 *
+	 * The policy exists to stop a *colleague's* sick leave becoming visible to the
+	 * team (§8); withholding the type from HR, who record sick leave in the first
+	 * place, protects nothing and actively misinforms — the client has no type to
+	 * label the absence with and has to fall back to a generic marker, so an HR
+	 * timeline of sick colleagues used to read as a row of holidays.
 	 *
 	 * @param string[] $employeeUids
 	 * @return array{events:list<array<string,mixed>>,byDate:array<string,int>,maxConcurrent:int,threshold:int,conflict:bool}
@@ -67,18 +74,30 @@ public function getCoverage(array $employeeUids, string $from, string $to, ?int
 		$statuses = [LeaveRequest::STATUS_APPROVED, LeaveRequest::STATUS_PENDING, LeaveRequest::STATUS_ESCALATED, LeaveRequest::STATUS_WITHDRAWAL_PENDING];
 		$requests = $this->requestMapper->findForEmployeesInRange($employeeUids, $from, $to, $statuses);
 
+		// Resolve the viewer's reach once rather than asking canView() per request: a
+		// company-wide month can hold hundreds of rows, and the answer depends only on
+		// who is looking, not on which request.
+		$viewerIsHr = !$revealTypes && $viewerUid !== null && $this->permission->isHr($viewerUid);
+		$viewerReports = (!$revealTypes && !$viewerIsHr && $viewerUid !== null)
+			? $this->managerResolver->getDirectReports($viewerUid)
+			: [];
+
 		$events = [];
 		$byDate = [];
 		foreach ($requests as $request) {
 			if ($excludeRequestId !== null && $request->getId() === $excludeRequestId) {
 				continue;
 			}
-			$ownEvent = $viewerUid !== null && $request->getEmployeeUid() === $viewerUid;
+			$employeeUid = $request->getEmployeeUid();
+			$maySeeType = $revealTypes
+				|| $viewerIsHr
+				|| ($viewerUid !== null && $employeeUid === $viewerUid)
+				|| in_array($employeeUid, $viewerReports, true);
 			$events[] = [
 				'requestId' => $request->getId(),
-				'employeeUid' => $request->getEmployeeUid(),
-				'displayName' => $this->displayName($request->getEmployeeUid()),
-				'typeId' => ($revealTypes || $ownEvent) ? $request->getTypeId() : null,
+				'employeeUid' => $employeeUid,
+				'displayName' => $this->displayName($employeeUid),
+				'typeId' => $maySeeType ? $request->getTypeId() : null,
 				'status' => $request->getStatus(),
 				'start' => $request->getStartDate(),
 				'end' => $request->getEndDate(),

js/absence-main.mjs.map

@@ -44,8 +44,12 @@ export const store = reactive({
 	leaveType(id) {
 		// A null/undefined id means the server withheld the leave type (neutral
 		// shared-calendar visibility): show a generic "Absent" marker, not "Unknown".
+		// The marker has to say nothing about *why* somebody is away. A palm tree here
+		// was not neutral — it read as a holiday, so a colleague on sick leave was
+		// shown sunbathing. Withholding the reason and then inventing a cheerful one
+		// is worse than either revealing it or saying nothing.
 		if (id === null || id === undefined) {
-			return { label: t('absence', 'Absent'), color: '#888', icon: '🌴' }
+			return { label: t('absence', 'Absent'), color: '#888', icon: '•' }
 		}
 		return this.leaveTypes.find((t) => t.id === id) || { label: t('absence', 'Unknown'), color: '#888', icon: '❔' }
 	},

lib/Service/CoverageService.php

@@ -153,11 +153,18 @@ export default {
 			const type = store.leaveType(r.typeId)
 			const range = formatRange(r.startDate, r.endDate)
 			if (r.startDate <= today) {
+				// The hero already renders the leave type's own icon next to this text, so
+				// the palm that used to be baked into the string was a second, type-blind
+				// one — it wished a holiday on whatever the absence actually was. Sick
+				// leave is not something to enjoy, either.
+				const headline = type.key === 'sick'
+					? t('absence', 'Get well soon.')
+					: t('absence', 'Enjoy your {type}!', { type: type.label.toLowerCase() })
 				return {
 					icon: type.icon,
 					color: type.color,
 					eyebrow: t('absence', 'You are off right now'),
-					headline: t('absence', 'Enjoy your {type}! 🌴', { type: type.label.toLowerCase() }),
+					headline,
 					sub: range,
 					live: false,
 				}

src/store.js

@@ -87,6 +87,85 @@ public function testRevealPolicyExposesType(): void {
 		self::assertSame(3, $result['events'][0]['typeId']);
 	}
 
+	public function testHrSeesTheTypeUnderTheNeutralPolicy(): void {
+		// HR record sick leave and can open any request to read its type, so hiding it
+		// on the who's-off timeline protects nothing — it only leaves the client with
+		// no type to label the absence with, which used to render as a holiday.
+		$this->config->method('getSharedCalendarVisibility')->willReturn(ConfigService::VISIBILITY_NEUTRAL);
+		$this->config->method('getMaxConcurrentAbsences')->willReturn(0);
+		$this->permission->method('isHr')->with('viewer')->willReturn(true);
+		$this->requestMapper->method('findForEmployeesInRange')->willReturn([
+			$this->request(2, 'colleague'),
+		]);
+
+		$result = $this->service->getCoverage(['colleague'], '2026-01-01', '2026-01-31', null, 'viewer');
+
+		self::assertSame(3, $result['events'][0]['typeId']);
+	}
+
+	public function testAManagerSeesTheTypeOfTheirOwnReports(): void {
+		$this->config->method('getSharedCalendarVisibility')->willReturn(ConfigService::VISIBILITY_NEUTRAL);
+		$this->config->method('getMaxConcurrentAbsences')->willReturn(0);
+		$this->permission->method('isHr')->willReturn(false);
+		$this->managerResolver->method('getDirectReports')->with('boss')->willReturn(['report']);
+		$this->requestMapper->method('findForEmployeesInRange')->willReturn([
+			$this->request(2, 'report'),
+			$this->request(3, 'somebody-elses-report'),
+		]);
+
+		$result = $this->service->getCoverage(['report', 'somebody-elses-report'], '2026-01-01', '2026-01-31', null, 'boss');
+		$byUid = [];
+		foreach ($result['events'] as $event) {
+			$byUid[$event['employeeUid']] = $event['typeId'];
+		}
+
+		self::assertSame(3, $byUid['report'], 'Their own report, whose requests they decide');
+		self::assertNull($byUid['somebody-elses-report'], 'Not their report — still withheld');
+	}
+
+	public function testAPeerStillLearnsNothingAboutAColleague(): void {
+		// The protection the policy exists for: a plain colleague sees that somebody is
+		// away and nothing about why.
+		$this->config->method('getSharedCalendarVisibility')->willReturn(ConfigService::VISIBILITY_NEUTRAL);
+		$this->config->method('getMaxConcurrentAbsences')->willReturn(0);
+		$this->permission->method('isHr')->willReturn(false);
+		$this->managerResolver->method('getDirectReports')->willReturn([]);
+		$this->requestMapper->method('findForEmployeesInRange')->willReturn([
+			$this->request(2, 'colleague'),
+		]);
+
+		$result = $this->service->getCoverage(['colleague'], '2026-01-01', '2026-01-31', null, 'peer');
+
+		self::assertNull($result['events'][0]['typeId']);
+	}
+
+	public function testAnAnonymousCallerLearnsNothing(): void {
+		// No viewer to check permissions against, so fail closed rather than reveal.
+		$this->config->method('getSharedCalendarVisibility')->willReturn(ConfigService::VISIBILITY_NEUTRAL);
+		$this->config->method('getMaxConcurrentAbsences')->willReturn(0);
+		$this->permission->expects(self::never())->method('isHr');
+		$this->requestMapper->method('findForEmployeesInRange')->willReturn([
+			$this->request(2, 'colleague'),
+		]);
+
+		$result = $this->service->getCoverage(['colleague'], '2026-01-01', '2026-01-31', null, null);
+
+		self::assertNull($result['events'][0]['typeId']);
+	}
+
+	public function testTheRevealPolicyDoesNotBotherResolvingPermissions(): void {
+		// Everyone sees every type anyway, so the group and manager lookups are waste.
+		$this->config->method('getSharedCalendarVisibility')->willReturn(ConfigService::VISIBILITY_REVEAL);
+		$this->config->method('getMaxConcurrentAbsences')->willReturn(0);
+		$this->permission->expects(self::never())->method('isHr');
+		$this->managerResolver->expects(self::never())->method('getDirectReports');
+		$this->requestMapper->method('findForEmployeesInRange')->willReturn([
+			$this->request(2, 'colleague'),
+		]);
+
+		self::assertSame(3, $this->service->getCoverage(['colleague'], '2026-01-01', '2026-01-31', null, 'viewer')['events'][0]['typeId']);
+	}
+
 	public function testRejectsInvalidRange(): void {
 		$this->expectException(ValidationException::class);
 		$this->service->getCoverage(['viewer'], '2026-13-99', '2026-01-31', null, 'viewer');