From f07578f5cf3da5250883cc883fe9ad89efc55ba5 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Mon, 10 Aug 2026 22:08:04 +0200 Subject: [PATCH] feat(notifications): carry what people wrote into the notification and the email MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comments an employee, manager or HR leave on a request are the substance of the workflow — the reason for asking, the reason for refusing, the question that holds a decision up — and until now almost none of it left the database. A comment produced no notification and no email at all, so it existed only behind the request's Comments tab, which nobody opens unless they already know there is something to read. A manager's question could sit unanswered until the request escalated on a timer. Adding a comment now notifies and mails the other people on the request: the employee and their line manager, plus HR once the request has been escalated and they are a party to it. The author is never notified of their own comment. The same text is now carried by the messages that already existed, attributed to whoever wrote it: - new request / escalation / reminder — the applicant's reason, which the manager was previously asked to decide without; - approved — the manager's approval note, which was dropped entirely; - rejected — the reason, which reached the email only, glued into the middle of the summary sentence, and never the notification; - withdrawal declined — the refusal reason, in place of the previous "See the comments on the request for the reason." The email quotes the text in full via addBodyListItem, which escapes it and keeps its line breaks, so a comment written as several paragraphs still reads as several paragraphs. A notification renders on one line and its parameters are copied to every recipient's row, so it carries a whitespace-collapsed 200-char opening instead; where a note is present it replaces the message boilerplate, because "Review it in Absence." says nothing the Review button does not while the reason is the reason to look at all. The replacement is the one deliberate exception: they are told the dates and nothing else. Cover duty does not come with a right to read a reason that may be medical, and a test pins that. Notifications stored before this change have neither parameter, so the notifier reads both defensively rather than failing to parse an old row. For the server log, comment bodies and decision comments were already there as `detail`. The gap was the applicant's own reason, which was never logged at all; it now goes into the request_created context as `reason`. Deliberately not into `detail` — that feeds the per-request history timeline, where the Details tab already shows the reason and a second copy would just be noise. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 10 +- SPECIFICATION.md | 17 +- lib/Notification/Notifier.php | 16 ++ lib/Service/NotificationService.php | 114 +++++++-- lib/Service/RequestService.php | 27 +- .../Unit/Service/NotificationServiceTest.php | 235 ++++++++++++++++++ tests/Unit/Service/RequestServiceTest.php | 51 ++++ 7 files changed, 447 insertions(+), 23 deletions(-) create mode 100644 tests/Unit/Service/NotificationServiceTest.php diff --git a/README.md b/README.md index 4665e04..41b73b8 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,10 @@ Built to the specification in [SPECIFICATION.md](./SPECIFICATION.md). and CSV export. - **Calendar sync**: approved leave is written to a personal and a shared team calendar via CalDAV. -- **Notifications, email and activity** for every step. +- **Notifications, email and activity** for every step — including whatever the + employee, manager or HR wrote: the reason on the request, the decision comment and + new comments are quoted in the notification and the email, so nobody has to open + the app to find out what was said. - Native Nextcloud UI (Vue 3 + `@nextcloud/vue`), dark-mode aware, translatable. ## Screenshots @@ -161,7 +164,10 @@ withdrawal requested/approved/rejected, escalation, comments, entitlement change carry-over rollover/expiry, leave-type and holiday changes, admin-config changes, and GDPR user-data purge — is written to **`nextcloud.log`** as a structured JSON entry tagged `"app":"absence"` with a machine-readable `action` and full context (actor, -request id, employee, type, dates, working days, status). +request id, employee, type, dates, working days, status). Where the action carried +free text, the text is part of the entry too: `detail` holds a comment body or a +decision comment, `reason` holds the note the applicant wrote when creating the +request. These entries are **always written regardless of the instance log level**: on install (and every update) the app adds `absence` to the system `log.condition.apps` list, diff --git a/SPECIFICATION.md b/SPECIFICATION.md index f74f9d8..f023c9b 100644 --- a/SPECIFICATION.md +++ b/SPECIFICATION.md @@ -522,7 +522,9 @@ All four channels are required. - **Nextcloud notifications:** implement `OCP\Notification\INotifier`. Events: new request (→ manager), decision made (→ employee), escalation (→ HR), - reminder (→ manager), withdrawal request (→ manager/HR), **replacement assigned** + reminder (→ manager), withdrawal request (→ manager/HR), **comment added** + (→ the employee and their manager, plus HR once the request has been escalated; + never back to the comment's own author), **replacement assigned** (→ replacement, on approval) and **replacement cancelled** (→ replacement, when approved leave is cancelled) — §5.1. These are pushed (a standard NC notification is delivered to push automatically). Provide actionable notifications (Approve/Reject @@ -530,13 +532,24 @@ All four channels are required. - **Email:** via `OCP\Mail\IMailer` with templated messages (`OCP\Mail\IEMailTemplate`) for each of the above events. Respect the user's configured email + language. +- **What people wrote travels with the message.** Free text on a request — the + applicant's `reason`, the `decision_comment`, the body of a comment (§3.6) — is + carried by the notification and the email that announce the event, attributed to + its author. A recipient must never have to open the app to find out what was + actually said. The email quotes the text in full; the notification, which renders + on one line, carries a whitespace-collapsed opening of it and where a note is + present it takes the place of boilerplate like "Review it in Absence.". + One deliberate exception: the replacement (§5.1) is told the dates only, never the + reason — cover duty does not come with a right to read it. - **Activity:** implement `OCP\Activity\IProvider` / setting so all state changes appear in the Activity app feed, filterable to an "Absence" activity type. Include activity for HR overrides and balance adjustments. - **Server-log audit trail (always-on):** every important action is written to `nextcloud.log` as a structured entry tagged `["app" => "absence"]` with a machine-readable `action` and full context (actor, request id, employee, type, - dates, working days, status). Covered actions: the full request lifecycle (create, + dates, working days, status) plus, where the action carried free text, that text + itself — `detail` for a comment body or decision comment, `reason` for the note + the applicant wrote on creation. Covered actions: the full request lifecycle (create, edit, superseding edit, HR edit, approve, reject, cancel, withdrawal request/approve/reject, escalate, comment), entitlement changes, bulk-set, carry-over rollover/expiry, leave-type and holiday changes, admin-config changes, diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index db26d50..bd44c47 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -44,6 +44,9 @@ public function prepare(INotification $notification, string $languageCode): INot $params = $notification->getSubjectParameters(); $employee = $this->displayName((string)($params['employee'] ?? '')); $requestId = (string)($params['requestId'] ?? $notification->getObjectId()); + // Notifications stored before notes were carried have neither key. + $note = trim((string)($params['note'] ?? '')); + $noteAuthor = $this->displayName((string)($params['noteAuthor'] ?? '')); [$subject, $message] = match ($notification->getSubject()) { NotificationService::SUBJECT_NEW_REQUEST => [ @@ -82,9 +85,22 @@ public function prepare(INotification $notification, string $languageCode): INot $l->t('No longer covering for %s', [$employee]), $l->t('Their leave was cancelled.'), ], + NotificationService::SUBJECT_COMMENT => [ + $noteAuthor !== '' + ? $l->t('%s commented on a leave request', [$noteAuthor]) + : $l->t('New comment on a leave request'), + '', + ], default => throw new UnknownNotificationException('Unknown subject'), }; + // What someone actually wrote beats the boilerplate that would otherwise fill + // this line: "Review it in Absence." says nothing the Review button doesn't, + // while the reason or decision comment is the reason to look at all. + if ($note !== '') { + $message = $note; + } + $notification->setParsedSubject($subject); if ($message !== '') { $notification->setParsedMessage($message); diff --git a/lib/Service/NotificationService.php b/lib/Service/NotificationService.php index 2c4c137..743d09b 100644 --- a/lib/Service/NotificationService.php +++ b/lib/Service/NotificationService.php @@ -32,6 +32,15 @@ class NotificationService { public const SUBJECT_WITHDRAWAL_REJECTED = 'withdrawal_rejected'; public const SUBJECT_REPLACEMENT_ASSIGNED = 'replacement_assigned'; public const SUBJECT_REPLACEMENT_CANCELLED = 'replacement_cancelled'; + public const SUBJECT_COMMENT = 'comment_added'; + + /** + * How much of a note survives into a Nextcloud notification. The free text is + * copied into every recipient's notification row and rendered on a single line + * there, so the notification carries a readable opening and the email — which + * has room for it — carries the whole thing. + */ + private const NOTE_PREVIEW_LENGTH = 200; public function __construct( private INotificationManager $notificationManager, @@ -44,22 +53,32 @@ public function __construct( } public function notifyNewRequest(LeaveRequest $request, string $managerUid): void { - $this->send($managerUid, self::SUBJECT_NEW_REQUEST, $request, true); + $this->send($managerUid, self::SUBJECT_NEW_REQUEST, $request, true, $request->getReason(), $request->getEmployeeUid()); } /** @param string[] $hrUids */ public function notifyEscalation(LeaveRequest $request, array $hrUids): void { foreach ($hrUids as $uid) { - $this->send($uid, self::SUBJECT_ESCALATION, $request, true); + $this->send($uid, self::SUBJECT_ESCALATION, $request, true, $request->getReason(), $request->getEmployeeUid()); } } public function notifyDecision(LeaveRequest $request, bool $approved): void { - $this->send($request->getEmployeeUid(), $approved ? self::SUBJECT_APPROVED : self::SUBJECT_REJECTED, $request, false); + // The decision comment is the whole substance of the message for a rejection + // and the manager's note on an approval — read it off the request the caller + // just wrote rather than making every call site pass it again. + $this->send( + $request->getEmployeeUid(), + $approved ? self::SUBJECT_APPROVED : self::SUBJECT_REJECTED, + $request, + false, + $request->getDecisionComment(), + $request->getDecidedBy(), + ); } public function notifyReminder(LeaveRequest $request, string $managerUid): void { - $this->send($managerUid, self::SUBJECT_REMINDER, $request, true); + $this->send($managerUid, self::SUBJECT_REMINDER, $request, true, $request->getReason(), $request->getEmployeeUid()); } /** @param string[] $recipientUids */ @@ -69,9 +88,29 @@ public function notifyWithdrawal(LeaveRequest $request, array $recipientUids): v } } - /** Tell the employee their withdrawal was declined — the leave stays approved. */ - public function notifyWithdrawalRejected(LeaveRequest $request): void { - $this->send($request->getEmployeeUid(), self::SUBJECT_WITHDRAWAL_REJECTED, $request, false); + /** + * Tell the employee their withdrawal was declined — the leave stays approved. + * The reason lives in a comment rather than on the request (see + * {@see RequestService::reject()}), so the caller has to hand it over. + */ + public function notifyWithdrawalRejected(LeaveRequest $request, ?string $comment = null, ?string $actorUid = null): void { + $this->send($request->getEmployeeUid(), self::SUBJECT_WITHDRAWAL_REJECTED, $request, false, $comment, $actorUid); + } + + /** + * Tell the other people on a request that someone commented on it. Without + * this a comment only exists behind the request's Comments tab, which nobody + * opens unless they already know there is something to read. + * + * @param string[] $recipientUids + */ + public function notifyComment(LeaveRequest $request, string $authorUid, string $body, array $recipientUids): void { + foreach (array_unique(array_filter($recipientUids)) as $uid) { + if ($uid === $authorUid) { + continue; + } + $this->send($uid, self::SUBJECT_COMMENT, $request, false, $body, $authorUid); + } } /** Tell the nominated replacement they now cover for the employee (§5.1). */ @@ -90,12 +129,20 @@ public function notifyReplacementCancelled(LeaveRequest $request): void { } } - private function send(string $recipientUid, string $subject, LeaveRequest $request, bool $actionable): void { - $this->sendNotification($recipientUid, $subject, $request, $actionable); - $this->sendEmail($recipientUid, $subject, $request); + /** + * @param ?string $note free text written by a person that the recipient would + * otherwise only find by opening the request: the + * applicant's reason, a decision comment, or a comment + * left on the request + * @param ?string $noteAuthorUid who wrote $note, so it can be attributed + */ + private function send(string $recipientUid, string $subject, LeaveRequest $request, bool $actionable, ?string $note = null, ?string $noteAuthorUid = null): void { + $note = trim((string)$note); + $this->sendNotification($recipientUid, $subject, $request, $actionable, $note, $noteAuthorUid); + $this->sendEmail($recipientUid, $subject, $request, $note, $noteAuthorUid); } - private function sendNotification(string $recipientUid, string $subject, LeaveRequest $request, bool $actionable): void { + private function sendNotification(string $recipientUid, string $subject, LeaveRequest $request, bool $actionable, string $note, ?string $noteAuthorUid): void { try { $notification = $this->notificationManager->createNotification(); $notification->setApp(ConfigService::APP_ID) @@ -106,6 +153,8 @@ private function sendNotification(string $recipientUid, string $subject, LeaveRe 'employee' => $request->getEmployeeUid(), 'requestId' => (string)$request->getId(), 'actionable' => $actionable, + 'note' => $this->preview($note), + 'noteAuthor' => (string)$noteAuthorUid, ]); $this->notificationManager->notify($notification); } catch (\Throwable $e) { @@ -113,7 +162,19 @@ private function sendNotification(string $recipientUid, string $subject, LeaveRe } } - private function sendEmail(string $recipientUid, string $subject, LeaveRequest $request): void { + /** + * Squeeze a note onto the single line a notification gives it: collapse the + * line breaks it may contain and cut it to length. The email carries the rest. + */ + private function preview(string $note): string { + $note = trim((string)preg_replace('/\s+/u', ' ', $note)); + if (mb_strlen($note) <= self::NOTE_PREVIEW_LENGTH) { + return $note; + } + return mb_substr($note, 0, self::NOTE_PREVIEW_LENGTH - 1) . '…'; + } + + private function sendEmail(string $recipientUid, string $subject, LeaveRequest $request, string $note, ?string $noteAuthorUid): void { $user = $this->userManager->get($recipientUid); if (!$user instanceof IUser) { return; @@ -125,13 +186,24 @@ private function sendEmail(string $recipientUid, string $subject, LeaveRequest $ try { $lang = $this->l10nFactory->getUserLanguage($user); $l = $this->l10nFactory->get(ConfigService::APP_ID, $lang); - [$heading, $body] = $this->emailContent($l, $subject, $request); + [$heading, $body] = $this->emailContent($l, $subject, $request, $noteAuthorUid); $template = $this->mailer->createEMailTemplate('absence.' . $subject); $template->setSubject($heading); $template->addHeader(); $template->addHeading($heading); $template->addBodyText($body); + // Quote the note verbatim under the summary, attributed. addBodyListItem + // escapes the text and keeps its line breaks, so a comment written as + // several lines still reads as several lines. + if ($note !== '') { + $template->addBodyListItem( + $note, + $noteAuthorUid !== null && $noteAuthorUid !== '' + ? $l->t('%s wrote:', [$this->displayName($noteAuthorUid)]) + : $l->t('Comment:'), + ); + } $template->addBodyButton( $l->t('Open Absence'), $this->urlGenerator->linkToRouteAbsolute('absence.page.index') . '#/requests/' . $request->getId(), @@ -150,8 +222,9 @@ private function sendEmail(string $recipientUid, string $subject, LeaveRequest $ /** * @return array{0:string,1:string} heading and body */ - private function emailContent(\OCP\IL10N $l, string $subject, LeaveRequest $request): array { + private function emailContent(\OCP\IL10N $l, string $subject, LeaveRequest $request, ?string $noteAuthorUid): array { $employee = $this->displayName($request->getEmployeeUid()); + $author = $noteAuthorUid !== null && $noteAuthorUid !== '' ? $this->displayName($noteAuthorUid) : ''; $range = $request->getStartDate() . ' – ' . $request->getEndDate(); return match ($subject) { self::SUBJECT_NEW_REQUEST => [ @@ -168,7 +241,8 @@ private function emailContent(\OCP\IL10N $l, string $subject, LeaveRequest $requ ], self::SUBJECT_REJECTED => [ $l->t('Your leave was declined'), - $l->t('Your leave request for %1$s was declined. %2$s', [$range, (string)$request->getDecisionComment()]), + // The reason follows as the quoted note; it is required on a rejection. + $l->t('Your leave request for %s was declined.', [$range]), ], self::SUBJECT_REMINDER => [ $l->t('Reminder: leave request from %s', [$employee]), @@ -181,8 +255,10 @@ private function emailContent(\OCP\IL10N $l, string $subject, LeaveRequest $requ self::SUBJECT_WITHDRAWAL_REJECTED => [ $l->t('Your withdrawal request was declined'), // The refusal reason is recorded as a comment on the request, not in - // decision_comment (that still holds the original approval note). - $l->t('Your request to withdraw the leave for %s was declined — the leave stays approved. See the comments on the request for the reason.', [$range]), + // decision_comment (that still holds the original approval note) — it + // is passed in as the note and quoted below, so there is no need to + // send the employee looking for it. + $l->t('Your request to withdraw the leave for %s was declined — the leave stays approved.', [$range]), ], self::SUBJECT_REPLACEMENT_ASSIGNED => [ $l->t('You are covering for %s', [$employee]), @@ -192,6 +268,10 @@ private function emailContent(\OCP\IL10N $l, string $subject, LeaveRequest $requ $l->t('No longer covering for %s', [$employee]), $l->t('%1$s\'s leave for %2$s was cancelled — you no longer need to cover.', [$employee, $range]), ], + self::SUBJECT_COMMENT => [ + $author !== '' ? $l->t('New comment from %s', [$author]) : $l->t('New comment on a leave request'), + $l->t('There is a new comment on the leave request of %1$s for %2$s.', [$employee, $range]), + ], default => [$l->t('Absence update'), $l->t('There is an update on a leave request.')], }; } diff --git a/lib/Service/RequestService.php b/lib/Service/RequestService.php index c55e8ca..566e128 100644 --- a/lib/Service/RequestService.php +++ b/lib/Service/RequestService.php @@ -345,7 +345,14 @@ public function create(string $actorUid, array $data): LeaveRequest { $this->notifications->notifyNewRequest($request, (string)$managerUid); $this->activity->publish(ActivityPublisher::SUBJECT_CREATED, $this->activityParams($request), [$employeeUid, (string)$managerUid], $request); } - $this->audit('request_created', $request, ['actor' => $actorUid, 'detail' => $this->createdDetail($request, $type, $onBehalf)]); + // `reason` is the employee's own note on the request; the log entry is the one + // place it can be reconstructed later, so it goes into the context rather than + // into `detail` (the Details tab already shows it in the request's history). + $this->audit('request_created', $request, [ + 'actor' => $actorUid, + 'detail' => $this->createdDetail($request, $type, $onBehalf), + 'reason' => $request->getReason(), + ]); return $request; } @@ -705,7 +712,7 @@ public function reject(string $actorUid, int $id, string $comment): LeaveRequest $this->notifications->dismiss($request); // A declined withdrawal is not an approval — tell the employee their // leave stands, not "your leave was approved, enjoy!". - $this->notifications->notifyWithdrawalRejected($request); + $this->notifications->notifyWithdrawalRejected($request, $comment, $actorUid); $this->activity->publish(ActivityPublisher::SUBJECT_APPROVED, $this->activityParams($request), [$request->getEmployeeUid(), $actorUid], $request); $this->audit('withdrawal_rejected', $request, ['actor' => $actorUid, 'detail' => $comment]); return $request; @@ -747,9 +754,25 @@ public function addComment(string $actorUid, int $id, string $body): RequestComm $comment->setCreatedAt($this->clock->now()); $comment = $this->commentMapper->insert($comment); $this->audit('comment_added', $request, ['actor' => $actorUid, 'detail' => $body]); + $this->notifications->notifyComment($request, $actorUid, $body, $this->commentRecipients($request)); return $comment; } + /** + * Who hears about a comment: the employee and their line manager, plus HR once + * the request has been through them, so a question HR asked does not sit + * unanswered. The author is filtered out by the notification service. + * + * @return string[] + */ + private function commentRecipients(LeaveRequest $request): array { + $recipients = [$request->getEmployeeUid(), (string)$request->getManagerUid()]; + if ($request->getEscalated()) { + $recipients = [...$recipients, ...$this->permission->getHrUids()]; + } + return array_values(array_unique(array_filter($recipients))); + } + // --------------------------------------------------------------- helpers ---- /** diff --git a/tests/Unit/Service/NotificationServiceTest.php b/tests/Unit/Service/NotificationServiceTest.php new file mode 100644 index 0000000..efb6fe0 --- /dev/null +++ b/tests/Unit/Service/NotificationServiceTest.php @@ -0,0 +1,235 @@ +notificationManager = $this->createMock(INotificationManager::class); + $this->mailer = $this->createMock(IMailer::class); + $this->userManager = $this->createMock(IUserManager::class); + $urlGenerator = $this->createMock(IURLGenerator::class); + $l10nFactory = $this->createMock(IFactory::class); + $logger = $this->createMock(LoggerInterface::class); + + $notification = $this->createMock(INotification::class); + $notification->method(self::anything())->willReturnSelf(); + $notification->method('setSubject')->willReturnCallback( + function (string $subject, array $parameters) use ($notification): INotification { + $this->pushed[] = $parameters; + return $notification; + }, + ); + $this->notificationManager->method('createNotification')->willReturn($notification); + + $l10n = $this->createMock(IL10N::class); + // Interpolate for real, so a mismatch between a format string and its + // arguments shows up here rather than in a recipient's inbox. + $l10n->method('t')->willReturnCallback( + static fn (string $text, $parameters = []): string => $parameters === [] ? $text : vsprintf($text, (array)$parameters), + ); + $l10nFactory->method('getUserLanguage')->willReturn('en'); + $l10nFactory->method('get')->willReturn($l10n); + + $this->template = $this->createMock(IEMailTemplate::class); + $this->template->method('addBodyText')->willReturnCallback( + function (string $text): void { + $this->emailBody[] = $text; + }, + ); + $this->template->method('addBodyListItem')->willReturnCallback( + function (string $text, string $metaInfo = ''): void { + $this->emailNotes[] = [$text, $metaInfo]; + }, + ); + $this->mailer->method('createEMailTemplate')->willReturn($this->template); + $this->mailer->method('createMessage')->willReturn($this->createMock(IMessage::class)); + + $this->service = new NotificationService( + $this->notificationManager, + $this->mailer, + $this->userManager, + $urlGenerator, + $l10nFactory, + $logger, + ); + } + + /** Everyone named here exists, has mail configured and has a display name. */ + private function withUsers(string ...$uids): void { + $this->userManager->method('get')->willReturnCallback( + function (string $uid) use ($uids): ?IUser { + if (!in_array($uid, $uids, true)) { + return null; + } + $user = $this->createMock(IUser::class); + $user->method('getEMailAddress')->willReturn($uid . '@example.com'); + $user->method('getDisplayName')->willReturn(ucfirst($uid)); + return $user; + }, + ); + } + + private function request(): LeaveRequest { + $request = new LeaveRequest(); + $request->setId(7); + $request->setEmployeeUid('emp'); + $request->setManagerUid('boss'); + $request->setStartDate('2026-02-10'); + $request->setEndDate('2026-02-12'); + $request->setStatus(LeaveRequest::STATUS_PENDING); + return $request; + } + + public function testACommentReachesTheOthersWithItsTextAndAuthor(): void { + $this->withUsers('emp', 'boss'); + + $this->service->notifyComment($this->request(), 'boss', 'Can you move this a day later?', ['emp', 'boss']); + + // One notification, to the employee: the author does not need telling. + self::assertCount(1, $this->pushed); + self::assertSame('Can you move this a day later?', $this->pushed[0]['note']); + self::assertSame('boss', $this->pushed[0]['noteAuthor']); + self::assertSame([['Can you move this a day later?', 'Boss wrote:']], $this->emailNotes); + } + + public function testALongCommentIsShortenedForTheNotificationButNotForTheEmail(): void { + // A notification renders on one line; the email has room for the whole thing. + $this->withUsers('emp', 'boss'); + $body = str_repeat('a', 500); + + $this->service->notifyComment($this->request(), 'boss', $body, ['emp']); + + self::assertSame(str_repeat('a', 199) . '…', $this->pushed[0]['note']); + self::assertSame($body, $this->emailNotes[0][0]); + } + + public function testAMultilineCommentIsFlattenedForTheNotificationOnly(): void { + $this->withUsers('emp', 'boss'); + + $this->service->notifyComment($this->request(), 'boss', "First line.\n\nSecond line.", ['emp']); + + self::assertSame('First line. Second line.', $this->pushed[0]['note']); + self::assertSame("First line.\n\nSecond line.", $this->emailNotes[0][0]); + } + + public function testTheApplicantsReasonTravelsWithTheNewRequestNotification(): void { + // Without it the manager is asked to decide on a request whose stated reason + // they can only read by opening the app. + $this->withUsers('emp', 'boss'); + $request = $this->request(); + $request->setReason('Family wedding abroad.'); + + $this->service->notifyNewRequest($request, 'boss'); + + self::assertSame('Family wedding abroad.', $this->pushed[0]['note']); + self::assertSame('emp', $this->pushed[0]['noteAuthor']); + self::assertSame([['Family wedding abroad.', 'Emp wrote:']], $this->emailNotes); + } + + public function testTheDecisionCommentTravelsWithTheOutcome(): void { + $this->withUsers('emp', 'boss'); + $request = $this->request(); + $request->setStatus(LeaveRequest::STATUS_APPROVED); + $request->setDecidedBy('boss'); + $request->setDecisionComment('Approved, but please brief Sam first.'); + + $this->service->notifyDecision($request, true); + + self::assertSame('Approved, but please brief Sam first.', $this->pushed[0]['note']); + self::assertSame('boss', $this->pushed[0]['noteAuthor']); + self::assertSame([['Approved, but please brief Sam first.', 'Boss wrote:']], $this->emailNotes); + } + + public function testARejectionEmailStatesTheReasonOnceOnly(): void { + // The reason used to be pasted into the summary sentence; it is now quoted + // below it, and quoting it twice would read as a mistake. + $this->withUsers('emp', 'boss'); + $request = $this->request(); + $request->setStatus(LeaveRequest::STATUS_REJECTED); + $request->setDecidedBy('boss'); + $request->setDecisionComment('Too many people out that week.'); + + $this->service->notifyDecision($request, false); + + self::assertSame(['Your leave request for 2026-02-10 – 2026-02-12 was declined.'], $this->emailBody); + self::assertSame([['Too many people out that week.', 'Boss wrote:']], $this->emailNotes); + } + + public function testADeclinedWithdrawalCarriesTheReasonItRecordedAsAComment(): void { + $this->withUsers('emp', 'boss'); + + $this->service->notifyWithdrawalRejected($this->request(), 'We have nobody to cover.', 'boss'); + + self::assertSame('We have nobody to cover.', $this->pushed[0]['note']); + self::assertSame([['We have nobody to cover.', 'Boss wrote:']], $this->emailNotes); + } + + public function testAMessageWithNothingWrittenOnItQuotesNothing(): void { + $this->withUsers('emp', 'boss'); + $request = $this->request(); + $request->setStatus(LeaveRequest::STATUS_APPROVED); + $request->setDecidedBy('boss'); + $request->setDecisionComment(null); + + $this->service->notifyDecision($request, true); + + self::assertSame('', $this->pushed[0]['note']); + self::assertSame([], $this->emailNotes); + } + + public function testTheReplacementIsNotToldWhyTheEmployeeIsAway(): void { + // Cover duty does not come with a right to read the reason, which can be + // medical. The replacement gets the dates and nothing else. + $this->withUsers('emp', 'stand-in'); + $request = $this->request(); + $request->setReplacementUid('stand-in'); + $request->setReason('Surgery.'); + + $this->service->notifyReplacementAssigned($request); + + self::assertSame('', $this->pushed[0]['note']); + self::assertSame([], $this->emailNotes); + } +} diff --git a/tests/Unit/Service/RequestServiceTest.php b/tests/Unit/Service/RequestServiceTest.php index 2e2a8d2..672d74d 100644 --- a/tests/Unit/Service/RequestServiceTest.php +++ b/tests/Unit/Service/RequestServiceTest.php @@ -334,4 +334,55 @@ public function testAddCommentRejectsOverlongBody(): void { $this->expectException(ValidationException::class); $this->service->addComment('emp', 5, str_repeat('a', 4001)); } + + public function testAddCommentNotifiesTheEmployeeAndTheManager(): void { + // A comment is otherwise only visible to whoever thinks to open the Comments + // tab, so a manager's question could sit unread until the request expired. + $request = $this->pendingOwnRequest(); + $request->setManagerUid('boss'); + $this->requestMapper->method('find')->with(5)->willReturn($request); + $this->permission->method('canView')->willReturn(true); + $this->commentMapper->method('insert')->willReturnArgument(0); + // Not escalated, so HR is not dragged into a conversation they are not part of. + $this->permission->expects(self::never())->method('getHrUids'); + + $this->notifications->expects(self::once())->method('notifyComment') + ->with($request, 'boss', 'Can you move this a day later?', ['emp', 'boss']); + + $this->service->addComment('boss', 5, 'Can you move this a day later?'); + } + + public function testAddCommentOnAnEscalatedRequestAlsoReachesHr(): void { + // Once HR has been pulled in they are a party to the discussion — a question + // they asked has to come back to them, not just to the line manager. + $request = $this->pendingOwnRequest(); + $request->setManagerUid('boss'); + $request->setEscalated(true); + $this->requestMapper->method('find')->with(5)->willReturn($request); + $this->permission->method('canView')->willReturn(true); + $this->permission->method('getHrUids')->willReturn(['hr1', 'hr2']); + $this->commentMapper->method('insert')->willReturnArgument(0); + + $this->notifications->expects(self::once())->method('notifyComment') + ->with($request, 'emp', 'Any news on this?', ['emp', 'boss', 'hr1', 'hr2']); + + $this->service->addComment('emp', 5, 'Any news on this?'); + } + + public function testDecliningAWithdrawalPassesTheReasonToTheEmployee(): void { + // The reason is recorded as a comment, not in decision_comment, so unless it + // travels with the notification the employee is told "declined" and nothing more. + $request = $this->pendingOwnRequest(); + $request->setManagerUid('boss'); + $request->setStatus(LeaveRequest::STATUS_WITHDRAWAL_PENDING); + $this->requestMapper->method('find')->with(5)->willReturn($request); + $this->requestMapper->method('update')->willReturnArgument(0); + $this->permission->method('canView')->willReturn(true); + $this->permission->method('canDecide')->willReturn(true); + + $this->notifications->expects(self::once())->method('notifyWithdrawalRejected') + ->with($request, 'We have nobody to cover that week.', 'boss'); + + $this->service->reject('boss', 5, 'We have nobody to cover that week.'); + } }