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