Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return approval state in API response #62

Merged
merged 1 commit into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions src/EntryPoints/REST/ApprovePageApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
) {
}

Expand All @@ -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 {
Expand All @@ -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 ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should return UserRealName and not the username since I am actually using user_real_name from user table and if the real name doesn't exist then perhaps maybe username. @malberts @JeroenDeDauw

] );
}

private function getUserNameFromUserId( ?int $userId ): ?string {
if ( $userId === null ) {
return null;
}

return $this->userIdentityLookup->getUserIdentityByUserId( $userId )?->getName();
}

public function newAuthorizationFailedResponse(): Response {
Expand Down
26 changes: 22 additions & 4 deletions src/EntryPoints/REST/UnapprovePageApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
) {
}

Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions src/PageApprovals.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}

Expand All @@ -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()
);
}

Expand Down
22 changes: 18 additions & 4 deletions tests/EntryPoints/REST/ApprovePageApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -48,7 +60,8 @@ private function newApprovePageApi(): ApprovePageApi {
$this->htmlRepository,
$this->newPageHtmlRetriever(),
$this->getServiceContainer()->getWikiPageFactory(),
$this->getServiceContainer()->getRevisionLookup()
$this->getServiceContainer()->getRevisionLookup(),
$this->getServiceContainer()->getUserIdentityLookup()
);
}

Expand Down Expand Up @@ -84,7 +97,8 @@ private function newApprovePageApiWithFailingAuthorizer(): ApprovePageApi {
$this->htmlRepository,
$this->newPageHtmlRetriever(),
$this->getServiceContainer()->getWikiPageFactory(),
$this->getServiceContainer()->getRevisionLookup()
$this->getServiceContainer()->getRevisionLookup(),
$this->getServiceContainer()->getUserIdentityLookup()
);
}

Expand Down
51 changes: 37 additions & 14 deletions tests/EntryPoints/REST/UnapprovePageApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}

Expand Down Expand Up @@ -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()
);
}

Expand All @@ -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
);
}

Expand Down
Loading