From b2f21bc9aebfce2afe4b2200d9b12203f118296f Mon Sep 17 00:00:00 2001 From: JoshuaLicense Date: Thu, 21 Dec 2023 09:44:56 +0000 Subject: [PATCH] fix: redesign query handlers for conversations (cherry picked from commit 8fcc7e4c24a892d51c965a3af220e65505e695ae) --- module/Api/config/query-map.config.php | 4 +- .../validation-map/messaging.config.php | 3 +- .../Conversations/ByApplicationToLicence.php | 45 +++++++++++ .../ByLicence.php} | 54 ++++++------- .../ByApplicationToLicenceTest.php | 48 ++++++++++++ .../ByLicenceTest.php} | 77 ++++++++----------- 6 files changed, 154 insertions(+), 77 deletions(-) create mode 100644 module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByApplicationToLicence.php rename module/Api/src/Domain/QueryHandler/Messaging/{ConversationList.php => Conversations/ByLicence.php} (82%) create mode 100644 test/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByApplicationToLicenceTest.php rename test/module/Api/src/Domain/QueryHandler/Messaging/{ConversationListTest.php => Conversations/ByLicenceTest.php} (54%) diff --git a/module/Api/config/query-map.config.php b/module/Api/config/query-map.config.php index b3e3208962..a051fa9bb8 100644 --- a/module/Api/config/query-map.config.php +++ b/module/Api/config/query-map.config.php @@ -727,5 +727,7 @@ Query\Cache\Single::class => QueryHandler\Cache\Single::class, Query\Cache\RecordList::class => QueryHandler\Cache\RecordList::class, - TransferQuery\Messaging\GetConversationList::class => QueryHandler\Messaging\ConversationList::class, + // Messaging + TransferQuery\Messaging\Conversations\ByLicence::class => QueryHandler\Messaging\Conversations\ByLicence::class, + TransferQuery\Messaging\Conversations\ByApplicationToLicence::class => QueryHandler\Messaging\Conversations\ByApplicationToLicence::class, ]; diff --git a/module/Api/config/validation-map/messaging.config.php b/module/Api/config/validation-map/messaging.config.php index 063ccdad67..60777a5751 100644 --- a/module/Api/config/validation-map/messaging.config.php +++ b/module/Api/config/validation-map/messaging.config.php @@ -4,5 +4,6 @@ use Dvsa\Olcs\Api\Domain\Validation\Handlers\Misc\NoValidationRequired; return [ - QueryHandler\Messaging\ConversationList::class => NoValidationRequired::class, + QueryHandler\Messaging\Conversations\ByLicence::class => NoValidationRequired::class, + QueryHandler\Messaging\Conversations\ByApplicationToLicence::class => NoValidationRequired::class, ]; diff --git a/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByApplicationToLicence.php b/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByApplicationToLicence.php new file mode 100644 index 0000000000..6bf168c104 --- /dev/null +++ b/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByApplicationToLicence.php @@ -0,0 +1,45 @@ +getApplicationRepository(); + + $application = $applicationRepository->fetchById($query->getApplication()); + + $licenceQuery = [ + 'page' => $query->getPage(), + 'limit' => $query->getLimit(), + 'licence' => $application->getLicence()->getId(), + ]; + + return $this->getQueryHandler()->handleQuery(GetConversationsByLicenceQuery::create($licenceQuery)); + } + + private function getApplicationRepository(): ApplicationRepo + { + $applicationRepository = $this->getRepo('Application'); + assert($applicationRepository instanceof ApplicationRepo); + return $applicationRepository; + } +} diff --git a/module/Api/src/Domain/QueryHandler/Messaging/ConversationList.php b/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByLicence.php similarity index 82% rename from module/Api/src/Domain/QueryHandler/Messaging/ConversationList.php rename to module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByLicence.php index 6ccf384f59..80fd3cc6b4 100644 --- a/module/Api/src/Domain/QueryHandler/Messaging/ConversationList.php +++ b/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByLicence.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Dvsa\Olcs\Api\Domain\QueryHandler\Messaging; +namespace Dvsa\Olcs\Api\Domain\QueryHandler\Messaging\Conversations; use Dvsa\Olcs\Api\Domain\QueryHandler\AbstractQueryHandler; use Dvsa\Olcs\Api\Domain\Repository\Conversation as ConversationRepo; @@ -10,39 +10,28 @@ use Dvsa\Olcs\Api\Domain\ToggleAwareTrait; use Dvsa\Olcs\Api\Domain\ToggleRequiredInterface; use Dvsa\Olcs\Api\Entity\System\FeatureToggle; -use Dvsa\Olcs\Transfer\Query\Messaging\GetConversationList as GetConversationListQuery; +use Dvsa\Olcs\Transfer\Query\Messaging\Conversations\ByLicence as GetConversationsByLicenceQuery; use Dvsa\Olcs\Transfer\Query\QueryInterface; -class ConversationList extends AbstractQueryHandler implements ToggleRequiredInterface +class ByLicence extends AbstractQueryHandler implements ToggleRequiredInterface { use ToggleAwareTrait; - protected $toggleConfig = [FeatureToggle::MESSAGING]; private const STATUS_CLOSED = "CLOSED"; private const STATUS_NEW_MESSAGE = "NEW_MESSAGE"; private const STATUS_OPEN = "OPEN"; + protected $toggleConfig = [FeatureToggle::MESSAGING]; protected $extraRepos = ['Conversation', 'Message']; public function handleQuery(QueryInterface $query) { - assert($query instanceof GetConversationListQuery); - $conversationRepository = $this->getRepo('Conversation'); - assert($conversationRepository instanceof ConversationRepo); + assert($query instanceof GetConversationsByLicenceQuery); + $conversationRepository = $this->getRepository(); $conversationsQuery = $conversationRepository->getBaseConversationListQuery($query); - if (!empty($query->getLicence())) { - $conversationsQuery = $conversationRepository->filterByLicenceId($conversationsQuery, $query->getLicence()); - } - - if (!empty($query->getApplication())) { - $conversationsQuery = $conversationRepository->filterByApplicationId($conversationsQuery, $query->getApplication()); - } - - if ($query->getApplyOpenMessageSorting()) { - $conversationsQuery = $conversationRepository->applyOrderByOpen($conversationsQuery); - } - + $conversationsQuery = $conversationRepository->filterByLicenceId($conversationsQuery, $query->getLicence()); + $conversationsQuery = $conversationRepository->applyOrderByOpen($conversationsQuery); $conversations = $conversationRepository->fetchPaginatedList($conversationsQuery); foreach ($conversations as $key => $value) { $unreadMessageCount = $this->getUnreadMessageCountForUser($value); @@ -51,9 +40,7 @@ public function handleQuery(QueryInterface $query) $conversations[$key]['latestMessage'] = $this->getLatestMessageMetadata($value['id']); } - if ($query->getApplyNewMessageSorting()) { - $conversations = $this->orderResultPrioritisingNewMessages($conversations); - } + $conversations = $this->orderResultPrioritisingNewMessages($conversations); return [ 'result' => $conversations, @@ -61,6 +48,14 @@ public function handleQuery(QueryInterface $query) ]; } + private function getUnreadMessageCountForUser($conversation): int + { + $messageRepository = $this->getRepo('Message'); + assert($messageRepository instanceof MessageRepo); + $results = $messageRepository->getUnreadMessagesByConversationIdAndUserId($conversation['id'], $this->getUser()->getId()); + return count($results); + } + private function stringifyMessageStatusForUser($conversation, $count): string { if ($conversation['isClosed']) { @@ -72,14 +67,6 @@ private function stringifyMessageStatusForUser($conversation, $count): string return self::STATUS_OPEN; } - private function getUnreadMessageCountForUser($conversation): int - { - $messageRepository = $this->getRepo('Message'); - assert($messageRepository instanceof MessageRepo); - $results = $messageRepository->getUnreadMessagesByConversationIdAndUserId($conversation['id'], $this->getUser()->getId()); - return count($results); - } - private function getLatestMessageMetadata($conversationId): array { $messageRepository = $this->getRepo('Message'); @@ -128,4 +115,11 @@ private function orderResultPrioritisingNewMessages($conversationList): array return array_merge($carry, $statusGroups[$status]); }, array()); } + + private function getRepository(): ConversationRepo + { + $conversationRepository = $this->getRepo('Conversation'); + assert($conversationRepository instanceof ConversationRepo); + return $conversationRepository; + } } diff --git a/test/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByApplicationToLicenceTest.php b/test/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByApplicationToLicenceTest.php new file mode 100644 index 0000000000..232b108c22 --- /dev/null +++ b/test/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByApplicationToLicenceTest.php @@ -0,0 +1,48 @@ +sut = new Handler(); + $this->mockRepo('Application', Repository\Application::class); + + parent::setUp(); + } + + public function testHandleQuery() + { + $query = Qry::create([ + 'application' => 1, + ]); + + $mockLicence = m::mock(Licence::class); + $mockLicence->shouldReceive('getId')->once()->andReturn(2); + $mockApplication = m::mock(Application::class); + $mockApplication->shouldReceive('getLicence')->once()->andReturn($mockLicence); + + $this->repoMap['Application']->shouldReceive('fetchById')->andReturn($mockApplication); + + $this->queryHandler->shouldReceive('handleQuery')->with(m::on( + function ($argument) { + $this->assertInstanceOf(ByLicence::class, $argument); + assert($argument instanceof ByLicence); + $this->assertEquals(2, $argument->getLicence(), 'Expected licence ID used in proxy call to ByLicence to match licence returned from application'); + return true; + } + ))->once(); + + $this->sut->handleQuery($query); + } +} diff --git a/test/module/Api/src/Domain/QueryHandler/Messaging/ConversationListTest.php b/test/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByLicenceTest.php similarity index 54% rename from test/module/Api/src/Domain/QueryHandler/Messaging/ConversationListTest.php rename to test/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByLicenceTest.php index 40c14eaa99..25a679fd37 100644 --- a/test/module/Api/src/Domain/QueryHandler/Messaging/ConversationListTest.php +++ b/test/module/Api/src/Domain/QueryHandler/Messaging/Conversations/ByLicenceTest.php @@ -4,19 +4,19 @@ use ArrayIterator; use Doctrine\ORM\QueryBuilder; -use Dvsa\Olcs\Api\Domain\QueryHandler\Messaging\ConversationList; +use Dvsa\Olcs\Api\Domain\QueryHandler\Messaging\Conversations\ByLicence; use Dvsa\Olcs\Api\Domain\Repository; use Dvsa\Olcs\Api\Entity\User\Permission; -use Dvsa\Olcs\Transfer\Query\Messaging\GetConversationList as Qry; +use Dvsa\Olcs\Transfer\Query\Messaging\Conversations\ByLicence as Qry; use Dvsa\OlcsTest\Api\Domain\QueryHandler\QueryHandlerTestCase; -use Mockery as m; use LmcRbacMvc\Service\AuthorizationService; +use Mockery as m; -class ConversationListTest extends QueryHandlerTestCase +class ByLicenceTest extends QueryHandlerTestCase { public function setUp(): void { - $this->sut = new ConversationList(); + $this->sut = new ByLicence(); $this->mockRepo('Conversation', Repository\Conversation::class); $this->mockRepo('Message', Repository\Message::class); @@ -29,70 +29,57 @@ public function setUp(): void public function testHandleQuery() { - $query = Qry::create([]); - - $conversations = new ArrayIterator([['id' => 1, 'isClosed' => false,],]); - $this->repoMap['Conversation']->shouldReceive('getBaseConversationListQuery')->andReturn(m::mock(QueryBuilder::class)); - $this->repoMap['Conversation']->shouldReceive('applyOrderByOpen')->once(); - $this->repoMap['Conversation']->shouldReceive('fetchPaginatedList')->once()->andReturn($conversations); - $this->repoMap['Conversation']->shouldReceive('fetchPaginatedCount')->once()->andReturn(0); - $this->repoMap['Message']->shouldReceive('getUnreadMessagesByConversationIdAndUserId')->andReturn([]); - $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->once()->andReturn($conversations[0]); - - $result = $this->sut->handleQuery($query); - - $this->assertArrayHasKey('result', $result); - $this->assertArrayHasKey('count', $result); - } - - public function testHandleQueryWithNoOrdering() - { - $query = Qry::create(['applyOpenMessageSorting' => false, 'applyNewMessageSorting' => false,]); + $query = Qry::create([ + 'licence' => 1, + ]); - $conversations = new ArrayIterator([['id' => 1, 'isClosed' => true,], ['id' => 2, 'isClosed' => false,],]); - $this->repoMap['Conversation']->shouldReceive('getBaseConversationListQuery')->andReturn(m::mock(QueryBuilder::class)); - $this->repoMap['Conversation']->shouldReceive('applyOrderByOpen')->never(); + $conversations = new ArrayIterator([['id' => 1, 'isClosed' => false,],['id' => 2, 'isClosed' => true,],]); + $mockQb = m::mock(QueryBuilder::class); + $this->repoMap['Conversation']->shouldReceive('getBaseConversationListQuery')->andReturn($mockQb); + $this->repoMap['Conversation']->shouldReceive('filterByLicenceId')->once()->with($mockQb, $query->getLicence())->andReturn($mockQb); + $this->repoMap['Conversation']->shouldReceive('applyOrderByOpen')->once()->with($mockQb)->andReturn($mockQb); $this->repoMap['Conversation']->shouldReceive('fetchPaginatedList')->once()->andReturn($conversations); $this->repoMap['Conversation']->shouldReceive('fetchPaginatedCount')->once()->andReturn(0); - $this->repoMap['Conversation']->shouldReceive('getLatestMessageMetadata')->with(1)->andReturn(['createdOn' => '2023-11-06T12:16:12+0000']); - $this->repoMap['Conversation']->shouldReceive('getLatestMessageMetadata')->with(2)->andReturn(['createdOn' => '2023-11-06T12:17:12+0000']); $this->repoMap['Message']->shouldReceive('getUnreadMessagesByConversationIdAndUserId')->andReturn([]); - $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->times(2)->andReturn($conversations[0], $conversations[1]); + $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->twice()->andReturn($conversations[0]); $result = $this->sut->handleQuery($query); $this->assertArrayHasKey('result', $result); $this->assertArrayHasKey('count', $result); - $this->assertEquals(1, $result['result'][0]['id']); - $this->assertEquals(2, $result['result'][1]['id']); + $this->assertCount(count($conversations), $result['result']); } - public function testHandleQueryWithMessageStatusOrdering() + public function testHandleConversationOrdering() { - $query = Qry::create(['applyOpenMessageSorting' => false, 'applyNewMessageSorting' => true,]); + $query = Qry::create([ + 'licence' => 1, + ]); - $conversations = new ArrayIterator([['id' => 1, 'isClosed' => true,], ['id' => 2, 'isClosed' => false,],['id' => 3, 'isClosed' => false,], ['id' => 4, 'isClosed' => false,],]); + $conversations = new ArrayIterator([$conversation1 = ['id' => 1, 'isClosed' => true,], $conversation2 = ['id' => 2, 'isClosed' => false,], $conversation3 = ['id' => 3, 'isClosed' => false,], $conversation4 = ['id' => 4, 'isClosed' => false,],]); $mockQb = m::mock(QueryBuilder::class); $this->repoMap['Conversation']->shouldReceive('getBaseConversationListQuery')->andReturn($mockQb); - $this->repoMap['Conversation']->shouldReceive('applyOrderByOpen')->never()->with($mockQb); + $this->repoMap['Conversation']->shouldReceive('filterByLicenceId')->once()->with($mockQb, $query->getLicence())->andReturn($mockQb); + $this->repoMap['Conversation']->shouldReceive('applyOrderByOpen')->once()->with($mockQb)->andReturn($mockQb); $this->repoMap['Conversation']->shouldReceive('fetchPaginatedList')->once()->andReturn($conversations); $this->repoMap['Conversation']->shouldReceive('fetchPaginatedCount')->once()->andReturn(0); $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->once()->with(1)->andReturn(['createdOn' => '2023-11-06T12:17:12+0000']); - $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->once()->with(2)->andReturn(['createdOn' => '2023-11-06T12:17:12+0000']); - $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->once()->with(3)->andReturn(['createdOn' => '2023-11-06T12:17:12+0000']); - $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->once()->with(4)->andReturn(['createdOn' => '2023-11-06T12:10:12+0000']); + $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->once()->with(2)->andReturn(['createdOn' => '2023-11-06T12:52:12+0000']); + $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->once()->with(3)->andReturn(['createdOn' => '2023-11-06T12:10:12+0000']); + $this->repoMap['Message']->shouldReceive('getLastMessageByConversationId')->once()->with(4)->andReturn(['createdOn' => '2023-11-06T12:30:12+0000']); $this->repoMap['Message']->shouldReceive('getUnreadMessagesByConversationIdAndUserId')->once()->with(1, 1)->andReturn([]); - $this->repoMap['Message']->shouldReceive('getUnreadMessagesByConversationIdAndUserId')->once()->with(2, 1)->andReturn([['id' => 3, 'createdOn' => '2023-11-06T12:17:12+0000'],]); - $this->repoMap['Message']->shouldReceive('getUnreadMessagesByConversationIdAndUserId')->once()->with(3, 1)->andReturn([['id' => 4, 'createdOn' => '2023-11-06T12:17:12+0000'],]); + $this->repoMap['Message']->shouldReceive('getUnreadMessagesByConversationIdAndUserId')->once()->with(2, 1)->andReturn([]); + $this->repoMap['Message']->shouldReceive('getUnreadMessagesByConversationIdAndUserId')->once()->with(3, 1)->andReturn([['id' => 4, 'createdOn' => '2023-11-06T12:10:12+0000'],]); $this->repoMap['Message']->shouldReceive('getUnreadMessagesByConversationIdAndUserId')->once()->with(4, 1)->andReturn([]); $result = $this->sut->handleQuery($query); $this->assertArrayHasKey('result', $result); $this->assertArrayHasKey('count', $result); - $this->assertEquals(2, $result['result'][0]['id']); - $this->assertEquals(3, $result['result'][1]['id']); - $this->assertEquals(4, $result['result'][2]['id']); - $this->assertEquals(1, $result['result'][3]['id']); + $this->assertCount(count($conversations), $result['result']); + $this->assertEquals($conversation3['id'], $result['result'][0]['id']); + $this->assertEquals($conversation2['id'], $result['result'][1]['id']); + $this->assertEquals($conversation4['id'], $result['result'][2]['id']); + $this->assertEquals($conversation1['id'], $result['result'][3]['id']); } }