' ).html( contents ).appendTo( $container );
- }
- } );
-
- pageIssue = pageIssuesParser.parse( $box.get( 0 ) );
-
- return {
- severity: pageIssue.severity,
- isMultiple: $box.parent().is( MULTIPLE_SELECTOR ),
- icon: pageIssue.icon.toHtmlString(),
- text: $container.html()
- };
- }
+ // T206179 should update this value to enable it
+ newTreatmentEnabled = QUERY_STRING_FLAG === 'b';
/**
* Create a link element that opens the issues overlay.
@@ -138,8 +50,7 @@
issueUrl = section === KEYWORD_ALL_SECTIONS ? '#/issues/' + KEYWORD_ALL_SECTIONS : '#/issues/' + section,
selector = 'table.ambox, table.tmbox, table.cmbox, table.fmbox',
issues = [],
- $link,
- severity;
+ $link;
if ( section === KEYWORD_ALL_SECTIONS ) {
$metadata = page.$( selector );
@@ -154,8 +65,8 @@
$this = $( this );
if ( $this.find( selector ).length === 0 ) {
- issue = extractMessage( $this );
- // Some issues after "extractMessage" has been run will have no text.
+ issue = pageIssuesParser.extract( $this );
+ // Some issues after "extract" has been run will have no text.
// For example in Template:Talk header the table will be removed and no issue found.
// These should not be rendered.
if ( issue.text ) {
@@ -166,14 +77,10 @@
// store it for later
allIssues[section] = issues;
- if ( $metadata.length && inline ) {
- severity = pageIssuesParser.maxSeverity(
- issues.map( function ( issue ) { return issue.severity; } )
- );
- new Icon( {
- glyphPrefix: 'minerva',
- name: pageIssuesParser.iconName( $metadata.get( 0 ), severity )
- } ).prependTo( $metadata.find( '.mbox-text' ) );
+ // If issues were extracted and there are inline amboxes, add learn more
+ // and icon to the UI element.
+ if ( issues.length && $metadata.length && inline ) {
+ issues[0].issue.icon.$el.prependTo( $metadata.eq( 0 ).find( '.mbox-text' ) );
$learnMore = $( '' )
.addClass( 'ambox-learn-more' )
.text( mw.msg( 'skin-minerva-issue-learn-more' ) );
@@ -185,34 +92,12 @@
$learnMore.appendTo( $metadata.find( '.mbox-text' ) );
}
$metadata.click( function () {
- var pageIssue = pageIssuesParser.parse( this );
- pageIssuesLogger.log( {
- action: 'issueClicked',
- issuesSeverity: [ pageIssue.severity ],
- sectionNumbers: [ section ]
- } );
overlayManager.router.navigate( issueUrl );
return false;
} );
} else {
$link = createLinkElement( labelText );
$link.attr( 'href', '#/issues/' + section );
- $link.click( function () {
- pageIssuesLogger.log( {
- action: 'issueClicked',
- issuesSeverity: [
- pageIssuesParser.maxSeverity(
- getIssues( '0' )
- .map( function ( issue ) { return issue.severity; } )
- )
- ],
- // In the old treatment, an issuesClicked event will always be '0'
- // as the old treatment is always associated with the lead section and we
- // are only sending one maximum severity for all of them.
- // An issuesClicked event should only ever be associated with one issue box.
- sectionNumbers: [ '0' ]
- } );
- } );
if ( $metadata.length ) {
$link.insertAfter( $( 'h1#section_0' ) );
$metadata.remove();
@@ -257,7 +142,7 @@
var lastIssue = allIssues[ section ][i - 1];
// If the last issue belongs to a "Multiple issues" template,
// and so does the current one, don't add the current one.
- if ( lastIssue && lastIssue.isMultiple && issue.isMultiple ) {
+ if ( lastIssue && lastIssue.grouped && issue.grouped ) {
acc[ acc.length - 1 ] = section;
} else {
acc.push( section );
@@ -316,40 +201,16 @@
}
}
- if ( isLoggingRequired( getIssues( KEYWORD_ALL_SECTIONS ) ) ) {
- // Enable logging of the PageIssues schema, setting up defaults.
- pageIssuesLogger.subscribe(
- newTreatmentEnabled,
- pageIssuesLogger.newPageIssueSchemaData(
- newTreatmentEnabled,
- CURRENT_NS,
- getIssues( KEYWORD_ALL_SECTIONS ).reduce( formatPageIssuesSeverity, [] ),
- getAllIssuesSections( allIssues )
- )
- );
-
- // Report that the page has been loaded.
- pageIssuesLogger.log( {
- action: 'pageLoaded'
- } );
- }
-
// Setup the overlay route.
overlayManager.add( new RegExp( '^/issues/(\\d+|' + KEYWORD_ALL_SECTIONS + ')$' ), function ( section ) {
return new PageIssuesOverlay(
- getIssues( section ), pageIssuesLogger, section, CURRENT_NS );
+ getIssues( section ), section, CURRENT_NS );
} );
}
M.define( 'skins.minerva.scripts/pageIssues', {
init: initPageIssues,
- // The logger requires initialization (subscription). Ideally, the logger would be
- // initialized and passed to initPageIssues() by the client. Since it's not, expose a log
- // method and hide the subscription call in cleanuptemplates.
- log: pageIssuesLogger.log,
test: {
- formatPageIssuesSeverity: formatPageIssuesSeverity,
- extractMessage: extractMessage,
getAllIssuesSections: getAllIssuesSections,
createBanner: createBanner
}
diff --git a/resources/skins.minerva.scripts/pageIssuesLogger.js b/resources/skins.minerva.scripts/pageIssuesLogger.js
deleted file mode 100644
index 3ce89bb..0000000
--- a/resources/skins.minerva.scripts/pageIssuesLogger.js
+++ /dev/null
@@ -1,111 +0,0 @@
-( function ( M, mwConfig, mwTrack, mwTrackSubscribe, mwUser ) {
- var
- util = M.require( 'mobile.startup/util' ),
- EVENT_PAGE_ISSUE_LOG = 'minerva.PageIssuesAB';
-
- /**
- * Defines default data for Schema:PageIssues that will be recorded with every event.
- * @param {boolean} newTreatmentEnabled
- * @param {number} namespaceId The namespace for the page that has issues.
- * @param {string[]} pageIssueSeverities An array of PageIssue severities.
- * @param {array} pageIssuesSections
- *
- * @return {Object} A Partial ' ).html( contents ).appendTo( $container );
+ }
+ } );
+
+ pageIssue = parse( $box.get( 0 ) );
+
+ return {
+ issue: pageIssue,
+ // For template compatibility with PageIssuesOverlay
+ iconString: pageIssue.icon.toHtmlString(),
+ text: $container.html()
+ };
+ }
+
/**
* @module skins.minerva.scripts/utils
*/
M.define( 'skins.minerva.scripts/pageIssuesParser', {
- /**
- * Extract an icon for use with the issue.
- * @param {JQuery.Object} $box element to extract the icon from
- * @return {Icon} representing the icon
- */
+ extract: extract,
parse: parse,
maxSeverity: maxSeverity,
iconName: iconName,
test: {
parseSeverity: parseSeverity,
- parseType: parseType
+ parseType: parseType,
+ parseGroup: parseGroup
}
} );
diff --git a/skin.json b/skin.json
index de786bb..b30f1a1 100644
--- a/skin.json
+++ b/skin.json
@@ -155,7 +155,6 @@
}
},
"EventLoggingSchemas": {
- "PageIssues": 18392542,
"WebClientError": 18340282
},
"ResourceModules": {
@@ -435,7 +434,6 @@
"resources/skins.minerva.scripts/errorLogging.js",
"resources/skins.minerva.scripts/preInit.js",
"resources/skins.minerva.scripts/DownloadIcon.js",
- "resources/skins.minerva.scripts/pageIssuesLogger.js",
"resources/skins.minerva.scripts/pageIssuesParser.js",
"resources/skins.minerva.scripts/AB.js",
"resources/skins.minerva.scripts/PageIssuesOverlay.js",
diff --git a/tests/qunit/skins.minerva.scripts/PageIssuesOverlay.test.js b/tests/qunit/skins.minerva.scripts/PageIssuesOverlay.test.js
deleted file mode 100644
index 11c7bac..0000000
--- a/tests/qunit/skins.minerva.scripts/PageIssuesOverlay.test.js
+++ /dev/null
@@ -1,57 +0,0 @@
-( function ( M ) {
- var PageIssuesOverlay = M.require( 'skins.minerva.scripts/PageIssuesOverlay' );
-
- QUnit.module( 'Minerva PageIssuesOverlay', {
- beforeEach: function () {
- this.logger = {
- log: this.sandbox.spy()
- };
- }
- } );
-
- QUnit.test( '#log (section=all)', function ( assert ) {
- var overlay = new PageIssuesOverlay( [], this.logger, 'all', 0 );
- overlay.onExit();
- assert.strictEqual( this.logger.log.calledOnce, true, 'Logger called once' );
- assert.strictEqual(
- this.logger.log.calledWith( {
- action: 'modalClose',
- issuesSeverity: []
- } ), true, 'sectionNumbers is not set (T202940)'
- );
- } );
-
- QUnit.test( '#log (section=1)', function ( assert ) {
- var overlay = new PageIssuesOverlay( [
- {
- severity: 'MEDIUM'
- }
- ], this.logger, '1', 0 );
- overlay.onExit();
- assert.strictEqual(
- this.logger.log.calledWith( {
- action: 'modalClose',
- issuesSeverity: [ 'MEDIUM' ],
- sectionNumbers: [ '1' ]
- } ), true, 'sectionNumbers is set'
- );
- } );
-
- QUnit.test( '#log (section=2) multiple issues', function ( assert ) {
- var overlay = new PageIssuesOverlay(
- [ {
- severity: 'MEDIUM'
- },
- {
- severity: 'LOW'
- } ], this.logger, '2', 0 );
- overlay.onExit();
- assert.strictEqual(
- this.logger.log.calledWith( {
- action: 'modalClose',
- issuesSeverity: [ 'MEDIUM', 'LOW' ],
- sectionNumbers: [ '2', '2' ]
- } ), true, 'sectionNumbers is set for each issue'
- );
- } );
-}( mw.mobileFrontend ) );
diff --git a/tests/qunit/skins.minerva.scripts/pageIssues.test.js b/tests/qunit/skins.minerva.scripts/pageIssues.test.js
index d6d03b5..63a1dc8 100644
--- a/tests/qunit/skins.minerva.scripts/pageIssues.test.js
+++ b/tests/qunit/skins.minerva.scripts/pageIssues.test.js
@@ -1,35 +1,30 @@
( function ( M ) {
var pageIssues = M.require( 'skins.minerva.scripts/pageIssues' ),
util = M.require( 'mobile.startup/util' ),
- pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' ),
- extractMessage = pageIssues.test.extractMessage,
createBanner = pageIssues.test.createBanner,
- formatPageIssuesSeverity = pageIssues.test.formatPageIssuesSeverity,
+ icon = {},
MEDIUM_ISSUE = {
- severity: 'MEDIUM',
- icon: 'i',
- text: 't'
- },
- MEDIUM_MULTIPLE_ISSUE = {
- severity: 'MEDIUM',
- isMultiple: true,
- icon: 'i',
- text: 't'
- },
- LOW_MULTIPLE_ISSUE = {
- severity: 'LOW',
- isMultiple: true,
- icon: 'i',
+ issue: {
+ severity: 'MEDIUM',
+ icon: icon
+ },
+ iconString: 'i',
text: 't'
},
LOW_ISSUE = {
- severity: 'LOW',
- icon: 'i',
+ issue: {
+ severity: 'LOW',
+ icon: icon
+ },
+ iconString: 'i',
text: 't'
},
HIGH_ISSUE = {
- severity: 'HIGH',
- icon: 'i',
+ issue: {
+ severity: 'HIGH',
+ icon: icon
+ },
+ iconString: 'i',
text: 't'
},
getAllIssuesSections = pageIssues.test.getAllIssuesSections,
@@ -67,93 +62,6 @@
assert.strictEqual( window.location.hash, '#/issues/' + SECTION );
} );
- // NOTE: Only for PageIssues AB
- QUnit.test( 'clicking on the product of createBanner() should trigger a custom event', function ( assert ) {
- var mockAction = {
- action: 'issueClicked',
- issuesSeverity: [ 'MEDIUM' ],
- sectionNumbers: [ SECTION ]
- };
- mw.trackSubscribe( 'minerva.PageIssuesAB', function ( topic, data ) {
- assert.deepEqual( mockAction, data );
- } );
- processedAmbox.click();
- } );
-
- QUnit.test( 'formatPageIssuesSeverity', function ( assert ) {
- var multipleIssues = [
- MEDIUM_MULTIPLE_ISSUE,
- LOW_MULTIPLE_ISSUE,
- LOW_MULTIPLE_ISSUE
- ],
- multipleSingleIssues = [
- LOW_ISSUE,
- HIGH_ISSUE,
- MEDIUM_ISSUE
- ],
- mixedMultipleSingle = [
- HIGH_ISSUE,
- LOW_MULTIPLE_ISSUE,
- MEDIUM_MULTIPLE_ISSUE,
- LOW_ISSUE,
- MEDIUM_ISSUE,
- HIGH_ISSUE
- ],
- testMultiple = multipleIssues.reduce( formatPageIssuesSeverity, [] ),
- testSingle = multipleSingleIssues.reduce( formatPageIssuesSeverity, [] ),
- testMixed = mixedMultipleSingle.reduce( formatPageIssuesSeverity, [] );
-
- assert.deepEqual( testMultiple, [ 'MEDIUM' ], 'Multiple issues return one maxSeverity value' );
- assert.deepEqual( testSingle, [ 'LOW', 'HIGH', 'MEDIUM' ], 'Single issues return each corresponding severity' );
- assert.deepEqual( testMixed, [ 'HIGH', 'MEDIUM', 'LOW', 'MEDIUM', 'HIGH' ], 'Mixed single/multiple return one value for multiples' );
-
- } );
-
- QUnit.test( 'extractMessage', function ( assert ) {
- this.sandbox.stub( pageIssuesParser, 'parse' ).returns(
- {
- severity: 'LOW',
- icon: {
- toHtmlString: function () {
- return ' Smelly Dirty Smelly Dirty