diff options
author | Joas Schilling <coding@schilljs.com> | 2024-06-24 16:28:43 +0200 |
---|---|---|
committer | backportbot[bot] <backportbot[bot]@users.noreply.github.com> | 2024-06-25 09:33:12 +0000 |
commit | 2bd5a534f0b21c356acf0dcedb816d288604fbb5 (patch) | |
tree | 850fd45022ebdd9994b15198e70f57e0496d5ed1 /apps/user_status | |
parent | 69561004c701f62bbbe753ea89e67d26a3fa7f01 (diff) | |
download | nextcloud-server-2bd5a534f0b21c356acf0dcedb816d288604fbb5.tar.gz nextcloud-server-2bd5a534f0b21c356acf0dcedb816d288604fbb5.zip |
fix(userstatus): Fix user status automation in real-life scenario
Order of applying:
- Out-of-office
- Availability
- Call
- Meeting
- User status
Signed-off-by: Joas Schilling <coding@schilljs.com>
Diffstat (limited to 'apps/user_status')
-rw-r--r-- | apps/user_status/lib/Service/StatusService.php | 34 | ||||
-rw-r--r-- | apps/user_status/tests/Unit/Service/StatusServiceTest.php | 74 |
2 files changed, 99 insertions, 9 deletions
diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php index d6e857520df..a291b4420c1 100644 --- a/apps/user_status/lib/Service/StatusService.php +++ b/apps/user_status/lib/Service/StatusService.php @@ -41,6 +41,7 @@ use OCP\IConfig; use OCP\IEmojiHelper; use OCP\IUserManager; use OCP\UserStatus\IUserStatus; +use Psr\Log\LoggerInterface; use function in_array; /** @@ -82,12 +83,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'; @@ -263,8 +267,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); @@ -284,7 +308,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 da11ec0943b..afa7fabe774 100644 --- a/apps/user_status/tests/Unit/Service/StatusServiceTest.php +++ b/apps/user_status/tests/Unit/Service/StatusServiceTest.php @@ -45,6 +45,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 { @@ -67,6 +68,9 @@ class StatusServiceTest extends TestCase { /** @var IUserManager|MockObject */ private $userManager; + /** @var LoggerInterface|MockObject */ + private $logger; + private StatusService $service; protected function setUp(): void { @@ -78,6 +82,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([ @@ -90,7 +95,8 @@ class StatusServiceTest extends TestCase { $this->predefinedStatusService, $this->emojiHelper, $this->config, - $this->userManager + $this->userManager, + $this->logger, ); } @@ -146,7 +152,8 @@ class StatusServiceTest extends TestCase { $this->predefinedStatusService, $this->emojiHelper, $this->config, - $this->userManager + $this->userManager, + $this->logger, ); $this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50)); @@ -165,7 +172,8 @@ class StatusServiceTest extends TestCase { $this->predefinedStatusService, $this->emojiHelper, $this->config, - $this->userManager + $this->userManager, + $this->logger, ); $this->assertEquals([], $this->service->findAllRecentStatusChanges(20, 50)); @@ -749,7 +757,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') @@ -825,4 +832,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); + } } |