Matthias Mullie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/228267
Change subject: Get rid of $wgFlowOccupyPages
......................................................................
Get rid of $wgFlowOccupyPages
We already didn’t need this anymore: every board is guaranteerd to have an
associated record in `page` already. We retroactively created missing pages
a while ago with maintenance/FlowUpdateWorkflowPageId.php
(I67ac1bc3def5cce143b7b08b5d4301fe741df09b). At that same time, we also
deployed changes to guarantee a new workflow could not be created without
an associated entry in `page`.
Since page/revision entries exist, we can read the content model from there
and no longer need $wgFlowOccupyPages.
Theoretically, there could be pages in $wgFlowOccupyPages where a topic was
never created. After this patch, such empty pages would then no longer be
recognized as Flow board. I think it’s unlikely there are any such pages
because:
- they were activated on request, so likely to be used
- we’ve been activating them using Special:EnableFlow for awhile now, which
creates a `page` entry
Meanwhile also completely removed a bit of code in DeletedContributionsQuery
hook, where we were trying to short-circuit when viewing a namespace that was
guaranteed not to have a Flow page, but that code was already not reliable
since $wgFlowOccupy* aren’t the only means of enabling Flow anymore
Bug: T105574
Change-Id: I27e8e45951bfe96453c2bd2424f643a343cb7e59
---
M Flow.php
M Hooks.php
M includes/Block/TopicList.php
M includes/Content/BoardContentHandler.php
M includes/TalkpageManager.php
M tests/browser/features/support/pages/flow_page.rb
M tests/browser/features/support/pages/new_wiki_page.rb
M tests/phpunit/TalkpageManagerTest.php
M tests/phpunit/api/ApiTestCase.php
9 files changed, 13 insertions(+), 73 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/67/228267/1
diff --git a/Flow.php b/Flow.php
index 52f49fe..5aafb01 100644
--- a/Flow.php
+++ b/Flow.php
@@ -273,9 +273,6 @@
// Maximum number of users that can be mentioned in one comment
$wgFlowMaxMentionCount = 100;
-// Pages to occupy is an array of normalised page names, e.g. array( 'User
talk:Zomg' ).
-$wgFlowOccupyPages = array();
-
// Namespaces to occupy is an array of NS_* constants, e.g. array(
NS_USER_TALK ).
$wgFlowOccupyNamespaces = array();
diff --git a/Hooks.php b/Hooks.php
index f194be5..abafd2c 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -50,17 +50,13 @@
*/
public static function getOccupationController() {
if ( self::$occupationController === null ) {
- global $wgFlowOccupyNamespaces,
- $wgFlowOccupyPages;
+ global $wgFlowOccupyNamespaces;
// NS_TOPIC is always occupied
$namespaces = $wgFlowOccupyNamespaces;
$namespaces[] = NS_TOPIC;
- self::$occupationController = new TalkpageManager(
- array_unique( $namespaces ),
- $wgFlowOccupyPages
- );
+ self::$occupationController = new TalkpageManager(
array_unique( $namespaces ) );
}
return self::$occupationController;
}
@@ -804,24 +800,6 @@
* @return bool
*/
public static function onDeletedContributionsQuery( &$data, $pager,
$offset, $limit, $descending ) {
- global $wgFlowOccupyNamespaces, $wgFlowOccupyPages;
-
- // Ignore when looking in a specific namespace where there is
no Flow
- if ( $pager->namespace !== '' ) {
- // Flow enabled on entire namespace(s)
- $namespaces = array_flip( $wgFlowOccupyNamespaces );
-
- // Flow enabled on specific pages - get those namespaces
- foreach ( $wgFlowOccupyPages as $page ) {
- $title = Title::newFromText( $page );
- $namespaces[$title->getNamespace()] = 1;
- }
-
- if ( !isset( $namespaces[$pager->namespace] ) ) {
- return true;
- }
- }
-
set_error_handler( new Flow\RecoverableErrorHandler, -1 );
try {
// Contributions may be on pages outside the set of
currently
diff --git a/includes/Block/TopicList.php b/includes/Block/TopicList.php
index f6ad034..e9f51aa 100644
--- a/includes/Block/TopicList.php
+++ b/includes/Block/TopicList.php
@@ -161,8 +161,8 @@
* Order of storage is important! We've been changing when we
stored
* workflow a couple of times. For now, it needs to be stored
first:
* * OccupationListener.php (workflow listener) must first
create the
- * board before NotificationListener.php (topic/post
listeners)
- * creates notifications (& mails) that link to the board
+ * Topic:Xyz page before NotificationListener.php (topic/post
+ * listeners) creates notifications (& mails) that link to it
* * ReferenceExtractor.php (run from ReferenceRecorder.php, a
post
* listener) needs to parse content with Parsoid & for that
it needs
* the board title. AbstractRevision::getContent() will
figure out
diff --git a/includes/Content/BoardContentHandler.php
b/includes/Content/BoardContentHandler.php
index ffa64ad..2a6c6dc 100644
--- a/includes/Content/BoardContentHandler.php
+++ b/includes/Content/BoardContentHandler.php
@@ -86,7 +86,7 @@
*
* @since 1.21
*
- * @return Content
+ * @return BoardContent
*/
public function makeEmptyContent() {
return new BoardContent;
diff --git a/includes/TalkpageManager.php b/includes/TalkpageManager.php
index c585592..74578e5 100644
--- a/includes/TalkpageManager.php
+++ b/includes/TalkpageManager.php
@@ -42,7 +42,7 @@
* Gives a user object used to manage talk pages
*
* @return User User to manage talkpages
- * @throws MWException If a user cannot be created.
+ * @throws \MWException If a user cannot be created.
*/
public function getTalkpageManager();
}
@@ -52,11 +52,6 @@
* @var int[]
*/
protected $occupiedNamespaces;
-
- /**
- * @var string[]
- */
- protected $occupiedPages;
/**
* @var Title[]
@@ -71,11 +66,9 @@
/**
* @param int[] $occupiedNamespaces See documentation for
$wgFlowOccupyNamespaces
- * @param string[] $occupiedPages See documentation for
$wgFlowOccupyPages
*/
- public function __construct( array $occupiedNamespaces, array
$occupiedPages ) {
+ public function __construct( array $occupiedNamespaces ) {
$this->occupiedNamespaces = $occupiedNamespaces;
- $this->occupiedPages = $occupiedPages;
}
/**
@@ -104,9 +97,6 @@
// told not to. Specifically, while creating the first
revision of a flow board,
// onContentHandlerDefaultModelFor calls this function,
and $title->exists() is already
// true at that point but we are still deciding which
content model to use.
- if ( in_array( $title->getPrefixedText(),
$this->occupiedPages ) ) {
- return true;
- }
if ( in_array( $title->getNamespace(),
$this->occupiedNamespaces ) ) {
return true;
}
@@ -236,7 +226,7 @@
*/
public function canBeUsedOn( Title $title ) {
return
- // automatically allowed (occupiedNamespaces &
occupiedPages)
+ // automatically allowed (occupiedNamespaces)
$this->isTalkpageOccupied( $title, false ) ||
// explicitly allowed via allowCreation()
in_array( $title->getPrefixedDBkey(),
$this->allowCreation );
diff --git a/tests/browser/features/support/pages/flow_page.rb
b/tests/browser/features/support/pages/flow_page.rb
index a72c65b..9dbb7ce 100644
--- a/tests/browser/features/support/pages/flow_page.rb
+++ b/tests/browser/features/support/pages/flow_page.rb
@@ -1,4 +1,4 @@
class FlowPage < AbstractFlowPage
page_url 'Talk:Flow_QA'
- # MEDIAWIKI_URL must have this in $wgFlowOccupyPages array or
$wgFlowOccupyNamespaces.
+ # MEDIAWIKI_URL must have this in $wgFlowOccupyNamespaces.
end
diff --git a/tests/browser/features/support/pages/new_wiki_page.rb
b/tests/browser/features/support/pages/new_wiki_page.rb
index dce072f..f3fa990 100644
--- a/tests/browser/features/support/pages/new_wiki_page.rb
+++ b/tests/browser/features/support/pages/new_wiki_page.rb
@@ -1,6 +1,6 @@
require_relative 'wiki_page'
class NewWikiPage < WikiPage
- # MEDIAWIKI_URL must have this in $wgFlowOccupyPages array or
$wgFlowOccupyNamespaces.
+ # MEDIAWIKI_URL must have this in $wgFlowOccupyNamespaces.
page_url "<%=params[:page]%>"
end
diff --git a/tests/phpunit/TalkpageManagerTest.php
b/tests/phpunit/TalkpageManagerTest.php
index 61b6817..b526143 100644
--- a/tests/phpunit/TalkpageManagerTest.php
+++ b/tests/phpunit/TalkpageManagerTest.php
@@ -12,8 +12,8 @@
* @covers TalkpageManager::isTalkpageOccupied
* @dataProvider provideIsTalkpageOccupied
*/
- public function testIsTalkpageOccupied( array $pages, array $ns, Title
$title, $checkContentModel, $expected ) {
- $manager = new TalkpageManager( $ns, $pages );
+ public function testIsTalkpageOccupied( array $ns, Title $title,
$checkContentModel, $expected ) {
+ $manager = new TalkpageManager( $ns );
$actual = $manager->isTalkpageOccupied( $title,
$checkContentModel );
$this->assertEquals( $expected, $actual );
}
@@ -35,63 +35,48 @@
public function provideIsTalkpageOccupied() {
return array(
array(
- array( 'Talk:Foo Bar' ),
- array(),
- Title::newFromText( 'Talk:Foo Bar' ),
- false,
- true
- ),
- array(
- array(),
array( NS_USER_TALK ),
Title::newFromText( 'User talk:Foo Bar' ),
false,
true
),
array(
- array(),
array( NS_USER_TALK ),
Title::newFromText( 'User talk:Foo/Bar' ),
false,
true
),
array(
- array(),
array( NS_USER_TALK ),
Title::newFromText( 'Talk:Foo Bar' ),
false,
false
),
array(
- array(),
array( NS_USER_TALK ),
$this->getMockTitle( false, 'wikitext',
NS_USER_TALK, 'User talk:FooBar' ),
true,
true
),
array(
- array(),
array( NS_USER_TALK ),
$this->getMockTitle( false, 'wikitext',
NS_USER_TALK, 'User talk:Foo/Bar' ),
true,
true
),
array(
- array(),
array( NS_USER_TALK ),
$this->getMockTitle( true, 'wikitext',
NS_USER_TALK, 'User talk:FooBar' ),
true,
false
),
array(
- array(),
array( NS_USER_TALK ),
$this->getMockTitle( true, 'wikitext',
NS_USER_TALK, 'User talk:Foo/Bar' ),
true,
false
),
array(
- array(),
array( NS_TALK ),
$this->getMockTitle( false, 'wikitext',
NS_USER_TALK, 'User talk:FooBar' ),
true,
@@ -99,20 +84,17 @@
),
array(
array(),
- array(),
$this->getMockTitle( false, 'wikitext',
NS_USER_TALK, 'User talk:FooBar' ),
true,
false
),
array(
array(),
- array(),
$this->getMockTitle( true, 'flow-board',
NS_USER_TALK, 'User talk:FooBar' ),
true,
true
),
array(
- array(),
array(),
$this->getMockTitle( true, 'flow-board',
NS_USER_TALK, 'User talk:Foo/Bar' ),
true,
diff --git a/tests/phpunit/api/ApiTestCase.php
b/tests/phpunit/api/ApiTestCase.php
index d7912f2..d38e950 100644
--- a/tests/phpunit/api/ApiTestCase.php
+++ b/tests/phpunit/api/ApiTestCase.php
@@ -28,14 +28,7 @@
);
protected function setUp() {
- $this->setMwGlobals( 'wgFlowOccupyPages', array(
- // For testing use; shared with browser tests
- 'Talk:Flow QA',
-
- // Don't do any write operations on this. It's
intentionally left
- // blank for testing read operations on unused (but
occupied) pages.
- 'Talk:Intentionally blank',
- ) );
+ $this->setMwGlobals( 'wgFlowOccupyNamespaces', array( 1 ) );
Container::reset();
parent::setUp();
--
To view, visit https://gerrit.wikimedia.org/r/228267
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I27e8e45951bfe96453c2bd2424f643a343cb7e59
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
