WMDE-leszek has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/363304 )
Change subject: Remove user permission logic from ItemMergeInteractor
......................................................................
Remove user permission logic from ItemMergeInteractor
Just ask for merge permission and let EntityPermissionChecker
figure out what MW permissions are actually required.
Bug: T166586
Change-Id: I8038a4262c349ce6f32a58a1f761395650971bc7
---
M repo/includes/Interactors/ItemMergeInteractor.php
M repo/tests/phpunit/includes/Interactors/ItemMergeInteractorTest.php
M repo/tests/phpunit/includes/Specials/SpecialMergeItemsTest.php
3 files changed, 27 insertions(+), 59 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/04/363304/1
diff --git a/repo/includes/Interactors/ItemMergeInteractor.php
b/repo/includes/Interactors/ItemMergeInteractor.php
index bec8369..7a50dfc 100644
--- a/repo/includes/Interactors/ItemMergeInteractor.php
+++ b/repo/includes/Interactors/ItemMergeInteractor.php
@@ -90,33 +90,16 @@
}
/**
- * Check all applicable permissions for redirecting the given $entityId.
+ * Check user's for the given entity ID.
*
* @param EntityId $entityId
- */
- private function checkPermissions( EntityId $entityId ) {
- $permissions = [
- 'edit',
- $entityId->getEntityType() . '-merge'
- ];
-
- foreach ( $permissions as $permission ) {
- $this->checkPermission( $entityId, $permission );
- }
- }
-
- /**
- * Check the given permissions for the given $entityId.
- *
- * @param EntityId $entityId
- * @param string $permission
*
* @throws ItemMergeException if the permission check fails
*/
- private function checkPermission( EntityId $entityId, $permission ) {
+ private function checkPermissions( EntityId $entityId ) {
$status =
$this->permissionChecker->getPermissionStatusForEntityId(
$this->user,
- $permission,
+ EntityPermissionChecker::PERMISSION_MERGE,
$entityId
);
diff --git
a/repo/tests/phpunit/includes/Interactors/ItemMergeInteractorTest.php
b/repo/tests/phpunit/includes/Interactors/ItemMergeInteractorTest.php
index 2a7d5db..e05e16a 100644
--- a/repo/tests/phpunit/includes/Interactors/ItemMergeInteractorTest.php
+++ b/repo/tests/phpunit/includes/Interactors/ItemMergeInteractorTest.php
@@ -85,13 +85,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' );
@@ -143,13 +143,13 @@
$changeOpsFactory,
$this->mockRepository,
$this->mockRepository,
- $this->getPermissionCheckers(),
+ $this->getPermissionChecker(),
$summaryFormatter,
$user,
new RedirectCreationInteractor(
$this->mockRepository,
$this->mockRepository,
- $this->getPermissionCheckers(),
+ $this->getPermissionChecker(),
$summaryFormatter,
$user,
$this->getMockEditFilterHookRunner(),
@@ -442,20 +442,10 @@
}
}
- public function permissionProvider() {
- return [
- 'edit' => [ 'edit' ],
- 'item-merge' => [ 'item-merge' ],
- ];
- }
-
- /**
- * @dataProvider permissionProvider
- */
- public function testSetRedirect_noPermission( $permission ) {
+ public function testSetRedirect_noPermission() {
$this->setExpectedException( ItemMergeException::class );
- $user = User::newFromName( 'UserWithoutPermission-' .
$permission );
+ $user = User::newFromName( 'UserWithoutPermission' );
$fromId = new ItemId( 'Q1' );
$toId = new ItemId( 'Q2' );
diff --git a/repo/tests/phpunit/includes/Specials/SpecialMergeItemsTest.php
b/repo/tests/phpunit/includes/Specials/SpecialMergeItemsTest.php
index 65340bd..8a633ea 100644
--- a/repo/tests/phpunit/includes/Specials/SpecialMergeItemsTest.php
+++ b/repo/tests/phpunit/includes/Specials/SpecialMergeItemsTest.php
@@ -246,8 +246,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 {
@@ -441,35 +441,30 @@
$this->assertContains( '<p
class="error">(wikibase-itemmerge-redirect)</p>', $html );
}
- public function permissionProvider() {
- return [
- 'edit' => [ 'edit' ],
- 'item-merge' => [ 'item-merge' ],
- ];
- }
-
- /**
- * @dataProvider permissionProvider
- */
- public function testNoPermission( $permission ) {
+ public function testNoSpecialPagePermission() {
$params = [
'fromid' => 'Q1',
'toid' => 'Q2'
];
$this->setMwGlobals( 'wgGroupPermissions', [ '*' => [
- 'item-merge' => $permission !== 'item-merge',
- 'edit' => $permission !== 'edit'
+ 'item-merge' => false,
] ] );
- $user = User::newFromName( 'UserWithoutPermission-' .
$permission );
+ $this->setExpectedException( PermissionsError::class );
- if ( $permission === 'item-merge' ) {
- $this->setExpectedException( PermissionsError::class );
- }
+ $html = $this->executeSpecialMergeItems( $params,
$GLOBALS['wgUser'] );
+ }
+
+ public function testMergePermission() {
+ $params = [
+ 'fromid' => 'Q1',
+ 'toid' => 'Q2'
+ ];
+
+ $user = User::newFromName( 'UserWithoutPermission' );
+
$html = $this->executeSpecialMergeItems( $params, $user );
- if ( $permission === 'edit' ) {
- $this->assertError(
'Wikibase\Repo\Interactors\ItemMergeException:wikibase-itemmerge-permissiondenied',
$html );
- }
+ $this->assertError(
'Wikibase\Repo\Interactors\ItemMergeException:wikibase-itemmerge-permissiondenied',
$html );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/363304
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8038a4262c349ce6f32a58a1f761395650971bc7
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
