fix(userstatus): set user status to 'In a meeting' if calendar is busy

Signed-off-by: Anna Larch <anna@nextcloud.com>

6 files changed, 64 insertions, 98 deletions

diff --git a/apps/user_status/lib/Controller/UserStatusController.php b/apps/user_status/lib/Controller/UserStatusController.php
index f2b1537e4f7..3beb8abc3ab 100644
--- a/apps/user_status/lib/Controller/UserStatusController.php
+++ b/apps/user_status/lib/Controller/UserStatusController.php
@@ -28,6 +28,7 @@ declare(strict_types=1);
  */
 namespace OCA\UserStatus\Controller;
 
+use OCA\DAV\CalDAV\Status\StatusService as CalendarStatusService;
 use OCA\UserStatus\Db\UserStatus;
 use OCA\UserStatus\Exception\InvalidClearAtException;
 use OCA\UserStatus\Exception\InvalidMessageIdException;
@@ -55,6 +56,7 @@ class UserStatusController extends OCSController {
 		private string $userId,
 		private LoggerInterface $logger,
 		private StatusService $service,
+		private CalendarStatusService $calendarStatusService,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -71,6 +73,7 @@ class UserStatusController extends OCSController {
 	 */
 	public function getStatus(): DataResponse {
 		try {
+			$this->calendarStatusService->processCalendarStatus($this->userId);
 			$userStatus = $this->service->findByUserId($this->userId);
 		} catch (DoesNotExistException $ex) {
 			throw new OCSNotFoundException('No status for the current user');
diff --git a/apps/user_status/lib/Listener/UserLiveStatusListener.php b/apps/user_status/lib/Listener/UserLiveStatusListener.php
index 0d0e6e3ebf0..b999c51d72f 100644
--- a/apps/user_status/lib/Listener/UserLiveStatusListener.php
+++ b/apps/user_status/lib/Listener/UserLiveStatusListener.php
@@ -25,6 +25,7 @@ declare(strict_types=1);
  */
 namespace OCA\UserStatus\Listener;
 
+use OCA\DAV\CalDAV\Status\StatusService as CalendarStatusService;
 use OCA\UserStatus\Connector\UserStatus as ConnectorUserStatus;
 use OCA\UserStatus\Db\UserStatus;
 use OCA\UserStatus\Db\UserStatusMapper;
@@ -48,7 +49,8 @@ class UserLiveStatusListener implements IEventListener {
 
 	public function __construct(UserStatusMapper $mapper,
 		StatusService $statusService,
-		ITimeFactory $timeFactory) {
+		ITimeFactory $timeFactory,
+		private CalendarStatusService $calendarStatusService) {
 		$this->mapper = $mapper;
 		$this->statusService = $statusService;
 		$this->timeFactory = $timeFactory;
@@ -65,6 +67,7 @@ class UserLiveStatusListener implements IEventListener {
 
 		$user = $event->getUser();
 		try {
+			$this->calendarStatusService->processCalendarStatus($user->getUID());
 			$userStatus = $this->statusService->findByUserId($user->getUID());
 		} catch (DoesNotExistException $ex) {
 			$userStatus = new UserStatus();
@@ -81,6 +84,12 @@ class UserLiveStatusListener implements IEventListener {
 			return;
 		}
 
+		// Don't overwrite the "away" calendar status if it's set
+		if($userStatus->getMessageId() === IUserStatus::MESSAGE_CALENDAR_BUSY) {
+			$event->setUserStatus(new ConnectorUserStatus($userStatus));
+			return;
+		}
+
 		$needsUpdate = false;
 
 		// If the current status is older than 5 minutes,
diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php
index 9582c403329..c623262eec6 100644
--- a/apps/user_status/lib/Service/StatusService.php
+++ b/apps/user_status/lib/Service/StatusService.php
@@ -27,8 +27,6 @@ declare(strict_types=1);
  */
 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;
 use OCA\UserStatus\Exception\InvalidClearAtException;
@@ -89,8 +87,7 @@ class StatusService {
 		private PredefinedStatusService $predefinedStatusService,
 		private IEmojiHelper $emojiHelper,
 		private IConfig $config,
-		private IUserManager $userManager,
-		private CalendarStatusService $calendarStatusService) {
+		private IUserManager $userManager) {
 		$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
 		$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
 		$this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
@@ -558,30 +555,4 @@ class StatusService {
 		// For users that matched restore the previous status
 		$this->mapper->restoreBackupStatuses($restoreIds);
 	}
-
-	/**
-	 * Calculate a users' status according to their calendar events
-	 *
-	 * There are 4 predefined types of FBTYPE - 'FREE', 'BUSY', 'BUSY-UNAVAILABLE', 'BUSY-TENTATIVE',
-	 * but 'X-' properties are possible
-	 *
-	 * @link https://icalendar.org/iCalendar-RFC-5545/3-2-9-free-busy-time-type.html
-	 *
-	 * The status will be changed for types
-	 *  - 'BUSY'
-	 *  - 'BUSY-TENTATIVE' (ex.: an event has been accepted tentatively)
-	 * and all FREEBUSY components without a type (implicitly a 'BUSY' status)
-	 *
-	 * 'X-' properties and BUSY-UNAVAILABLE is not handled
-	 *
-	 * @param string $userId
-	 * @return CalendarStatus|null
-	 */
-	public function getCalendarStatus(string $userId): ?CalendarStatus {
-		$user = $this->userManager->get($userId);
-		if ($user === null) {
-			return null;
-		}
-		return $this->calendarStatusService->processCalendarAvailability($user);
-	}
 }
diff --git a/apps/user_status/tests/Unit/Controller/UserStatusControllerTest.php b/apps/user_status/tests/Unit/Controller/UserStatusControllerTest.php
index cabcc63b291..6161eb100ec 100644
--- a/apps/user_status/tests/Unit/Controller/UserStatusControllerTest.php
+++ b/apps/user_status/tests/Unit/Controller/UserStatusControllerTest.php
@@ -26,6 +26,7 @@ declare(strict_types=1);
  */
 namespace OCA\UserStatus\Tests\Controller;
 
+use OCA\DAV\CalDAV\Status\StatusService as CalendarStatusService;
 use OCA\UserStatus\Controller\UserStatusController;
 use OCA\UserStatus\Db\UserStatus;
 use OCA\UserStatus\Exception\InvalidClearAtException;
@@ -47,7 +48,10 @@ class UserStatusControllerTest extends TestCase {
 	private $logger;
 
 	/** @var StatusService|\PHPUnit\Framework\MockObject\MockObject */
-	private $service;
+	private $statusService;
+
+	/** @var CalendarStatusService|\PHPUnit\Framework\MockObject\MockObject $calendarStatusService */
+	private $calendarStatusService;
 
 	/** @var UserStatusController */
 	private $controller;
@@ -58,15 +62,23 @@ class UserStatusControllerTest extends TestCase {
 		$request = $this->createMock(IRequest::class);
 		$userId = 'john.doe';
 		$this->logger = $this->createMock(LoggerInterface::class);
-		$this->service = $this->createMock(StatusService::class);
-
-		$this->controller = new UserStatusController('user_status', $request, $userId, $this->logger, $this->service);
+		$this->statusService = $this->createMock(StatusService::class);
+		$this->calendarStatusService = $this->createMock(CalendarStatusService::class);
+
+		$this->controller = new UserStatusController(
+			'user_status',
+			$request,
+			$userId,
+			$this->logger,
+			$this->statusService,
+			$this->calendarStatusService,
+		);
 	}
 
 	public function testGetStatus(): void {
 		$userStatus = $this->getUserStatus();
 
-		$this->service->expects($this->once())
+		$this->statusService->expects($this->once())
 			->method('findByUserId')
 			->with('john.doe')
 			->willReturn($userStatus);
@@ -85,7 +97,10 @@ class UserStatusControllerTest extends TestCase {
 	}
 
 	public function testGetStatusDoesNotExist(): void {
-		$this->service->expects($this->once())
+		$this->calendarStatusService->expects(self::once())
+			->method('processCalendarStatus')
+			->with('john.doe');
+		$this->statusService->expects($this->once())
 			->method('findByUserId')
 			->with('john.doe')
 			->willThrowException(new DoesNotExistException(''));
@@ -121,12 +136,12 @@ class UserStatusControllerTest extends TestCase {
 		$userStatus = $this->getUserStatus();
 
 		if ($expectException) {
-			$this->service->expects($this->once())
+			$this->statusService->expects($this->once())
 				->method('setStatus')
 				->with('john.doe', $statusType, null, true)
 				->willThrowException($exception);
 		} else {
-			$this->service->expects($this->once())
+			$this->statusService->expects($this->once())
 				->method('setStatus')
 				->with('john.doe', $statusType, null, true)
 				->willReturn($userStatus);
@@ -187,12 +202,12 @@ class UserStatusControllerTest extends TestCase {
 		$userStatus = $this->getUserStatus();
 
 		if ($expectException) {
-			$this->service->expects($this->once())
+			$this->statusService->expects($this->once())
 				->method('setPredefinedMessage')
 				->with('john.doe', $messageId, $clearAt)
 				->willThrowException($exception);
 		} else {
-			$this->service->expects($this->once())
+			$this->statusService->expects($this->once())
 				->method('setPredefinedMessage')
 				->with('john.doe', $messageId, $clearAt)
 				->willReturn($userStatus);
@@ -259,28 +274,28 @@ class UserStatusControllerTest extends TestCase {
 		$userStatus = $this->getUserStatus();
 
 		if ($expectException) {
-			$this->service->expects($this->once())
+			$this->statusService->expects($this->once())
 				->method('setCustomMessage')
 				->with('john.doe', $statusIcon, $message, $clearAt)
 				->willThrowException($exception);
 		} else {
 			if ($expectSuccessAsReset) {
-				$this->service->expects($this->never())
+				$this->statusService->expects($this->never())
 					->method('setCustomMessage');
-				$this->service->expects($this->once())
+				$this->statusService->expects($this->once())
 					->method('clearMessage')
 					->with('john.doe');
-				$this->service->expects($this->once())
+				$this->statusService->expects($this->once())
 					->method('findByUserId')
 					->with('john.doe')
 					->willReturn($userStatus);
 			} else {
-				$this->service->expects($this->once())
+				$this->statusService->expects($this->once())
 					->method('setCustomMessage')
 					->with('john.doe', $statusIcon, $message, $clearAt)
 					->willReturn($userStatus);
 
-				$this->service->expects($this->never())
+				$this->statusService->expects($this->never())
 					->method('clearMessage');
 			}
 		}
@@ -326,7 +341,7 @@ class UserStatusControllerTest extends TestCase {
 	}
 
 	public function testClearMessage(): void {
-		$this->service->expects($this->once())
+		$this->statusService->expects($this->once())
 			->method('clearMessage')
 			->with('john.doe');
 
diff --git a/apps/user_status/tests/Unit/Listener/UserLiveStatusListenerTest.php b/apps/user_status/tests/Unit/Listener/UserLiveStatusListenerTest.php
index 5bc60ca71f9..0f637b75411 100644
--- a/apps/user_status/tests/Unit/Listener/UserLiveStatusListenerTest.php
+++ b/apps/user_status/tests/Unit/Listener/UserLiveStatusListenerTest.php
@@ -26,6 +26,7 @@ declare(strict_types=1);
  */
 namespace OCA\UserStatus\Tests\Listener;
 
+use OCA\DAV\CalDAV\Status\StatusService as CalendarStatusService;
 use OCA\UserStatus\Db\UserStatus;
 use OCA\UserStatus\Db\UserStatusMapper;
 use OCA\UserStatus\Listener\UserDeletedListener;
@@ -36,27 +37,37 @@ use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\GenericEvent;
 use OCP\IUser;
 use OCP\User\Events\UserLiveStatusEvent;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class UserLiveStatusListenerTest extends TestCase {
 
-	/** @var UserStatusMapper|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var UserStatusMapper|MockObject */
 	private $mapper;
-	/** @var StatusService|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var StatusService|MockObject */
 	private $statusService;
-	/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var ITimeFactory|MockObject */
 	private $timeFactory;
 
 	/** @var UserDeletedListener */
 	private $listener;
 
+	private CalendarStatusService|MockObject $calendarStatusService;
+
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->mapper = $this->createMock(UserStatusMapper::class);
 		$this->statusService = $this->createMock(StatusService::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
-		$this->listener = new UserLiveStatusListener($this->mapper, $this->statusService, $this->timeFactory);
+		$this->calendarStatusService = $this->createMock(CalendarStatusService::class);
+
+		$this->listener = new UserLiveStatusListener(
+			$this->mapper,
+			$this->statusService,
+			$this->timeFactory,
+			$this->calendarStatusService,
+		);
 	}
 
 	/**
diff --git a/apps/user_status/tests/Unit/Service/StatusServiceTest.php b/apps/user_status/tests/Unit/Service/StatusServiceTest.php
index 2de041712bd..da11ec0943b 100644
--- a/apps/user_status/tests/Unit/Service/StatusServiceTest.php
+++ b/apps/user_status/tests/Unit/Service/StatusServiceTest.php
@@ -28,9 +28,6 @@ namespace OCA\UserStatus\Tests\Service;
 
 use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
 use OC\DB\Exceptions\DbalException;
-use OC\User\User;
-use OCA\DAV\CalDAV\Status\Status;
-use OCA\DAV\CalDAV\Status\StatusService as CalendarStatusService;
 use OCA\UserStatus\Db\UserStatus;
 use OCA\UserStatus\Db\UserStatusMapper;
 use OCA\UserStatus\Exception\InvalidClearAtException;
@@ -45,7 +42,6 @@ use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\DB\Exception;
 use OCP\IConfig;
 use OCP\IEmojiHelper;
-use OCP\IUser;
 use OCP\IUserManager;
 use OCP\UserStatus\IUserStatus;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -71,9 +67,6 @@ class StatusServiceTest extends TestCase {
 	/** @var IUserManager|MockObject  */
 	private $userManager;
 
-	/** @var CalendarStatusService|MockObject  */
-	private $calendarStatusService;
-
 	private StatusService $service;
 
 	protected function setUp(): void {
@@ -84,8 +77,6 @@ class StatusServiceTest extends TestCase {
 		$this->predefinedStatusService = $this->createMock(PredefinedStatusService::class);
 		$this->emojiHelper = $this->createMock(IEmojiHelper::class);
 		$this->userManager = $this->createMock(IUserManager::class);
-		$this->calendarStatusService = $this->createMock(CalendarStatusService::class);
-
 		$this->config = $this->createMock(IConfig::class);
 
 		$this->config->method('getAppValue')
@@ -99,8 +90,7 @@ class StatusServiceTest extends TestCase {
 			$this->predefinedStatusService,
 			$this->emojiHelper,
 			$this->config,
-			$this->userManager,
-			$this->calendarStatusService,
+			$this->userManager
 		);
 	}
 
@@ -156,8 +146,7 @@ class StatusServiceTest extends TestCase {
 			$this->predefinedStatusService,
 			$this->emojiHelper,
 			$this->config,
-			$this->userManager,
-			$this->calendarStatusService,
+			$this->userManager
 		);
 
 		$this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50));
@@ -176,8 +165,7 @@ class StatusServiceTest extends TestCase {
 			$this->predefinedStatusService,
 			$this->emojiHelper,
 			$this->config,
-			$this->userManager,
-			$this->calendarStatusService,
+			$this->userManager
 		);
 
 		$this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50));
@@ -837,35 +825,4 @@ class StatusServiceTest extends TestCase {
 
 		$this->service->revertMultipleUserStatus(['john', 'nobackup', 'backuponly', 'nobackupanddnd'], 'call');
 	}
-
-	public function testCalendarAvailabilityNoUser(): void {
-		$userId = 'admin';
-
-		$this->userManager->expects(self::once())
-			->method('get')
-			->with($userId)
-			->willReturn(null);
-		$this->calendarStatusService->expects(self::never())
-			->method('processCalendarAvailability');
-
-		$this->service->getCalendarStatus($userId);
-	}
-
-	public function testCalendarAvailabilityNoStatus(): void {
-		$user = $this->createConfiguredMock(IUser::class, [
-			'getUID' => 'admin',
-			'getEMailAddress' => 'test@test.com',
-		]);
-
-		$this->userManager->expects(self::once())
-			->method('get')
-			->with($user->getUID())
-			->willReturn($user);
-		$this->calendarStatusService->expects(self::once())
-			->method('processCalendarAvailability')
-			->with($user)
-			->willReturn(null);
-
-		$this->service->getCalendarStatus($user->getUID());
-	}
 }