RCFilters: Don't allow underscore in filter or group names

This is reserved for the client-side which joins 'someGroup'
and 'somefilter' to make 'someGroup__somefilter' as an internal
ID.

Change-Id: I1b6ca9f337dd48e10705c46ef5027c3156254e01

diff --git a/includes/changes/ChangesListFilter.php b/includes/changes/ChangesListFilter.php
index 4ac6387..cd74154 100644 (file)
--- a/includes/changes/ChangesListFilter.php
+++ b/includes/changes/ChangesListFilter.php
@@ -110,6 +110,8 @@ abstract class ChangesListFilter {
         */
        protected $priority;
 
+       const RESERVED_NAME_CHAR = '_';
+
        /**
         * Create a new filter with the specified configuration.
         *
@@ -122,7 +124,8 @@ abstract class ChangesListFilter {
         *
         * @param array $filterDefinition ChangesListFilter definition
         *
-        * $filterDefinition['name'] string Name of filter
+        * $filterDefinition['name'] string Name of filter; use lowercase with no
+        *  punctuation
         * $filterDefinition['cssClassSuffix'] string CSS class suffix, used to mark
         *  that a particular row belongs to this filter (when a row is included by the
         *  filter) (optional)
@@ -151,6 +154,13 @@ abstract class ChangesListFilter {
                                'ChangesListFilterGroup this filter belongs to' );
                }
 
+               if ( strpos( $filterDefinition['name'], self::RESERVED_NAME_CHAR ) !== false ) {
+                       throw new MWException( 'Filter names may not contain \'' .
+                               self::RESERVED_NAME_CHAR .
+                               '\'.  Use the naming convention: \'lowercase\''
+                       );
+               }
+
                $this->name = $filterDefinition['name'];
 
                if ( isset( $filterDefinition['cssClassSuffix'] ) ) {

diff --git a/includes/changes/ChangesListFilterGroup.php b/includes/changes/ChangesListFilterGroup.php
index a4cc287..30ab552 100644 (file)
--- a/includes/changes/ChangesListFilterGroup.php
+++ b/includes/changes/ChangesListFilterGroup.php
@@ -123,11 +123,13 @@ abstract class ChangesListFilterGroup {
 
        const DEFAULT_PRIORITY = -100;
 
+       const RESERVED_NAME_CHAR = '_';
+
        /**
         * Create a new filter group with the specified configuration
         *
         * @param array $groupDefinition Configuration of group
-        * * $groupDefinition['name'] string Group name
+        * * $groupDefinition['name'] string Group name; use camelCase with no punctuation
         * * $groupDefinition['title'] string i18n key for title (optional, can be omitted
         * *  only if none of the filters in the group display in the structured UI)
         * * $groupDefinition['type'] string A type constant from a subclass of this one
@@ -142,6 +144,13 @@ abstract class ChangesListFilterGroup {
         * *  changes list entries are filtered out.
         */
        public function __construct( array $groupDefinition ) {
+               if ( strpos( $groupDefinition['name'], self::RESERVED_NAME_CHAR ) !== false ) {
+                       throw new MWException( 'Group names may not contain \'' .
+                               self::RESERVED_NAME_CHAR .
+                               '\'.  Use the naming convention: \'camelCase\''
+                       );
+               }
+
                $this->name = $groupDefinition['name'];
 
                if ( isset( $groupDefinition['title'] ) ) {

diff --git a/tests/phpunit/includes/changes/ChangesListFilterGroupTest.php b/tests/phpunit/includes/changes/ChangesListFilterGroupTest.php
index 718d4b3..f712a2f 100644 (file)
--- a/tests/phpunit/includes/changes/ChangesListFilterGroupTest.php
+++ b/tests/phpunit/includes/changes/ChangesListFilterGroupTest.php
@@ -4,6 +4,23 @@
  * @covers ChangesListFilterGroup
  */
 class ChangesListFilterGroupTest extends MediaWikiTestCase {
+       // @codingStandardsIgnoreStart
+       /**
+        * @expectedException MWException
+        * @expectedExceptionMessage Group names may not contain '_'.  Use the naming convention: 'camelCase'
+        */
+       // @codingStandardsIgnoreEnd
+       public function testReservedCharacter() {
+               new MockChangesListFilterGroup(
+                       [
+                               'type' => 'some_type',
+                               'name' => 'group_name',
+                               'priority' => 1,
+                               'filters' => [],
+                       ]
+               );
+       }
+
        public function testAutoPriorities() {
                $group = new MockChangesListFilterGroup(
                        [

diff --git a/tests/phpunit/includes/changes/ChangesListFilterTest.php b/tests/phpunit/includes/changes/ChangesListFilterTest.php
index b63482f..c212560 100644 (file)
--- a/tests/phpunit/includes/changes/ChangesListFilterTest.php
+++ b/tests/phpunit/includes/changes/ChangesListFilterTest.php
@@ -24,6 +24,22 @@ class ChangesListFilterTest extends MediaWikiTestCase {
 
        }
 
+       // @codingStandardsIgnoreStart
+       /**
+        * @expectedException MWException
+        * @expectedExceptionMessage Filter names may not contain '_'.  Use the naming convention: 'lowercase'
+        */
+       // @codingStandardsIgnoreEnd
+       public function testReservedCharacter() {
+               $filter = new MockChangesListFilter(
+                       [
+                               'group' => $this->group,
+                               'name' => 'some_name',
+                               'priority' => 1,
+                       ]
+               );
+       }
+
        /**
         * @expectedException MWException
         * @expectedExceptionMessage Supersets can only be defined for filters in the same group