From fbdf733be0df1bf00f8903a98ccabdf519db55d2 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 21 Sep 2023 17:05:06 +0200 Subject: [PATCH] fix(userstatus): Track message timestamp too Signed-off-by: Christoph Wurst --- apps/user_status/appinfo/info.xml | 2 +- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Dashboard/UserStatusWidget.php | 2 +- apps/user_status/lib/Db/UserStatus.php | 6 + apps/user_status/lib/Db/UserStatusMapper.php | 3 +- .../Version1008Date20230921144701.php | 67 +++++++++ .../user_status/lib/Service/StatusService.php | 6 + .../Service/StatusServiceIntegrationTest.php | 134 ++++++++++++++++++ .../tests/Unit/Db/UserStatusMapperTest.php | 2 + 10 files changed, 221 insertions(+), 3 deletions(-) create mode 100644 apps/user_status/lib/Migration/Version1008Date20230921144701.php create mode 100644 apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php diff --git a/apps/user_status/appinfo/info.xml b/apps/user_status/appinfo/info.xml index ee9f4a46781f3..6808c07ec7f87 100644 --- a/apps/user_status/appinfo/info.xml +++ b/apps/user_status/appinfo/info.xml @@ -5,7 +5,7 @@ User status User status - 1.8.0 + 1.8.1 agpl Georg Ehrke UserStatus diff --git a/apps/user_status/composer/composer/autoload_classmap.php b/apps/user_status/composer/composer/autoload_classmap.php index 50a2459e53b8f..a0e3572a87bcb 100644 --- a/apps/user_status/composer/composer/autoload_classmap.php +++ b/apps/user_status/composer/composer/autoload_classmap.php @@ -31,6 +31,7 @@ 'OCA\\UserStatus\\Migration\\Version0002Date20200902144824' => $baseDir . '/../lib/Migration/Version0002Date20200902144824.php', 'OCA\\UserStatus\\Migration\\Version1000Date20201111130204' => $baseDir . '/../lib/Migration/Version1000Date20201111130204.php', 'OCA\\UserStatus\\Migration\\Version1003Date20210809144824' => $baseDir . '/../lib/Migration/Version1003Date20210809144824.php', + 'OCA\\UserStatus\\Migration\\Version1008Date20230921144701' => $baseDir . '/../lib/Migration/Version1008Date20230921144701.php', 'OCA\\UserStatus\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php', 'OCA\\UserStatus\\Service\\JSDataService' => $baseDir . '/../lib/Service/JSDataService.php', 'OCA\\UserStatus\\Service\\PredefinedStatusService' => $baseDir . '/../lib/Service/PredefinedStatusService.php', diff --git a/apps/user_status/composer/composer/autoload_static.php b/apps/user_status/composer/composer/autoload_static.php index 317f9d18969e1..58e77db6721b6 100644 --- a/apps/user_status/composer/composer/autoload_static.php +++ b/apps/user_status/composer/composer/autoload_static.php @@ -46,6 +46,7 @@ class ComposerStaticInitUserStatus 'OCA\\UserStatus\\Migration\\Version0002Date20200902144824' => __DIR__ . '/..' . '/../lib/Migration/Version0002Date20200902144824.php', 'OCA\\UserStatus\\Migration\\Version1000Date20201111130204' => __DIR__ . '/..' . '/../lib/Migration/Version1000Date20201111130204.php', 'OCA\\UserStatus\\Migration\\Version1003Date20210809144824' => __DIR__ . '/..' . '/../lib/Migration/Version1003Date20210809144824.php', + 'OCA\\UserStatus\\Migration\\Version1008Date20230921144701' => __DIR__ . '/..' . '/../lib/Migration/Version1008Date20230921144701.php', 'OCA\\UserStatus\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php', 'OCA\\UserStatus\\Service\\JSDataService' => __DIR__ . '/..' . '/../lib/Service/JSDataService.php', 'OCA\\UserStatus\\Service\\PredefinedStatusService' => __DIR__ . '/..' . '/../lib/Service/PredefinedStatusService.php', diff --git a/apps/user_status/lib/Dashboard/UserStatusWidget.php b/apps/user_status/lib/Dashboard/UserStatusWidget.php index 89b9c05f8053d..6c0dd356730d9 100644 --- a/apps/user_status/lib/Dashboard/UserStatusWidget.php +++ b/apps/user_status/lib/Dashboard/UserStatusWidget.php @@ -165,7 +165,7 @@ static function (UserStatus $status) use ($userId, $since): bool { : $status->getStatus(), 'icon' => $status->getCustomIcon(), 'message' => $status->getCustomMessage(), - 'timestamp' => $status->getStatusTimestamp(), + 'timestamp' => $status->getStatusMessageTimestamp(), ]; }, $recentStatusUpdates); } diff --git a/apps/user_status/lib/Db/UserStatus.php b/apps/user_status/lib/Db/UserStatus.php index d4e24e7559772..92b3df740c251 100644 --- a/apps/user_status/lib/Db/UserStatus.php +++ b/apps/user_status/lib/Db/UserStatus.php @@ -52,6 +52,8 @@ * @method void setClearAt(int|null $clearAt) * @method setIsBackup(bool $true): void * @method getIsBackup(): bool + * @method int getStatusMessageTimestamp() + * @method void setStatusMessageTimestamp(int $statusTimestamp) */ class UserStatus extends Entity { @@ -82,6 +84,9 @@ class UserStatus extends Entity { /** @var bool $isBackup */ public $isBackup; + /** @var int */ + protected $statusMessageTimestamp = 0; + public function __construct() { $this->addType('userId', 'string'); $this->addType('status', 'string'); @@ -92,5 +97,6 @@ public function __construct() { $this->addType('customMessage', 'string'); $this->addType('clearAt', 'int'); $this->addType('isBackup', 'boolean'); + $this->addType('statusMessageTimestamp', 'int'); } } diff --git a/apps/user_status/lib/Db/UserStatusMapper.php b/apps/user_status/lib/Db/UserStatusMapper.php index d40c6a2986050..3621c1bfa9666 100644 --- a/apps/user_status/lib/Db/UserStatusMapper.php +++ b/apps/user_status/lib/Db/UserStatusMapper.php @@ -75,8 +75,9 @@ public function findAllRecent(?int $limit = null, ?int $offset = null): array { $qb ->select('*') ->from($this->tableName) - ->orderBy('status_timestamp', 'DESC') + ->orderBy('status_message_timestamp', 'DESC') ->where($qb->expr()->andX( + $qb->expr()->neq('status_message_timestamp', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT), $qb->expr()->orX( $qb->expr()->notIn('status', $qb->createNamedParameter([IUserStatus::ONLINE, IUserStatus::AWAY, IUserStatus::OFFLINE], IQueryBuilder::PARAM_STR_ARRAY)), $qb->expr()->isNotNull('message_id'), diff --git a/apps/user_status/lib/Migration/Version1008Date20230921144701.php b/apps/user_status/lib/Migration/Version1008Date20230921144701.php new file mode 100644 index 0000000000000..3b653f68eebd7 --- /dev/null +++ b/apps/user_status/lib/Migration/Version1008Date20230921144701.php @@ -0,0 +1,67 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\UserStatus\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version1008Date20230921144701 extends SimpleMigrationStep { + + public function __construct(private IDBConnection $connection) { + } + + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $statusTable = $schema->getTable('user_status'); + if (!($statusTable->hasColumn('status_message_timestamp'))) { + $statusTable->addColumn('status_message_timestamp', Types::INTEGER, [ + 'notnull' => true, + 'length' => 11, + 'unsigned' => true, + 'default' => 0, + ]); + } + if (!$statusTable->hasIndex('user_status_mtstmp_ix')) { + $statusTable->addIndex(['status_message_timestamp'], 'user_status_mtstmp_ix'); + } + + return $schema; + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $qb = $this->connection->getQueryBuilder(); + + $update = $qb->update('user_status') + ->set('status_message_timestamp', 'status_timestamp'); + + $update->executeStatement(); + } +} diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php index b5dd3cd361a8f..f9ae769a5a9d5 100644 --- a/apps/user_status/lib/Service/StatusService.php +++ b/apps/user_status/lib/Service/StatusService.php @@ -237,6 +237,7 @@ public function setPredefinedMessage(string $userId, $userStatus->setCustomIcon(null); $userStatus->setCustomMessage(null); $userStatus->setClearAt($clearAt); + $userStatus->setStatusMessageTimestamp($this->timeFactory->now()->getTimestamp()); if ($userStatus->getId() === null) { return $this->mapper->insert($userStatus); @@ -291,6 +292,7 @@ public function setUserStatus(string $userId, $userStatus->setCustomIcon(null); $userStatus->setCustomMessage(null); $userStatus->setClearAt(null); + $userStatus->setStatusMessageTimestamp($this->timeFactory->now()->getTimestamp()); if ($userStatus->getId() !== null) { $this->mapper->update($userStatus); @@ -340,6 +342,7 @@ public function setCustomMessage(string $userId, $userStatus->setCustomIcon($statusIcon); $userStatus->setCustomMessage($message); $userStatus->setClearAt($clearAt); + $userStatus->setStatusMessageTimestamp($this->timeFactory->now()->getTimestamp()); if ($userStatus->getId() === null) { return $this->mapper->insert($userStatus); @@ -384,6 +387,7 @@ public function clearMessage(string $userId): bool { $userStatus->setCustomMessage(null); $userStatus->setCustomIcon(null); $userStatus->setClearAt(null); + $userStatus->setStatusMessageTimestamp(0); $this->mapper->update($userStatus); return true; @@ -465,6 +469,7 @@ private function cleanStatusMessage(UserStatus $status): void { $status->setCustomIcon(null); $status->setCustomMessage(null); $status->setClearAt(null); + $status->setStatusMessageTimestamp(0); $this->mapper->update($status); } @@ -478,6 +483,7 @@ private function addDefaultMessage(UserStatus $status): void { if ($predefinedMessage !== null) { $status->setCustomMessage($predefinedMessage['message']); $status->setCustomIcon($predefinedMessage['icon']); + $status->setStatusMessageTimestamp($this->timeFactory->now()->getTimestamp()); } } diff --git a/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php b/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php new file mode 100644 index 0000000000000..798e591f97543 --- /dev/null +++ b/apps/user_status/tests/Integration/Service/StatusServiceIntegrationTest.php @@ -0,0 +1,134 @@ + + * + * @author 2023 Christoph Wurst + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\UserStatus\Tests\Integration\BackgroundJob; + +use OCA\UserStatus\Service\StatusService; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IDBConnection; +use OCP\Server; +use OCP\UserStatus\IUserStatus; +use Test\TestCase; +use function sleep; +use function time; + +/** + * @group DB + */ +class StatusServiceIntegrationTest extends TestCase { + + private StatusService $service; + + protected function setUp(): void { + parent::setUp(); + + $this->service = Server::get(StatusService::class); + + $db = Server::get(IDBConnection::class); + $qb = $db->getQueryBuilder(); + $qb->delete('user_status')->executeStatement(); + } + + public function testNoStatusYet(): void { + $this->expectException(DoesNotExistException::class); + + $this->service->findByUserId('test123'); + } + + public function testCustomStatusMessageTimestamp(): void { + $this->service->setCustomMessage( + 'test123', + '🍕', + 'Lunch', + null, + ); + + $status = $this->service->findByUserId('test123'); + + self::assertSame('Lunch', $status->getCustomMessage()); + self::assertGreaterThanOrEqual(time(), $status->getStatusMessageTimestamp()); + } + + public function testOnlineStatusKeepsMessageTimestamp(): void { + $this->service->setStatus( + 'test123', + IUserStatus::OFFLINE, + time() + 1000, + false, + ); + $this->service->setCustomMessage( + 'test123', + '🍕', + 'Lunch', + null, + ); + $timeAfterInsert = time(); + sleep(1); + $this->service->setStatus( + 'test123', + IUserStatus::ONLINE, + time() + 2000, + false, + ); + $status = $this->service->findByUserId('test123'); + + self::assertSame('Lunch', $status->getCustomMessage()); + self::assertLessThanOrEqual($timeAfterInsert, $status->getStatusMessageTimestamp()); + } + + public function testCreateRestoreBackupAutomatically(): void { + $this->service->setStatus( + 'test123', + IUserStatus::ONLINE, + null, + false, + ); + $this->service->setUserStatus( + 'test123', + IUserStatus::DND, + 'meeting', + true, + ); + self::assertSame( + 'meeting', + $this->service->findByUserId('test123')->getMessageId(), + ); + + $this->service->revertUserStatus( + 'test123', + 'meeting', + ); + self::assertSame( + IUserStatus::ONLINE, + $this->service->findByUserId('test123')->getStatus(), + ); + } + + public function testCi(): void { + // TODO: remove if CI turns red + self::assertTrue(false); + } + +} diff --git a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php index 577277cfd618f..38913e5f8f5d1 100644 --- a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php +++ b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php @@ -228,6 +228,7 @@ private function insertSampleStatuses(): void { $userStatus2->setUserId('user1'); $userStatus2->setStatus('dnd'); $userStatus2->setStatusTimestamp(5000); + $userStatus2->setStatusMessageTimestamp(5000); $userStatus2->setIsUserDefined(true); $userStatus2->setCustomIcon('💩'); $userStatus2->setCustomMessage('Do not disturb'); @@ -237,6 +238,7 @@ private function insertSampleStatuses(): void { $userStatus3->setUserId('user2'); $userStatus3->setStatus('away'); $userStatus3->setStatusTimestamp(6000); + $userStatus3->setStatusMessageTimestamp(6000); $userStatus3->setIsUserDefined(false); $userStatus3->setCustomIcon('🏝'); $userStatus3->setCustomMessage('On vacation');