From c5b89ef21085eaad0e0e6c2b4f57eefb55e22480 Mon Sep 17 00:00:00 2001 From: waximabbax Date: Thu, 11 Jul 2024 07:17:33 +0500 Subject: [PATCH 1/2] Show UI conditionally --- src/Adapters/DatabaseApproverRepository.php | 16 ++++++++++++++++ src/Application/ApproverRepository.php | 5 +++++ .../UseCases/ApprovalUiQuery/ApprovalUiQuery.php | 14 +++++++++++--- src/PageApprovals.php | 3 ++- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/Adapters/DatabaseApproverRepository.php b/src/Adapters/DatabaseApproverRepository.php index 4cae841..11ed614 100644 --- a/src/Adapters/DatabaseApproverRepository.php +++ b/src/Adapters/DatabaseApproverRepository.php @@ -75,6 +75,22 @@ public function setApproverCategories( int $userId, array $categoryNames ): void ); } + public function getAllAssignedCategories(): array { + $result = $this->database->select( + 'approver_config', + [ 'ac_categories' ], + [], + __METHOD__ + ); + + $allCategories = []; + foreach ( $result as $row ) { + $allCategories = array_merge( $allCategories, $this->deserializeCategories( $row->ac_categories ) ); + } + + return array_unique( $allCategories ); + } + private function serializeCategories( array $categories ): string { return implode( '|', array_unique( array_map( fn ( string $category ) => $this->normalizeCategoryTitle( $category ), diff --git a/src/Application/ApproverRepository.php b/src/Application/ApproverRepository.php index 9daaa8a..9a9f202 100644 --- a/src/Application/ApproverRepository.php +++ b/src/Application/ApproverRepository.php @@ -16,6 +16,11 @@ public function getApproverCategories( int $userId ): array; */ public function getApproversWithCategories(): array; + /** + * @return string[] + */ + public function getAllAssignedCategories(): array; + /** * @param string[] $categoryNames */ diff --git a/src/Application/UseCases/ApprovalUiQuery/ApprovalUiQuery.php b/src/Application/UseCases/ApprovalUiQuery/ApprovalUiQuery.php index 1b44b6a..bd41c57 100644 --- a/src/Application/UseCases/ApprovalUiQuery/ApprovalUiQuery.php +++ b/src/Application/UseCases/ApprovalUiQuery/ApprovalUiQuery.php @@ -8,12 +8,14 @@ use ProfessionalWiki\PageApprovals\Application\ApprovalAuthorizer; use ProfessionalWiki\PageApprovals\Application\ApprovalLog; use ProfessionalWiki\PageApprovals\Application\ApprovalState; +use ProfessionalWiki\PageApprovals\Application\ApproverRepository; class ApprovalUiQuery { public function __construct( private readonly ApprovalLog $approvalLog, - private readonly ApprovalAuthorizer $approvalAuthorizer + private readonly ApprovalAuthorizer $approvalAuthorizer, + private readonly ApproverRepository $approverRepository ) { } @@ -35,12 +37,18 @@ private function getApprovalState( OutputPage $out, bool $showUi ): ?ApprovalSta if ( $showUi ) { return $this->approvalLog->getApprovalState( pageId: $out->getWikiPage()->getId() ); // TODO: test } - return null; } private function isApprovablePage( OutputPage $out ): bool { - return $out->isArticle() // TODO: test + $hasAssignedCategories = !empty( + array_intersect( + $out->getCategories(), + $this->approverRepository->getAllAssignedCategories() + ) + ); // TODO: test for case sensitive categories + return $hasAssignedCategories + && $out->isArticle() // TODO: test && $out->getRevisionId() !== null // Exclude non-existing pages // TODO: test && $out->isRevisionCurrent(); // TODO: test } diff --git a/src/PageApprovals.php b/src/PageApprovals.php index 430d6cd..03ef083 100644 --- a/src/PageApprovals.php +++ b/src/PageApprovals.php @@ -106,7 +106,8 @@ public function getApproverRepository(): ApproverRepository { public function newApprovalUiQuery(): ApprovalUiQuery { return new ApprovalUiQuery( approvalLog: $this->getApprovalLog(), - approvalAuthorizer: $this->newPageApprovalAuthorizer() + approvalAuthorizer: $this->newPageApprovalAuthorizer(), + approverRepository: $this->getApproverRepository() ); } From e6cbf04f4ae3aaa909a6cc35037a13177fdf8205 Mon Sep 17 00:00:00 2001 From: waximabbax Date: Thu, 11 Jul 2024 20:35:35 +0500 Subject: [PATCH 2/2] Adding Unit Tests --- phpstan-baseline.neon | 2 +- psalm-baseline.xml | 5 +- src/Adapters/DatabaseApproverRepository.php | 9 ++-- src/Adapters/InMemoryApproverRepository.php | 11 +++++ src/Application/ApproverRepository.php | 2 +- .../ApprovalUiQuery/ApprovalUiQuery.php | 17 ++++--- .../DatabaseApproverRepositoryTest.php | 12 +++++ .../UseCases/ApprovalUiQueryTest.php | 4 +- tests/EntryPoints/PageApprovalsHooksTest.php | 49 +++++++++++++------ 9 files changed, 83 insertions(+), 28 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index b2c5f34..537be6d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -32,7 +32,7 @@ parameters: - message: "#^Cannot access property \\$categories on array\\|bool\\|stdClass\\.$#" - count: 1 + count: 2 path: src/Adapters/DatabaseApproverRepository.php - diff --git a/psalm-baseline.xml b/psalm-baseline.xml index beec9f7..987d055 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + getCategories()]]> @@ -26,11 +26,14 @@ deserializeCategories( $row->ac_categories )]]> ]]> + + ac_categories]]> categories]]> + categories]]> userId]]> diff --git a/src/Adapters/DatabaseApproverRepository.php b/src/Adapters/DatabaseApproverRepository.php index 11ed614..6cbc195 100644 --- a/src/Adapters/DatabaseApproverRepository.php +++ b/src/Adapters/DatabaseApproverRepository.php @@ -75,17 +75,20 @@ public function setApproverCategories( int $userId, array $categoryNames ): void ); } - public function getAllAssignedCategories(): array { + /** + * @return string[] + */ + public function getAllCategories(): array { $result = $this->database->select( 'approver_config', - [ 'ac_categories' ], + [ 'ac_categories AS categories' ], [], __METHOD__ ); $allCategories = []; foreach ( $result as $row ) { - $allCategories = array_merge( $allCategories, $this->deserializeCategories( $row->ac_categories ) ); + $allCategories = array_merge( $allCategories, $this->deserializeCategories( (string)$row->categories ) ); } return array_unique( $allCategories ); diff --git a/src/Adapters/InMemoryApproverRepository.php b/src/Adapters/InMemoryApproverRepository.php index d101578..69bfb60 100644 --- a/src/Adapters/InMemoryApproverRepository.php +++ b/src/Adapters/InMemoryApproverRepository.php @@ -28,6 +28,17 @@ public function getApproversWithCategories(): array { return $this->approversCategories; } + /** + * @return string[] + */ + public function getAllCategories(): array { + $allCategories = []; + foreach ( $this->approversCategories as $categories ) { + $allCategories = array_merge( $allCategories, $categories ); + } + return array_values( array_unique( $allCategories ) ); + } + /** * @param string[] $categoryNames */ diff --git a/src/Application/ApproverRepository.php b/src/Application/ApproverRepository.php index 9a9f202..ba92a62 100644 --- a/src/Application/ApproverRepository.php +++ b/src/Application/ApproverRepository.php @@ -19,7 +19,7 @@ public function getApproversWithCategories(): array; /** * @return string[] */ - public function getAllAssignedCategories(): array; + public function getAllCategories(): array; /** * @param string[] $categoryNames diff --git a/src/Application/UseCases/ApprovalUiQuery/ApprovalUiQuery.php b/src/Application/UseCases/ApprovalUiQuery/ApprovalUiQuery.php index bd41c57..dc5dec5 100644 --- a/src/Application/UseCases/ApprovalUiQuery/ApprovalUiQuery.php +++ b/src/Application/UseCases/ApprovalUiQuery/ApprovalUiQuery.php @@ -41,16 +41,19 @@ private function getApprovalState( OutputPage $out, bool $showUi ): ?ApprovalSta } private function isApprovablePage( OutputPage $out ): bool { - $hasAssignedCategories = !empty( - array_intersect( - $out->getCategories(), - $this->approverRepository->getAllAssignedCategories() - ) - ); // TODO: test for case sensitive categories - return $hasAssignedCategories + return $this->pageHasApprovers( $out ) && $out->isArticle() // TODO: test && $out->getRevisionId() !== null // Exclude non-existing pages // TODO: test && $out->isRevisionCurrent(); // TODO: test } + private function pageHasApprovers( OutputPage $out ): bool { + return !empty( + array_intersect( + $out->getCategories(), + $this->approverRepository->getAllCategories() + ) + ); + } + } diff --git a/tests/Adapters/DatabaseApproverRepositoryTest.php b/tests/Adapters/DatabaseApproverRepositoryTest.php index df6e8f6..3496bc9 100644 --- a/tests/Adapters/DatabaseApproverRepositoryTest.php +++ b/tests/Adapters/DatabaseApproverRepositoryTest.php @@ -29,6 +29,18 @@ public function testGetApproverCategoriesReturnsEmptyArrayForNonexistentUser(): $this->assertSame( [], $repository->getApproverCategories( 404 ) ); } + public function testGetAllCategories(): void { + $repository = $this->newRepository(); + + $repository->setApproverCategories( 1, [ 'Category1', 'Category2' ] ); + + $this->assertSame( + [ 'Category1', 'Category2' ], + $repository->getAllCategories(), + 'getAllCategories should return all unique categories across all approvers' + ); + } + public function testSetAndGetApproverCategories(): void { $repository = $this->newRepository(); $userId = 1; diff --git a/tests/Application/UseCases/ApprovalUiQueryTest.php b/tests/Application/UseCases/ApprovalUiQueryTest.php index b9963f1..cda0e83 100644 --- a/tests/Application/UseCases/ApprovalUiQueryTest.php +++ b/tests/Application/UseCases/ApprovalUiQueryTest.php @@ -5,6 +5,7 @@ namespace ProfessionalWiki\PageApprovals\Tests\Application\UseCases; use ProfessionalWiki\PageApprovals\Adapters\InMemoryApprovalLog; +use ProfessionalWiki\PageApprovals\Adapters\InMemoryApproverRepository; use ProfessionalWiki\PageApprovals\Application\UseCases\ApprovalUiQuery\ApprovalUiQuery; use ProfessionalWiki\PageApprovals\Tests\PageApprovalsIntegrationTest; use ProfessionalWiki\PageApprovals\Tests\TestDoubles\SucceedingApprovalAuthorizer; @@ -39,7 +40,8 @@ public function testPageWithNoApprovals(): void { private function newApprovalUiQuery(): ApprovalUiQuery { return new ApprovalUiQuery( new InMemoryApprovalLog(), - new SucceedingApprovalAuthorizer() + new SucceedingApprovalAuthorizer(), + new InMemoryApproverRepository() ); } diff --git a/tests/EntryPoints/PageApprovalsHooksTest.php b/tests/EntryPoints/PageApprovalsHooksTest.php index 93a10a2..bd67c5b 100644 --- a/tests/EntryPoints/PageApprovalsHooksTest.php +++ b/tests/EntryPoints/PageApprovalsHooksTest.php @@ -8,7 +8,9 @@ use ProfessionalWiki\PageApprovals\EntryPoints\PageApprovalsHooks; use ProfessionalWiki\PageApprovals\Tests\PageApprovalsIntegrationTest; use RequestContext; -use Title; +use ProfessionalWiki\PageApprovals\PageApprovals; +use ParserOutput; +use WikiPage; /** * @group Database @@ -16,25 +18,44 @@ */ class PageApprovalsHooksTest extends PageApprovalsIntegrationTest { - public function testOnOutputPageBeforeHTML() { - $title = Title::newFromText( 'PageApprovalsIntegrationTest' ); - $this->insertPage( $title ); + private OutputPage $out; - $context = new RequestContext(); - $context->setTitle( $title ); - $context->setUser( $this->getTestUser()->getUser() ); + protected function setUp(): void { + parent::setUp(); + $this->tablesUsed[] = 'approver_config'; - $out = new OutputPage( $context ); - $out->setTitle( $title ); - $out->setArticleFlag( true ); - $out->setRevisionId( $this->getServiceContainer()->getWikiPageFactory()->newFromTitle( $title )->getLatest() ); + $page = $this->createPageWithText( 'Test page content' ); - PageApprovalsHooks::onOutputPageBeforeHTML( $out ); + $this->out = new OutputPage( new RequestContext() ); + $this->out->setTitle( $page->getTitle() ); + $this->out->setArticleFlag( true ); + $this->out->setRevisionId( $page->getLatest() ); + } + + public function testOnOutputPageBeforeHTMLUiIsNotShown() { + PageApprovalsHooks::onOutputPageBeforeHTML( $this->out ); + + $this->assertStringNotContainsString( + 'page-approval-container', + $this->out->getHTML(), + 'The page approval status should not be displayed without a matching category.' + ); + } + + public function testOnOutputPageBeforeHTMLUiIsShown() { + $parserOutput = new ParserOutput( 'Test page content' ); + $parserOutput->addCategory( 'ApprovalCategory', 'ApprovalCategory' ); + $this->out->addParserOutput( $parserOutput ); + + $approverRepository = PageApprovals::getInstance()->getApproverRepository(); + $approverRepository->setApproverCategories( 1, [ 'ApprovalCategory' ] ); + + PageApprovalsHooks::onOutputPageBeforeHTML( $this->out ); $this->assertStringContainsString( 'page-approval-container', - $out->getHTML(), - 'The page approval status should be displayed.' + $this->out->getHTML(), + 'The page approval status should be displayed with a matching category.' ); }