From a3095303d58eac0bc6782922699116a2a31cbf15 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Wed, 17 Jan 2024 16:15:15 +0100 Subject: [PATCH 1/2] fix(userstatus): CALL status should overwrite MEETING status Signed-off-by: Anna Larch --- apps/dav/lib/CalDAV/Status/StatusService.php | 35 +++++----- .../user_status/lib/Service/StatusService.php | 33 ++++++--- .../Service/StatusServiceIntegrationTest.php | 67 +++++++++++++++++++ 3 files changed, 110 insertions(+), 25 deletions(-) diff --git a/apps/dav/lib/CalDAV/Status/StatusService.php b/apps/dav/lib/CalDAV/Status/StatusService.php index 3fcd4957fa231..58f2d5883fcb2 100644 --- a/apps/dav/lib/CalDAV/Status/StatusService.php +++ b/apps/dav/lib/CalDAV/Status/StatusService.php @@ -77,12 +77,13 @@ public function processCalendarStatus(string $userId): void { return; } - $userStatusTimestamp = null; - $currentStatus = null; try { $currentStatus = $this->userStatusService->findByUserId($userId); - $userStatusTimestamp = $currentStatus->getIsUserDefined() ? $currentStatus->getStatusTimestamp() : null; + // Was the status set by anything other than the calendar automation? + $userStatusTimestamp = $currentStatus->getIsUserDefined() && $currentStatus->getMessageId() !== IUserStatus::MESSAGE_CALENDAR_BUSY ? $currentStatus->getStatusTimestamp() : null; } catch (DoesNotExistException) { + $userStatusTimestamp = null; + $currentStatus = null; } if($currentStatus !== null && $currentStatus->getMessageId() === IUserStatus::MESSAGE_CALL @@ -123,19 +124,21 @@ public function processCalendarStatus(string $userId): void { return; } - // One event that fulfills all status conditions is enough - // 1. Not an OOO event - // 2. Current user status was not set after the start of this event - // 3. Event is not set to be transparent - $count = count($applicableEvents); - $this->logger->debug("Found $count applicable event(s), changing user status", ['user' => $userId]); - $this->userStatusService->setUserStatus( - $userId, - IUserStatus::AWAY, - IUserStatus::MESSAGE_CALENDAR_BUSY, - true - ); - + // Only update the status if it's neccesary otherwise we mess up the timestamp + if($currentStatus !== null && $currentStatus->getMessageId() !== IUserStatus::MESSAGE_CALENDAR_BUSY) { + // One event that fulfills all status conditions is enough + // 1. Not an OOO event + // 2. Current user status (that is not a calendar status) was not set after the start of this event + // 3. Event is not set to be transparent + $count = count($applicableEvents); + $this->logger->debug("Found $count applicable event(s), changing user status", ['user' => $userId]); + $this->userStatusService->setUserStatus( + $userId, + IUserStatus::AWAY, + IUserStatus::MESSAGE_CALENDAR_BUSY, + true + ); + } } private function getCalendarEvents(User $user): array { diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php index c623262eec65d..813f6b9d4e171 100644 --- a/apps/user_status/lib/Service/StatusService.php +++ b/apps/user_status/lib/Service/StatusService.php @@ -172,8 +172,6 @@ public function setStatus(string $userId, throw new InvalidStatusTypeException('Status-type "' . $status . '" is not supported'); } - - if ($statusTimestamp === null) { $statusTimestamp = $this->timeFactory->getTime(); } @@ -257,21 +255,38 @@ public function setUserStatus(string $userId, throw new InvalidMessageIdException('Message-Id "' . $messageId . '" is not supported'); } + try { + $userStatus = $this->mapper->findByUserId($userId); + } catch (DoesNotExistException $e) { + // We don't need to do anything + $userStatus = new UserStatus(); + $userStatus->setUserId($userId); + } + + // CALL trumps CALENDAR status, but we don't need to do anything but overwrite the message + if ($userStatus->getMessageId() === IUserStatus::MESSAGE_CALENDAR_BUSY && $messageId === IUserStatus::MESSAGE_CALL) { + $userStatus->setStatus($status); + $userStatus->setStatusTimestamp($this->timeFactory->getTime()); + $userStatus->setIsUserDefined(true); + $userStatus->setIsBackup(false); + $userStatus->setMessageId($messageId); + $userStatus->setCustomIcon(null); + $userStatus->setCustomMessage($customMessage); + $userStatus->setClearAt(null); + $userStatus->setStatusMessageTimestamp($this->timeFactory->now()->getTimestamp()); + return $this->mapper->update($userStatus); + } + if ($createBackup) { if ($this->backupCurrentStatus($userId) === false) { return null; // Already a status set automatically => abort. } // If we just created the backup + // we need to create a new status to insert + // Unfortunatley there's no way to unset the DB ID on an Entity $userStatus = new UserStatus(); $userStatus->setUserId($userId); - } else { - try { - $userStatus = $this->mapper->findByUserId($userId); - } catch (DoesNotExistException $ex) { - $userStatus = new UserStatus(); - $userStatus->setUserId($userId); - } } $userStatus->setStatus($status); diff --git a/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php b/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php index 182cac562179d..9179f066fa85d 100644 --- a/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php +++ b/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php @@ -126,6 +126,73 @@ public function testCreateRestoreBackupAutomatically(): void { ); } + public function testCallOverwritesMeetingStatus(): void { + $this->service->setStatus( + 'test123', + IUserStatus::ONLINE, + null, + false, + ); + $this->service->setUserStatus( + 'test123', + IUserStatus::AWAY, + IUserStatus::MESSAGE_CALENDAR_BUSY, + true, + ); + self::assertSame( + 'meeting', + $this->service->findByUserId('test123')->getMessageId(), + ); + + $this->service->setUserStatus( + 'test123', + IUserStatus::AWAY, + IUserStatus::MESSAGE_CALL, + true, + ); + self::assertSame( + IUserStatus::AWAY, + $this->service->findByUserId('test123')->getStatus(), + ); + + self::assertSame( + IUserStatus::MESSAGE_CALL, + $this->service->findByUserId('test123')->getMessageId(), + ); + } + + public function testOtherAutomationsDoNotOverwriteEachOther(): void { + $this->service->setStatus( + 'test123', + IUserStatus::ONLINE, + null, + false, + ); + $this->service->setUserStatus( + 'test123', + IUserStatus::AWAY, + IUserStatus::MESSAGE_CALENDAR_BUSY, + true, + ); + self::assertSame( + 'meeting', + $this->service->findByUserId('test123')->getMessageId(), + ); + + $nostatus = $this->service->setUserStatus( + 'test123', + IUserStatus::AWAY, + IUserStatus::MESSAGE_AVAILABILITY, + true, + ); + + self::assertNull($nostatus); + self::assertSame( + IUserStatus::MESSAGE_CALENDAR_BUSY, + $this->service->findByUserId('test123')->getMessageId(), + ); + } + public function testCi(): void { // TODO: remove if CI turns red self::assertTrue(false); From aa24c671dcad93b56d7ff864f3595249116b6263 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 25 Jan 2024 11:05:48 +0100 Subject: [PATCH 2/2] fix(userstatus): Also set the user status when the user has no status at all Signed-off-by: Joas Schilling --- apps/dav/lib/CalDAV/Status/StatusService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/CalDAV/Status/StatusService.php b/apps/dav/lib/CalDAV/Status/StatusService.php index 58f2d5883fcb2..24e929f9d0e73 100644 --- a/apps/dav/lib/CalDAV/Status/StatusService.php +++ b/apps/dav/lib/CalDAV/Status/StatusService.php @@ -125,7 +125,7 @@ public function processCalendarStatus(string $userId): void { } // Only update the status if it's neccesary otherwise we mess up the timestamp - if($currentStatus !== null && $currentStatus->getMessageId() !== IUserStatus::MESSAGE_CALENDAR_BUSY) { + if($currentStatus === null || $currentStatus->getMessageId() !== IUserStatus::MESSAGE_CALENDAR_BUSY) { // One event that fulfills all status conditions is enough // 1. Not an OOO event // 2. Current user status (that is not a calendar status) was not set after the start of this event