diff options
4 files changed, 101 insertions, 21 deletions
diff --git a/apps/dav/lib/BackgroundJob/UserStatusAutomation.php b/apps/dav/lib/BackgroundJob/UserStatusAutomation.php index e17fa604ab9..1593be49a5d 100644 --- a/apps/dav/lib/BackgroundJob/UserStatusAutomation.php +++ b/apps/dav/lib/BackgroundJob/UserStatusAutomation.php @@ -201,9 +201,7 @@ class UserStatusAutomation extends TimedJob { return; } - $this->logger->debug('User is currently NOT available, reverting call status if applicable and then setting DND'); - // The DND status automation is more important than the "Away - In call" so we also restore that one if it exists. - $this->manager->revertUserStatus($userId, IUserStatus::MESSAGE_CALL, IUserStatus::AWAY); + $this->logger->debug('User is currently NOT available, reverting call and meeting status if applicable and then setting DND'); $this->manager->setUserStatus($userId, IUserStatus::MESSAGE_AVAILABILITY, IUserStatus::DND, true); $this->logger->debug('User status automation ran'); } @@ -232,10 +230,8 @@ class UserStatusAutomation extends TimedJob { } $this->logger->debug('User is currently in an OOO period, reverting other automated status and setting OOO DND status'); - // Revert both a possible 'CALL - away' and 'office hours - DND' status - $this->manager->revertUserStatus($user->getUID(), IUserStatus::MESSAGE_CALL, IUserStatus::DND); - $this->manager->revertUserStatus($user->getUID(), IUserStatus::MESSAGE_AVAILABILITY, IUserStatus::DND); $this->manager->setUserStatus($user->getUID(), IUserStatus::MESSAGE_OUT_OF_OFFICE, IUserStatus::DND, true, $ooo->getShortMessage()); + // Run at the end of an ooo period to return to availability / regular user status // If it's overwritten by a custom status in the meantime, there's nothing we can do about it $this->setLastRunToNextToggleTime($user->getUID(), $ooo->getEndDate()); diff --git a/apps/dav/tests/unit/BackgroundJob/UserStatusAutomationTest.php b/apps/dav/tests/unit/BackgroundJob/UserStatusAutomationTest.php index fb3f9255f1a..7e30ec3d2fe 100644 --- a/apps/dav/tests/unit/BackgroundJob/UserStatusAutomationTest.php +++ b/apps/dav/tests/unit/BackgroundJob/UserStatusAutomationTest.php @@ -108,8 +108,6 @@ class UserStatusAutomationTest extends TestCase { ->willReturn(new \DateTime($currentTime, new \DateTimeZone('UTC'))); $this->logger->expects(self::exactly(4)) ->method('debug'); - $this->statusManager->expects(self::exactly(2)) - ->method('revertUserStatus'); if (!$isAvailable) { $this->statusManager->expects(self::once()) ->method('setUserStatus') @@ -172,8 +170,6 @@ END:VCALENDAR'); ->willReturn('yes'); $this->time->method('getDateTime') ->willReturn(new \DateTime('2023-02-24 13:58:24.479357', new \DateTimeZone('UTC'))); - $this->statusManager->expects($this->exactly(3)) - ->method('revertUserStatus'); $this->jobList->expects($this->once()) ->method('remove') ->with(UserStatusAutomation::class, ['userId' => 'user']); @@ -207,8 +203,6 @@ END:VCALENDAR'); $this->coordinator->expects(self::once()) ->method('isInEffect') ->willReturn(true); - $this->statusManager->expects($this->exactly(2)) - ->method('revertUserStatus'); $this->statusManager->expects(self::once()) ->method('setUserStatus') ->with('user', IUserStatus::MESSAGE_OUT_OF_OFFICE, IUserStatus::DND, true, $ooo->getShortMessage()); diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php index 3246777b46b..2a761a0f304 100644 --- a/apps/user_status/lib/Service/StatusService.php +++ b/apps/user_status/lib/Service/StatusService.php @@ -22,6 +22,7 @@ use OCP\IConfig; use OCP\IEmojiHelper; use OCP\IUserManager; use OCP\UserStatus\IUserStatus; +use Psr\Log\LoggerInterface; use function in_array; /** @@ -63,12 +64,15 @@ class StatusService { /** @var int */ public const MAXIMUM_MESSAGE_LENGTH = 80; - public function __construct(private UserStatusMapper $mapper, + public function __construct( + private UserStatusMapper $mapper, private ITimeFactory $timeFactory, private PredefinedStatusService $predefinedStatusService, private IEmojiHelper $emojiHelper, private IConfig $config, - private IUserManager $userManager) { + private IUserManager $userManager, + private LoggerInterface $logger, + ) { $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'; @@ -244,8 +248,28 @@ class StatusService { $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) { + $updateStatus = false; + if ($messageId === IUserStatus::MESSAGE_OUT_OF_OFFICE) { + // OUT_OF_OFFICE trumps AVAILABILITY, CALL and CALENDAR status + $updateStatus = $userStatus->getMessageId() === IUserStatus::MESSAGE_AVAILABILITY || $userStatus->getMessageId() === IUserStatus::MESSAGE_CALL || $userStatus->getMessageId() === IUserStatus::MESSAGE_CALENDAR_BUSY; + } elseif ($messageId === IUserStatus::MESSAGE_AVAILABILITY) { + // AVAILABILITY trumps CALL and CALENDAR status + $updateStatus = $userStatus->getMessageId() === IUserStatus::MESSAGE_CALL || $userStatus->getMessageId() === IUserStatus::MESSAGE_CALENDAR_BUSY; + } elseif ($messageId === IUserStatus::MESSAGE_CALL) { + // CALL trumps CALENDAR status + $updateStatus = $userStatus->getMessageId() === IUserStatus::MESSAGE_CALENDAR_BUSY; + } + + if ($messageId === IUserStatus::MESSAGE_OUT_OF_OFFICE || $messageId === IUserStatus::MESSAGE_AVAILABILITY || $messageId === IUserStatus::MESSAGE_CALL || $messageId === IUserStatus::MESSAGE_CALENDAR_BUSY) { + if ($updateStatus) { + $this->logger->debug('User ' . $userId . ' is currently NOT available, overwriting status [status: ' . $userStatus->getStatus() . ', messageId: ' . json_encode($userStatus->getMessageId()) . ']', ['app' => 'dav']); + } else { + $this->logger->debug('User ' . $userId . ' is currently NOT available, but we are NOT overwriting status [status: ' . $userStatus->getStatus() . ', messageId: ' . json_encode($userStatus->getMessageId()) . ']', ['app' => 'dav']); + } + } + + // There should be a backup already or none is needed. So we take a shortcut. + if ($updateStatus) { $userStatus->setStatus($status); $userStatus->setStatusTimestamp($this->timeFactory->getTime()); $userStatus->setIsUserDefined(true); @@ -265,7 +289,7 @@ class StatusService { // 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 + // Unfortunately there's no way to unset the DB ID on an Entity $userStatus = new UserStatus(); $userStatus->setUserId($userId); } diff --git a/apps/user_status/tests/Unit/Service/StatusServiceTest.php b/apps/user_status/tests/Unit/Service/StatusServiceTest.php index ee764cfc97c..710b3081b17 100644 --- a/apps/user_status/tests/Unit/Service/StatusServiceTest.php +++ b/apps/user_status/tests/Unit/Service/StatusServiceTest.php @@ -27,6 +27,7 @@ use OCP\IEmojiHelper; use OCP\IUserManager; use OCP\UserStatus\IUserStatus; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; use Test\TestCase; class StatusServiceTest extends TestCase { @@ -49,6 +50,9 @@ class StatusServiceTest extends TestCase { /** @var IUserManager|MockObject */ private $userManager; + /** @var LoggerInterface|MockObject */ + private $logger; + private StatusService $service; protected function setUp(): void { @@ -60,6 +64,7 @@ class StatusServiceTest extends TestCase { $this->emojiHelper = $this->createMock(IEmojiHelper::class); $this->userManager = $this->createMock(IUserManager::class); $this->config = $this->createMock(IConfig::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->config->method('getAppValue') ->willReturnMap([ @@ -72,7 +77,8 @@ class StatusServiceTest extends TestCase { $this->predefinedStatusService, $this->emojiHelper, $this->config, - $this->userManager + $this->userManager, + $this->logger, ); } @@ -128,7 +134,8 @@ class StatusServiceTest extends TestCase { $this->predefinedStatusService, $this->emojiHelper, $this->config, - $this->userManager + $this->userManager, + $this->logger, ); $this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50)); @@ -147,7 +154,8 @@ class StatusServiceTest extends TestCase { $this->predefinedStatusService, $this->emojiHelper, $this->config, - $this->userManager + $this->userManager, + $this->logger, ); $this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50)); @@ -731,7 +739,6 @@ class StatusServiceTest extends TestCase { } public function testBackup(): void { - $e = new Exception('', Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION); $this->mapper->expects($this->once()) ->method('createBackupStatus') ->with('john') @@ -807,4 +814,63 @@ class StatusServiceTest extends TestCase { $this->service->revertMultipleUserStatus(['john', 'nobackup', 'backuponly', 'nobackupanddnd'], 'call'); } + + public function dataSetUserStatus(): array { + return [ + [IUserStatus::MESSAGE_CALENDAR_BUSY, '', false], + + // Call > Meeting + [IUserStatus::MESSAGE_CALENDAR_BUSY, IUserStatus::MESSAGE_CALL, false], + [IUserStatus::MESSAGE_CALL, IUserStatus::MESSAGE_CALENDAR_BUSY, true], + + // Availability > Call&Meeting + [IUserStatus::MESSAGE_CALENDAR_BUSY, IUserStatus::MESSAGE_AVAILABILITY, false], + [IUserStatus::MESSAGE_CALL, IUserStatus::MESSAGE_AVAILABILITY, false], + [IUserStatus::MESSAGE_AVAILABILITY, IUserStatus::MESSAGE_CALENDAR_BUSY, true], + [IUserStatus::MESSAGE_AVAILABILITY, IUserStatus::MESSAGE_CALL, true], + + // Out-of-office > Availability&Call&Meeting + [IUserStatus::MESSAGE_AVAILABILITY, IUserStatus::MESSAGE_OUT_OF_OFFICE, false], + [IUserStatus::MESSAGE_CALENDAR_BUSY, IUserStatus::MESSAGE_OUT_OF_OFFICE, false], + [IUserStatus::MESSAGE_CALL, IUserStatus::MESSAGE_OUT_OF_OFFICE, false], + [IUserStatus::MESSAGE_OUT_OF_OFFICE, IUserStatus::MESSAGE_AVAILABILITY, true], + [IUserStatus::MESSAGE_OUT_OF_OFFICE, IUserStatus::MESSAGE_CALENDAR_BUSY, true], + [IUserStatus::MESSAGE_OUT_OF_OFFICE, IUserStatus::MESSAGE_CALL, true], + ]; + } + + /** + * @dataProvider dataSetUserStatus + */ + public function testSetUserStatus(string $messageId, string $oldMessageId, bool $expectedUpdateShortcut): void { + $previous = new UserStatus(); + $previous->setId(1); + $previous->setStatus(IUserStatus::AWAY); + $previous->setStatusTimestamp(1337); + $previous->setIsUserDefined(false); + $previous->setMessageId($oldMessageId); + $previous->setUserId('john'); + $previous->setIsBackup(false); + + $this->mapper->expects($this->once()) + ->method('findByUserId') + ->with('john') + ->willReturn($previous); + + $e = DbalException::wrap($this->createMock(UniqueConstraintViolationException::class)); + $this->mapper->expects($expectedUpdateShortcut ? $this->never() : $this->once()) + ->method('createBackupStatus') + ->willThrowException($e); + + $this->mapper->expects($this->any()) + ->method('update') + ->willReturnArgument(0); + + $this->predefinedStatusService->expects($this->once()) + ->method('isValidId') + ->with($messageId) + ->willReturn(true); + + $this->service->setUserStatus('john', IUserStatus::DND, $messageId, true); + } } |