Merge PageIssue and IssueSummary type defs

We have two type defs - IssueSummary and PageIssue.
I'd like to consolidate these two types by making
IssueSummary a combination of the two

Change-Id: Ic831b463fa66b0cacdd0b9b79aff741e55c0ec24
This commit is contained in:
jdlrobson 2018-10-16 15:46:28 -07:00
parent 13aeb1e8f5
commit 878989bd85
3 changed files with 66 additions and 49 deletions

View File

@ -2,7 +2,7 @@
{{#issues}} {{#issues}}
<li> <li>
<div class="issue-notice" data-severity="{{severity}}"> <div class="issue-notice" data-severity="{{severity}}">
{{{icon}}} {{{iconString}}}
<div class="issue-details">{{{text}}}</div> <div class="issue-details">{{{text}}}</div>
</div> </div>
</li> </li>

View File

@ -9,7 +9,6 @@
NS_TALK = 1, NS_TALK = 1,
NS_CATEGORY = 14, NS_CATEGORY = 14,
CURRENT_NS = config.get( 'wgNamespaceNumber' ), CURRENT_NS = config.get( 'wgNamespaceNumber' ),
Icon = M.require( 'mobile.startup/Icon' ),
pageIssuesLogger = M.require( 'skins.minerva.scripts/pageIssuesLogger' ), pageIssuesLogger = M.require( 'skins.minerva.scripts/pageIssuesLogger' ),
pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' ), pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' ),
PageIssuesOverlay = M.require( 'skins.minerva.scripts/PageIssuesOverlay' ), PageIssuesOverlay = M.require( 'skins.minerva.scripts/PageIssuesOverlay' ),
@ -28,9 +27,9 @@
/** /**
* @typedef {Object} IssueSummary * @typedef {Object} IssueSummary
* @prop {string} severity A PageIssue.severity. * @prop {PageIssue} issue
* @prop {Boolean} isMultiple Whether or not the issue is part of a "multiple issues" template. * @prop {string} iconString a string representation of icon.
* @prop {string} icon HTML string. * This is kept for template compatibility (our views do not yet support composition).
* @prop {string} text HTML string. * @prop {string} text HTML string.
*/ */
@ -53,16 +52,17 @@
*/ */
function formatPageIssuesSeverity( formattedArr, currentItem, currentIndex, pageIssues ) { function formatPageIssuesSeverity( formattedArr, currentItem, currentIndex, pageIssues ) {
var lastItem = pageIssues[ currentIndex - 1 ], var lastItem = pageIssues[ currentIndex - 1 ],
issue = currentItem.issue,
lastFormattedIndex = formattedArr.length - 1, lastFormattedIndex = formattedArr.length - 1,
lastFormattedValue = formattedArr[ lastFormattedIndex ]; lastFormattedValue = formattedArr[ lastFormattedIndex ];
// If the last and current item `isMultiple`, fold the maxSeverity // If the last and current item `grouped`, fold the maxSeverity
// of the two items into a single value. // of the two items into a single value.
if ( lastItem && lastItem.isMultiple && currentItem.isMultiple ) { if ( lastItem && lastItem.issue && lastItem.issue.grouped && issue.grouped ) {
formattedArr[ lastFormattedIndex ] = pageIssuesParser.maxSeverity( formattedArr[ lastFormattedIndex ] = pageIssuesParser.maxSeverity(
[ lastFormattedValue, currentItem.severity ] [ lastFormattedValue, issue.severity ]
); );
} else { } else {
formattedArr.push( currentItem.severity ); formattedArr.push( issue.severity );
} }
return formattedArr; return formattedArr;
} }
@ -93,9 +93,9 @@
pageIssue = pageIssuesParser.parse( $box.get( 0 ) ); pageIssue = pageIssuesParser.parse( $box.get( 0 ) );
return { return {
severity: pageIssue.severity, issue: pageIssue,
isMultiple: pageIssue.grouped, // For template compatibility with PageIssuesOverlay
icon: pageIssue.icon.toHtmlString(), iconString: pageIssue.icon.toHtmlString(),
text: $container.html() text: $container.html()
}; };
} }
@ -137,8 +137,7 @@
issueUrl = section === KEYWORD_ALL_SECTIONS ? '#/issues/' + KEYWORD_ALL_SECTIONS : '#/issues/' + section, issueUrl = section === KEYWORD_ALL_SECTIONS ? '#/issues/' + KEYWORD_ALL_SECTIONS : '#/issues/' + section,
selector = 'table.ambox, table.tmbox, table.cmbox, table.fmbox', selector = 'table.ambox, table.tmbox, table.cmbox, table.fmbox',
issues = [], issues = [],
$link, $link;
severity;
if ( section === KEYWORD_ALL_SECTIONS ) { if ( section === KEYWORD_ALL_SECTIONS ) {
$metadata = page.$( selector ); $metadata = page.$( selector );
@ -165,14 +164,10 @@
// store it for later // store it for later
allIssues[section] = issues; allIssues[section] = issues;
if ( $metadata.length && inline ) { // If issues were extracted and there are inline amboxes, add learn more
severity = pageIssuesParser.maxSeverity( // and icon to the UI element.
issues.map( function ( issue ) { return issue.severity; } ) if ( issues.length && $metadata.length && inline ) {
); issues[0].issue.icon.$el.prependTo( $metadata.eq( 0 ).find( '.mbox-text' ) );
new Icon( {
glyphPrefix: 'minerva',
name: pageIssuesParser.iconName( $metadata.get( 0 ), severity )
} ).prependTo( $metadata.find( '.mbox-text' ) );
$learnMore = $( '<span>' ) $learnMore = $( '<span>' )
.addClass( 'ambox-learn-more' ) .addClass( 'ambox-learn-more' )
.text( mw.msg( 'skin-minerva-issue-learn-more' ) ); .text( mw.msg( 'skin-minerva-issue-learn-more' ) );
@ -256,7 +251,7 @@
var lastIssue = allIssues[ section ][i - 1]; var lastIssue = allIssues[ section ][i - 1];
// If the last issue belongs to a "Multiple issues" template, // If the last issue belongs to a "Multiple issues" template,
// and so does the current one, don't add the current one. // 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; acc[ acc.length - 1 ] = section;
} else { } else {
acc.push( section ); acc.push( section );

View File

@ -3,32 +3,48 @@
util = M.require( 'mobile.startup/util' ), util = M.require( 'mobile.startup/util' ),
extractMessage = pageIssues.test.extractMessage, extractMessage = pageIssues.test.extractMessage,
createBanner = pageIssues.test.createBanner, createBanner = pageIssues.test.createBanner,
icon = {},
formatPageIssuesSeverity = pageIssues.test.formatPageIssuesSeverity, formatPageIssuesSeverity = pageIssues.test.formatPageIssuesSeverity,
MEDIUM_ISSUE = { MEDIUM_ISSUE = {
severity: 'MEDIUM', issue: {
icon: 'i', severity: 'MEDIUM',
icon: icon
},
iconString: 'i',
text: 't' text: 't'
}, },
MEDIUM_MULTIPLE_ISSUE = { MEDIUM_MULTIPLE_ISSUE = {
severity: 'MEDIUM', issue: {
isMultiple: true, severity: 'MEDIUM',
icon: 'i', grouped: true,
icon: icon
},
iconString: 'i',
text: 't' text: 't'
}, },
LOW_MULTIPLE_ISSUE = { LOW_MULTIPLE_ISSUE = {
severity: 'LOW', issue: {
isMultiple: true, severity: 'LOW',
icon: 'i', grouped: true,
icon: icon
},
iconString: 'i',
text: 't' text: 't'
}, },
LOW_ISSUE = { LOW_ISSUE = {
severity: 'LOW', issue: {
icon: 'i', severity: 'LOW',
icon: icon
},
iconString: 'i',
text: 't' text: 't'
}, },
HIGH_ISSUE = { HIGH_ISSUE = {
severity: 'HIGH', issue: {
icon: 'i', severity: 'HIGH',
icon: icon
},
iconString: 'i',
text: 't' text: 't'
}, },
getAllIssuesSections = pageIssues.test.getAllIssuesSections, getAllIssuesSections = pageIssues.test.getAllIssuesSections,
@ -115,24 +131,30 @@
'<div class="mbox-text">Smelly</div>' '<div class="mbox-text">Smelly</div>'
).appendTo( '<div class="mw-collapsible-content" />' ), ).appendTo( '<div class="mw-collapsible-content" />' ),
{ {
severity: 'DEFAULT', issue: {
isMultiple: true, severity: 'DEFAULT',
icon: this.sandbox.match.typeOf( 'string' ), grouped: true,
icon: icon
},
iconString: this.sandbox.match.typeOf( 'string' ),
text: '<p>Smelly</p>' text: '<p>Smelly</p>'
}, },
'When the box is a child of mw-collapsible-content it isMultiple' 'When the box is a child of mw-collapsible-content it grouped'
], ],
[ [
$( '<div />' ).html( $( '<div />' ).html(
'<div class="mbox-text">Dirty</div>' '<div class="mbox-text">Dirty</div>'
), ),
{ {
severity: 'DEFAULT', issue: {
isMultiple: false, severity: 'DEFAULT',
icon: this.sandbox.match.typeOf( 'string' ), grouped: false,
icon: icon
},
iconString: this.sandbox.match.typeOf( 'string' ),
text: '<p>Dirty</p>' text: '<p>Dirty</p>'
}, },
'When the box is not child of mw-collapsible-content it !isMultiple' 'When the box is not child of mw-collapsible-content it !grouped'
] ]
].forEach( function ( test ) { ].forEach( function ( test ) {
sinon.assert.match( // eslint-disable-line no-undef sinon.assert.match( // eslint-disable-line no-undef
@ -155,17 +177,17 @@
}; };
multipleIssues = { multipleIssues = {
0: [ 0: [
util.extend( {}, MEDIUM_ISSUE, { isMultiple: true } ), util.extend( {}, MEDIUM_ISSUE, { grouped: true } ),
util.extend( {}, LOW_ISSUE, { isMultiple: true } ), util.extend( {}, LOW_ISSUE, { grouped: true } ),
util.extend( {}, MEDIUM_ISSUE, { isMultiple: true } ) util.extend( {}, MEDIUM_ISSUE, { grouped: true } )
] ]
}; };
multipleIssuesWithDeletion = { multipleIssuesWithDeletion = {
0: [ 0: [
HIGH_ISSUE, HIGH_ISSUE,
util.extend( {}, MEDIUM_ISSUE, { isMultiple: true } ), util.extend( {}, MEDIUM_ISSUE, { grouped: true } ),
util.extend( {}, LOW_ISSUE, { isMultiple: true } ), util.extend( {}, LOW_ISSUE, { grouped: true } ),
util.extend( {}, MEDIUM_ISSUE, { isMultiple: true } ) util.extend( {}, MEDIUM_ISSUE, { grouped: true } )
] ]
}; };
allIssuesNewTreatment = { allIssuesNewTreatment = {