AndyRussG has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/151568

Change subject: Fix banner name filter and put it in URLs
......................................................................

Fix banner name filter and put it in URLs

Correct bug on Special::CentralNoticeBanners in which banner name
filters disappeared when paging about. Instead of trying to store
the filter in session data, pass it around in a URL query parameter.
This also solves a bug requesting filters in a URL query parameter.

Bug: 39212
Bug: 53753
Change-Id: I7d01c1e4b340ca3f9beee6942e9ceb8768bb208f
---
M CentralNotice.modules.php
M includes/CNBannerPager.php
M modules/ext.centralNotice.adminUi.bannerManager/bannermanager.css
M modules/ext.centralNotice.adminUi.bannerManager/bannermanager.js
M special/SpecialCentralNoticeBanners.php
5 files changed, 139 insertions(+), 37 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CentralNotice 
refs/changes/68/151568/1

diff --git a/CentralNotice.modules.php b/CentralNotice.modules.php
index f87d233..ce6db2e 100644
--- a/CentralNotice.modules.php
+++ b/CentralNotice.modules.php
@@ -67,7 +67,8 @@
        'remoteExtPath' => 'CentralNotice/modules',
        'dependencies' => array(
                'ext.centralNotice.adminUi',
-               'jquery.ui.dialog'
+               'jquery.ui.dialog',
+               'mediawiki.Uri'
        ),
        'scripts'       => 
'ext.centralNotice.adminUi.bannerManager/bannermanager.js',
        'styles'        => 
'ext.centralNotice.adminUi.bannerManager/bannermanager.css',
diff --git a/includes/CNBannerPager.php b/includes/CNBannerPager.php
index abfef35..a0f9db5 100644
--- a/includes/CNBannerPager.php
+++ b/includes/CNBannerPager.php
@@ -18,14 +18,15 @@
        protected $formSection = null;
 
        /**
-        * @param IContextSource $hostTitle
+        * @param SpecialCentralNoticeBanners $hostSpecialPage
         * @param string         $formSection
         * @param array          $prependPrototypes
         * @param array          $appendPrototypes
         * @param string         $bannerFilter
         * @param bool           $editable
         */
-       function __construct( $hostTitle, $formSection = null, 
$prependPrototypes = array(),
+       function __construct( SpecialCentralNoticeBanners $hostSpecialPage,
+               $formSection = null, $prependPrototypes = array(),
                $appendPrototypes = array(), $bannerFilter = '', $editable = 
false
        ) {
                $this->editable = $editable;
@@ -36,8 +37,7 @@
                $this->appendPrototypes = $appendPrototypes;
                $this->formSection = $formSection;
 
-               $this->viewPage = $hostTitle;
-
+               $this->hostSpecialPage = $hostSpecialPage;
                // Override paging defaults
                list( $this->mLimit, $this->mOffset ) = 
$this->mRequest->getLimitOffset( 20, '' );
                $this->mLimitsShown = array( 20, 50, 100 );
@@ -193,6 +193,20 @@
                }
                return $retval;
        }
+
+       /**
+        * Make a self-link. Overriding to add filter as a query parameter.
+        * @see Pager::makeLink()
+        */
+       public function makeLink( $text, array $query = null, $type = null ) {
+
+               $filterQuery = 
$this->hostSpecialPage->getFilterUrlParamAsArray();
+
+               $query = ( $query === null ) ?
+                       $filterQuery : array_merge( $query, $filterQuery );
+
+               return parent::makeLink( $text, $query, $type );
+       }
 }
 
 class HTMLBannerPagerNavigation extends HTMLFormField {
diff --git a/modules/ext.centralNotice.adminUi.bannerManager/bannermanager.css 
b/modules/ext.centralNotice.adminUi.bannerManager/bannermanager.css
index b68cbf2..2594126 100644
--- a/modules/ext.centralNotice.adminUi.bannerManager/bannermanager.css
+++ b/modules/ext.centralNotice.adminUi.bannerManager/bannermanager.css
@@ -45,7 +45,7 @@
 
 /* Style the search sub form */
 #cn-formsection-header-banner-search,
-#mw-htmlform-banner-search .mw-htmlform-field-HTMLSubmitField {
+#mw-htmlform-banner-search .mw-htmlform-field-HTMLButtonField {
        float: right;
 }
 #mw-htmlform-banner-search .mw-htmlform-field-HTMLTextField {
diff --git a/modules/ext.centralNotice.adminUi.bannerManager/bannermanager.js 
b/modules/ext.centralNotice.adminUi.bannerManager/bannermanager.js
index 829b9bf..7a9aea6 100644
--- a/modules/ext.centralNotice.adminUi.bannerManager/bannermanager.js
+++ b/modules/ext.centralNotice.adminUi.bannerManager/bannermanager.js
@@ -23,7 +23,10 @@
  * @file
  */
 ( function ( $, mw ) {
-       mw.centralNotice.adminUi.bannerManagement = {
+
+       var bm;
+
+       bm = mw.centralNotice.adminUi.bannerManagement = {
                /**
                 * State tracking variable for the number of items currently 
selected
                 * @protected
@@ -90,7 +93,7 @@
                        dialogObj.dialog({
                                title: mw.message(
                                        'centralnotice-delete-banner-title',
-                                       
mw.centralNotice.adminUi.bannerManagement.selectedItemCount
+                                       bm.selectedItemCount
                                ).text(),
                                resizable: false,
                                modal: true,
@@ -118,7 +121,7 @@
                        dialogObj.dialog({
                                title: mw.message(
                                        'centralnotice-archive-banner-title',
-                                       
mw.centralNotice.adminUi.bannerManagement.selectedItemCount
+                                       bm.selectedItemCount
                                ).text(),
                                resizable: false,
                                modal: true,
@@ -132,14 +135,13 @@
                checkAllStateAltered: function() {
                        var checkBoxes = $( 'input.cn-bannerlist-check-applyto' 
);
                        if ( $( '#mw-input-wpselectAllBanners' ).prop( 
'checked' ) ) {
-                               
mw.centralNotice.adminUi.bannerManagement.selectedItemCount =
-                                       
mw.centralNotice.adminUi.bannerManagement.totalSelectableItems;
+                               bm.selectedItemCount = bm.totalSelectableItems;
                                checkBoxes.each( function() { $( this ).prop( 
'checked', true ); } );
                        } else {
-                               
mw.centralNotice.adminUi.bannerManagement.selectedItemCount = 0;
+                               bm.selectedItemCount = 0;
                                checkBoxes.each( function() { $( this ).prop( 
'checked', false ); } );
                        }
-                       
mw.centralNotice.adminUi.bannerManagement.checkedCountUpdated();
+                       bm.checkedCountUpdated();
                },
 
                /**
@@ -147,11 +149,11 @@
                 */
                selectCheckStateAltered: function() {
                        if ( $( this ).prop( 'checked' ) === true ) {
-                               
mw.centralNotice.adminUi.bannerManagement.selectedItemCount++;
+                               bm.selectedItemCount++;
                        } else {
-                               
mw.centralNotice.adminUi.bannerManagement.selectedItemCount--;
+                               bm.selectedItemCount--;
                        }
-                       
mw.centralNotice.adminUi.bannerManagement.checkedCountUpdated();
+                       bm.checkedCountUpdated();
                },
 
                /**
@@ -161,14 +163,12 @@
                        var selectAllCheck = $( '#mw-input-wpselectAllBanners' 
),
                                deleteButton = $(' 
#mw-input-wpdeleteSelectedBanners' );
 
-                       if ( 
mw.centralNotice.adminUi.bannerManagement.selectedItemCount ==
-                               
mw.centralNotice.adminUi.bannerManagement.totalSelectableItems
-                       ) {
+                       if ( bm.selectedItemCount == bm.totalSelectableItems ) {
                                // Everything selected
                                selectAllCheck.prop( 'checked', true );
                                selectAllCheck.prop( 'indeterminate', false );
                                deleteButton.prop( 'disabled', false );
-                       } else if ( 
mw.centralNotice.adminUi.bannerManagement.selectedItemCount === 0 ) {
+                       } else if ( bm.selectedItemCount === 0 ) {
                                // Nothing selected
                                selectAllCheck.prop( 'checked', false );
                                selectAllCheck.prop( 'indeterminate', false );
@@ -179,21 +179,66 @@
                                selectAllCheck.prop( 'indeterminate', true );
                                deleteButton.prop( 'disabled', false );
                        }
+               },
+
+               /**
+                * Reload the page with a URL query for the requested banner 
name
+                * filter (or lack thereof).
+                */
+               applyFilter: function() {
+
+                       var newUri, filterStr;
+                       filterStr = $( '#mw-input-wpbannerNameFilter' ).val();
+                       newUri = new mw.Uri();
+
+                       // If there's a filter, reload with a filter query 
param.
+                       // If there's no filter, reload with no such param.
+                       if ( filterStr.length > 0 ) {
+                               filterStr = bm.sanitizeFilterStr( filterStr );
+                               newUri.extend( {filter: filterStr} );
+                       } else {
+                               delete newUri.query.filter;
+                       }
+
+                       window.location.replace( newUri.toString() );
+               },
+
+               /**
+                * Filter text box keypress handler; applies the filter when 
enter is
+                * pressed.
+                */
+               filterTextBoxKeypress: function( e ) {
+                       if ( e.which == 13 ) {
+                               bm.applyFilter();
+                               return false;
+                       }
+               },
+
+               /**
+                * Remove characters not allowed in banner names. See 
server-side
+                * Banner::isValidBannerName() and
+                * SpecialCentralNotice::sanitizeSearchTerms().
+                */
+               sanitizeFilterStr: function( $origFilterStr ) {
+                       return $origFilterStr.replace(/[^0-9a-zA-Z_\-]/g, '');
                }
        };
 
        // Attach event handlers
-       $( '#mw-input-wpaddNewBanner' ).click( 
mw.centralNotice.adminUi.bannerManagement.doAddBannerDialog );
-       $( '#mw-input-wpdeleteSelectedBanners' ).click( 
mw.centralNotice.adminUi.bannerManagement.doRemoveBanners );
-       $( '#mw-input-wparchiveSelectedBanners' ).click( 
mw.centralNotice.adminUi.bannerManagement.doArchiveBanners );
-       $( '#mw-input-wpselectAllBanners' ).click( 
mw.centralNotice.adminUi.bannerManagement.checkAllStateAltered );
+       $( '#mw-input-wpaddNewBanner' ).click( bm.doAddBannerDialog );
+       $( '#mw-input-wpdeleteSelectedBanners' ).click( bm.doRemoveBanners );
+       $( '#mw-input-wparchiveSelectedBanners' ).click( bm.doArchiveBanners );
+       $( '#mw-input-wpselectAllBanners' ).click( bm.checkAllStateAltered );
+       $( '#mw-input-wpfilterApply' ).click( bm.applyFilter );
+       $( '#mw-input-wpbannerNameFilter' ).keypress( bm.filterTextBoxKeypress 
);
+
        $( 'input.cn-bannerlist-check-applyto' ).each( function() {
-               $( this ).click( 
mw.centralNotice.adminUi.bannerManagement.selectCheckStateAltered );
-               
mw.centralNotice.adminUi.bannerManagement.totalSelectableItems++;
+               $( this ).click( bm.selectCheckStateAltered );
+               bm.totalSelectableItems++;
        } );
 
        // Some initial display work
-       mw.centralNotice.adminUi.bannerManagement.checkAllStateAltered();
+       bm.checkAllStateAltered();
        $( '#cn-js-error-warn' ).hide();
 
 } )( jQuery, mediaWiki );
diff --git a/special/SpecialCentralNoticeBanners.php 
b/special/SpecialCentralNoticeBanners.php
index 452b6e5..a18cabc 100644
--- a/special/SpecialCentralNoticeBanners.php
+++ b/special/SpecialCentralNoticeBanners.php
@@ -23,15 +23,10 @@
                wfSetupSession();
 
                // Load things that may have been serialized into the session
-               $this->bannerFilterString = $this->getCNSessionVar( 
'bannerFilterString', '' );
                $this->bannerLanguagePreview = $this->getCNSessionVar(
                        'bannerLanguagePreview',
                        $this->getLanguage()->getCode()
                );
-       }
-
-       function __destruct() {
-               $this->setCNSessionVar( 'bannerFilterString', 
$this->bannerFilterString );
        }
 
        /**
@@ -136,17 +131,20 @@
         */
        protected function generateBannerListForm( $filter = '' ) {
                // --- Create the banner search form --- //
+
+               // Note: filter is normally set via JS, not form submission. 
But we
+               // leave the info in the submitted form, in any case.
                $formDescriptor = array(
-                       'bannerNameLike' => array(
+                       'bannerNameFilter' => array(
                                'section' => 'header/banner-search',
                                'class' => 'HTMLTextField',
                                'placeholder' => wfMessage( 
'centralnotice-filter-template-prompt' ),
                                'filter-callback' => array( $this, 
'sanitizeSearchTerms' ),
                                'default' => $filter,
                        ),
-                       'filterSubmit' => array(
+                       'filterApply' => array(
                                'section' => 'header/banner-search',
-                               'class' => 'HTMLSubmitField',
+                               'class' => 'HTMLButtonField',
                                'default' => wfMessage( 
'centralnotice-filter-template-submit' )->text(),
                        )
                );
@@ -191,7 +189,7 @@
 
                // --- Add all the banners via the fancy pager object ---
                $pager = new CNBannerPager(
-                       $this->getPageTitle(),
+                       $this,
                        'banner-list',
                        array(
                                 'applyTo' => array(
@@ -220,7 +218,8 @@
         * @return null|string|array
         */
        public function processBannerList( $formData ) {
-               $this->bannerFilterString = $formData[ 'bannerNameLike' ];
+
+               $this->setFilterFromUrl();
 
                if ( $formData[ 'action' ] && $this->editable ) {
                        switch ( strtolower( $formData[ 'action' ] ) ) {
@@ -275,6 +274,16 @@
                                        }
                                        if ( $failed ) {
                                                return 'some banners were not 
deleted';
+
+                                       } else {
+
+                                               // Go back to the special 
banners page, including the
+                                               // same filter that was already 
set in the URL.
+                                               $this->getOutput()->redirect(
+                                                       
SpecialPage::getTitleFor( 'CentralNoticeBanners' )
+                                                       ->getFullURL( 
$this->getFilterUrlParamAsArray() ) );
+
+                                               
$this->bannerFormRedirectRequired = true;
                                        }
                                        break;
                        }
@@ -287,6 +296,39 @@
        }
 
        /**
+        * Use a URL parameter to set the filter string for the banner list.
+        */
+       protected function setFilterFromUrl() {
+
+               // This is the normal param on visible URLs.
+               $filterParam = $this->getRequest()->getVal( 'filter', null );
+
+               // If the form was posted the filter parameter'll have a 
different name.
+               if ( $filterParam === null ) {
+                       $filterParam =
+                               $this->getRequest()->getVal( 
'wpbannerNameFilter', null );
+               }
+
+               // Clean, clean...
+               if ( $filterParam !== null ) {
+                       $this->bannerFilterString
+                               = static::sanitizeSearchTerms( $filterParam );
+               }
+       }
+
+       /**
+        * Return an array for use in constructing a URL query part with or 
without
+        * a filter parameter, as required.
+        *
+        * @return array
+        */
+       public function getFilterUrlParamAsArray() {
+
+               return $this->bannerFilterString ?
+                       array( 'filter' => $this->bannerFilterString ) : 
array();
+       }
+
+       /**
         * Display the banner editor and process edits
         */
        protected function showBannerEditor() {

-- 
To view, visit https://gerrit.wikimedia.org/r/151568
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7d01c1e4b340ca3f9beee6942e9ceb8768bb208f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: AndyRussG <andrew.green...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to