Hygiene: move page issue group parsing to parser

Separate the page issue grouping concern so that changes to parsing
don't concern everything else and vice-versa.

Bug: T203449, T202349
Change-Id: I7bddb0c53310805ece71b8f7821b1d6ce05cfae9
This commit is contained in:
Stephen Niedzielski 2018-09-06 12:39:58 -06:00 committed by Niedzielski
parent ed5cdcee1f
commit f07985c6de
4 changed files with 46 additions and 20 deletions

View File

@ -75,7 +75,6 @@
*/
function extractMessage( $box ) {
var SELECTOR = '.mbox-text, .ambox-text',
MULTIPLE_SELECTOR = '.mw-collapsible-content',
$container = $( '<div>' ),
pageIssue;
@ -95,7 +94,7 @@
return {
severity: pageIssue.severity,
isMultiple: $box.parent().is( MULTIPLE_SELECTOR ),
isMultiple: pageIssue.grouped,
icon: pageIssue.icon.toHtmlString(),
text: $container.html()
};

View File

@ -2,6 +2,7 @@
/**
* @typedef PageIssue
* @prop {string} severity A SEVERITY_LEVEL key.
* @prop {boolean} grouped True if part of a group of multiple issues, false if singular.
* @prop {Icon} icon
*/
@ -78,6 +79,7 @@
].join( '|' ) )
// ..And everything else that doesn't match is mapped to a "SEVERITY" type.
},
GROUPED_PARENT_REGEX = /mw-collapsible-content/,
// Variants supported by specific types. The "severity icon" supports all severities but the
// type icons only support one each by ResourceLoader.
TYPE_SEVERITY = {
@ -117,6 +119,14 @@
};
}
/**
* @param {Element} box
* @return {boolean} True if part of a group of multiple issues, false if singular.
*/
function parseGroup( box ) {
return !!box.parentNode && GROUPED_PARENT_REGEX.test( box.parentNode.className );
}
/**
* @param {Element} box
* @param {string} severity An SEVERITY_LEVEL key.
@ -147,6 +157,7 @@
var severity = parseSeverity( box );
return {
severity: severity,
grouped: parseGroup( box ),
icon: new Icon( {
glyphPrefix: 'minerva',
name: iconName( box, severity )
@ -168,7 +179,8 @@
iconName: iconName,
test: {
parseSeverity: parseSeverity,
parseType: parseType
parseType: parseType,
parseGroup: parseGroup
}
} );

View File

@ -1,7 +1,6 @@
( 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,
@ -109,26 +108,16 @@
} );
QUnit.test( 'extractMessage', function ( assert ) {
this.sandbox.stub( pageIssuesParser, 'parse' ).returns(
{
severity: 'LOW',
icon: {
toHtmlString: function () {
return '<icon />';
}
}
}
);
QUnit.test( 'extractMessage', function () {
[
[
$( '<div />' ).html(
'<div class="mbox-text">Smelly</div>'
).appendTo( '<div class="mw-collapsible-content" />' ),
{
severity: 'LOW',
severity: 'DEFAULT',
isMultiple: true,
icon: '<icon />',
icon: this.sandbox.match.typeOf( 'string' ),
text: '<p>Smelly</p>'
},
'When the box is a child of mw-collapsible-content it isMultiple'
@ -138,15 +127,15 @@
'<div class="mbox-text">Dirty</div>'
),
{
severity: 'LOW',
severity: 'DEFAULT',
isMultiple: false,
icon: '<icon />',
icon: this.sandbox.match.typeOf( 'string' ),
text: '<p>Dirty</p>'
},
'When the box is not child of mw-collapsible-content it !isMultiple'
]
].forEach( function ( test ) {
assert.deepEqual(
sinon.assert.match( // eslint-disable-line no-undef
extractMessage( test[ 0 ] ),
test[ 1 ],
test[ 2 ]

View File

@ -70,6 +70,32 @@
} );
} );
QUnit.test( 'parseGroup', function ( assert ) {
var tests = [
[ undefined, false, 'orphaned' ],
[ '', false, 'ungrouped' ],
[ 'mw-collapsible-content', true, 'grouped' ]
];
tests.forEach( function ( params, i ) {
var
parentClassName = params[0],
expect = params[1],
test = params[2],
parent,
box = newBox( '' );
if ( parentClassName !== undefined ) {
parent = document.createElement( 'div' );
parent.className = parentClassName;
parent.appendChild( box );
}
assert.strictEqual(
pageIssuesParser.test.parseGroup( box ),
expect,
'Result should be the correct grouping; case ' + i + ': ' + test + '.'
);
} );
} );
QUnit.test( 'iconName', function ( assert ) {
var tests = [
[ '', 'DEFAULT', 'issue-generic-defaultColor' ],