From aa64ed24e92aafa9c96586ad1cbc94384e433445 Mon Sep 17 00:00:00 2001 From: waximabbax Date: Tue, 6 Aug 2024 20:08:21 +0500 Subject: [PATCH 1/4] Fixing addition of multiple categories for multiple users --- .../Specials/SpecialManageApprovers.php | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/EntryPoints/Specials/SpecialManageApprovers.php b/src/EntryPoints/Specials/SpecialManageApprovers.php index 17a2339..a79f753 100644 --- a/src/EntryPoints/Specials/SpecialManageApprovers.php +++ b/src/EntryPoints/Specials/SpecialManageApprovers.php @@ -30,16 +30,16 @@ public function execute( $subPage ): void { $this->checkPermissions(); $this->checkReadOnly(); - $approversWithCategories = new GetApproversWithCategories( $this->approverRepository ); - $approversCategories = $approversWithCategories->getApproversWithCategories(); - $request = $this->getRequest(); if ( $request->wasPosted() ) { - $this->handlePostRequest( $request, $approversCategories ); + $this->handlePostRequest( $request ); $this->getOutput()->redirect( $this->getPageTitle()->getLocalURL() ); return; } + $approversWithCategories = new GetApproversWithCategories( $this->approverRepository ); + $approversCategories = $approversWithCategories->getApproversWithCategories(); + $this->renderHtml( $approversCategories ); $this->getOutput()->addModuleStyles( 'ext.pageApprovals.manageApprovers.styles' ); @@ -50,10 +50,7 @@ private function isAdmin(): bool { return in_array( 'sysop', $userGroups ); } - /** - * @param array $approversCategories - */ - private function handlePostRequest( WebRequest $request, array $approversCategories ): void { + private function handlePostRequest( WebRequest $request ): void { $action = $request->getText( 'action' ); $username = $request->getText( 'username' ); $category = $request->getText( 'category' ); @@ -65,17 +62,17 @@ private function handlePostRequest( WebRequest $request, array $approversCategor return; } - $userWithCategories = array_filter( $approversCategories, fn( $approver ) => $approver->username === $username - ); - $currentCategories = $userWithCategories[0]->categories ?? []; - - $this->processCategoryAction( $action, $category, $userId, $currentCategories ); + $this->processCategoryAction( $action, $category, $userId ); } /** - * @param string[] $currentCategories + * @param string $action + * @param string $category + * @param int $userId */ - private function processCategoryAction( string $action, string $category, int $userId, array $currentCategories ): void { + private function processCategoryAction( string $action, string $category, int $userId ): void { + $currentCategories = $this->approverRepository->getApproverCategories( $userId ); + switch ( $action ) { case 'add': $currentCategories[] = $category; @@ -107,6 +104,7 @@ private function renderHtml( array $approversCategories ): void { /** * @param array $approvers + * * @return array> */ private function approversToViewModel( array $approvers ): array { From 2cf220302ec2552be123610851cf947183409b3c Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Wed, 7 Aug 2024 11:48:23 +0200 Subject: [PATCH 2/4] Rename test class --- ...pproverCategoriesTest.php => SpecialManageApproversTest.php} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/EntryPoints/Specials/{SpecialApproverCategoriesTest.php => SpecialManageApproversTest.php} (97%) diff --git a/tests/EntryPoints/Specials/SpecialApproverCategoriesTest.php b/tests/EntryPoints/Specials/SpecialManageApproversTest.php similarity index 97% rename from tests/EntryPoints/Specials/SpecialApproverCategoriesTest.php rename to tests/EntryPoints/Specials/SpecialManageApproversTest.php index 2211c3c..566fb5d 100644 --- a/tests/EntryPoints/Specials/SpecialApproverCategoriesTest.php +++ b/tests/EntryPoints/Specials/SpecialManageApproversTest.php @@ -13,7 +13,7 @@ * @group Database * @covers \ProfessionalWiki\PageApprovals\EntryPoints\Specials\SpecialManageApprovers */ -class SpecialApproverCategoriesTest extends SpecialPageTestBase { +class SpecialManageApproversTest extends SpecialPageTestBase { protected function newSpecialPage(): SpecialManageApprovers { return new SpecialManageApprovers( From fd8c47801cf27c55b6d6205649e1b2cc026f54a9 Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Wed, 7 Aug 2024 11:54:32 +0200 Subject: [PATCH 3/4] Add failing test case --- .../Specials/SpecialManageApproversTest.php | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/tests/EntryPoints/Specials/SpecialManageApproversTest.php b/tests/EntryPoints/Specials/SpecialManageApproversTest.php index 566fb5d..c16a0cc 100644 --- a/tests/EntryPoints/Specials/SpecialManageApproversTest.php +++ b/tests/EntryPoints/Specials/SpecialManageApproversTest.php @@ -4,6 +4,7 @@ use FauxRequest; use PermissionsError; +use ProfessionalWiki\PageApprovals\Application\ApproverRepository; use ProfessionalWiki\PageApprovals\EntryPoints\Specials\SpecialManageApprovers; use ProfessionalWiki\PageApprovals\PageApprovals; use SpecialPageTestBase; @@ -15,9 +16,16 @@ */ class SpecialManageApproversTest extends SpecialPageTestBase { + private ApproverRepository $approverRepository; + + protected function setUp(): void { + parent::setUp(); + $this->approverRepository = PageApprovals::getInstance()->getApproverRepository(); + } + protected function newSpecialPage(): SpecialManageApprovers { return new SpecialManageApprovers( - PageApprovals::getInstance()->getApproverRepository(), + $this->approverRepository, $this->getServiceContainer()->getUserGroupManager(), $this->getServiceContainer()->getUserFactory() ); @@ -103,4 +111,46 @@ public function testAddAndDeleteCategoryAction(): void { $this->assertStringNotContainsString( 'TestCategory', $this->viewPage(), 'Category should be deleted' ); } + public function testCanAddAnotherCategoryWhenMultipleUsersHaveMultipleCategories(): void { + $user1 = self::getTestUser( [ 'Group1' ] )->getUser(); + $user2 = self::getTestUser( [ 'Group2' ] )->getUser(); + $user3 = self::getTestUser( [ 'Group3' ] )->getUser(); + + $this->approverRepository->setApproverCategories( + $user1->getId(), + [ 'TestCategory1', 'TestCategory2' ] + ); + $this->approverRepository->setApproverCategories( + $user2->getId(), + [ 'TestCategory2', 'TestCategory3' ] + ); + $this->approverRepository->setApproverCategories( + $user3->getId(), + [ 'TestCategory3' ] + ); + + $this->post( + request: [ + 'action' => 'add', + 'username' => $user3->getName(), + 'category' => 'TestCategory4' + ] + ); + + $this->assertSame( + [ 'TestCategory1', 'TestCategory2' ], + $this->approverRepository->getApproverCategories( $user1->getId() ) + ); + + $this->assertSame( + [ 'TestCategory2', 'TestCategory3' ], + $this->approverRepository->getApproverCategories( $user2->getId() ) + ); + + $this->assertSame( + [ 'TestCategory3', 'TestCategory4' ], + $this->approverRepository->getApproverCategories( $user3->getId() ) + ); + } + } From d88d6019f841c2523b13550733c2d8249d245edc Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Wed, 7 Aug 2024 18:00:19 +0200 Subject: [PATCH 4/4] Remove unnecessary comments --- src/EntryPoints/Specials/SpecialManageApprovers.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/EntryPoints/Specials/SpecialManageApprovers.php b/src/EntryPoints/Specials/SpecialManageApprovers.php index a79f753..5efc1d9 100644 --- a/src/EntryPoints/Specials/SpecialManageApprovers.php +++ b/src/EntryPoints/Specials/SpecialManageApprovers.php @@ -65,11 +65,6 @@ private function handlePostRequest( WebRequest $request ): void { $this->processCategoryAction( $action, $category, $userId ); } - /** - * @param string $action - * @param string $category - * @param int $userId - */ private function processCategoryAction( string $action, string $category, int $userId ): void { $currentCategories = $this->approverRepository->getApproverCategories( $userId ); @@ -104,7 +99,6 @@ private function renderHtml( array $approversCategories ): void { /** * @param array $approvers - * * @return array> */ private function approversToViewModel( array $approvers ): array {