Merge pull request #43107 from nextcloud/backport/42896/stable28

[stable28] fix(userstatus): CALL status should overwrite MEETING status
nextcloud · Jan 25, 2024 · ddc22c8 · ddc22c8
2 parents 4d43b6e + aa24c67
commit ddc22c8
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 25 deletions.
diff --git 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
@@ -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
@@ -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);