WMDE-leszek has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/363303 )
Change subject: Remove user permission logic from RedirectCreationInteractor
......................................................................
Remove user permission logic from RedirectCreationInteractor
Just ask for redirect permission and let EntityPermissionChecker
figure out what MW permissions are actually required.
Bug: T166586
Change-Id: Id75ea5934edc94aa5e371c824319b0cf5cb26369
---
M repo/includes/Interactors/RedirectCreationInteractor.php
M repo/tests/phpunit/includes/Api/CreateRedirectTest.php
M repo/tests/phpunit/includes/Interactors/RedirectCreationInteractorTest.php
M repo/tests/phpunit/includes/Specials/SpecialRedirectEntityTest.php
4 files changed, 19 insertions(+), 58 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/03/363303/1
diff --git a/repo/includes/Interactors/RedirectCreationInteractor.php
b/repo/includes/Interactors/RedirectCreationInteractor.php
index 3fe8528..cde809f 100644
--- a/repo/includes/Interactors/RedirectCreationInteractor.php
+++ b/repo/includes/Interactors/RedirectCreationInteractor.php
@@ -117,33 +117,18 @@
}
/**
- * Check all applicable permissions for redirecting the given $entityId.
+ * Check user's permissions for the given entity ID.
*
* @param EntityId $entityId
- *
- * @throws RedirectCreationException if a permission check fails
- */
- private function checkPermissions( EntityId $entityId ) {
- $permissions = [
- 'edit',
- $entityId->getEntityType() . '-redirect'
- ];
-
- foreach ( $permissions as $permission ) {
- $this->checkPermission( $entityId, $permission );
- }
- }
-
- /**
- * Check the given permissions for the given $entityId.
- *
- * @param EntityId $entityId
- * @param string $permission
*
* @throws RedirectCreationException if the permission check fails
*/
- private function checkPermission( EntityId $entityId, $permission ) {
- $status =
$this->permissionChecker->getPermissionStatusForEntityId( $this->user,
$permission, $entityId );
+ private function checkPermissions( EntityId $entityId ) {
+ $status =
$this->permissionChecker->getPermissionStatusForEntityId(
+ $this->user,
+ EntityPermissionChecker::PERMISSION_REDIRECT,
+ $entityId
+ );
if ( !$status->isOK() ) {
// XXX: This is silly, we really want to pass the
Status object to the API error handler.
diff --git a/repo/tests/phpunit/includes/Api/CreateRedirectTest.php
b/repo/tests/phpunit/includes/Api/CreateRedirectTest.php
index 688e532..2430ada 100644
--- a/repo/tests/phpunit/includes/Api/CreateRedirectTest.php
+++ b/repo/tests/phpunit/includes/Api/CreateRedirectTest.php
@@ -80,8 +80,8 @@
$permissionChecker->expects( $this->any() )
->method( 'getPermissionStatusForEntityId' )
- ->will( $this->returnCallback( function( User $user,
$permission ) {
- if ( $user->getName() ===
'UserWithoutPermission' && $permission === 'edit' ) {
+ ->will( $this->returnCallback( function( User $user ) {
+ if ( $user->getName() ===
'UserWithoutPermission' ) {
return Status::newFatal(
'permissiondenied' );
} else {
return Status::newGood();
diff --git
a/repo/tests/phpunit/includes/Interactors/RedirectCreationInteractorTest.php
b/repo/tests/phpunit/includes/Interactors/RedirectCreationInteractorTest.php
index 5bc21e6..0b9edc3 100644
--- a/repo/tests/phpunit/includes/Interactors/RedirectCreationInteractorTest.php
+++ b/repo/tests/phpunit/includes/Interactors/RedirectCreationInteractorTest.php
@@ -69,13 +69,13 @@
/**
* @return EntityPermissionChecker
*/
- private function getPermissionCheckers() {
+ private function getPermissionChecker() {
$permissionChecker = $this->getMock(
EntityPermissionChecker::class );
$permissionChecker->expects( $this->any() )
->method( 'getPermissionStatusForEntityId' )
- ->will( $this->returnCallback( function( User $user,
$permission, EntityId $id ) {
- $userWithoutPermissionName =
'UserWithoutPermission-' . $permission;
+ ->will( $this->returnCallback( function( User $user ) {
+ $userWithoutPermissionName =
'UserWithoutPermission';
if ( $user->getName() ===
$userWithoutPermissionName ) {
return Status::newFatal(
'permissiondenied' );
@@ -137,7 +137,7 @@
$interactor = new RedirectCreationInteractor(
$this->mockRepository,
$this->mockRepository,
- $this->getPermissionCheckers(),
+ $this->getPermissionChecker(),
$summaryFormatter,
$user,
$this->getMockEditFilterHookRunner( $efHookCalls,
$efHookStatus ),
@@ -218,20 +218,10 @@
}
}
- public function permissionProvider() {
- return [
- 'edit' => [ 'edit' ],
- 'item-redirect' => [ 'item-redirect' ],
- ];
- }
-
- /**
- * @dataProvider permissionProvider
- */
- public function testSetRedirect_noPermission( $permission ) {
+ public function testSetRedirect_noPermission() {
$this->setExpectedException( RedirectCreationException::class );
- $user = User::newFromName( 'UserWithoutPermission-' .
$permission );
+ $user = User::newFromName( 'UserWithoutPermission' );
$interactor = $this->newInteractor( null, null, $user );
$interactor->createRedirect( new ItemId( 'Q11' ), new ItemId(
'Q12' ), false );
diff --git a/repo/tests/phpunit/includes/Specials/SpecialRedirectEntityTest.php
b/repo/tests/phpunit/includes/Specials/SpecialRedirectEntityTest.php
index e8e5328..aa6a167 100644
--- a/repo/tests/phpunit/includes/Specials/SpecialRedirectEntityTest.php
+++ b/repo/tests/phpunit/includes/Specials/SpecialRedirectEntityTest.php
@@ -189,8 +189,8 @@
$permissionChecker->expects( $this->any() )
->method( 'getPermissionStatusForEntityId' )
- ->will( $this->returnCallback( function( User $user,
$permission, EntityId $id ) {
- $name = 'UserWithoutPermission-' . $permission;
+ ->will( $this->returnCallback( function( User $user ) {
+ $name = 'UserWithoutPermission';
if ( $user->getName() === $name ) {
return Status::newFatal(
'permissiondenied' );
} else {
@@ -283,27 +283,13 @@
$this->assertError(
'Wikibase\Repo\Interactors\RedirectCreationException:no-such-entity', $html );
}
- public function permissionProvider() {
- return [
- 'edit' => [ 'edit' ],
- 'item-redirect' => [ 'item-redirect' ],
- ];
- }
-
- /**
- * @dataProvider permissionProvider
- */
- public function testNoPermission( $permission ) {
+ public function testNoPermission() {
$params = [
'fromid' => 'Q1',
'toid' => 'Q2'
];
- $this->setMwGlobals( 'wgGroupPermissions', [ '*' => [
- 'item-redirect' => $permission !== 'item-redirect',
- 'edit' => $permission !== 'edit'
- ] ] );
- $user = User::newFromName( 'UserWithoutPermission-' .
$permission );
+ $user = User::newFromName( 'UserWithoutPermission' );
$html = $this->executeSpecialEntityRedirect( $params, $user );
$this->assertError(
'Wikibase\Repo\Interactors\RedirectCreationException:permissiondenied', $html );
--
To view, visit https://gerrit.wikimedia.org/r/363303
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id75ea5934edc94aa5e371c824319b0cf5cb26369
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: WMDE-leszek <leszek.mani...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
