diff --git a/lib/Controller/AppointmentController.php b/lib/Controller/AppointmentController.php index 5a57a8ef..fffb7386 100644 --- a/lib/Controller/AppointmentController.php +++ b/lib/Controller/AppointmentController.php @@ -190,15 +190,6 @@ public function index( $canSeeResponseCounts, ); - // Add checkin summary to each appointment the user may see at least - // the aggregate numbers for — it only ever carries counts, no names. - foreach ($page['appointments'] as &$appointment) { - if ($appointment['myPermissions']['canSeeResponseCounts']) { - $appointment['checkinSummary'] = $this->checkinService->getCheckinSummary($appointment['id']); - } - } - unset($appointment); - return new DataResponse($query->isPaged() ? $page : $page['appointments']); } @@ -799,12 +790,6 @@ public function show(int $id): DataResponse { return new DataResponse(['error' => 'Appointment not found or not visible'], 404); } - // Add checkin summary if user may see at least the aggregate - // numbers for this appointment — it only ever carries counts. - if ($appointment['myPermissions']['canSeeResponseCounts']) { - $appointment['checkinSummary'] = $this->checkinService->getCheckinSummary($id); - } - return new DataResponse($appointment); } catch (\Exception $e) { return new DataResponse(['error' => 'Appointment not found'], 404); diff --git a/lib/Db/AppointmentAttachmentMapper.php b/lib/Db/AppointmentAttachmentMapper.php index 1b46a33f..2e985433 100644 --- a/lib/Db/AppointmentAttachmentMapper.php +++ b/lib/Db/AppointmentAttachmentMapper.php @@ -6,6 +6,7 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; /** @@ -34,20 +35,35 @@ public function find(int $id): AppointmentAttachment { } /** - * @param int $appointmentId - * @return array + * @return list */ public function findByAppointment(int $appointmentId): array { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('appointment_id', $qb->createNamedParameter($appointmentId)) - ) - ->orderBy('added_at', 'ASC'); + return $this->findByAppointments([$appointmentId])[$appointmentId] ?? []; + } - return $this->findEntities($qb); + /** + * The attachments of these appointments, bucketed by appointment, oldest first. + * + * @param list $appointmentIds + * @return array> + */ + public function findByAppointments(array $appointmentIds): array { + $byAppointment = []; + + foreach (array_chunk($appointmentIds, 1000) as $chunk) { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->in('appointment_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->orderBy('added_at', 'ASC') + ->addOrderBy('id', 'ASC'); + + foreach ($this->findEntities($qb) as $attachment) { + $byAppointment[$attachment->getAppointmentId()][] = $attachment; + } + } + + return $byAppointment; } /** diff --git a/lib/Db/AppointmentMapper.php b/lib/Db/AppointmentMapper.php index b4616b25..309380e7 100644 --- a/lib/Db/AppointmentMapper.php +++ b/lib/Db/AppointmentMapper.php @@ -327,6 +327,37 @@ public function findBySeriesId(string $seriesId): array { return $this->findEntities($qb); } + /** + * How many active appointments each series has, for the series a page of + * the list touches — the count is all the list shows. + * + * @param list $seriesIds + * @return array series id → count + */ + public function countBySeriesIds(array $seriesIds): array { + $counts = []; + + foreach (array_chunk($seriesIds, 1000) as $chunk) { + $qb = $this->db->getQueryBuilder(); + $qb->select('series_id') + ->selectAlias($qb->createFunction('COUNT(*)'), 'count') + ->from($this->getTableName()) + ->where($qb->expr()->eq('is_active', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->in('series_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_STR_ARRAY))) + ->groupBy('series_id'); + + $result = $qb->executeQuery(); + /** @var list> $raw IResult::fetchAll() is declared mixed */ + $raw = $result->fetchAll(); + $result->closeCursor(); + foreach ($raw as $row) { + $counts[(string)$row['series_id']] = (int)$row['count']; + } + } + + return $counts; + } + /** * Find active appointments in a series from a given position onward. * diff --git a/lib/Db/AttendanceResponseMapper.php b/lib/Db/AttendanceResponseMapper.php index 67f8a738..c914dd75 100644 --- a/lib/Db/AttendanceResponseMapper.php +++ b/lib/Db/AttendanceResponseMapper.php @@ -83,20 +83,65 @@ public function findByUserForAppointments(string $userId, array $appointmentIds) } /** - * @param int $appointmentId - * @return list + * Every response of these appointments, bucketed by appointment, the + * latest answer first within each bucket. + * + * @param list $appointmentIds + * @return array> */ - public function findByAppointment(int $appointmentId): array { - $qb = $this->db->getQueryBuilder(); + public function findByAppointments(array $appointmentIds): array { + $byAppointment = []; - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('appointment_id', $qb->createNamedParameter($appointmentId)) - ) - ->orderBy('responded_at', 'DESC'); + foreach (array_chunk($appointmentIds, 1000) as $chunk) { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->in('appointment_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->orderBy('responded_at', 'DESC') + ->addOrderBy('id', 'ASC'); - return $this->findEntities($qb); + foreach ($this->findEntities($qb) as $response) { + $byAppointment[$response->getAppointmentId()][] = $response; + } + } + + return $byAppointment; + } + + /** + * Who a manager booked, per appointment — the one column the scheduling + * filters of the list read, for every appointment of the list at once. + * + * @param list $appointmentIds + * @return array> appointment id → booked user ids + */ + public function findBookedUserIds(array $appointmentIds): array { + $byAppointment = []; + + foreach (array_chunk($appointmentIds, 1000) as $chunk) { + $qb = $this->db->getQueryBuilder(); + $qb->select('appointment_id', 'user_id') + ->from($this->getTableName()) + ->where($qb->expr()->eq('booking_status', $qb->createNamedParameter(BookingService::STATUS_BOOKED))) + ->andWhere($qb->expr()->in('appointment_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + + $result = $qb->executeQuery(); + /** @var list> $raw IResult::fetchAll() is declared mixed */ + $raw = $result->fetchAll(); + $result->closeCursor(); + foreach ($raw as $row) { + $byAppointment[(int)$row['appointment_id']][] = (string)$row['user_id']; + } + } + + return $byAppointment; + } + + /** + * @return list + */ + public function findByAppointment(int $appointmentId): array { + return $this->findByAppointments([$appointmentId])[$appointmentId] ?? []; } /** diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index 491afa58..920822fe 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -29,6 +29,10 @@ class Notifier implements INotifier { private IUserManager $userManager; private EventMessages $eventMessages; + /** @var array */ + private array $maybeOffered = []; + private ?string $iconUrl = null; + public function __construct( IFactory $l10nFactory, IURLGenerator $urlGenerator, @@ -115,9 +119,7 @@ public function prepare(INotification $notification, string $languageCode): INot // TRANSLATORS Push notification body for a reminder, shown under a "Response missing" subject that already names the appointment and its date. The app speaks as "we", the people organizing — not about them in the third person. People who cannot come tend to stay silent rather than answer, which leaves the planning guessing, hence the second sentence. Sample German: "Wir warten noch auf dich. Ein Nein hilft uns genauso wie ein Ja.". $l->t('We are still waiting. A no helps just as much as a yes.') ); - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); // Add quick response action buttons if ($appointmentId > 0) { @@ -140,9 +142,7 @@ public function prepare(INotification $notification, string $languageCode): INot // TRANSLATORS Push notification body for a newly created appointment, shown under a "New appointment" subject that already names it and its date. The app speaks as "we", the people organizing — not about them in the third person. "Make it" means being able to attend. Organizers only switch this notification on when they need answers fast, so keep the urgency of the first sentence: it is the whole reason the message was sent. Sample German: "Wir brauchen schnell deine Rückmeldung. Bist du dabei?". $l->t('We need your answer quickly. Let us know whether you can make it.') ); - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); // Add quick response action buttons if ($appointmentId > 0) { @@ -154,18 +154,14 @@ public function prepare(INotification $notification, string $languageCode): INot $rendered = $this->eventMessages->forCancelled($l, $notification->getSubjectParameters(), $notification->getUser()); $notification->setParsedSubject($rendered['subject']); $notification->setParsedMessage($rendered['message']); - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); return $notification; case 'appointment_reactivated': $rendered = $this->eventMessages->forReactivated($l, $notification->getSubjectParameters(), $notification->getUser()); $notification->setParsedSubject($rendered['subject']); $notification->setParsedMessage($rendered['message']); - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); return $notification; case 'appointment_updated': @@ -182,9 +178,7 @@ public function prepare(INotification $notification, string $languageCode): INot $notification->setParsedMessage( $this->eventMessages->describeUpdate((array)($parameters['changed'] ?? []), $l) ); - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); // A moved appointment is exactly when someone needs to revise their answer. if ($appointmentId > 0) { @@ -199,9 +193,7 @@ public function prepare(INotification $notification, string $languageCode): INot : $this->eventMessages->forBookingDeclined($l, $notification->getSubjectParameters(), $notification->getUser()); $notification->setParsedSubject($rendered['subject']); $notification->setParsedMessage($rendered['message']); - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); return $notification; case 'waitlist_promoted': case 'waitlist_not_promoted': @@ -227,9 +219,7 @@ public function prepare(INotification $notification, string $languageCode): INot $l->t('The appointment stayed full, so you no longer need to keep the date free. Thanks for answering.') ); } - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); return $notification; case 'response_submitted': case 'response_changed': @@ -249,18 +239,14 @@ public function prepare(INotification $notification, string $languageCode): INot // TRANSLATORS Push notification body for several appointments added at once, shown under a subject that already gives the number and one example name. The app speaks as "we", the people organizing — not about them in the third person. The recipient answers each appointment separately, which is why this says "which ones" rather than asking for a single answer. Sample German: "Sag uns bitte, bei welchen Terminen du dabei bist.". $l->t('Let us know which ones you can make.') ); - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); return $notification; case 'appointments_series_updated': $rendered = $this->eventMessages->forSeriesUpdated($l, $notification->getSubjectParameters()); $notification->setParsedSubject($rendered['subject']); $notification->setParsedMessage($rendered['message']); - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); return $notification; default: @@ -345,9 +331,7 @@ private function prepareResponseChangeNotification(INotification $notification, } $notification->setParsedSubject($subject); - $notification->setIcon($this->urlGenerator->getAbsoluteURL( - $this->urlGenerator->imagePath('attendance', 'app-dark.svg') - )); + $notification->setIcon($this->iconUrl()); return $notification; } @@ -370,6 +354,16 @@ private function restoreObjectForClientCompatibility(INotification $notification } } + /** + * The same for every notification of the app, while imagePath() walks the + * theme folders (or asks the memcache) on every call. + */ + private function iconUrl(): string { + return $this->iconUrl ??= $this->urlGenerator->getAbsoluteURL( + $this->urlGenerator->imagePath('attendance', 'app-dark.svg'), + ); + } + /** * Notifications carry raw user IDs; resolving them here rather than at send * time keeps renamed accounts correct. Unknown accounts keep the ID. @@ -403,8 +397,15 @@ private function translateResponseValue(string $value, \OCP\IL10N $l): string { * Whether this appointment still offers "Maybe". A notification can outlive * the appointment it points at, and an unknown one keeps all three buttons * rather than silently dropping one. + * + * Remembered per appointment: a wave prepares one notification per + * recipient, and all of them ask about the same appointment. */ private function isMaybeOffered(int $appointmentId): bool { + return $this->maybeOffered[$appointmentId] ??= $this->lookupMaybeOffered($appointmentId); + } + + private function lookupMaybeOffered(int $appointmentId): bool { try { return $this->responsePolicyService->isMaybeAllowed( $this->appointmentMapper->find($appointmentId), diff --git a/lib/Service/AppointmentSerializer.php b/lib/Service/AppointmentSerializer.php index aa624ad1..968e96f2 100644 --- a/lib/Service/AppointmentSerializer.php +++ b/lib/Service/AppointmentSerializer.php @@ -28,12 +28,11 @@ public function serialize(Appointment $appointment): array { /** @var array $data */ $data = $appointment->jsonSerialize(); $data['allowMaybe'] = $this->responsePolicyService->isMaybeAllowed($appointment); - // Only an appointment with a limit pays for the queue query, which is - // why every reader asks limitOf() first. Without one there is nothing to - // count and nothing that can be full. - $hasLimit = $this->capacityService->limitOf($appointment) !== null; - $data['occupancy'] = $hasLimit ? $this->capacityService->occupancy($appointment) : 0; - $data['isFull'] = $hasLimit && $this->capacityService->isFull($appointment); + // Handed on to isFull() so the queue is counted once; without a limit + // occupancy() answers 0 without a query. + $occupancy = $this->capacityService->occupancy($appointment); + $data['occupancy'] = $occupancy; + $data['isFull'] = $this->capacityService->isFull($appointment, $occupancy); return $data; } } diff --git a/lib/Service/AppointmentService.php b/lib/Service/AppointmentService.php index d9358d8f..de82d968 100644 --- a/lib/Service/AppointmentService.php +++ b/lib/Service/AppointmentService.php @@ -88,6 +88,7 @@ public function __construct( AppointmentSerializer $appointmentSerializer, VacationService $vacationService, private TimezoneService $timezoneService, + private CheckinService $checkinService, ) { $this->appointmentMapper = $appointmentMapper; $this->responseMapper = $responseMapper; @@ -1151,19 +1152,24 @@ public function getAppointmentWithUserResponse(int $id, string $userId, bool $in $appointmentData = $this->serializeAppointment($appointment); $appointmentData = $this->enrichVisibilityData($appointmentData); - $appointmentData = $this->enrichSeriesCount($appointmentData, $appointment); + $appointmentData = $this->enrichSeriesCount($appointmentData, $appointment, $this->seriesCountsFor([$appointment])); // Only expose userResponse when the user actually answered yes/no/maybe. // A row with response=NULL exists when an admin checked the user in // before they ever responded; treat that as "no response" (matches list endpoint). $userResponse = $this->getUserResponse($appointment->getId(), $userId); $appointmentData['userResponse'] = $this->serializeUserResponse($userResponse, $appointment); - $responseSummary = $this->buildResponseSummaryFor($myPermissions, $appointment->getId()); + // Everybody's responses feed the summary tier and the check-in counts alike. + $responses = $myPermissions['canSeeResponseCounts'] ? $this->responseMapper->findByAppointment($appointment->getId()) : []; + $responseSummary = $this->buildResponseSummaryFor($myPermissions, $appointment, $responses); if ($responseSummary !== null) { $appointmentData['responseSummary'] = $responseSummary; } $appointmentData['attachments'] = $this->attachmentService->getAttachments($appointment->getId()); $appointmentData['myPermissions'] = $myPermissions; $appointmentData = $this->hideTalkRoomFromOutsiders($appointmentData, $myPermissions['canEdit'], $userResponse); + if ($myPermissions['canSeeResponseCounts']) { + $appointmentData['checkinSummary'] = $this->checkinService->getCheckinSummary($appointment, $responses); + } return $appointmentData; } @@ -1190,15 +1196,15 @@ private function hideTalkRoomFromOutsiders(array $appointmentData, bool $canMana * the aggregate counts, or nothing. * * @param array{isOrganizer: bool, isAttendee: bool, canEdit: bool, canSeeResponses: bool, canSeeResponseCounts: bool, canSeeComments: bool, canSeeAuditLog: bool} $myPermissions + * @param list $responses All responses of the appointment */ - private function buildResponseSummaryFor(array $myPermissions, int $appointmentId): ?array { - if ($myPermissions['canSeeResponses']) { - return $this->responseSummaryService->getResponseSummary($appointmentId, $myPermissions['canSeeComments']); - } - if ($myPermissions['canSeeResponseCounts']) { - return $this->responseSummaryService->getResponseCounts($appointmentId); + private function buildResponseSummaryFor(array $myPermissions, Appointment $appointment, array $responses): ?array { + if (!$myPermissions['canSeeResponseCounts']) { + return null; } - return null; + return $myPermissions['canSeeResponses'] + ? $this->responseSummaryService->getResponseSummary($appointment, $responses, $myPermissions['canSeeComments']) + : $this->responseSummaryService->getResponseCounts($appointment, $responses); } /** @@ -1672,10 +1678,7 @@ private function ownAppointments(array $appointments, string $userId): array { * @return list> */ private function buildNavigationData(array $own, string $userId): array { - $responses = $this->responseMapper->findByUserForAppointments( - $userId, - array_map(static fn (array $entry): int => $entry['appointment']->getId(), $own), - ); + $responses = $this->responseMapper->findByUserForAppointments($userId, self::rowIds($own)); $result = []; foreach ($own as ['appointment' => $appointment, 'inAudience' => $inAudience]) { @@ -1785,16 +1788,28 @@ public function getAppointmentPage( static fn (array $row): bool => !self::hasAnswered($responses[$row['appointment']->getId()] ?? null), )); } + // Who holds a place where, once for all rows rather than once per row. + $booked = $query->notScheduledOut || $query->onlyScheduled + ? $this->bookingService->bookedUserIdsFor(self::rowIds($rows)) + : []; if ($query->notScheduledOut) { $rows = array_values(array_filter( $rows, - fn (array $row): bool => !$this->bookingService->isScheduledOut($row['appointment'], $userId), + fn (array $row): bool => !$this->bookingService->isScheduledOut( + $row['appointment'], + $userId, + $booked[$row['appointment']->getId()] ?? [], + ), )); } if ($query->onlyScheduled) { $rows = array_values(array_filter( $rows, - fn (array $row): bool => $this->bookingService->isScheduledIn($row['appointment'], $userId), + fn (array $row): bool => $this->bookingService->isScheduledIn( + $row['appointment'], + $userId, + $booked[$row['appointment']->getId()] ?? [], + ), )); } @@ -1828,11 +1843,9 @@ public function getAppointmentPage( $this->loadUserResponses($responses, $userId, $rows); $globalManage = $this->permissionService->canManageAppointments($userId); - $appointments = []; - foreach ($rows as ['appointment' => $appointment, 'isAttendee' => $isAttendee, 'isPast' => $isPast]) { - $userResponse = $responses[$appointment->getId()] ?? null; - - $myPermissions = $this->buildMyPermissions( + $permissions = []; + foreach ($rows as ['appointment' => $appointment, 'isAttendee' => $isAttendee]) { + $permissions[$appointment->getId()] = $this->buildMyPermissions( $appointment, $userId, $globalManage, @@ -1841,19 +1854,39 @@ public function getAppointmentPage( $includeResponseCounts, $isAttendee, ); + } + + // Page-wide reads; everybody's responses only for the rows that get a + // summary or check-in counts out of them. + $summarized = array_keys(array_filter($permissions, static fn (array $p): bool => $p['canSeeResponseCounts'])); + $allResponses = $summarized === [] ? [] : $this->responseMapper->findByAppointments($summarized); + $attachments = $this->attachmentService->getAttachmentsForAppointments(self::rowIds($rows)); + $seriesCounts = $this->seriesCountsFor(array_column($rows, 'appointment')); + + $appointments = []; + foreach ($rows as ['appointment' => $appointment, 'isPast' => $isPast]) { + $id = $appointment->getId(); + $userResponse = $responses[$id] ?? null; + $rowResponses = $allResponses[$id] ?? []; + $myPermissions = $permissions[$id]; $appointmentData = $this->serializeAppointment($appointment); $appointmentData = $this->enrichVisibilityData($appointmentData); - $appointmentData = $this->enrichSeriesCount($appointmentData, $appointment); + $appointmentData = $this->enrichSeriesCount($appointmentData, $appointment, $seriesCounts); $appointmentData['userResponse'] = $this->serializeUserResponse($userResponse, $appointment); - $responseSummary = $this->buildResponseSummaryFor($myPermissions, $appointment->getId()); + $responseSummary = $this->buildResponseSummaryFor($myPermissions, $appointment, $rowResponses); if ($responseSummary !== null) { $appointmentData['responseSummary'] = $responseSummary; } - $appointmentData['attachments'] = $this->attachmentService->getAttachments($appointment->getId()); + $appointmentData['attachments'] = $attachments[$id] ?? []; $appointmentData['myPermissions'] = $myPermissions; $appointmentData['isPast'] = $isPast; - $appointments[] = $this->hideTalkRoomFromOutsiders($appointmentData, $myPermissions['canEdit'], $userResponse); + $appointmentData = $this->hideTalkRoomFromOutsiders($appointmentData, $myPermissions['canEdit'], $userResponse); + // Only ever counts, no names — safe for the same people who see the response counts. + if ($myPermissions['canSeeResponseCounts']) { + $appointmentData['checkinSummary'] = $this->checkinService->getCheckinSummary($appointment, $rowResponses); + } + $appointments[] = $appointmentData; } return [ @@ -2396,18 +2429,41 @@ private function normalizeAttendeeLimit(?int $maxAttendees): ?int { /** * Enrich appointment data with series count. + * + * @param array $seriesCounts As counted by seriesCountsFor() */ - private function enrichSeriesCount(array $appointmentData, Appointment $appointment): array { + private function enrichSeriesCount(array $appointmentData, Appointment $appointment, array $seriesCounts): array { $seriesId = $appointment->getSeriesId(); - if ($seriesId !== null) { - $siblings = $this->appointmentMapper->findBySeriesId($seriesId); - $appointmentData['seriesCount'] = count($siblings); - } else { - $appointmentData['seriesCount'] = 0; - } + $appointmentData['seriesCount'] = $seriesId === null ? 0 : ($seriesCounts[$seriesId] ?? 0); return $appointmentData; } + /** + * The size of every series these appointments belong to, in one query. + * + * @param list $appointments + * @return array series id → count + */ + private function seriesCountsFor(array $appointments): array { + $seriesIds = []; + foreach ($appointments as $appointment) { + $seriesId = $appointment->getSeriesId(); + if ($seriesId !== null) { + $seriesIds[] = $seriesId; + } + } + + return $this->appointmentMapper->countBySeriesIds(array_values(array_unique($seriesIds))); + } + + /** + * @param list $rows + * @return list + */ + private static function rowIds(array $rows): array { + return array_map(static fn (array $row): int => $row['appointment']->getId(), $rows); + } + /** * Validate that end datetime is after start datetime. */ diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php index 8e2258fa..168b000e 100644 --- a/lib/Service/AttachmentService.php +++ b/lib/Service/AttachmentService.php @@ -86,13 +86,32 @@ public function removeAttachment(int $appointmentId, int $fileId): void { * @return array */ public function getAttachments(int $appointmentId): array { - $attachments = $this->attachmentMapper->findByAppointment($appointmentId); + return array_map( + fn (AppointmentAttachment $attachment): array => $this->serializeAttachment($attachment), + $this->attachmentMapper->findByAppointment($appointmentId), + ); + } + + /** + * getAttachments() for a whole page of appointments in one query. + * + * @param list $appointmentIds + * @return array> appointment id → attachments; absent when it has none + */ + public function getAttachmentsForAppointments(array $appointmentIds): array { + return array_map( + fn (array $attachments): array => array_map( + fn (AppointmentAttachment $attachment): array => $this->serializeAttachment($attachment), + $attachments, + ), + $this->attachmentMapper->findByAppointments($appointmentIds), + ); + } - return array_map(function (AppointmentAttachment $attachment) { - $data = $attachment->jsonSerialize(); - $data['downloadUrl'] = $this->getAttachmentDownloadUrl($attachment->getFileId()); - return $data; - }, $attachments); + private function serializeAttachment(AppointmentAttachment $attachment): array { + $data = $attachment->jsonSerialize(); + $data['downloadUrl'] = $this->getAttachmentDownloadUrl($attachment->getFileId()); + return $data; } /** diff --git a/lib/Service/BookingService.php b/lib/Service/BookingService.php index 4d2c1ed1..c082f44f 100644 --- a/lib/Service/BookingService.php +++ b/lib/Service/BookingService.php @@ -118,25 +118,17 @@ public function setBookingStatus(int $appointmentId, string $userId, ?string $st * Everything softer stays visible: an open inquiry has decided nothing yet, * and an appointment nobody was scheduled for is one where the manager * simply doesn't use the feature — it stays visible to everyone. + * + * @param ?list $bookedUserIds Who holds a place, pre-loaded by bookedUserIdsFor() to save a query */ - public function isScheduledOut(Appointment $appointment, string $userId): bool { + public function isScheduledOut(Appointment $appointment, string $userId, ?array $bookedUserIds = null): bool { if (!$this->isEnabled() || !$appointment->isClosed()) { return false; } - $anyoneBooked = false; - foreach ($this->responseMapper->findByAppointment($appointment->getId()) as $response) { - if ($response->getBookingStatus() !== self::STATUS_BOOKED) { - continue; - } - // Own booking wins outright — no need to look at the rest. - if ($response->getUserId() === $userId) { - return false; - } - $anyoneBooked = true; - } + $bookedUserIds ??= $this->bookedUserIdsOf($appointment); - return $anyoneBooked; + return $bookedUserIds !== [] && !in_array($userId, $bookedUserIds, true); } /** @@ -149,19 +141,42 @@ public function isScheduledOut(Appointment $appointment, string $userId): bool { * anything once closing has made the verdict final. So a caller filtering * for "appointments I have a place in" sees the open ones too — those are * exactly the upcoming dates the user has to turn up for. + * + * @param ?list $bookedUserIds Who holds a place, pre-loaded by bookedUserIdsFor() to save a query */ - public function isScheduledIn(Appointment $appointment, string $userId): bool { + public function isScheduledIn(Appointment $appointment, string $userId, ?array $bookedUserIds = null): bool { if (!$this->isEnabled()) { return false; } - foreach ($this->responseMapper->findByAppointment($appointment->getId()) as $response) { - if ($response->getUserId() === $userId) { - return $response->getBookingStatus() === self::STATUS_BOOKED; - } + $bookedUserIds ??= $this->bookedUserIdsOf($appointment); + + return in_array($userId, $bookedUserIds, true); + } + + /** + * Who holds a place in each of these appointments, in one query — what the + * list's scheduling filters would otherwise ask per row. Empty while + * planning is off: nobody can hold a place then. + * + * @param list $appointmentIds + * @return array> appointment id → booked user ids; absent when nobody is booked + */ + public function bookedUserIdsFor(array $appointmentIds): array { + if (!$this->isEnabled()) { + return []; } - return false; + return $this->responseMapper->findBookedUserIds($appointmentIds); + } + + /** + * @return list + */ + private function bookedUserIdsOf(Appointment $appointment): array { + $id = $appointment->getId(); + + return $this->responseMapper->findBookedUserIds([$id])[$id] ?? []; } /** diff --git a/lib/Service/CapacityService.php b/lib/Service/CapacityService.php index 8dcc29af..9cf85741 100644 --- a/lib/Service/CapacityService.php +++ b/lib/Service/CapacityService.php @@ -78,10 +78,12 @@ public function occupancy(Appointment $appointment): int { /** * Whether a further self-service yes would have to wait. + * + * @param ?int $occupancy pre-counted by occupancy(), to save a query */ - public function isFull(Appointment $appointment): bool { + public function isFull(Appointment $appointment, ?int $occupancy = null): bool { $limit = $this->limitOf($appointment); - return $limit !== null && $this->occupancy($appointment) >= $limit; + return $limit !== null && ($occupancy ?? $this->occupancy($appointment)) >= $limit; } /** diff --git a/lib/Service/CheckinService.php b/lib/Service/CheckinService.php index 2edac630..ffc76b9d 100644 --- a/lib/Service/CheckinService.php +++ b/lib/Service/CheckinService.php @@ -4,6 +4,7 @@ namespace OCA\Attendance\Service; +use OCA\Attendance\Db\Appointment; use OCA\Attendance\Db\AppointmentMapper; use OCA\Attendance\Db\AttendanceResponse; use OCA\Attendance\Db\AttendanceResponseMapper; @@ -171,8 +172,7 @@ public function getCheckinData(int $appointmentId): array { */ private function buildGroupList(array $whitelistedGroups): array { if (empty($whitelistedGroups)) { - $allGroups = $this->groupManager->search(''); - $groups = array_map(fn ($group) => $group->getGID(), $allGroups); + $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( @@ -188,15 +188,13 @@ private function buildGroupList(array $whitelistedGroups): array { } /** - * Get checkin summary counts for an appointment. + * Check-in counts for an appointment. Takes the responses rather than + * reading them: the list attaches one per row and reads them in bulk. * - * @param int $appointmentId The appointment ID + * @param list $responses All responses of the appointment * @return array Checkin summary with attended, absent, notCheckedIn counts and hasCheckins flag */ - public function getCheckinSummary(int $appointmentId): array { - $appointment = $this->appointmentMapper->find($appointmentId); - $responses = $this->responseMapper->findByAppointment($appointmentId); - + public function getCheckinSummary(Appointment $appointment, array $responses): array { // Get whitelisted groups for filtering $whitelistedGroups = $this->configService->getWhitelistedGroups(); diff --git a/lib/Service/ResponseSummaryService.php b/lib/Service/ResponseSummaryService.php index 78080ffc..0004a00a 100644 --- a/lib/Service/ResponseSummaryService.php +++ b/lib/Service/ResponseSummaryService.php @@ -5,9 +5,7 @@ namespace OCA\Attendance\Service; use OCA\Attendance\Db\Appointment; -use OCA\Attendance\Db\AppointmentMapper; use OCA\Attendance\Db\AttendanceResponse; -use OCA\Attendance\Db\AttendanceResponseMapper; use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; @@ -18,8 +16,6 @@ * Optimized to avoid N+1 query patterns through caching and batch operations. */ class ResponseSummaryService { - private AppointmentMapper $appointmentMapper; - private AttendanceResponseMapper $responseMapper; private ConfigService $configService; private VisibilityService $visibilityService; private PermissionService $permissionService; @@ -29,8 +25,6 @@ class ResponseSummaryService { private CapacityService $capacityService; public function __construct( - AppointmentMapper $appointmentMapper, - AttendanceResponseMapper $responseMapper, ConfigService $configService, VisibilityService $visibilityService, PermissionService $permissionService, @@ -39,8 +33,6 @@ public function __construct( GuestService $guestService, CapacityService $capacityService, ) { - $this->appointmentMapper = $appointmentMapper; - $this->responseMapper = $responseMapper; $this->configService = $configService; $this->visibilityService = $visibilityService; $this->permissionService = $permissionService; @@ -51,9 +43,10 @@ public function __construct( } /** - * Get response summary for an appointment. + * Get response summary for an appointment. Takes the responses rather than + * reading them: the list builds one summary per row and reads them in bulk. * - * @param int $appointmentId The appointment ID + * @param list $responses All responses of the appointment * @param bool $includeComments Whether to include the free-text comment / * checkinComment fields in the serialized * responses. Gated by the caller's @@ -61,10 +54,7 @@ public function __construct( * to users who may not read comments. * @return array The response summary */ - public function getResponseSummary(int $appointmentId, bool $includeComments = false): array { - $appointment = $this->appointmentMapper->find($appointmentId); - $responses = $this->responseMapper->findByAppointment($appointmentId); - + public function getResponseSummary(Appointment $appointment, array $responses, bool $includeComments = false): array { // Pre-fetch and cache data to avoid N+1 queries $cache = $this->buildCache($appointment, $responses); @@ -108,11 +98,10 @@ public function getResponseSummary(int $appointmentId, bool $includeComments = f * default, so it tallies straight over the responses and the relevant * users — the same shape (and non-responder semantics) as the check-in * summary. + * + * @param list $responses All responses of the appointment */ - public function getResponseCounts(int $appointmentId): array { - $appointment = $this->appointmentMapper->find($appointmentId); - $responses = $this->responseMapper->findByAppointment($appointmentId); - + public function getResponseCounts(Appointment $appointment, array $responses): array { /** @var array{yes: int, no: int, maybe: int, no_response: int} $counts */ $counts = $this->initializeSummary(); /** @var array $respondedUserIds */ @@ -199,24 +188,16 @@ private function buildCache(Appointment $appointment, array $responses): array { /** @var array> $groupUsers */ $groupUsers = []; if ($groupsMode === ConfigService::RESPONSE_SUMMARY_MODE_SPECIFIC) { - foreach ($sectionGroups as $groupId) { - $group = $this->groupManager->get($groupId); - if ($group) { - $groupUsers[$groupId] = $group->getUsers(); - } - } + $sectionGroupIds = $sectionGroups; } elseif (!empty($appointmentVisibleGroupsLower)) { - foreach ($visibilitySettings['groups'] as $groupId) { - $group = $this->groupManager->get($groupId); - if ($group) { - $groupUsers[$groupId] = $group->getUsers(); - } - } + $sectionGroupIds = $visibilitySettings['groups']; } elseif ($groupsMode === ConfigService::RESPONSE_SUMMARY_MODE_ALL) { - $allGroups = $this->groupManager->search(''); - foreach ($allGroups as $group) { - $groupUsers[$group->getGID()] = $group->getUsers(); - } + $sectionGroupIds = $this->visibilityService->getAllGroupIds(); + } else { + $sectionGroupIds = []; + } + foreach ($sectionGroupIds as $groupId) { + $groupUsers[$groupId] = $this->visibilityService->getGroupMembers($groupId); } // Pre-fetch whitelisted team members and info diff --git a/lib/Service/VisibilityService.php b/lib/Service/VisibilityService.php index 66553a60..b2840f96 100644 --- a/lib/Service/VisibilityService.php +++ b/lib/Service/VisibilityService.php @@ -33,6 +33,14 @@ class VisibilityService { /** @var array, groups: array, teams: array}> * Keyed by the raw JSON columns, so an in-request edit misses naturally. */ private array $visibilitySettingsCache = []; + /** @var array> groupId → members. */ + private array $groupMembersCache = []; + /** @var ?list every group of the instance. */ + private ?array $allGroupIds = null; + /** @var ?array every account, keyed by UID. */ + private ?array $allUsers = null; + /** @var array teamId → info, null when unknown. */ + private array $teamInfoCache = []; public function __construct( IGroupManager $groupManager, @@ -185,11 +193,8 @@ public function getRelevantUsersForAppointment(Appointment $appointment, array $ // Add users from visible groups foreach ($settings['groups'] as $groupId) { - $group = $this->groupManager->get($groupId); - if ($group) { - foreach ($group->getUsers() as $user) { - $relevantUsers[$user->getUID()] = $user; - } + foreach ($this->getGroupMembers($groupId) as $user) { + $relevantUsers[$user->getUID()] = $user; } } @@ -213,24 +218,23 @@ public function getRelevantUsersForAppointment(Appointment $appointment, array $ // If whitelisted groups are configured, only load users from those groups if (!empty($whitelistedGroups)) { foreach ($whitelistedGroups as $groupId) { - $group = $this->groupManager->get($groupId); - if ($group) { - foreach ($group->getUsers() as $user) { - $relevantUsers[$user->getUID()] = $user; - } + foreach ($this->getGroupMembers($groupId) as $user) { + $relevantUsers[$user->getUID()] = $user; } } return $relevantUsers; } - // Case 3: No restrictions at all - must load all users - // This is unavoidable but should be rare in production (admins usually configure whitelisted groups) - $allUsers = $this->userManager->search(''); - foreach ($allUsers as $user) { - $relevantUsers[$user->getUID()] = $user; + // Case 3: No restrictions at all - must load all users. Once per request: + // it is the same everyone for every open appointment of a list. + if ($this->allUsers === null) { + $this->allUsers = []; + foreach ($this->userManager->search('') as $user) { + $this->allUsers[$user->getUID()] = $user; + } } - return $relevantUsers; + return $this->allUsers; } /** @@ -290,6 +294,36 @@ public function hasRestrictedVisibility(Appointment $appointment): bool { return !empty($settings['users']) || !empty($settings['groups']) || !empty($settings['teams']); } + /** + * Cached per request: the group manager forgets a group's members whenever + * getUserGroups() hands out a fresh group object — once per responder in a list. + * + * @return list<\OCP\IUser> + */ + public function getGroupMembers(string $groupId): array { + if (!isset($this->groupMembersCache[$groupId])) { + $group = $this->groupManager->get($groupId); + $this->groupMembersCache[$groupId] = $group !== null ? array_values($group->getUsers()) : []; + } + return $this->groupMembersCache[$groupId]; + } + + /** + * Every group of the instance, read once per request — the "all" summary + * mode and the check-in filter section a whole list by them. + * + * @return list + */ + public function getAllGroupIds(): array { + if ($this->allGroupIds === null) { + $this->allGroupIds = []; + foreach ($this->groupManager->search('') as $group) { + $this->allGroupIds[] = $group->getGID(); + } + } + return $this->allGroupIds; + } + /** * Check if a user is a member of a team (circle). * @@ -397,19 +431,26 @@ public function getTeamInfo(string $teamId): ?array { if ($this->teamsManager === null) { return null; } + // Same per-request lifetime as the member cache: the list names a + // team on every appointment addressed to it. + if (array_key_exists($teamId, $this->teamInfoCache)) { + return $this->teamInfoCache[$teamId]; + } try { $this->teamsManager->startSuperSession(); $circle = $this->teamsManager->getCircle($teamId); $this->teamsManager->stopSession(); - return [ + $info = [ 'id' => $teamId, 'label' => (string)$circle->getDisplayName(), 'type' => 'team', ]; } catch (\Exception $e) { - return null; + $info = null; } + + return $this->teamInfoCache[$teamId] = $info; } } diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 4f45ab7f..27ec954c 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1523,9 +1523,6 @@ getName()]]> - getFileId()]]> - getFileName()]]> - getFilePath()]]> getRelativePath($file->getPath()) ?? $file->getPath()]]> @@ -1534,14 +1531,10 @@ - - - - @@ -1677,7 +1670,6 @@ - getCheckinComment()]]> @@ -1852,30 +1844,17 @@ - - - - getFileId()]]]> - getFileId()]]]> getId()]]]> - - - - - - - - getFileId()]]> diff --git a/tests/unit/Notification/NotifierTest.php b/tests/unit/Notification/NotifierTest.php index 0df71b9d..b813f1c5 100644 --- a/tests/unit/Notification/NotifierTest.php +++ b/tests/unit/Notification/NotifierTest.php @@ -5,6 +5,7 @@ namespace OCA\Attendance\Tests\Unit\Notification; use OCA\Attendance\Activity\EventMessages; +use OCA\Attendance\Db\Appointment; use OCA\Attendance\Db\AppointmentMapper; use OCA\Attendance\Db\AttendanceResponse; use OCA\Attendance\Db\AttendanceResponseMapper; @@ -108,12 +109,12 @@ private function mockResponseChangeNotification(string $subject, array $subjectP return $notification; } - private function mockAppointmentNotification(string $subject, array $subjectParameters): INotification|MockObject { + private function mockAppointmentNotification(string $subject, array $subjectParameters, string $user = 'alice'): INotification|MockObject { $notification = $this->createMock(INotification::class); $notification->method('getApp')->willReturn('attendance'); $notification->method('getSubject')->willReturn($subject); $notification->method('getSubjectParameters')->willReturn($subjectParameters); - $notification->method('getUser')->willReturn('alice'); + $notification->method('getUser')->willReturn($user); $notification->method('setParsedSubject')->willReturnSelf(); $notification->method('setParsedMessage')->willReturnSelf(); $notification->method('setIcon')->willReturnSelf(); @@ -245,6 +246,34 @@ public function testUpdateNotificationReachesUsersWhoAlreadyResponded(): void { $this->assertSame($notification, $result); } + /** + * A wave prepares one notification per recipient, all about the same + * appointment — the push path then has to read it once, not once per + * person, and the icon is the same for everyone anyway. + */ + public function testWaveReadsTheAppointmentAndIconOnce(): void { + $this->appointmentMapper->expects($this->once()) + ->method('find') + ->with(42) + ->willReturn(new Appointment()); + $this->urlGenerator->expects($this->once()) + ->method('imagePath') + ->with('attendance', 'app-dark.svg') + ->willReturn('/apps/attendance/img/app-dark.svg'); + + foreach (['alice', 'bob', 'carol'] as $recipient) { + $notification = $this->mockAppointmentNotification('appointment_updated', [ + 'appointmentId' => 42, + 'name' => 'Rehearsal', + 'startDatetime' => '2030-08-01 18:00:00', + 'changed' => ['time'], + ], $recipient); + $notification->expects($this->exactly(3))->method('addParsedAction'); + + $this->notifier->prepare($notification, 'de'); + } + } + public function testSeriesUpdateNotificationCountsAndDescribes(): void { $notification = $this->mockAppointmentNotification('appointments_series_updated', [ 'count' => 12, diff --git a/tests/unit/Service/AppointmentServiceTest.php b/tests/unit/Service/AppointmentServiceTest.php index 97ede1ba..1cbd2040 100644 --- a/tests/unit/Service/AppointmentServiceTest.php +++ b/tests/unit/Service/AppointmentServiceTest.php @@ -18,6 +18,7 @@ use OCA\Attendance\Service\AuditEventService; use OCA\Attendance\Service\BookingService; use OCA\Attendance\Service\CapacityService; +use OCA\Attendance\Service\CheckinService; use OCA\Attendance\Service\ConfigService; use OCA\Attendance\Service\DeadlineUpdate; use OCA\Attendance\Service\GuestService; @@ -98,6 +99,8 @@ class AppointmentServiceTest extends TestCase { private CapacityService $capacityService; /** @var VacationService|MockObject */ private $vacationService; + /** @var CheckinService|MockObject */ + private $checkinService; private AppointmentService $service; @@ -130,6 +133,7 @@ protected function setUp(): void { ); $this->responsePolicyService = new ResponsePolicyService($this->configService, $this->capacityService, $this->visibilityService); $this->vacationService = $this->createMock(VacationService::class); + $this->checkinService = $this->createMock(CheckinService::class); $userConfig = $this->createMock(IUserConfig::class); $userConfig->method('getValueString')->willReturn('Europe/Berlin'); @@ -159,6 +163,7 @@ protected function setUp(): void { new AppointmentSerializer($this->responsePolicyService, $this->capacityService), $this->vacationService, $timezoneService, + $this->checkinService, ); } @@ -1958,7 +1963,6 @@ public function testNotScheduledOutFilterDropsTheAppointmentFromTheList(): void $this->appointmentMapper->method('findUpcoming')->willReturn([$scheduledOut]); $this->visibilityService->method('canUserSeeAppointment')->willReturn(true); $this->visibilityService->method('isUserTargetAttendee')->willReturn(true); - $this->attachmentService->method('getAttachments')->willReturn([]); $this->bookingService->method('isScheduledOut')->willReturn(true); // Without the filter the appointment is still listed — being scheduled @@ -1979,7 +1983,6 @@ public function testNotScheduledOutFilterImpliesOnlyForMe(): void { $this->permissionService->method('canSeeAppointmentViaSeeAll')->willReturn(true); $this->permissionService->method('isOrganizer')->willReturn(false); $this->visibilityService->method('isUserTargetAttendee')->willReturn(false); - $this->attachmentService->method('getAttachments')->willReturn([]); $this->assertSame([], $this->service->getAppointmentsWithUserResponses( 'alice', @@ -1998,7 +2001,6 @@ public function testOnlyForMeKeepsAppointmentsTheUserOrganizes(): void { $this->permissionService->method('canSeeAppointmentViaSeeAll')->willReturn(false); $this->permissionService->method('isOrganizer')->willReturn(true); $this->visibilityService->method('isUserTargetAttendee')->willReturn(false); - $this->attachmentService->method('getAttachments')->willReturn([]); $appointments = $this->service->getAppointmentsWithUserResponses('bob', onlyForMe: true); @@ -2014,7 +2016,6 @@ public function testUnansweredInboxSkipsAppointmentsTheUserOnlyOrganizes(): void $this->permissionService->method('canSeeAppointmentViaSeeAll')->willReturn(false); $this->permissionService->method('isOrganizer')->willReturn(true); $this->visibilityService->method('isUserTargetAttendee')->willReturn(false); - $this->attachmentService->method('getAttachments')->willReturn([]); // Organizing an appointment is not being asked to answer it. $this->assertSame([], $this->service->getAppointmentsWithUserResponses('bob', unansweredOnly: true)); @@ -2163,7 +2164,10 @@ public function testPageReturnsTheSliceAndCountsEverything(): void { $this->listAsAttendee(array_map(fn (int $id) => $this->createAppointment($id, "Meeting $id"), [1, 2, 3, 4, 5])); // The costly per-appointment work must stay with the page itself. - $this->attachmentService->expects($this->exactly(2))->method('getAttachments')->willReturn([]); + $this->attachmentService->expects($this->once()) + ->method('getAttachmentsForAppointments') + ->with([3, 4]) + ->willReturn([]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(limit: 2, offset: 2)); @@ -2171,9 +2175,119 @@ public function testPageReturnsTheSliceAndCountsEverything(): void { $this->assertSame(5, $page['total']); } + /** + * The single appointment reads everybody's responses once and hands them + * to the summary and the check-in counts alike; the series size comes from + * the count query rather than from hydrating every sibling. + */ + public function testSingleAppointmentReadsResponsesOnceForSummaryAndCheckin(): void { + $appointment = $this->createAppointment(7, 'Rehearsal'); + $appointment->setSeriesId('series-a'); + $this->appointmentMapper->method('find')->with(7)->willReturn($appointment); + $this->visibilityService->method('canUserSeeAppointment')->willReturn(true); + $this->visibilityService->method('isUserTargetAttendee')->willReturn(true); + $this->responseMapper->method('findByAppointmentAndUser')->willReturn($this->answer(7, 'yes')); + + $answers = [$this->answer(7, 'yes')]; + $this->responseMapper->expects($this->once())->method('findByAppointment')->with(7)->willReturn($answers); + $this->appointmentMapper->expects($this->once())->method('countBySeriesIds')->with(['series-a'])->willReturn(['series-a' => 3]); + $this->appointmentMapper->expects($this->never())->method('findBySeriesId'); + $this->responseSummaryService->expects($this->once()) + ->method('getResponseCounts') + ->with($appointment, $answers) + ->willReturn(['yes' => 1]); + $this->checkinService->expects($this->once()) + ->method('getCheckinSummary') + ->with($appointment, $answers) + ->willReturn(['attended' => 0, 'absent' => 0, 'notCheckedIn' => 1, 'hasCheckins' => false]); + + $data = $this->service->getAppointmentWithUserResponse(7, 'alice', includeResponseCounts: true); + + $this->assertSame(3, $data['seriesCount']); + $this->assertSame(['yes' => 1], $data['responseSummary']); + $this->assertSame(1, $data['checkinSummary']['notCheckedIn']); + } + + /** + * What the rows of a page need — everybody's responses, attachments, + * series sizes — is read in bulk, and the summary gets the row's entity + * instead of re-reading it by id. + */ + public function testPageReadsPerRowDataInBulk(): void { + $first = $this->createAppointment(1, 'First'); + $first->setSeriesId('series-a'); + $second = $this->createAppointment(2, 'Second'); + $second->setSeriesId('series-a'); + $this->listAsAttendee([$first, $second]); + + $this->responseMapper->expects($this->once()) + ->method('findByAppointments') + ->with([1, 2]) + ->willReturn([2 => [$this->answer(2, 'yes')]]); + $this->attachmentService->expects($this->once()) + ->method('getAttachmentsForAppointments') + ->with([1, 2]) + ->willReturn([1 => [['fileId' => 7]]]); + $this->appointmentMapper->expects($this->once()) + ->method('countBySeriesIds') + ->with(['series-a']) + ->willReturn(['series-a' => 2]); + $this->appointmentMapper->expects($this->never())->method('find'); + $this->appointmentMapper->expects($this->never())->method('findBySeriesId'); + $this->responseSummaryService->expects($this->exactly(2)) + ->method('getResponseSummary') + ->willReturnCallback(static fn (Appointment $appointment, array $responses): array => ['yes' => count($responses)]); + $this->checkinService->expects($this->exactly(2)) + ->method('getCheckinSummary') + ->willReturn(['attended' => 0, 'absent' => 0, 'notCheckedIn' => 0, 'hasCheckins' => false]); + + $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(limit: 20), includeResponseSummary: true); + + [$firstData, $secondData] = $page['appointments']; + $this->assertSame(['yes' => 0], $firstData['responseSummary']); + $this->assertSame(['yes' => 1], $secondData['responseSummary']); + $this->assertSame([['fileId' => 7]], $firstData['attachments']); + $this->assertSame([], $secondData['attachments']); + $this->assertSame(2, $firstData['seriesCount']); + $this->assertFalse($firstData['checkinSummary']['hasCheckins']); + } + + /** + * Without a summary tier nobody's responses are read at all, and the + * check-in counts stay off the payload. + */ + public function testPageSkipsEverybodysResponsesWhenTheViewerGetsNoSummary(): void { + $this->listAsAttendee([$this->createAppointment(1, 'Only one')]); + + $this->responseMapper->expects($this->never())->method('findByAppointments'); + $this->checkinService->expects($this->never())->method('getCheckinSummary'); + + $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(limit: 20)); + + $this->assertArrayNotHasKey('responseSummary', $page['appointments'][0]); + $this->assertArrayNotHasKey('checkinSummary', $page['appointments'][0]); + } + + /** + * The scheduling filters ask once who holds a place where, instead of + * reading every row's responses one by one. + */ + public function testSchedulingFiltersReadTheBookingsOnce(): void { + $this->listAsAttendee([$this->createAppointment(1, 'Booked'), $this->createAppointment(2, 'Not booked')]); + $this->bookingService->expects($this->once()) + ->method('bookedUserIdsFor') + ->with([1, 2]) + ->willReturn([1 => ['alice']]); + $this->bookingService->method('isScheduledIn') + ->willReturnCallback(static fn (Appointment $appointment, string $userId, ?array $booked): bool => in_array($userId, $booked ?? [], true)); + + $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(onlyScheduled: true, limit: 20)); + + $this->assertSame([1], $this->pageIds($page)); + } + public function testPageBeyondTheEndIsEmptyButKeepsTheTotal(): void { $this->listAsAttendee([$this->createAppointment(1, 'Only one')]); - $this->attachmentService->method('getAttachments')->willReturn([]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(limit: 20, offset: 20)); @@ -2183,7 +2297,6 @@ public function testPageBeyondTheEndIsEmptyButKeepsTheTotal(): void { public function testUnpagedQueryReturnsEverything(): void { $this->listAsAttendee(array_map(fn (int $id) => $this->createAppointment($id, "Meeting $id"), [1, 2, 3])); - $this->attachmentService->method('getAttachments')->willReturn([]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire()); @@ -2196,7 +2309,6 @@ public function testAllTimeframeListsUpcomingBeforePast(): void { [$this->createAppointment(1, 'Next week'), $this->createAppointment(2, 'Next month')], [$this->createAppointment(3, 'Yesterday'), $this->createAppointment(4, 'Last year')], ); - $this->attachmentService->method('getAttachments')->willReturn([]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire( AppointmentListQuery::TIMEFRAME_ALL, @@ -2217,7 +2329,6 @@ public function testCancelledLastMovesCancelledAppointmentsBehindThePastOnes(): [$cancelled, $this->createAppointment(2, 'Upcoming')], [$this->createAppointment(3, 'Past')], ); - $this->attachmentService->method('getAttachments')->willReturn([]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire( AppointmentListQuery::TIMEFRAME_ALL, @@ -2237,7 +2348,6 @@ public function testStatusFilterTellsOpenClosedAndCancelledApart(): void { $cancelled->setClosedAt('2026-01-01 10:00:00'); $cancelled->setCancelledAt('2026-01-01 10:00:00'); $this->listAsAttendee([$open, $closed, $cancelled]); - $this->attachmentService->method('getAttachments')->willReturn([]); foreach (['open' => [1], 'closed' => [2], 'cancelled' => [3]] as $status => $expected) { $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(status: $status, limit: 20)); @@ -2247,7 +2357,6 @@ public function testStatusFilterTellsOpenClosedAndCancelledApart(): void { public function testResponseFilterMatchesTheUsersOwnAnswer(): void { $this->listAsAttendee(array_map(fn (int $id) => $this->createAppointment($id, "Meeting $id"), [1, 2, 3, 4])); - $this->attachmentService->method('getAttachments')->willReturn([]); $this->responseMapper->method('findByUserForAppointments')->willReturn([ 1 => $this->answer(1, 'yes'), 2 => $this->answer(2, 'no'), @@ -2267,7 +2376,6 @@ public function testSearchMatchesNameAndDescriptionIgnoringCase(): void { $byDescription = $this->createAppointment(2, 'Probe'); $byDescription->setDescription('Generalprobe für das KONZERT'); $this->listAsAttendee([$byName, $byDescription, $this->createAppointment(3, 'Vorstandssitzung')]); - $this->attachmentService->method('getAttachments')->willReturn([]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(search: ' konzert ', limit: 20)); @@ -2283,7 +2391,6 @@ public function testLocationAndCategoryFiltersKeepTheOtherOptionsOnOffer(): void $church->setLocation('Church'); $church->setCategoryId(4); $this->listAsAttendee([$hall, $church, $this->createAppointment(3, 'Nowhere in particular')]); - $this->attachmentService->method('getAttachments')->willReturn([]); $byLocation = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(locations: ['Hall'], limit: 20)); $byCategory = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(categoryIds: [4], limit: 20)); @@ -2310,7 +2417,6 @@ private function listWithMixedRoles(): void { ->willReturnCallback(static fn (Appointment $appointment): bool => $appointment->getId() === 1); $this->permissionService->method('isOrganizer') ->willReturnCallback(static fn (Appointment $appointment): bool => $appointment->getId() === 2); - $this->attachmentService->method('getAttachments')->willReturn([]); } public function testRoleFilterKeepsTheAppointmentsOfThatRole(): void { @@ -2326,7 +2432,6 @@ public function testRoleFilterKeepsTheAppointmentsOfThatRole(): void { public function testRoleThatDoesNotSplitTheListIsNeitherOfferedNorApplied(): void { // Invited everywhere and organizing nothing: no role tells anything apart. $this->listAsAttendee([$this->createAppointment(1, 'First'), $this->createAppointment(2, 'Second')]); - $this->attachmentService->method('getAttachments')->willReturn([]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(role: 'organizer', limit: 20)); @@ -2346,7 +2451,6 @@ public function testUnansweredCountIgnoresTheFiltersOfTheQuery(): void { $closed, $cancelled, ]); - $this->attachmentService->method('getAttachments')->willReturn([]); $this->responseMapper->method('findByUserForAppointments')->willReturn([2 => $this->answer(2, 'yes')]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(search: 'nothing matches this', limit: 20)); @@ -2357,7 +2461,6 @@ public function testUnansweredCountIgnoresTheFiltersOfTheQuery(): void { public function testUnansweredCountIsLeftOutWhenOnlyPastAppointmentsAreListed(): void { $this->listAsAttendee([$this->createAppointment(1, 'Upcoming')], [$this->createAppointment(2, 'Past')]); - $this->attachmentService->method('getAttachments')->willReturn([]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire( AppointmentListQuery::TIMEFRAME_PAST, @@ -2369,7 +2472,6 @@ public function testUnansweredCountIsLeftOutWhenOnlyPastAppointmentsAreListed(): public function testUnansweredOnlyPageDropsWhatTheUserAnswered(): void { $this->listAsAttendee(array_map(fn (int $id) => $this->createAppointment($id, "Meeting $id"), [1, 2, 3])); - $this->attachmentService->method('getAttachments')->willReturn([]); $this->responseMapper->method('findByUserForAppointments')->willReturn([2 => $this->answer(2, 'maybe')]); $page = $this->service->getAppointmentPage('alice', AppointmentListQuery::fromWire(unansweredOnly: true, limit: 20)); @@ -2380,7 +2482,6 @@ public function testUnansweredOnlyPageDropsWhatTheUserAnswered(): void { public function testListReadsTheUsersResponsesInOneQuery(): void { $this->listAsAttendee(array_map(fn (int $id) => $this->createAppointment($id, "Meeting $id"), [1, 2, 3])); - $this->attachmentService->method('getAttachments')->willReturn([]); $this->responseMapper->expects($this->once()) ->method('findByUserForAppointments') diff --git a/tests/unit/Service/BookingServiceTest.php b/tests/unit/Service/BookingServiceTest.php index 0d67fb3d..deb5a953 100644 --- a/tests/unit/Service/BookingServiceTest.php +++ b/tests/unit/Service/BookingServiceTest.php @@ -136,7 +136,7 @@ public function testIsEnabledReflectsConfig(): void { public function testNotifyOnCloseSkippedWhenFeatureDisabled(): void { $this->configService->method('isBookingEnabled')->willReturn(false); - $this->responseMapper->expects($this->never())->method('findByAppointment'); + $this->responseMapper->expects($this->never())->method('findBookedUserIds'); $this->notificationService->expects($this->never())->method('sendBookingNotification'); $sent = $this->service->notifyOnClose($this->appointment()); @@ -259,7 +259,7 @@ private function closedAppointment(): Appointment { public function testIsScheduledOutIsFalseWhenPlanningIsDisabled(): void { $this->configService->method('isBookingEnabled')->willReturn(false); - $this->responseMapper->expects($this->never())->method('findByAppointment'); + $this->responseMapper->expects($this->never())->method('findBookedUserIds'); $this->assertFalse($this->service->isScheduledOut($this->closedAppointment(), 'alice')); } @@ -268,7 +268,7 @@ public function testIsScheduledOutIsFalseWhileInquiryIsStillOpen(): void { // Nothing has been decided yet — hiding here would take away the very // appointment the user still needs to answer. $this->configService->method('isBookingEnabled')->willReturn(true); - $this->responseMapper->expects($this->never())->method('findByAppointment'); + $this->responseMapper->expects($this->never())->method('findBookedUserIds'); $this->assertFalse($this->service->isScheduledOut($this->appointment(), 'alice')); } @@ -276,51 +276,65 @@ public function testIsScheduledOutIsFalseWhileInquiryIsStillOpen(): void { public function testIsScheduledOutIsFalseWhenNobodyWasScheduled(): void { // The manager closed without using the feature — everyone keeps seeing it. $this->configService->method('isBookingEnabled')->willReturn(true); - $this->responseMapper->method('findByAppointment')->willReturn([ - $this->response('alice', 'yes'), - $this->response('bob', 'yes'), - ]); + $this->responseMapper->method('findBookedUserIds')->with([5])->willReturn([]); $this->assertFalse($this->service->isScheduledOut($this->closedAppointment(), 'alice')); } public function testIsScheduledOutIsFalseForTheScheduledUser(): void { $this->configService->method('isBookingEnabled')->willReturn(true); - $this->responseMapper->method('findByAppointment')->willReturn([ - $this->response('bob', 'yes', BookingService::STATUS_BOOKED), - $this->response('alice', 'yes', BookingService::STATUS_BOOKED), - ]); + $this->responseMapper->method('findBookedUserIds')->with([5])->willReturn([5 => ['bob', 'alice']]); $this->assertFalse($this->service->isScheduledOut($this->closedAppointment(), 'alice')); } public function testIsScheduledOutIsTrueWhenSomebodyElseGotThePlace(): void { $this->configService->method('isBookingEnabled')->willReturn(true); - $this->responseMapper->method('findByAppointment')->willReturn([ - $this->response('bob', 'yes', BookingService::STATUS_BOOKED), - $this->response('alice', 'yes', BookingService::STATUS_DECLINED), - ]); + $this->responseMapper->method('findBookedUserIds')->with([5])->willReturn([5 => ['bob']]); $this->assertTrue($this->service->isScheduledOut($this->closedAppointment(), 'alice')); } + public function testScheduledFiltersUsePreloadedBookingsWithoutAQuery(): void { + $this->configService->method('isBookingEnabled')->willReturn(true); + $this->responseMapper->expects($this->never())->method('findBookedUserIds'); + + $this->assertTrue($this->service->isScheduledOut($this->closedAppointment(), 'alice', ['bob'])); + $this->assertFalse($this->service->isScheduledOut($this->closedAppointment(), 'alice', ['bob', 'alice'])); + $this->assertFalse($this->service->isScheduledOut($this->closedAppointment(), 'alice', [])); + $this->assertTrue($this->service->isScheduledIn($this->closedAppointment(), 'alice', ['alice'])); + $this->assertFalse($this->service->isScheduledIn($this->closedAppointment(), 'alice', ['bob'])); + } + + public function testBookedUserIdsForSkipsTheQueryWhilePlanningIsOff(): void { + $this->configService->method('isBookingEnabled')->willReturn(false); + $this->responseMapper->expects($this->never())->method('findBookedUserIds'); + + $this->assertSame([], $this->service->bookedUserIdsFor([1, 2])); + } + + public function testBookedUserIdsForReadsAllAppointmentsAtOnce(): void { + $this->configService->method('isBookingEnabled')->willReturn(true); + $this->responseMapper->expects($this->once()) + ->method('findBookedUserIds') + ->with([1, 2]) + ->willReturn([1 => ['alice']]); + + $this->assertSame([1 => ['alice']], $this->service->bookedUserIdsFor([1, 2])); + } + // --- isScheduledIn: the "only my scheduled" filter --- public function testIsScheduledInIsFalseWhenPlanningIsOff(): void { $this->configService->method('isBookingEnabled')->willReturn(false); - $this->responseMapper->method('findByAppointment')->willReturn([ - $this->response('alice', 'yes', BookingService::STATUS_BOOKED), - ]); + $this->responseMapper->method('findBookedUserIds')->with([5])->willReturn([5 => ['alice']]); $this->assertFalse($this->service->isScheduledIn($this->closedAppointment(), 'alice')); } public function testIsScheduledInIsTrueForTheBookedUser(): void { $this->configService->method('isBookingEnabled')->willReturn(true); - $this->responseMapper->method('findByAppointment')->willReturn([ - $this->response('bob', 'yes'), - $this->response('alice', 'yes', BookingService::STATUS_BOOKED), - ]); + $this->responseMapper->method('findBookedUserIds')->with([5])->willReturn([5 => ['alice']]); $this->assertTrue($this->service->isScheduledIn($this->closedAppointment(), 'alice')); } @@ -330,28 +344,21 @@ public function testIsScheduledInCountsAnOpenInquiry(): void { // before closing — unlike not being booked. The date is one the user // has to turn up for either way. $this->configService->method('isBookingEnabled')->willReturn(true); - $this->responseMapper->method('findByAppointment')->willReturn([ - $this->response('alice', 'yes', BookingService::STATUS_BOOKED), - ]); + $this->responseMapper->method('findBookedUserIds')->with([5])->willReturn([5 => ['alice']]); $this->assertTrue($this->service->isScheduledIn($this->appointment(), 'alice')); } public function testIsScheduledInIsFalseForSomebodyElsesPlace(): void { $this->configService->method('isBookingEnabled')->willReturn(true); - $this->responseMapper->method('findByAppointment')->willReturn([ - $this->response('bob', 'yes', BookingService::STATUS_BOOKED), - $this->response('alice', 'yes', BookingService::STATUS_DECLINED), - ]); + $this->responseMapper->method('findBookedUserIds')->with([5])->willReturn([5 => ['bob']]); $this->assertFalse($this->service->isScheduledIn($this->closedAppointment(), 'alice')); } public function testIsScheduledInIsFalseWithoutAnyResponse(): void { $this->configService->method('isBookingEnabled')->willReturn(true); - $this->responseMapper->method('findByAppointment')->willReturn([ - $this->response('bob', 'yes', BookingService::STATUS_BOOKED), - ]); + $this->responseMapper->method('findBookedUserIds')->with([5])->willReturn([5 => ['bob']]); $this->assertFalse($this->service->isScheduledIn($this->closedAppointment(), 'alice')); } @@ -389,7 +396,7 @@ public function testEffectiveBookingStatusIsOpenWhenNoWaveHasRun(): void { public function testEffectiveBookingStatusCostsNoQuery(): void { // It reads two columns of a row the caller already holds; a list // endpoint calls this once per appointment. - $this->responseMapper->expects($this->never())->method('findByAppointment'); + $this->responseMapper->expects($this->never())->method('findBookedUserIds'); $this->configService->expects($this->never())->method('isBookingEnabled'); $this->service->effectiveBookingStatus($this->response('alice', 'no')); diff --git a/tests/unit/Service/CapacityServiceTest.php b/tests/unit/Service/CapacityServiceTest.php index db9019b3..e3f4326d 100644 --- a/tests/unit/Service/CapacityServiceTest.php +++ b/tests/unit/Service/CapacityServiceTest.php @@ -61,6 +61,14 @@ private function queue(array $userIds, array $notified = []): void { $this->responseMapper->method('findClaimedSpots')->willReturn($rows); } + public function testIsFullTakesAPreCountedOccupancyWithoutAQuery(): void { + $this->responseMapper->expects($this->never())->method('findClaimedSpots'); + + $this->assertTrue($this->service->isFull($this->appointment(2), 2)); + $this->assertFalse($this->service->isFull($this->appointment(2), 1)); + $this->assertFalse($this->service->isFull($this->appointment(null), 5)); + } + public function testNoLimitMeansNoQueryAndNobodyWaiting(): void { $this->responseMapper->expects($this->never())->method('findClaimedSpots'); $appointment = $this->appointment(null); diff --git a/tests/unit/Service/CheckinServiceTest.php b/tests/unit/Service/CheckinServiceTest.php new file mode 100644 index 00000000..65a06d39 --- /dev/null +++ b/tests/unit/Service/CheckinServiceTest.php @@ -0,0 +1,86 @@ +appointmentMapper = $this->createMock(AppointmentMapper::class); + $this->responseMapper = $this->createMock(AttendanceResponseMapper::class); + $this->visibilityService = $this->createMock(VisibilityService::class); + + $this->service = new CheckinService( + $this->appointmentMapper, + $this->responseMapper, + $this->createMock(ConfigService::class), + $this->visibilityService, + $this->createMock(IGroupManager::class), + $this->createMock(GuestService::class), + $this->createMock(AuditEventService::class), + $this->createMock(CapacityService::class), + $this->createMock(AppointmentSerializer::class), + ); + } + + private function user(string $uid): IUser|MockObject { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($uid); + return $user; + } + + private function checkin(string $userId, string $state): AttendanceResponse { + $response = new AttendanceResponse(); + $response->setUserId($userId); + $response->setCheckinState($state); + return $response; + } + + /** + * The list hands over the row's entity and its responses; the summary + * counts the audience by check-in state without reading anything itself. + */ + public function testSummaryCountsTheAudienceByCheckinState(): void { + $appointment = new Appointment(); + $appointment->setId(7); + $this->visibilityService->method('getTargetAttendees')->willReturn([ + 'alice' => $this->user('alice'), + 'bob' => $this->user('bob'), + 'carol' => $this->user('carol'), + 'dave' => $this->user('dave'), + ]); + $this->appointmentMapper->expects($this->never())->method('find'); + $this->responseMapper->expects($this->never())->method('findByAppointment'); + + $summary = $this->service->getCheckinSummary($appointment, [ + $this->checkin('alice', 'yes'), + $this->checkin('bob', 'no'), + $this->checkin('carol', ''), + // Checked in, but no longer in the audience — not counted. + $this->checkin('mallory', 'yes'), + ]); + + $this->assertSame(['attended' => 1, 'absent' => 1, 'notCheckedIn' => 2, 'hasCheckins' => true], $summary); + } +} diff --git a/tests/unit/Service/ResponseSummaryServiceTest.php b/tests/unit/Service/ResponseSummaryServiceTest.php index cf6fb0e3..669880bd 100644 --- a/tests/unit/Service/ResponseSummaryServiceTest.php +++ b/tests/unit/Service/ResponseSummaryServiceTest.php @@ -5,7 +5,6 @@ namespace OCA\Attendance\Tests\Unit\Service; use OCA\Attendance\Db\Appointment; -use OCA\Attendance\Db\AppointmentMapper; use OCA\Attendance\Db\AttendanceResponse; use OCA\Attendance\Db\AttendanceResponseMapper; use OCA\Attendance\Service\AuditEventService; @@ -24,8 +23,6 @@ use PHPUnit\Framework\TestCase; class ResponseSummaryServiceTest extends TestCase { - /** @var AppointmentMapper|MockObject */ - private $appointmentMapper; /** @var AttendanceResponseMapper|MockObject */ private $responseMapper; @@ -51,7 +48,6 @@ class ResponseSummaryServiceTest extends TestCase { private ResponseSummaryService $service; protected function setUp(): void { - $this->appointmentMapper = $this->createMock(AppointmentMapper::class); $this->responseMapper = $this->createMock(AttendanceResponseMapper::class); $this->configService = $this->createMock(ConfigService::class); $this->visibilityService = $this->createMock(VisibilityService::class); @@ -61,8 +57,6 @@ protected function setUp(): void { $this->guestService = $this->createMock(GuestService::class); $this->service = new ResponseSummaryService( - $this->appointmentMapper, - $this->responseMapper, $this->configService, $this->visibilityService, $this->permissionService, @@ -91,9 +85,6 @@ public function testGetResponseSummaryWithNumericGroupIdDoesNotThrowTypeError(): $appointment->setVisibleGroups('[]'); $appointment->setVisibleTeams('[]'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([]); - // "All groups" mode, which triggers array_keys() on a group-keyed // cache where PHP has coerced "123" into int 123. $this->configService->method('getWhitelistedGroups')->willReturn([]); @@ -106,13 +97,9 @@ public function testGetResponseSummaryWithNumericGroupIdDoesNotThrowTypeError(): $this->visibilityService->method('isUserTargetAttendee')->willReturn(true); $this->visibilityService->method('getTargetAttendees')->willReturn([]); - $numericGroup = $this->createMock(IGroup::class); - $numericGroup->method('getGID')->willReturn('123'); - $numericGroup->method('getUsers')->willReturn([]); - - $this->groupManager->method('search')->with('')->willReturn([$numericGroup]); + $this->visibilityService->method('getAllGroupIds')->willReturn(['123']); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, []); $this->assertIsArray($summary); $this->assertArrayHasKey('by_group', $summary); @@ -132,9 +119,6 @@ public function testGetResponseSummaryWithNumericUserIdDoesNotThrowTypeError(): $appointment->setVisibleGroups('[]'); $appointment->setVisibleTeams('[]'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([]); - $this->configService->method('getWhitelistedGroups')->willReturn(['staff']); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_SPECIFIC); $this->configService->method('getWhitelistedTeams')->willReturn([]); @@ -149,9 +133,8 @@ public function testGetResponseSummaryWithNumericUserIdDoesNotThrowTypeError(): $staffGroup = $this->createMock(IGroup::class); $staffGroup->method('getGID')->willReturn('staff'); - $staffGroup->method('getUsers')->willReturn([$numericUser]); - $this->groupManager->method('get')->with('staff')->willReturn($staffGroup); + $this->visibilityService->method('getGroupMembers')->with('staff')->willReturn([$numericUser]); $this->groupManager->method('getUserGroups')->willReturn([$staffGroup]); // Keyed by the numeric-string UID; PHP coerces the key to int 456. @@ -159,7 +142,7 @@ public function testGetResponseSummaryWithNumericUserIdDoesNotThrowTypeError(): ->willReturn(['456' => $numericUser]); $this->visibilityService->method('isUserTargetAttendee')->willReturn(true); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, []); $this->assertIsArray($summary); $this->assertSame(1, $summary['no_response']); @@ -178,9 +161,6 @@ public function testGetResponseSummaryIncludesDirectlyAddressedUserOutsideAnyWhi $appointment->setVisibleGroups('[]'); $appointment->setVisibleTeams('[]'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([]); - $this->configService->method('getWhitelistedGroups')->willReturn(['staff']); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_SPECIFIC); $this->configService->method('getWhitelistedTeams')->willReturn([]); @@ -196,17 +176,12 @@ public function testGetResponseSummaryIncludesDirectlyAddressedUserOutsideAnyWhi $newHire->method('getUID')->willReturn('new_hire'); $newHire->method('getDisplayName')->willReturn('New Hire'); - $staffGroup = $this->createMock(IGroup::class); - $staffGroup->method('getGID')->willReturn('staff'); - $staffGroup->method('getUsers')->willReturn([]); - - $this->groupManager->method('get')->with('staff')->willReturn($staffGroup); $this->groupManager->method('getUserGroups')->willReturn([]); $this->visibilityService->method('getTargetAttendees') ->willReturn(['new_hire' => $newHire]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, []); // The user must appear in the global non-responder count and in the // Others bucket (no visible section to render under). @@ -216,6 +191,56 @@ public function testGetResponseSummaryIncludesDirectlyAddressedUserOutsideAnyWhi $this->assertContains('new_hire', $othersIds); } + /** + * Section members come from the visibility service, which keeps them + * across a whole page of summaries, and the summary asks once per group. + */ + public function testGetResponseSummaryTakesGroupMembersFromTheVisibilityService(): void { + $appointment = new Appointment(); + $appointment->setId(4); + $appointment->setVisibleUsers('[]'); + $appointment->setVisibleGroups('[]'); + $appointment->setVisibleTeams('[]'); + + $response = new AttendanceResponse(); + $response->setId(1); + $response->setAppointmentId(4); + $response->setUserId('alice'); + $response->setResponse('yes'); + + $this->configService->method('getWhitelistedGroups')->willReturn(['staff']); + $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_SPECIFIC); + $this->configService->method('getWhitelistedTeams')->willReturn([]); + + $this->visibilityService->method('getVisibilitySettings') + ->willReturn(['users' => [], 'groups' => [], 'teams' => []]); + $this->visibilityService->method('hasRestrictedVisibility')->willReturn(false); + $this->visibilityService->method('isUserTargetAttendee')->willReturn(true); + + $alice = $this->createMock(IUser::class); + $alice->method('getUID')->willReturn('alice'); + $alice->method('getDisplayName')->willReturn('Alice'); + $bob = $this->createMock(IUser::class); + $bob->method('getUID')->willReturn('bob'); + $bob->method('getDisplayName')->willReturn('Bob'); + $staffGroup = $this->createMock(IGroup::class); + $staffGroup->method('getGID')->willReturn('staff'); + + $this->visibilityService->expects($this->once()) + ->method('getGroupMembers') + ->with('staff') + ->willReturn([$alice, $bob]); + $this->visibilityService->method('getTargetAttendees')->willReturn(['alice' => $alice, 'bob' => $bob]); + $this->userManager->method('get')->willReturnMap([['alice', $alice]]); + $this->groupManager->method('getUserGroups')->willReturn([$staffGroup]); + + $summary = $this->service->getResponseSummary($appointment, [$response]); + + $this->assertSame(1, $summary['yes']); + $this->assertSame(1, $summary['by_group']['staff']['yes']); + $this->assertSame(['bob'], array_column($summary['by_group']['staff']['non_responding_users'], 'userId')); + } + /** * Regression for issue #229: somebody who never answered but was checked in * has a response row carrying only a check-in state. That row is no answer, @@ -235,9 +260,6 @@ public function testGetResponseSummaryKeepsTheCheckinOfSomebodyWhoNeverAnswered( $checkinOnly->setResponse(null); $checkinOnly->setCheckinState('yes'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->willReturn([$checkinOnly]); - $this->configService->method('getWhitelistedGroups')->willReturn([]); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_NONE); $this->configService->method('getWhitelistedTeams')->willReturn([]); @@ -258,7 +280,7 @@ public function testGetResponseSummaryKeepsTheCheckinOfSomebodyWhoNeverAnswered( $this->visibilityService->method('getTargetAttendees') ->willReturn(['wilfried' => $wilfried, 'no_show' => $noShow]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, [$checkinOnly]); // Both never answered; only one of them turned up. $this->assertSame(2, $summary['no_response']); @@ -288,9 +310,6 @@ public function testGetResponseSummaryListsVisibleGroupMembersOutsideWhitelistUn $response->setUserId('alice'); $response->setResponse('yes'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - // The whitelist names a different group than the appointment targets. $this->configService->method('getWhitelistedGroups')->willReturn(['staff']); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_SPECIFIC); @@ -310,18 +329,14 @@ public function testGetResponseSummaryListsVisibleGroupMembersOutsideWhitelistUn $boardGroup = $this->createMock(IGroup::class); $boardGroup->method('getGID')->willReturn('board'); - $staffGroup = $this->createMock(IGroup::class); - $staffGroup->method('getGID')->willReturn('staff'); - $staffGroup->method('getUsers')->willReturn([]); - $this->groupManager->method('get')->with('staff')->willReturn($staffGroup); $this->userManager->method('get')->with('alice')->willReturn($alice); $this->groupManager->method('getUserGroups')->willReturn([$boardGroup]); $this->visibilityService->method('getTargetAttendees') ->willReturn(['alice' => $alice, 'bob' => $bob]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, [$response]); // 'board' is not whitelisted, so no group section renders for it … $this->assertSame([], $summary['by_group']); @@ -348,9 +363,6 @@ public function testGetResponseSummaryListsVisibleTeamMembersOutsideWhitelistUnd $appointment->setVisibleGroups('[]'); $appointment->setVisibleTeams(json_encode(['team-1'])); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([]); - $this->configService->method('getWhitelistedGroups')->willReturn(['staff']); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_SPECIFIC); $this->configService->method('getWhitelistedTeams')->willReturn([]); @@ -364,18 +376,13 @@ public function testGetResponseSummaryListsVisibleTeamMembersOutsideWhitelistUnd $carol->method('getUID')->willReturn('carol'); $carol->method('getDisplayName')->willReturn('Carol'); - $staffGroup = $this->createMock(IGroup::class); - $staffGroup->method('getGID')->willReturn('staff'); - $staffGroup->method('getUsers')->willReturn([]); - - $this->groupManager->method('get')->with('staff')->willReturn($staffGroup); $this->groupManager->method('getUserGroups')->willReturn([]); // getTargetAttendees now folds team members into the audience. $this->visibilityService->method('getTargetAttendees') ->willReturn(['carol' => $carol]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, []); $this->assertSame([], $summary['by_group']); $this->assertSame([], $summary['by_team']); @@ -407,9 +414,6 @@ public function testGetResponseSummaryStripsCommentsWhenNotPermitted(): void { $response->setComment('Secret personal reason'); $response->setCheckinComment('Secret checkin note'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - // No grouping → alice lands in the "others" bucket. $this->configService->method('getWhitelistedGroups')->willReturn([]); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_NONE); @@ -427,10 +431,9 @@ public function testGetResponseSummaryStripsCommentsWhenNotPermitted(): void { $this->userManager->method('get')->with('alice')->willReturn($alice); $this->groupManager->method('getUserGroups')->willReturn([]); - $this->groupManager->method('search')->with('')->willReturn([]); // Without permission: comment fields stripped. - $stripped = $this->service->getResponseSummary($appointmentId, false); + $stripped = $this->service->getResponseSummary($appointment, [$response], false); $this->assertCount(1, $stripped['others']['responses']); $strippedResponse = $stripped['others']['responses'][0]; $this->assertArrayNotHasKey('comment', $strippedResponse); @@ -439,7 +442,7 @@ public function testGetResponseSummaryStripsCommentsWhenNotPermitted(): void { $this->assertSame('Alice', $strippedResponse['userName']); // With permission: comment fields retained. - $full = $this->service->getResponseSummary($appointmentId, true); + $full = $this->service->getResponseSummary($appointment, [$response], true); $fullResponse = $full['others']['responses'][0]; $this->assertSame('Secret personal reason', $fullResponse['comment']); $this->assertSame('Secret checkin note', $fullResponse['checkinComment']); @@ -465,9 +468,6 @@ public function testGetResponseCountsCarriesOnlyAggregateNumbers(): void { $response->setResponse('yes'); $response->setComment('Secret personal reason'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - $this->configService->method('getWhitelistedGroups')->willReturn([]); $this->visibilityService->method('isUserTargetAttendee')->willReturn(true); @@ -482,7 +482,7 @@ public function testGetResponseCountsCarriesOnlyAggregateNumbers(): void { $this->visibilityService->method('getTargetAttendees') ->willReturn(['alice' => $alice, 'bob' => $bob]); - $counts = $this->service->getResponseCounts($appointmentId); + $counts = $this->service->getResponseCounts($appointment, [$response]); $this->assertSame(1, $counts['yes']); $this->assertSame(0, $counts['no']); @@ -521,9 +521,6 @@ public function testGetResponseSummaryKeepsWhitelistedGroupingWhenVisibilityRest $response->setUserId('alice'); $response->setResponse('yes'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - // Grouping is configured by instrument, access is restricted by status. $this->configService->method('getWhitelistedGroups')->willReturn(['sopranos', 'altos']); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_SPECIFIC); @@ -545,14 +542,12 @@ public function testGetResponseSummaryKeepsWhitelistedGroupingWhenVisibilityRest $activeGroup->method('getGID')->willReturn('active'); $sopranosGroup = $this->createMock(IGroup::class); $sopranosGroup->method('getGID')->willReturn('sopranos'); - $sopranosGroup->method('getUsers')->willReturn([$alice]); $altosGroup = $this->createMock(IGroup::class); $altosGroup->method('getGID')->willReturn('altos'); - $altosGroup->method('getUsers')->willReturn([$bob]); - $this->groupManager->method('get')->willReturnMap([ - ['sopranos', $sopranosGroup], - ['altos', $altosGroup], + $this->visibilityService->method('getGroupMembers')->willReturnMap([ + ['sopranos', [$alice]], + ['altos', [$bob]], ]); $this->groupManager->method('getUserGroups')->willReturnCallback( fn (IUser $user) => $user->getUID() === 'alice' @@ -564,7 +559,7 @@ public function testGetResponseSummaryKeepsWhitelistedGroupingWhenVisibilityRest $this->visibilityService->method('getTargetAttendees') ->willReturn(['alice' => $alice, 'bob' => $bob]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, [$response]); // Both instrument sections render; the restriction group gets none. $this->assertSame(['sopranos', 'altos'], array_keys($summary['by_group'])); @@ -598,9 +593,6 @@ public function testGetResponseSummaryKeepsWhitelistedTeamSectionsWhenVisibility $response->setUserId('carol'); $response->setResponse('yes'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - $this->configService->method('getWhitelistedGroups')->willReturn([]); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_NONE); $this->configService->method('getWhitelistedTeams')->willReturn(['team-b']); @@ -620,12 +612,11 @@ public function testGetResponseSummaryKeepsWhitelistedTeamSectionsWhenVisibility $this->userManager->method('get')->willReturnMap([['carol', $carol]]); $this->groupManager->method('getUserGroups')->willReturn([]); - $this->groupManager->method('search')->with('')->willReturn([]); $this->visibilityService->method('getTargetAttendees') ->willReturn(['carol' => $carol]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, [$response]); $this->assertSame(['team-b'], array_keys($summary['by_team'])); $this->assertSame('Team B', $summary['by_team']['team-b']['displayName']); @@ -656,9 +647,6 @@ public function testGetResponseSummaryWithoutWhitelistKeepsGroupingByRestriction $response->setUserId('dave'); $response->setResponse('yes'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - $this->configService->method('getWhitelistedGroups')->willReturn([]); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_NONE); $this->configService->method('getWhitelistedTeams')->willReturn([]); @@ -674,19 +662,20 @@ public function testGetResponseSummaryWithoutWhitelistKeepsGroupingByRestriction $boardGroup = $this->createMock(IGroup::class); $boardGroup->method('getGID')->willReturn('board'); - $boardGroup->method('getUsers')->willReturn([$dave]); $staffGroup = $this->createMock(IGroup::class); $staffGroup->method('getGID')->willReturn('staff'); - $staffGroup->method('getUsers')->willReturn([$dave]); - $this->groupManager->method('search')->with('')->willReturn([$boardGroup, $staffGroup]); + $this->visibilityService->method('getGroupMembers')->willReturnMap([ + ['board', [$dave]], + ['staff', [$dave]], + ]); $this->groupManager->method('getUserGroups')->willReturn([$boardGroup, $staffGroup]); $this->userManager->method('get')->willReturnMap([['dave', $dave]]); $this->visibilityService->method('getTargetAttendees') ->willReturn(['dave' => $dave]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, [$response]); $this->assertSame(['board'], array_keys($summary['by_group'])); $this->assertSame(1, $summary['by_group']['board']['yes']); @@ -714,9 +703,6 @@ public function testGetResponseSummaryKeepsResponseFromFormerTargetAttendee(): v $response->setUserId('member11'); $response->setResponse('yes'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - $this->configService->method('getWhitelistedGroups')->willReturn([]); $this->configService->method('getWhitelistedTeams')->willReturn([]); @@ -735,16 +721,10 @@ public function testGetResponseSummaryKeepsResponseFromFormerTargetAttendee(): v $this->userManager->method('get')->willReturnMap([['member11', $member11]]); $this->groupManager->method('getUserGroups')->willReturn([]); - $this->groupManager->method('search')->with('')->willReturn([]); - - $team1Group = $this->createMock(IGroup::class); - $team1Group->method('getGID')->willReturn('team1'); - $team1Group->method('getUsers')->willReturn([]); - $this->groupManager->method('get')->with('team1')->willReturn($team1Group); $this->visibilityService->method('getTargetAttendees')->willReturn([]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, [$response]); $this->assertSame(1, $summary['yes']); $this->assertSame(1, $summary['others']['yes']); @@ -771,9 +751,6 @@ public function testGetResponseSummaryFiltersNonAttendeeAdminResponse(): void { $response->setUserId('admin1'); $response->setResponse('yes'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - $this->configService->method('getWhitelistedGroups')->willReturn([]); $this->configService->method('getWhitelistedTeams')->willReturn([]); @@ -785,10 +762,9 @@ public function testGetResponseSummaryFiltersNonAttendeeAdminResponse(): void { $this->permissionService->method('canManageAppointments')->willReturn(true); $this->groupManager->method('getUserGroups')->willReturn([]); - $this->groupManager->method('search')->with('')->willReturn([]); $this->visibilityService->method('getTargetAttendees')->willReturn([]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, [$response]); $this->assertSame(0, $summary['yes']); $this->assertSame(0, $summary['others']['yes']); @@ -815,9 +791,6 @@ public function testGetResponseSummaryNoneModeDoesNotGroupByMemberGroups(): void $response->setUserId('eve'); $response->setResponse('yes'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - $this->configService->method('getWhitelistedGroups')->willReturn([]); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_NONE); $this->configService->method('getWhitelistedTeams')->willReturn([]); @@ -839,11 +812,10 @@ public function testGetResponseSummaryNoneModeDoesNotGroupByMemberGroups(): void $this->userManager->method('get')->willReturnMap([['eve', $eve]]); $this->groupManager->method('getUserGroups')->willReturn([$choirGroup, $adminsGroup]); - $this->groupManager->method('search')->with('')->willReturn([$choirGroup, $adminsGroup]); $this->visibilityService->method('getTargetAttendees')->willReturn(['eve' => $eve]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, [$response]); $this->assertSame([], $summary['by_group']); $this->assertSame(1, $summary['others']['yes']); @@ -870,9 +842,6 @@ public function testGetResponseSummaryAllModeGroupsByEveryMemberGroup(): void { $response->setUserId('eve'); $response->setResponse('yes'); - $this->appointmentMapper->method('find')->with($appointmentId)->willReturn($appointment); - $this->responseMapper->method('findByAppointment')->with($appointmentId)->willReturn([$response]); - $this->configService->method('getWhitelistedGroups')->willReturn([]); $this->configService->method('getResponseSummaryGroupsMode')->willReturn(ConfigService::RESPONSE_SUMMARY_MODE_ALL); $this->configService->method('getWhitelistedTeams')->willReturn([]); @@ -893,11 +862,11 @@ public function testGetResponseSummaryAllModeGroupsByEveryMemberGroup(): void { $this->userManager->method('get')->willReturnMap([['eve', $eve]]); $this->groupManager->method('getUserGroups')->willReturn([$choirGroup, $adminsGroup]); - $this->groupManager->method('search')->with('')->willReturn([$choirGroup, $adminsGroup]); + $this->visibilityService->method('getAllGroupIds')->willReturn(['choir', 'admins']); $this->visibilityService->method('getTargetAttendees')->willReturn(['eve' => $eve]); - $summary = $this->service->getResponseSummary($appointmentId); + $summary = $this->service->getResponseSummary($appointment, [$response]); $this->assertSame(['admins', 'choir'], array_keys($summary['by_group'])); $this->assertSame(1, $summary['by_group']['choir']['yes']); diff --git a/tests/unit/Service/VisibilityServiceTest.php b/tests/unit/Service/VisibilityServiceTest.php index 6da0ab75..c84f2dee 100644 --- a/tests/unit/Service/VisibilityServiceTest.php +++ b/tests/unit/Service/VisibilityServiceTest.php @@ -167,6 +167,54 @@ public function testOwnAppointmentCoversOrganizersOutsideTheAudience(): void { $this->assertFalse($service->isUserOwnAppointment($appointment, 'mallory')); } + /** + * A list resolves every appointment's audience from the same groups: the + * group manager is asked once per group, however many appointments. + */ + public function testGroupMembersAreReadOncePerRequest(): void { + $alice = $this->createMock(IUser::class); + $alice->method('getUID')->willReturn('alice'); + $choir = $this->createMock(IGroup::class); + $choir->method('getUsers')->willReturn(['alice' => $alice]); + $this->groupManager->expects($this->once())->method('get')->with('choir')->willReturn($choir); + + $service = $this->partialMock(['getTeamMembers']); + + $first = new Appointment(); + $first->setVisibleUsers('[]'); + $first->setVisibleGroups(json_encode(['choir'])); + $first->setVisibleTeams('[]'); + $second = clone $first; + + $this->assertSame(['alice' => $alice], $service->getRelevantUsersForAppointment($first)); + $this->assertSame(['alice' => $alice], $service->getRelevantUsersForAppointment($second)); + $this->assertSame([$alice], $service->getGroupMembers('choir')); + } + + /** + * "Everyone" and "every group" are the same for every appointment of a + * request, so the instance-wide lookups run once. + */ + public function testInstanceWideLookupsRunOncePerRequest(): void { + $alice = $this->createMock(IUser::class); + $alice->method('getUID')->willReturn('alice'); + $this->userManager->expects($this->once())->method('search')->with('')->willReturn([$alice]); + $choir = $this->createMock(IGroup::class); + $choir->method('getGID')->willReturn('choir'); + $this->groupManager->expects($this->once())->method('search')->with('')->willReturn([$choir]); + + $service = $this->partialMock(['getTeamMembers']); + $open = new Appointment(); + $open->setVisibleUsers('[]'); + $open->setVisibleGroups('[]'); + $open->setVisibleTeams('[]'); + + $this->assertSame(['alice' => $alice], $service->getRelevantUsersForAppointment($open)); + $this->assertSame(['alice' => $alice], $service->getRelevantUsersForAppointment(clone $open)); + $this->assertSame(['choir'], $service->getAllGroupIds()); + $this->assertSame(['choir'], $service->getAllGroupIds()); + } + public function testEnrichAudienceLabelsWhatItKnowsAndFallsBackToTheId(): void { $alice = $this->createMock(IUser::class); $alice->method('getDisplayName')->willReturn('Alice');