jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/342979 )
Change subject: RCFilters: Prevent duplicate filter names
......................................................................
RCFilters: Prevent duplicate filter names
Explicitly block two filters in the same group from having the same
name.
Before, it would be left to registerFilter, which would just cause
the second one to win.
Also, avoid a getFilter warning when the filter does not exist.
Do the same for getFilterGroup on ChangesListSpecialPage
Finally, a minor related doc fix.
Change-Id: I6b3880a5c7cc381c169bbd969cd4814559b49c91
---
M docs/hooks.txt
M includes/changes/ChangesListFilter.php
M includes/changes/ChangesListFilterGroup.php
M includes/specialpage/ChangesListSpecialPage.php
M tests/phpunit/includes/changes/ChangesListFilterGroupTest.php
M tests/phpunit/includes/changes/ChangesListFilterTest.php
6 files changed, 67 insertions(+), 9 deletions(-)
Approvals:
Sbisson: Looks good to me, approved
jenkins-bot: Verified
diff --git a/docs/hooks.txt b/docs/hooks.txt
index 149ee4b..1be9a03 100644
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -1015,10 +1015,11 @@
'ChangesListSpecialPageStructuredFilters': Called to allow extensions to
register
filters for pages inheriting from ChangesListSpecialPage (in core:
RecentChanges,
RecentChangesLinked, and Watchlist). Generally, you will want to construct
-new ChangesListBooleanFilter or ChangesListStringOptionsFilter objects. You
can
-then either add them to existing ChangesListFilterGroup objects (accessed
through
-$special->getFilterGroup), or create your own. If you create new groups, you
-must register them with $special->registerFilterGroup.
+new ChangesListBooleanFilter or ChangesListStringOptionsFilter objects.
+
+When constructing them, you specify which group they belong to. You can reuse
+existing groups (accessed through $special->getFilterGroup), or create your
own.
+If you create new groups, you must register them with
$special->registerFilterGroup.
$special: ChangesListSpecialPage instance
'ChangeTagAfterDelete': Called after a change tag has been deleted (that is,
diff --git a/includes/changes/ChangesListFilter.php
b/includes/changes/ChangesListFilter.php
index cd74154..22e797d 100644
--- a/includes/changes/ChangesListFilter.php
+++ b/includes/changes/ChangesListFilter.php
@@ -113,7 +113,8 @@
const RESERVED_NAME_CHAR = '_';
/**
- * Create a new filter with the specified configuration.
+ * Creates a new filter with the specified configuration, and registers
it to the
+ * specified group.
*
* It infers which UI (it can be either or both) to display the filter
on based on
* which messages are provided.
@@ -161,6 +162,11 @@
);
}
+ if ( $this->group->getFilter( $filterDefinition['name'] ) ) {
+ throw new MWException( 'Two filters in a group cannot
have the ' .
+ "same name: '{$filterDefinition['name']}'" );
+ }
+
$this->name = $filterDefinition['name'];
if ( isset( $filterDefinition['cssClassSuffix'] ) ) {
diff --git a/includes/changes/ChangesListFilterGroup.php
b/includes/changes/ChangesListFilterGroup.php
index 30ab552..d2ad204 100644
--- a/includes/changes/ChangesListFilterGroup.php
+++ b/includes/changes/ChangesListFilterGroup.php
@@ -315,10 +315,10 @@
* Get filter by name
*
* @param string $name Filter name
- * @return ChangesListFilter Specified filter
+ * @return ChangesListFilter|null Specified filter, or null if it is
not registered
*/
public function getFilter( $name ) {
- return $this->filters[$name];
+ return isset( $this->filters[$name] ) ? $this->filters[$name] :
null;
}
/**
diff --git a/includes/specialpage/ChangesListSpecialPage.php
b/includes/specialpage/ChangesListSpecialPage.php
index e92f697..8f702ba 100644
--- a/includes/specialpage/ChangesListSpecialPage.php
+++ b/includes/specialpage/ChangesListSpecialPage.php
@@ -663,10 +663,12 @@
*
* @param string $groupName Name of group
*
- * @return ChangesListFilterGroup
+ * @return ChangesListFilterGroup|null Group, or null if not registered
*/
public function getFilterGroup( $groupName ) {
- return $this->filterGroups[$groupName];
+ return isset( $this->filterGroups[$groupName] ) ?
+ $this->filterGroups[$groupName] :
+ null;
}
// Currently, this intentionally only includes filters that display
diff --git a/tests/phpunit/includes/changes/ChangesListFilterGroupTest.php
b/tests/phpunit/includes/changes/ChangesListFilterGroupTest.php
index f712a2f..465a9d1 100644
--- a/tests/phpunit/includes/changes/ChangesListFilterGroupTest.php
+++ b/tests/phpunit/includes/changes/ChangesListFilterGroupTest.php
@@ -51,4 +51,29 @@
)
);
}
+
+ // Get without warnings
+ public function testGetFilter() {
+ $group = new MockChangesListFilterGroup(
+ [
+ 'type' => 'some_type',
+ 'name' => 'groupName',
+ 'isFullCoverage' => true,
+ 'priority' => 1,
+ 'filters' => [
+ [ 'name' => 'foo' ],
+ ],
+ ]
+ );
+
+ $this->assertEquals(
+ 'foo',
+ $group->getFilter( 'foo' )->getName()
+ );
+
+ $this->assertEquals(
+ null,
+ $group->getFilter( 'bar' )
+ );
+ }
}
diff --git a/tests/phpunit/includes/changes/ChangesListFilterTest.php
b/tests/phpunit/includes/changes/ChangesListFilterTest.php
index c212560..1d87aeb 100644
--- a/tests/phpunit/includes/changes/ChangesListFilterTest.php
+++ b/tests/phpunit/includes/changes/ChangesListFilterTest.php
@@ -40,6 +40,30 @@
);
}
+ // @codingStandardsIgnoreStart
+ /**
+ * @expectedException MWException
+ * @expectedExceptionMessage Two filters in a group cannot have the
same name: 'somename'
+ */
+ // @codingStandardsIgnoreEnd
+ public function testDuplicateName() {
+ new MockChangesListFilter(
+ [
+ 'group' => $this->group,
+ 'name' => 'somename',
+ 'priority' => 1,
+ ]
+ );
+
+ new MockChangesListFilter(
+ [
+ 'group' => $this->group,
+ 'name' => 'somename',
+ 'priority' => 2,
+ ]
+ );
+ }
+
/**
* @expectedException MWException
* @expectedExceptionMessage Supersets can only be defined for filters
in the same group
--
To view, visit https://gerrit.wikimedia.org/r/342979
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6b3880a5c7cc381c169bbd969cd4814559b49c91
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Mattflaschen <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Mooeypoo <[email protected]>
Gerrit-Reviewer: Sbisson <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
