0
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2025-02-24 17:06:50 +00:00

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>
This commit is contained in:
Joas Schilling 2024-06-24 16:28:43 +02:00 committed by backportbot[bot]
parent 69561004c7
commit 2bd5a534f0
4 changed files with 101 additions and 21 deletions
apps
dav
lib/BackgroundJob
tests/unit/BackgroundJob
user_status
lib/Service
tests/Unit/Service

View file

@ -216,9 +216,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');
}
@ -247,10 +245,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());

View file

@ -125,8 +125,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')
@ -189,8 +187,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']);
@ -224,8 +220,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());

View file

@ -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);
}

View file

@ -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);
}
}