Change delimiter for multiple namespaces and tags
authorMatthew Flaschen <mflaschen@wikimedia.org>
Mon, 8 May 2017 05:06:12 +0000 (01:06 -0400)
committerMatthew Flaschen <mflaschen@wikimedia.org>
Tue, 9 May 2017 03:03:46 +0000 (23:03 -0400)
It's ; for namespaces (since that is generally what we're using,
e.g. for STRING_OPTIONS), and | for tags.  I would have
preferred them to all be consistent, but there is one revision
that has a ;, and this seems better than modifying the DB.

Comma is already used as a delimiter between separate parameters
(e.g. hideanons and namepsace).

Also, fix multiple namespaces for parseParameters and add test
cases.

Bug: T164132
Bug: T164133
Change-Id: Iad061e0bc17e3522a3f5d330ac2c8bf9cf0e614f

includes/changetags/ChangeTags.php
includes/specialpage/ChangesListSpecialPage.php
includes/specials/SpecialRecentchanges.php
tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php
tests/phpunit/includes/specials/SpecialRecentchangesTest.php

index 46cae8f..a07d3c9 100644 (file)
@@ -660,7 +660,7 @@ class ChangeTags {
 
                        $tables[] = 'change_tag';
                        $join_conds['change_tag'] = [ 'INNER JOIN', $join_cond ];
-                       $conds['ct_tag'] = explode( ',', $filter_tag );
+                       $conds['ct_tag'] = explode( '|', $filter_tag );
                }
        }
 
@@ -947,9 +947,12 @@ class ChangeTags {
                        return Status::newFatal( 'tags-create-no-name' );
                }
 
-               // tags cannot contain commas (used as a delimiter in tag_summary table) or
-               // slashes (would break tag description messages in MediaWiki namespace)
-               if ( strpos( $tag, ',' ) !== false || strpos( $tag, '/' ) !== false ) {
+               // tags cannot contain commas (used as a delimiter in tag_summary table),
+               // pipe (used as a delimiter between multiple tags in
+               // modifyDisplayQuery), or slashes (would break tag description messages in
+               // MediaWiki namespace)
+               if ( strpos( $tag, ',' ) !== false || strpos( $tag, '|' ) !== false
+                       || strpos( $tag, '/' ) !== false ) {
                        return Status::newFatal( 'tags-create-invalid-chars' );
                }
 
index 7e70df2..51212ba 100644 (file)
@@ -1078,7 +1078,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 
                // Namespace filtering
                if ( $opts[ 'namespace' ] !== '' ) {
-                       $namespaces = explode( ',', $opts[ 'namespace' ] );
+                       $namespaces = explode( ';', $opts[ 'namespace' ] );
 
                        if ( $opts[ 'associated' ] ) {
                                $associatedNamespaces = array_map(
index c10dbdd..f09dae7 100644 (file)
@@ -291,7 +291,7 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
                        if ( preg_match( '/^days=(\d+)$/', $bit, $m ) ) {
                                $opts['days'] = $m[1];
                        }
-                       if ( preg_match( '/^namespace=(\d+)$/', $bit, $m ) ) {
+                       if ( preg_match( '/^namespace=(.*)$/', $bit, $m ) ) {
                                $opts['namespace'] = $m[1];
                        }
                        if ( preg_match( '/^tagfilter=(.*)$/', $bit, $m ) ) {
index c8c65dc..e2209eb 100644 (file)
@@ -136,7 +136,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                                "rc_namespace IN ('1','2','3')",
                        ],
                        [
-                               'namespace' => '1,2,3',
+                               'namespace' => '1;2;3',
                        ],
                        "rc conditions with multiple namespaces"
                );
@@ -148,7 +148,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                                "rc_namespace IN ('0','1','4','5','6','7')",
                        ],
                        [
-                               'namespace' => '1,4,7',
+                               'namespace' => '1;4;7',
                                'associated' => 1,
                        ],
                        "rc conditions with multiple namespaces and associated"
@@ -161,7 +161,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                                "rc_namespace NOT IN ('2','3','8','9')",
                        ],
                        [
-                               'namespace' => '2,3,9',
+                               'namespace' => '2;3;9',
                                'associated' => 1,
                                'invert' => 1
                        ],
@@ -175,7 +175,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                                "rc_namespace NOT IN ('1','2','3')",
                        ],
                        [
-                               'namespace' => '1,2,3',
+                               'namespace' => '1;2;3',
                                'invert' => 1,
                        ],
                        "rc conditions with multiple namespaces inverted"
index 8f3f585..85becff 100644 (file)
@@ -27,9 +27,13 @@ class SpecialRecentchangesTest extends AbstractChangesListSpecialPageTestCase {
 
                        [ 'days=3', [ 'days' => '3' ] ],
 
-                       [ 'namespace=5', [ 'namespace' => 5 ] ],
+                       [ 'namespace=5', [ 'namespace' => '5' ] ],
+
+                       [ 'namespace=5|3', [ 'namespace' => '5|3' ] ],
 
                        [ 'tagfilter=foo', [ 'tagfilter' => 'foo' ] ],
+
+                       [ 'tagfilter=foo;bar', [ 'tagfilter' => 'foo;bar' ] ],
                ];
        }