For page-issues pageLoaded and editClicked events, treat

"multiple issues" templates as one issue.

When logging the `issuesSeverity` and `sectionNumbers` field,
any issues that are part of a "multiple issues" template only send
one value.

Adds an `isMultiple` property to IssueSummary to determine which
issues are part of a multiple-issues template.

Bug: T203050
Change-Id: I7d55dfead72439df4accadcdc8623a080e1321c2
This commit is contained in:
Jan Drewniak 2018-08-29 14:59:37 +02:00 committed by jdlrobson
parent 2cbd57c2f3
commit 815f3d99ee
2 changed files with 113 additions and 10 deletions

View file

@ -26,6 +26,7 @@
/**
* @typedef {Object} IssueSummary
* @prop {string} severity A PageIssue.severity.
* @prop {Boolean} isMultiple Whether or not the issue is part of a "multiple issues" template.
* @prop {string} icon HTML string.
* @prop {string} text HTML string.
*/
@ -36,11 +37,27 @@
}
/**
* @param {IssueSummary} summary
* @return {string}
* Array.reduce callback that returns the severity of page issues.
* In the case that a page-issue is part of a "multiple issues" template,
* returns the maximum severity for that group of issues.
*
* @param {array} acc - the return array containing severities
* @param {IssueSummary} summary current IssueSummary object
* @param {number} currentIndex current index of pageIssues
* @param {array} pageIssues array of pageIssues
*
* @return {array} acc
*/
function formatPageIssuesSeverity( summary ) {
return summary.severity;
function formatPageIssuesSeverity( acc, summary, currentIndex, pageIssues ) {
var lastItem = pageIssues[ currentIndex - 1 ];
if ( lastItem && lastItem.isMultiple && summary.isMultiple ) {
acc[ acc.length - 1 ] = pageIssuesParser.maxSeverity(
[ lastItem.severity, summary.severity ]
);
} else {
acc.push( summary.severity );
}
return acc;
}
/**
@ -50,11 +67,12 @@
* @return {IssueSummary}
*/
function extractMessage( $box ) {
var selector = '.mbox-text, .ambox-text',
var SELECTOR = '.mbox-text, .ambox-text',
MULTIPLE_SELECTOR = '.mw-collapsible-content',
$container = $( '<div>' ),
pageIssue;
$box.find( selector ).each( function () {
$box.find( SELECTOR ).each( function () {
var contents,
$this = $( this );
// Clean up talk page boxes
@ -70,6 +88,7 @@
return {
severity: pageIssue.severity,
isMultiple: $box.parent().is( MULTIPLE_SELECTOR ),
icon: pageIssue.icon.toHtmlString(),
text: $container.html()
};
@ -216,14 +235,23 @@
/**
* Returns an array containing the section of each page issue.
* In the case that several page issues are grouped in a 'multiple issues' template,
* returns the section of those issues as one item.
* @param {Object} allIssues mapping section {Number} to {IssueSummary}
* @return {array}
*/
function getAllIssuesSections( allIssues ) {
return Object.keys( allIssues ).reduce( function ( acc, section ) {
if ( allIssues[ section ].length ) {
allIssues[ section ].forEach( function () {
acc.push( section );
allIssues[ section ].forEach( function ( issue, i ) {
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 ) {
acc[ acc.length - 1 ] = section;
} else {
acc.push( section );
}
} );
}
return acc;
@ -282,7 +310,7 @@
pageIssuesLogger.newPageIssueSchemaData(
newTreatmentEnabled,
CURRENT_NS,
getIssues( KEYWORD_ALL_SECTIONS ).map( formatPageIssuesSeverity ),
getIssues( KEYWORD_ALL_SECTIONS ).reduce( formatPageIssuesSeverity, [] ),
getAllIssuesSections( allIssues )
)
);
@ -306,6 +334,7 @@
// the subscription call in cleanuptemplates.
log: pageIssuesLogger.log,
test: {
extractMessage: extractMessage,
getAllIssuesSections: getAllIssuesSections,
createBanner: createBanner
}

View file

@ -1,5 +1,8 @@
( 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,
MEDIUM_ISSUE = {
severity: 'MEDIUM',
@ -61,8 +64,54 @@
} );
} );
QUnit.test( 'extractMessage', function ( assert ) {
this.sandbox.stub( pageIssuesParser, 'parse' ).returns(
{
severity: 'LOW',
icon: {
toHtmlString: function () {
return '<icon />';
}
}
}
);
[
[
$( '<div />' ).html(
'<div class="mbox-text">Smelly</div>'
).appendTo( '<div class="mw-collapsible-content" />' ),
{
severity: 'LOW',
isMultiple: true,
icon: '<icon />',
text: '<p>Smelly</p>'
},
'When the box is a child of mw-collapsible-content it isMultiple'
],
[
$( '<div />' ).html(
'<div class="mbox-text">Dirty</div>'
),
{
severity: 'LOW',
isMultiple: false,
icon: '<icon />',
text: '<p>Dirty</p>'
},
'When the box is not child of mw-collapsible-content it !isMultiple'
]
].forEach( function ( test ) {
assert.deepEqual(
extractMessage( test[ 0 ] ),
test[ 1 ],
test[ 2 ]
);
} );
} );
QUnit.test( 'getAllIssuesSections', function ( assert ) {
var allIssuesOldTreatment, allIssuesNewTreatment;
var multipleIssuesWithDeletion,
multipleIssues, allIssuesOldTreatment, allIssuesNewTreatment;
allIssuesOldTreatment = {
0: [
MEDIUM_ISSUE,
@ -70,6 +119,21 @@
MEDIUM_ISSUE
]
};
multipleIssues = {
0: [
util.extend( {}, MEDIUM_ISSUE, { isMultiple: true } ),
util.extend( {}, LOW_ISSUE, { isMultiple: true } ),
util.extend( {}, MEDIUM_ISSUE, { isMultiple: true } )
]
};
multipleIssuesWithDeletion = {
0: [
HIGH_ISSUE,
util.extend( {}, MEDIUM_ISSUE, { isMultiple: true } ),
util.extend( {}, LOW_ISSUE, { isMultiple: true } ),
util.extend( {}, MEDIUM_ISSUE, { isMultiple: true } )
]
};
allIssuesNewTreatment = {
0: [
HIGH_ISSUE,
@ -90,5 +154,15 @@
[ '0', '0', '0', '1' ],
'section numbers correctly extracted from new treatment'
);
assert.deepEqual(
getAllIssuesSections( multipleIssues ),
[ '0' ],
'multiple issues are packed into one entry since there is one box'
);
assert.deepEqual(
getAllIssuesSections( multipleIssuesWithDeletion ),
[ '0', '0' ],
'while multiple issues are grouped, non-multiple issues are still reported'
);
} );
}( mw.mobileFrontend ) );