Merge pull request #27375 from nextcloud/bugfix/noid/dont-update-offline-status

Don't update statuses to offline again and again

apps/user_status/lib/Db/UserStatusMapper.php

@@ -137,6 +137,7 @@ class UserStatusMapper extends QBMapper {
 			->set('is_user_defined', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))
 			->set('status_timestamp', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT))
 			->where($qb->expr()->lte('status_timestamp', $qb->createNamedParameter($olderThan, IQueryBuilder::PARAM_INT)))
+			->andWhere($qb->expr()->neq('status', $qb->createNamedParameter(IUserStatus::OFFLINE)))
 			->andWhere($qb->expr()->orX(
 				$qb->expr()->eq('is_user_defined', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL), IQueryBuilder::PARAM_BOOL),
 				$qb->expr()->eq('status', $qb->createNamedParameter(IUserStatus::ONLINE))

apps/user_status/lib/Service/StatusService.php

@@ -356,6 +356,10 @@ class StatusService {
 	 * @param UserStatus $status
 	 */
 	private function cleanStatus(UserStatus $status): void {
+		if ($status->getStatus() === IUserStatus::OFFLINE && !$status->getIsUserDefined()) {
+			return;
+		}
+
 		$status->setStatus(IUserStatus::OFFLINE);
 		$status->setStatusTimestamp($this->timeFactory->getTime());
 		$status->setIsUserDefined(false);

apps/user_status/tests/Unit/Db/UserStatusMapperTest.php

@@ -188,6 +188,7 @@ class UserStatusMapperTest extends TestCase {
 
 	public function clearStatusesOlderThanDataProvider(): array {
 		return [
+			['offline', false, 6000, false],
 			['online', true, 6000, false],
 			['online', true, 4000, true],
 			['online', false, 6000, false],

apps/user_status/tests/Unit/Service/StatusServiceTest.php

@@ -38,6 +38,7 @@ use OCA\UserStatus\Service\PredefinedStatusService;
 use OCA\UserStatus\Service\StatusService;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\UserStatus\IUserStatus;
 use Test\TestCase;
 
 class StatusServiceTest extends TestCase {
@@ -624,4 +625,44 @@ class StatusServiceTest extends TestCase {
 		$actual = $this->service->removeUserStatus('john.doe');
 		$this->assertFalse($actual);
 	}
+
+	public function testCleanStatusAutomaticOnline(): void {
+		$status = new UserStatus();
+		$status->setStatus(IUserStatus::ONLINE);
+		$status->setStatusTimestamp(1337);
+		$status->setIsUserDefined(false);
+
+		$this->mapper->expects(self::once())
+			->method('update')
+			->with($status);
+
+		parent::invokePrivate($this->service, 'cleanStatus', [$status]);
+	}
+
+	public function testCleanStatusCustomOffline(): void {
+		$status = new UserStatus();
+		$status->setStatus(IUserStatus::OFFLINE);
+		$status->setStatusTimestamp(1337);
+		$status->setIsUserDefined(true);
+
+		$this->mapper->expects(self::once())
+			->method('update')
+			->with($status);
+
+		parent::invokePrivate($this->service, 'cleanStatus', [$status]);
+	}
+
+	public function testCleanStatusCleanedAlready(): void {
+		$status = new UserStatus();
+		$status->setStatus(IUserStatus::OFFLINE);
+		$status->setStatusTimestamp(1337);
+		$status->setIsUserDefined(false);
+
+		// Don't update the status again and again when no value changed
+		$this->mapper->expects(self::never())
+			->method('update')
+			->with($status);
+
+		parent::invokePrivate($this->service, 'cleanStatus', [$status]);
+	}
 }