fixup! feat(user status): automate user status for events

nextcloud · Nov 8, 2023 · 67467e8 · 67467e8
1 parent bdb1fd8
commit 67467e8
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 68 deletions.
diff --git a/apps/dav/lib/CalDAV/Status/StatusService.php b/apps/dav/lib/CalDAV/Status/StatusService.php
@@ -156,7 +156,7 @@ public function processCalendarAvailability(User $user, ?string $availability):
 		if(!empty($query->getCalendarUris())) {
 			// Query the next hour
 			$dtStart = $now;
-			$dtEnd = \DateTimeImmutable::createFromMutable($this->timeFactory->getDateTime('+1 hour'));
+			$dtEnd = \DateTimeImmutable::createFromMutable($this->timeFactory->getDateTime('+10 minutes'));
 			$query->setTimerangeStart($dtStart);
 			$query->setTimerangeEnd($dtEnd);
 			$calendarEvents = $this->calendarManager->searchForPrincipal($query);

diff --git a/apps/dav/tests/unit/CalDAV/Status/StatusServiceTest.php b/apps/dav/tests/unit/CalDAV/Status/StatusServiceTest.php
@@ -96,8 +96,6 @@ public function testNoEmail(): void {
 			->method('searchForPrincipal');
 		$this->generator->expects(self::never())
 			->method('getVCalendar');
-		$this->generator->expects(self::never())
-			->method('setBaseObject');
 		$this->generator->expects(self::never())
 			->method('setObjects');
 		$this->generator->expects(self::never())
@@ -156,8 +154,6 @@ public function testNoAcl(): void {
 			->method('searchForPrincipal');
 		$this->generator->expects(self::never())
 			->method('getVCalendar');
-		$this->generator->expects(self::never())
-			->method('setBaseObject');
 		$this->generator->expects(self::never())
 			->method('setObjects');
 		$this->generator->expects(self::never())
@@ -216,8 +212,6 @@ public function testNoInbox(): void {
 			->method('searchForPrincipal');
 		$this->generator->expects(self::never())
 			->method('getVCalendar');
-		$this->generator->expects(self::never())
-			->method('setBaseObject');
 		$this->generator->expects(self::never())
 			->method('setObjects');
 		$this->generator->expects(self::never())
@@ -283,8 +277,6 @@ public function testNoPrivilegesAcl(): void {
 			->method('searchForPrincipal');
 		$this->generator->expects(self::never())
 			->method('getVCalendar');
-		$this->generator->expects(self::never())
-			->method('setBaseObject');
 		$this->generator->expects(self::never())
 			->method('setObjects');
 		$this->generator->expects(self::never())
@@ -349,8 +341,6 @@ public function testNotAuthenticated(): void {
 			->method('searchForPrincipal');
 		$this->generator->expects(self::never())
 			->method('getVCalendar');
-		$this->generator->expects(self::never())
-			->method('setBaseObject');
 		$this->generator->expects(self::never())
 			->method('setObjects');
 		$this->generator->expects(self::never())
@@ -420,8 +410,6 @@ public function testNoCalendars(): void {
 			->method('searchForPrincipal');
 		$this->generator->expects(self::never())
 			->method('getVCalendar');
-		$this->generator->expects(self::never())
-			->method('setBaseObject');
 		$this->generator->expects(self::never())
 			->method('setObjects');
 		$this->generator->expects(self::never())
@@ -498,8 +486,6 @@ public function testEmptyAvailabilityAndNoSearchCalendars(): void {
 			->method('searchForPrincipal');
 		$this->generator->expects(self::never())
 			->method('getVCalendar');
-		$this->generator->expects(self::never())
-			->method('setBaseObject');
 		$this->generator->expects(self::never())
 			->method('setObjects');
 		$this->generator->expects(self::never())
@@ -603,8 +589,6 @@ public function testEmptyAvailabilityAndSearchCalendarsNoResults(): void {
 			->willReturn([]);
 		$this->generator->expects(self::never())
 			->method('getVCalendar');
-		$this->generator->expects(self::never())
-			->method('setBaseObject');
 		$this->generator->expects(self::never())
 			->method('setObjects');
 		$this->generator->expects(self::never())
@@ -718,9 +702,6 @@ public function testAvailabilityAndSearchCalendarsNoResults(): void {
 			->willReturn($vCalendar);
 		$vCalendar->expects(self::never())
 			->method('add');
-		$this->generator->expects(self::once())
-			->method('setBaseObject')
-			->with($vCalendar);
 		$this->generator->expects(self::once())
 			->method('setObjects')
 			->with($vCalendar);
@@ -827,7 +808,7 @@ public function testAvailabilityAndSearchCalendarsStatusOnline(): void {
 			->willReturn([$calendar]);
 		$this->timeFactory->expects(self::once())
 			->method('getDateTime')
-			->with('+1 hour')
+			->with('+5 minutes')
 			->willReturn($inAnHour);
 		$query->expects(self::once())
 			->method('setTimerangeStart')
@@ -844,9 +825,6 @@ public function testAvailabilityAndSearchCalendarsStatusOnline(): void {
 			->willReturn($vCalendar);
 		$vCalendar->expects(self::never())
 			->method('add');
-		$this->generator->expects(self::once())
-			->method('setBaseObject')
-			->with($vCalendar);
 		$this->generator->expects(self::once())
 			->method('setObjects')
 			->with($vCalendar);
@@ -954,7 +932,7 @@ public function testAvailabilityAndSearchCalendarsStatusBusyNoFBType(): void {
 			->willReturn([$calendar]);
 		$this->timeFactory->expects(self::once())
 			->method('getDateTime')
-			->with('+1 hour')
+			->with('+5 minutes')
 			->willReturn($inAnHour);
 		$query->expects(self::once())
 			->method('setTimerangeStart')
@@ -971,9 +949,6 @@ public function testAvailabilityAndSearchCalendarsStatusBusyNoFBType(): void {
 			->willReturn($vCalendar);
 		$vCalendar->expects(self::never())
 			->method('add');
-		$this->generator->expects(self::once())
-			->method('setBaseObject')
-			->with($vCalendar);
 		$this->generator->expects(self::once())
 			->method('setObjects')
 			->with($vCalendar);
@@ -1081,7 +1056,7 @@ public function testAvailabilityAndSearchCalendarsStatusBusy(): void {
 			->willReturn([$calendar]);
 		$this->timeFactory->expects(self::once())
 			->method('getDateTime')
-			->with('+1 hour')
+			->with('+5 minutes')
 			->willReturn($inAnHour);
 		$query->expects(self::once())
 			->method('setTimerangeStart')
@@ -1098,9 +1073,6 @@ public function testAvailabilityAndSearchCalendarsStatusBusy(): void {
 			->willReturn($vCalendar);
 		$vCalendar->expects(self::never())
 			->method('add');
-		$this->generator->expects(self::once())
-			->method('setBaseObject')
-			->with($vCalendar);
 		$this->generator->expects(self::once())
 			->method('setObjects')
 			->with($vCalendar);
@@ -1212,7 +1184,7 @@ public function testAvailabilityAndSearchCalendarsStatusBusyUnavailable(): void
 			->willReturn([$calendar]);
 		$this->timeFactory->expects(self::once())
 			->method('getDateTime')
-			->with('+1 hour')
+			->with('+5 minutes')
 			->willReturn($inAnHour);
 		$query->expects(self::once())
 			->method('setTimerangeStart')
@@ -1229,9 +1201,6 @@ public function testAvailabilityAndSearchCalendarsStatusBusyUnavailable(): void
 			->willReturn($vCalendar);
 		$vCalendar->expects(self::never())
 			->method('add');
-		$this->generator->expects(self::once())
-			->method('setBaseObject')
-			->with($vCalendar);
 		$this->generator->expects(self::once())
 			->method('setObjects')
 			->with($vCalendar);
@@ -1340,7 +1309,7 @@ public function testAvailabilityAndSearchCalendarsStatusBusyTentative(): void {
 			->willReturn([$calendar]);
 		$this->timeFactory->expects(self::once())
 			->method('getDateTime')
-			->with('+1 hour')
+			->with('+5 minutes')
 			->willReturn($inAnHour);
 		$query->expects(self::once())
 			->method('setTimerangeStart')
@@ -1357,9 +1326,6 @@ public function testAvailabilityAndSearchCalendarsStatusBusyTentative(): void {
 			->willReturn($vCalendar);
 		$vCalendar->expects(self::never())
 			->method('add');
-		$this->generator->expects(self::once())
-			->method('setBaseObject')
-			->with($vCalendar);
 		$this->generator->expects(self::once())
 			->method('setObjects')
 			->with($vCalendar);
@@ -1468,7 +1434,7 @@ public function testAvailabilityAndSearchCalendarsStatusBusyUnknownProperty(): v
 			->willReturn([$calendar]);
 		$this->timeFactory->expects(self::once())
 			->method('getDateTime')
-			->with('+1 hour')
+			->with('+5 minutes')
 			->willReturn($inAnHour);
 		$query->expects(self::once())
 			->method('setTimerangeStart')
@@ -1485,9 +1451,6 @@ public function testAvailabilityAndSearchCalendarsStatusBusyUnknownProperty(): v
 			->willReturn($vCalendar);
 		$vCalendar->expects(self::never())
 			->method('add');
-		$this->generator->expects(self::once())
-			->method('setBaseObject')
-			->with($vCalendar);
 		$this->generator->expects(self::once())
 			->method('setObjects')
 			->with($vCalendar);

diff --git a/apps/user_status/lib/Listener/UserLiveStatusListener.php b/apps/user_status/lib/Listener/UserLiveStatusListener.php
@@ -74,7 +74,7 @@ public function handle(Event $event): void {
 			$userStatus->setIsUserDefined(false);
 		}
 
-		// If the status is user-defined and one of the persistent statuses, we
+		// If the status is user-defined and one of the persistent status, we
 		// will not override it.
 		if ($userStatus->getIsUserDefined() &&
 			\in_array($userStatus->getStatus(), StatusService::PERSISTENT_STATUSES, true)) {

diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php
@@ -27,6 +27,7 @@
  */
 namespace OCA\UserStatus\Service;
 
+use OCA\DAV\CalDAV\Status\Status as CalendarStatus;
 use OCA\DAV\CalDAV\Status\StatusService as CalendarStatusService;
 use OCA\UserStatus\Db\UserStatus;
 use OCA\UserStatus\Db\UserStatusMapper;
@@ -137,9 +138,37 @@ public function findAllRecentStatusChanges(?int $limit = null, ?int $offset = nu
 	 * @return UserStatus
 	 * @throws DoesNotExistException
 	 */
-	public function findByUserId(string $userId):UserStatus {
-		$this->getCalendarStatus($userId);
-		return $this->processStatus($this->mapper->findByUserId($userId));
+	public function findByUserId(string $userId): UserStatus {
+		$userStatus = $this->mapper->findByUserId($userId);
+		// If the status is user-defined and one of the persistent statuses, we
+		// will not override it.
+		if ($userStatus->getIsUserDefined() && \in_array($userStatus->getStatus(), StatusService::PERSISTENT_STATUSES, true)) {
+			return $this->processStatus($userStatus);
+		}
+
+		$calendarStatus = $this->getCalendarStatus($userId);
+		// We found no status from the calendar, proceed with the existing status
+		if($calendarStatus === null) {
+			return $this->processStatus($userStatus);
+		}
+
+		// if we have the same status result for the calendar and the current status,
+		// and a custom message to boot, we leave the existing status alone
+		// as to not overwrite a custom message / emoji
+		if($userStatus->getIsUserDefined()
+			&& $calendarStatus->getStatus() === $userStatus->getStatus()
+			&& !empty($userStatus->getCustomMessage())) {
+			return $this->processStatus($userStatus);
+		}
+
+		// If the new status is null, there's already an identical status in place
+		$newUserStatus = $this->setUserStatus($userId,
+			$calendarStatus->getStatus(),
+			$calendarStatus->getMessage() ?? IUserStatus::MESSAGE_AVAILABILITY,
+			true,
+			$calendarStatus->getCustomMessage() ?? '');
+
+		return $newUserStatus === null ? $this->processStatus($userStatus) : $this->processStatus($newUserStatus);
 	}
 
 	/**
@@ -175,6 +204,9 @@ public function setStatus(string $userId,
 		if (!in_array($status, self::PRIORITY_ORDERED_STATUSES, true)) {
 			throw new InvalidStatusTypeException('Status-type "' . $status . '" is not supported');
 		}
+
+
+
 		if ($statusTimestamp === null) {
 			$statusTimestamp = $this->timeFactory->getTime();
 		}
@@ -247,7 +279,7 @@ public function setUserStatus(string $userId,
 									string $status,
 									string $messageId,
 								 	bool $createBackup,
-									string $customMessage = null): void {
+									string $customMessage = null): ?UserStatus {
 		// Check if status-type is valid
 		if (!in_array($status, self::PRIORITY_ORDERED_STATUSES, true)) {
 			throw new InvalidStatusTypeException('Status-type "' . $status . '" is not supported');
@@ -259,7 +291,7 @@ public function setUserStatus(string $userId,
 
 		if ($createBackup) {
 			if ($this->backupCurrentStatus($userId) === false) {
-				return; // Already a status set automatically => abort.
+				return null; // Already a status set automatically => abort.
 			}
 
 			// If we just created the backup
@@ -285,10 +317,9 @@ public function setUserStatus(string $userId,
 		$userStatus->setStatusMessageTimestamp($this->timeFactory->now()->getTimestamp());
 
 		if ($userStatus->getId() !== null) {
-			$this->mapper->update($userStatus);
-			return;
+			return $this->mapper->update($userStatus);
 		}
-		$this->mapper->insert($userStatus);
+		return $this->mapper->insert($userStatus);
 	}
 
 	/**
@@ -571,25 +602,14 @@ public function revertMultipleUserStatus(array $userIds, string $messageId): voi
 	 * @param string $userId
 	 * @return void
 	 */
-	public function getCalendarStatus(string $userId): void {
+	public function getCalendarStatus(string $userId): ?CalendarStatus {
 		$user = $this->userManager->get($userId);
-		if($user === null) {
-			return;
+		if ($user === null) {
+			return null;
 		}
 
 		$availability = $this->mapper->getAvailabilityFromPropertiesTable($userId);
 
-		$status = $this->calendarStatusService->processCalendarAvailability($user, $availability);
-		if($status === null) {
-			return;
-		}
-
-		if($status->getMessage() !== null) {
-			$this->setUserStatus($userId, $status->getStatus(), $status->getMessage(), false, $status->getCustomMessage());
-			return;
-		}
-
-		$this->setStatus($userId, $status->getStatus(), $this->timeFactory->getTime(), true);
-
+		return $this->calendarStatusService->processCalendarAvailability($user, $availability);
 	}
 }