From 3ae13a2302e735a0e999ae42e8256d411cf55fcb Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Mon, 1 Jul 2024 02:22:57 +0200 Subject: [PATCH] Return approval state in API response --- src/EntryPoints/REST/ApprovePageApi.php | 26 ++++++++-- src/EntryPoints/REST/UnapprovePageApi.php | 26 ++++++++-- src/PageApprovals.php | 6 ++- tests/EntryPoints/REST/ApprovePageApiTest.php | 22 ++++++-- .../EntryPoints/REST/UnapprovePageApiTest.php | 51 ++++++++++++++----- 5 files changed, 103 insertions(+), 28 deletions(-) diff --git a/src/EntryPoints/REST/ApprovePageApi.php b/src/EntryPoints/REST/ApprovePageApi.php index 57a4473..f85a76e 100644 --- a/src/EntryPoints/REST/ApprovePageApi.php +++ b/src/EntryPoints/REST/ApprovePageApi.php @@ -8,9 +8,11 @@ use MediaWiki\Rest\Response; use MediaWiki\Rest\SimpleHandler; use MediaWiki\Revision\RevisionLookup; +use MediaWiki\User\UserIdentityLookup; use ProfessionalWiki\PageApprovals\Adapters\PageHtmlRetriever; use ProfessionalWiki\PageApprovals\Application\ApprovalAuthorizer; use ProfessionalWiki\PageApprovals\Application\ApprovalLog; +use ProfessionalWiki\PageApprovals\Application\ApprovalState; use ProfessionalWiki\PageApprovals\Application\HtmlRepository; use Wikimedia\ParamValidator\ParamValidator; use WikiPage; @@ -23,7 +25,8 @@ public function __construct( private HtmlRepository $htmlRepository, private PageHtmlRetriever $pageHtmlRetriever, private WikiPageFactory $wikiPageFactory, - private RevisionLookup $revisionLookup + private RevisionLookup $revisionLookup, + private UserIdentityLookup $userIdentityLookup ) { } @@ -49,7 +52,7 @@ public function run( int $revisionId ): Response { $this->htmlRepository->saveApprovedHtml( $page->getId(), $html ); } - return $this->newSuccessResponse(); + return $this->newSuccessResponse( $this->approvalLog->getApprovalState( $page->getId() ) ); } private function getPageFromRevisionId( int $revisionId ): ?WikiPage { @@ -66,8 +69,23 @@ private function revisionIsLatest( int $revisionId, WikiPage $page ): bool { return $revisionId === $page->getRevisionRecord()?->getId(); } - public function newSuccessResponse(): Response { - return $this->getResponseFactory()->createNoContent(); + public function newSuccessResponse( ?ApprovalState $state ): Response { + if ( $state === null ) { + return $this->getResponseFactory()->createNoContent(); + } + + return $this->getResponseFactory()->createJson( [ + 'approvalTimestamp' => $state->approvalTimestamp, + 'approver' => $this->getUserNameFromUserId( $state->approverId ), + ] ); + } + + private function getUserNameFromUserId( ?int $userId ): ?string { + if ( $userId === null ) { + return null; + } + + return $this->userIdentityLookup->getUserIdentityByUserId( $userId )?->getName(); } public function newAuthorizationFailedResponse(): Response { diff --git a/src/EntryPoints/REST/UnapprovePageApi.php b/src/EntryPoints/REST/UnapprovePageApi.php index 59cd474..c2a26e9 100644 --- a/src/EntryPoints/REST/UnapprovePageApi.php +++ b/src/EntryPoints/REST/UnapprovePageApi.php @@ -8,8 +8,10 @@ use MediaWiki\Rest\Response; use MediaWiki\Rest\SimpleHandler; use MediaWiki\Revision\RevisionLookup; +use MediaWiki\User\UserIdentityLookup; use ProfessionalWiki\PageApprovals\Application\ApprovalAuthorizer; use ProfessionalWiki\PageApprovals\Application\ApprovalLog; +use ProfessionalWiki\PageApprovals\Application\ApprovalState; use Wikimedia\ParamValidator\ParamValidator; use WikiPage; @@ -19,7 +21,8 @@ public function __construct( private ApprovalAuthorizer $authorizer, private ApprovalLog $approvalLog, private WikiPageFactory $wikiPageFactory, - private RevisionLookup $revisionLookup + private RevisionLookup $revisionLookup, + private UserIdentityLookup $userIdentityLookup ) { } @@ -40,7 +43,7 @@ public function run( int $revisionId ): Response { $this->approvalLog->unapprovePage( $page->getId(), $this->getAuthority()->getUser()->getId() ); - return $this->newSuccessResponse(); + return $this->newSuccessResponse( $this->approvalLog->getApprovalState( $page->getId() ) ); } private function getPageFromRevisionId( int $revisionId ): ?WikiPage { @@ -57,8 +60,23 @@ private function revisionIsLatest( int $revisionId, WikiPage $page ): bool { return $revisionId === $page->getRevisionRecord()?->getId(); } - public function newSuccessResponse(): Response { - return $this->getResponseFactory()->createNoContent(); + public function newSuccessResponse( ?ApprovalState $state ): Response { + if ( $state === null ) { + return $this->getResponseFactory()->createNoContent(); + } + + return $this->getResponseFactory()->createJson( [ + 'approvalTimestamp' => $state->approvalTimestamp, + 'approver' => $this->getUserNameFromUserId( $state->approverId ), + ] ); + } + + private function getUserNameFromUserId( ?int $userId ): ?string { + if ( $userId === null ) { + return null; + } + + return $this->userIdentityLookup->getUserIdentityByUserId( $userId )?->getName(); } public function newAuthorizationFailedResponse(): Response { diff --git a/src/PageApprovals.php b/src/PageApprovals.php index 5b25ceb..cc036ff 100644 --- a/src/PageApprovals.php +++ b/src/PageApprovals.php @@ -39,7 +39,8 @@ public static function newApprovePageApi(): ApprovePageApi { self::getInstance()->getHtmlRepository(), self::getInstance()->getPageHtmlRetriever(), MediaWikiServices::getInstance()->getWikiPageFactory(), - MediaWikiServices::getInstance()->getRevisionLookup() + MediaWikiServices::getInstance()->getRevisionLookup(), + MediaWikiServices::getInstance()->getUserIdentityLookup() ); } @@ -48,7 +49,8 @@ public static function newUnapprovePageApi(): UnapprovePageApi { self::getInstance()->newPageApprovalAuthorizer(), self::getInstance()->getApprovalLog(), MediaWikiServices::getInstance()->getWikiPageFactory(), - MediaWikiServices::getInstance()->getRevisionLookup() + MediaWikiServices::getInstance()->getRevisionLookup(), + MediaWikiServices::getInstance()->getUserIdentityLookup() ); } diff --git a/tests/EntryPoints/REST/ApprovePageApiTest.php b/tests/EntryPoints/REST/ApprovePageApiTest.php index 2f0dd2d..25bee1b 100644 --- a/tests/EntryPoints/REST/ApprovePageApiTest.php +++ b/tests/EntryPoints/REST/ApprovePageApiTest.php @@ -33,12 +33,24 @@ protected function setUp(): void { } public function testApprovePageHappyPath(): void { + $user = $this->getTestUser(); + $page = $this->createPageWithCategories()->getRevisionRecord(); + $response = $this->executeHandler( $this->newApprovePageApi(), - $this->createValidRequestData( $this->createPageWithCategories()->getRevisionRecord()->getId() ), + $this->createValidRequestData( $page->getId() ), + authority: $user->getAuthority() ); + $state = $this->approvalLog->getApprovalState( $page->getId() ); - $this->assertSame( 204, $response->getStatusCode() ); + $this->assertSame( 200, $response->getStatusCode() ); + $this->assertSame( + [ + 'approvalTimestamp' => $state->approvalTimestamp, + 'approver' => $user->getUser()->getName(), + ], + json_decode( $response->getBody()->getContents(), true ) + ); } private function newApprovePageApi(): ApprovePageApi { @@ -48,7 +60,8 @@ private function newApprovePageApi(): ApprovePageApi { $this->htmlRepository, $this->newPageHtmlRetriever(), $this->getServiceContainer()->getWikiPageFactory(), - $this->getServiceContainer()->getRevisionLookup() + $this->getServiceContainer()->getRevisionLookup(), + $this->getServiceContainer()->getUserIdentityLookup() ); } @@ -84,7 +97,8 @@ private function newApprovePageApiWithFailingAuthorizer(): ApprovePageApi { $this->htmlRepository, $this->newPageHtmlRetriever(), $this->getServiceContainer()->getWikiPageFactory(), - $this->getServiceContainer()->getRevisionLookup() + $this->getServiceContainer()->getRevisionLookup(), + $this->getServiceContainer()->getUserIdentityLookup() ); } diff --git a/tests/EntryPoints/REST/UnapprovePageApiTest.php b/tests/EntryPoints/REST/UnapprovePageApiTest.php index 7cab74d..b0118a3 100644 --- a/tests/EntryPoints/REST/UnapprovePageApiTest.php +++ b/tests/EntryPoints/REST/UnapprovePageApiTest.php @@ -20,21 +20,45 @@ class UnapprovePageApiTest extends PageApprovalsIntegrationTest { use HandlerTestTrait; use MockAuthorityTrait; + private InMemoryApprovalLog $approvalLog; + + protected function setUp(): void { + parent::setUp(); + + $this->approvalLog = new InMemoryApprovalLog(); + } + public function testUnapprovePageHappyPath(): void { + $user = $this->getTestUser(); + $page = $this->createPageWithCategories(); + + $this->approvalLog->approvePage( $page->getId(), 1 ); + $response = $this->executeHandler( $this->newUnapprovePageApi(), - $this->createValidRequestData( $this->createPageWithCategories()->getRevisionRecord()->getId() ), + $this->createValidRequestData( $page->getRevisionRecord()->getId() ), + authority: $user->getAuthority() ); - $this->assertSame( 204, $response->getStatusCode() ); + $state = $this->approvalLog->getApprovalState( $page->getId() ); + + $this->assertSame( 200, $response->getStatusCode() ); + $this->assertSame( + [ + 'approvalTimestamp' => $state->approvalTimestamp, + 'approver' => $user->getUser()->getName(), + ], + json_decode( $response->getBody()->getContents(), true ) + ); } - private function newUnapprovePageApi( ?ApprovalLog $approvalLog = null ): UnapprovePageApi { + private function newUnapprovePageApi(): UnapprovePageApi { return new UnapprovePageApi( new SucceedingApprovalAuthorizer(), - $approvalLog ?? new InMemoryApprovalLog(), + $this->approvalLog, $this->getServiceContainer()->getWikiPageFactory(), - $this->getServiceContainer()->getRevisionLookup() + $this->getServiceContainer()->getRevisionLookup(), + $this->getServiceContainer()->getUserIdentityLookup() ); } @@ -64,7 +88,8 @@ private function newUnapprovePageApiWithFailingAuthorizer( ?ApprovalLog $approva new FailingApprovalAuthorizer(), $approvalLog ?? new InMemoryApprovalLog(), $this->getServiceContainer()->getWikiPageFactory(), - $this->getServiceContainer()->getRevisionLookup() + $this->getServiceContainer()->getRevisionLookup(), + $this->getServiceContainer()->getUserIdentityLookup() ); } @@ -78,37 +103,35 @@ public function testUnapprovalFailsForMissingPageId(): void { } public function testPageIsUnapproved(): void { - $approvalLog = new InMemoryApprovalLog(); $page = $this->createPageWithCategories(); - $approvalLog->approvePage( $page->getId(), 1 ); + $this->approvalLog->approvePage( $page->getId(), 1 ); $this->executeHandler( - $this->newUnapprovePageApi( $approvalLog ), + $this->newUnapprovePageApi(), $this->createValidRequestData( $page->getRevisionRecord()->getId() ), ); $this->assertFalse( - $approvalLog->getApprovalState( $page->getId() )->isApproved + $this->approvalLog->getApprovalState( $page->getId() )->isApproved ); } public function testAPIUserIsInApprovalState(): void { - $approvalLog = new InMemoryApprovalLog(); $page = $this->createPageWithCategories(); $user = $this->mockRegisteredUltimateAuthority(); - $approvalLog->approvePage( $page->getId(), 1 ); + $this->approvalLog->approvePage( $page->getId(), 1 ); $this->executeHandler( - $this->newUnapprovePageApi( $approvalLog ), + $this->newUnapprovePageApi(), $this->createValidRequestData( $page->getRevisionRecord()->getId() ), authority: $user ); $this->assertSame( $user->getUser()->getId(), - $approvalLog->getApprovalState( $page->getId() )->approverId + $this->approvalLog->getApprovalState( $page->getId() )->approverId ); }