jenkins-bot has submitted this change and it was merged.
Change subject: Handle banner loader errors on client
......................................................................
Handle banner loader errors on client
Bug: T149107
Change-Id: Iab80d0462becf1e7a7a2d9f2df662c7a68aed8bf
---
M resources/subscribing/ext.centralNotice.display.js
M resources/subscribing/ext.centralNotice.display.state.js
M special/SpecialBannerLoader.php
M tests/qunit/subscribing/ext.centralNotice.display.tests.js
4 files changed, 59 insertions(+), 11 deletions(-)
Approvals:
Ejegg: Looks good to me, approved
jenkins-bot: Verified
diff --git a/resources/subscribing/ext.centralNotice.display.js
b/resources/subscribing/ext.centralNotice.display.js
index 5536e05..ba4777d 100644
--- a/resources/subscribing/ext.centralNotice.display.js
+++ b/resources/subscribing/ext.centralNotice.display.js
@@ -153,10 +153,14 @@
);
// The returned javascript will call
mw.centralNotice.insertBanner()
+ // or mw.centralNotice.handleBannerLoaderError() (if an error
was
+ // handled on the server).
$.ajax( {
url: url.toString(),
dataType: 'script',
cache: true
+ } ).fail( function ( jqXHR, status, error ) {
+ cn.handleBannerLoaderError( status + ': ' + error );
} );
}
@@ -309,6 +313,20 @@
}
/**
+ * Stuff we have to do following the call to fetch a banner (successful
+ * or not)
+ */
+ function processAfterBannerFetch() {
+
+ // If we're testing a banner, don't call
Special:RecordImpression or
+ // run mixin hooks.
+ if ( !cn.internal.state.getData().testingBanner ) {
+ runPostBannerMixinHooks();
+ recordImpression();
+ }
+ }
+
+ /**
* CentralNotice base public object, exposed as mw.centralNotice. Note:
* other CN modules may add properties to this object, and we add some
* dynamically. These additional properties are:
@@ -402,12 +420,7 @@
state.setBannerShown();
}
- // If we're testing a banner, don't call
Special:RecordImpression or
- // run mixin hooks.
- if ( !state.getData().testingBanner ) {
- runPostBannerMixinHooks();
- recordImpression();
- }
+ processAfterBannerFetch();
},
/**
@@ -531,6 +544,16 @@
} );
},
+ /**
+ * Handle a banner loader error, with an optional message
+ * @param {string} [msg]
+ */
+ handleBannerLoaderError: function ( msg ) {
+ cn.internal.state.setBannerLoaderError( msg );
+ bannerLoadedDeferredObj.reject(
cn.internal.state.getData() );
+ processAfterBannerFetch();
+ },
+
hideBannerWithCloseButton: function () {
// Hide the banner element
$( '#centralNotice' ).hide();
diff --git a/resources/subscribing/ext.centralNotice.display.state.js
b/resources/subscribing/ext.centralNotice.display.state.js
index a2b11fb..2d18e1f 100644
--- a/resources/subscribing/ext.centralNotice.display.state.js
+++ b/resources/subscribing/ext.centralNotice.display.state.js
@@ -31,7 +31,8 @@
NO_BANNER_AVAILABLE: new Status( 'no_banner_available',
3 ),
BANNER_CHOSEN: new Status( 'banner_chosen', 4 ),
BANNER_LOADED_BUT_HIDDEN: new Status(
'banner_loaded_but_hidden', 5 ),
- BANNER_SHOWN: new Status( 'banner_shown', 6 )
+ BANNER_SHOWN: new Status( 'banner_shown', 6 ),
+ BANNER_LOADER_ERROR: new Status( 'banner_loader_error',
7 )
},
// Until T114078 is closed, we minify banner history logs. This
lookup
@@ -363,6 +364,17 @@
},
/**
+ * Set a banner loader error, with an optional message
+ * @param {string} [msg]
+ */
+ setBannerLoaderError: function ( msg ) {
+ if ( msg ) {
+ state.data.errorMsg = msg;
+ }
+ setStatus( STATUSES.BANNER_LOADER_ERROR );
+ },
+
+ /**
* Register that the current page view is included in a test.
*
* @param {string} identifier A string to identify the test.
Should not contain
diff --git a/special/SpecialBannerLoader.php b/special/SpecialBannerLoader.php
index 3c4dc39..dfaeb61 100644
--- a/special/SpecialBannerLoader.php
+++ b/special/SpecialBannerLoader.php
@@ -29,13 +29,20 @@
try {
$this->getParams();
- echo $this->getJsNotice( $this->bannerName );
+ $out = $this->getJsNotice( $this->bannerName );
+
} catch ( EmptyBannerException $e ) {
- echo "mw.centralNotice.insertBanner( false );";
+ $out = "mw.centralNotice.handleBannerLoaderError(
'Empty banner' );";
+
} catch ( Exception $e ) {
- wfDebugLog( 'CentralNotice', $e->getMessage() );
- echo "mw.centralNotice.insertBanner( false /* due to
internal exception */ );";
+ $msg = $e->getMessage();
+ $msgParamStr = $msg ? " '{$msg}' " : '';
+ $out =
"mw.centralNotice.handleBannerLoaderError({$msgParamStr});";
+
+ wfDebugLog( 'CentralNotice', $msg );
}
+
+ echo $out;
}
function getParams() {
diff --git a/tests/qunit/subscribing/ext.centralNotice.display.tests.js
b/tests/qunit/subscribing/ext.centralNotice.display.tests.js
index 9bfa2c3..08ba6b7 100644
--- a/tests/qunit/subscribing/ext.centralNotice.display.tests.js
+++ b/tests/qunit/subscribing/ext.centralNotice.display.tests.js
@@ -181,6 +181,7 @@
mw.centralNotice.internal.state.urlParams.banner =
'test_banner';
$.ajax = function ( params ) {
assert.ok( params.url.match(
/Special(?:[:]|%3A)BannerLoader.*[?&]banner=test_banner/ ) );
+ return $.Deferred();
};
mw.centralNotice.displayTestingBanner();
@@ -196,6 +197,7 @@
$.ajax = function ( params ) {
assert.ok( params.url.match(
/Special(?:[:]|%3A)BannerLoader.*[?&]banner=banner1/ ) );
+ return $.Deferred();
};
mw.centralNotice.chooseAndMaybeDisplay();
@@ -204,6 +206,7 @@
$.ajax = function ( params ) {
assert.ok( params.url.match(
/Special(?:[:]|%3A)BannerLoader.*[?&]banner=banner2/ ) );
+ return $.Deferred();
};
mw.centralNotice.chooseAndMaybeDisplay();
} );
@@ -216,6 +219,7 @@
$.ajax = function ( params ) {
assert.ok( params.url.match(
/Special(?:[:]|%3A)BannerLoader.*[?&]banner=banner1/ ) );
+ return $.Deferred();
};
mw.centralNotice.chooseAndMaybeDisplay();
@@ -224,6 +228,7 @@
$.ajax = function ( params ) {
assert.ok( params.url.match(
/Special(?:[:]|%3A)BannerLoader.*[?&]banner=banner2/ ) );
+ return $.Deferred();
};
mw.centralNotice.chooseAndMaybeDisplay();
} );
@@ -232,6 +237,7 @@
var mixin = new mw.centralNotice.Mixin( 'testMixin' );
$.ajax = function () {
mw.centralNotice.reallyInsertBanner( bannerData );
+ return $.Deferred();
};
mixin.setPreBannerHandler(
--
To view, visit https://gerrit.wikimedia.org/r/318159
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iab80d0462becf1e7a7a2d9f2df662c7a68aed8bf
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: AndyRussG <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
Gerrit-Reviewer: Cdentinger <[email protected]>
Gerrit-Reviewer: Ejegg <[email protected]>
Gerrit-Reviewer: Ssmith <[email protected]>
Gerrit-Reviewer: XenoRyet <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
