Drop wgMinervaPageActions

Remove code per spike outcome (T226490#7438685)

- This configuration option is never used in Wikimedia production
- isTalkAllowed content body is merged into isAllowed. Resulting code
is more readable.
- isAllowed('talk') still returns false on main page for anonymous users
which is the current behavior. Even on sv.m.wikipedia.org where talk pages
are shown to anonymous users, we do not show the talk page on the main page.
Add a comment pointing to the associated phabricator ticket

Bug: T226490
Change-Id: I9f34817c5ad654a6a66fe6156850a3e3fee881a7
This commit is contained in:
jdlrobson 2021-10-18 14:04:03 -07:00 committed by Jdlrobson
parent d138fa5cdb
commit 5a7a9c9c31
5 changed files with 31 additions and 47 deletions

View file

@ -67,15 +67,6 @@ When enabled and hacks.less exists, hacks.less workarounds are included in style
When enabled a donate link will be added to the main menu. The donate link uses the `sitesupport` and `sitesupport-url` mediawiki messages.
#### $wgMinervaPageActions
* Type: `Array`
* Default: `['edit', 'talk', 'watch', 'switch-language']`
Controls which page actions, if any, are displayed. Allowed: `edit`, `watch`, `talk`, and
`switch-language`.
#### $wgMinervaPageIssuesNewTreatment
* Type: `Array`

View file

@ -35,7 +35,6 @@ use User;
* A wrapper for all available Minerva permissions.
*/
final class MinervaPagePermissions implements IMinervaPagePermissions {
/**
* @var Title Current page title
*/
@ -117,10 +116,6 @@ final class MinervaPagePermissions implements IMinervaPagePermissions {
*
* Actions isn't allowed when:
* <ul>
* <li>
* the action is disabled (by removing it from the <code>MinervaPageActions</code>
* configuration variable; or
* </li>
* <li>the user is on the main page</li>
* </ul>
*
@ -143,13 +138,20 @@ final class MinervaPagePermissions implements IMinervaPagePermissions {
// T206406: Enable "Talk" or "Discussion" button on Main page, also, not forgetting
// the "switch-language" button. But disable "edit" and "watch" actions.
if ( $this->title->isMainPage() ) {
if ( !in_array( $action, $this->config->get( 'MinervaPageActions' ) ) ) {
return false;
}
if ( $action === self::SWITCH_LANGUAGE ) {
return !$wgHideInterlanguageLinks;
}
return $action === self::TALK;
// Only the talk page is allowed on the main page provided user is registered.
// talk page permission is disabled on mobile for anons
// https://phabricator.wikimedia.org/T54165
return $action === self::TALK && $this->user->isRegistered();
}
if ( $action === self::TALK ) {
return (
$this->title->isTalkPage() ||
$this->title->canHaveTalkPage()
);
}
if ( $action === self::HISTORY && $this->title->exists() ) {
@ -164,10 +166,6 @@ final class MinervaPagePermissions implements IMinervaPagePermissions {
return $this->canEditOrCreate();
}
if ( !in_array( $action, $this->config->get( 'MinervaPageActions' ) ) ) {
return false;
}
if ( $action === self::CONTENT_EDIT ) {
return $this->isCurrentPageContentModelEditable();
}
@ -198,20 +196,15 @@ final class MinervaPagePermissions implements IMinervaPagePermissions {
return $this->canProtect();
}
return true;
// Unknown action has been passed.
return false;
}
/**
* @inheritDoc
*/
public function isTalkAllowed() {
if ( !$this->title ) {
return false;
}
return $this->isAllowed( self::TALK ) &&
( $this->title->isTalkPage() || $this->title->canHaveTalkPage() ) &&
$this->user->isRegistered();
return $this->isAllowed( self::TALK );
}
/**

View file

@ -660,6 +660,10 @@ class SkinMinerva extends SkinMustache {
$subjectPage->isMainPage();
if ( !$this->getUserPageHelper()->isUserPage() &&
$this->getPermissions()->isTalkAllowed() && $talkAtBottom &&
// When showing talk at the bottom we restrict this so it is not shown to anons
// https://phabricator.wikimedia.org/T54165
// This whole code block can be removed when SkinOptions::TALK_AT_TOP is always true
$this->getUser()->isRegistered() &&
!$this->isTalkPageWithViewAction()
) {
$namespaces = $tpl->data['content_navigation']['namespaces'];

View file

@ -41,17 +41,6 @@
"MinervaApplyKnownTemplateHacks": {
"value": true
},
"MinervaPageActions": {
"value": [
"edit",
"talk",
"watch",
"switch-language",
"move",
"delete",
"protect"
]
},
"MinervaAlwaysShowLanguageButton": {
"value": true
},

View file

@ -23,7 +23,7 @@ class MinervaPagePermissionsTest extends MediaWikiIntegrationTestCase {
private function buildPermissionsObject(
Title $title,
array $actions = null,
$actions = null, /* unused */
array $options = [],
ContentHandler $contentHandler = null,
User $user = null,
@ -56,7 +56,6 @@ class MinervaPagePermissionsTest extends MediaWikiIntegrationTestCase {
$context = new RequestContext();
$context->setTitle( $title );
$context->setConfig( new \HashConfig( [
'MinervaPageActions' => $actions,
'MinervaAlwaysShowLanguageButton' => $alwaysShowLanguageButton
] ) );
$context->setUser( $user );
@ -87,10 +86,19 @@ class MinervaPagePermissionsTest extends MediaWikiIntegrationTestCase {
* @covers ::isAllowed
*/
public function testWatchAndEditNotAllowedOnMainPage() {
$userMock = $this->getMockBuilder( User::class )
->disableOriginalConstructor()
->onlyMethods( [ 'isRegistered' ] )
->getMock();
$userMock->expects( $this->once() )
->method( 'isRegistered' )
->willReturn( false );
$permsAnon = $this->buildPermissionsObject( Title::newMainPage(), null, [], null, $userMock );
$perms = $this->buildPermissionsObject( Title::newMainPage() );
$this->assertFalse( $perms->isAllowed( IMinervaPagePermissions::WATCH ) );
$this->assertFalse( $perms->isAllowed( IMinervaPagePermissions::CONTENT_EDIT ) );
$this->assertFalse( $permsAnon->isAllowed( IMinervaPagePermissions::TALK ) );
// Check to make sure 'talk' and 'switch-language' are enabled on the Main page.
$this->assertTrue( $perms->isAllowed( IMinervaPagePermissions::TALK ) );
@ -103,9 +111,8 @@ class MinervaPagePermissionsTest extends MediaWikiIntegrationTestCase {
public function testInvalidPageActionsArentAllowed() {
$perms = $this->buildPermissionsObject( Title::newFromText( 'test' ), [] );
// By default, the "talk" and "watch" page actions are allowed but are now deemed invalid.
$this->assertFalse( $perms->isAllowed( IMinervaPagePermissions::TALK ) );
$this->assertFalse( $perms->isAllowed( IMinervaPagePermissions::WATCH ) );
$this->assertFalse( $perms->isAllowed( 'blah' ) );
$this->assertFalse( $perms->isAllowed( 'wah' ) );
}
/**
@ -283,7 +290,7 @@ class MinervaPagePermissionsTest extends MediaWikiIntegrationTestCase {
* @covers ::isAllowed
*/
public function testMoveAndDeleteAndProtectNotAllowedByDefault() {
$perms = $this->buildPermissionsObject( Title::newFromText( 'test' ), [] );
$perms = $this->buildPermissionsObject( Title::newFromText( 'test' ), null );
$this->assertFalse( $perms->isAllowed( IMinervaPagePermissions::MOVE ) );
$this->assertFalse( $perms->isAllowed( IMinervaPagePermissions::DELETE ) );
$this->assertFalse( $perms->isAllowed( IMinervaPagePermissions::PROTECT ) );