Ejegg has submitted this change and it was merged.
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(-)
Approvals:
Ejegg: Looks good to me, approved
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/155318
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7d01c1e4b340ca3f9beee6942e9ceb8768bb208f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: wmf_deploy
Gerrit-Owner: Awight <[email protected]>
Gerrit-Reviewer: AndyRussG <[email protected]>
Gerrit-Reviewer: Ejegg <[email protected]>
Gerrit-Reviewer: Mwalker <[email protected]>
Gerrit-Reviewer: Ssmith <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
