From cb2ad9374efcb95d0085bfbb8a75f9f59146f79d Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Tue, 16 Oct 2018 16:02:22 -0700 Subject: [PATCH] extract moved to parser The extractMessage function has a lot to do with parsing - so this and its tests are moved into the pageIssuesParser. Change-Id: I62d79fbba166eff2c3ca573ef94ff86a269a7f9a --- resources/skins.minerva.scripts/pageIssues.js | 46 +------------------ .../skins.minerva.scripts/pageIssuesParser.js | 46 +++++++++++++++++-- .../skins.minerva.scripts/pageIssues.test.js | 42 ----------------- .../pageIssuesParser.test.js | 45 +++++++++++++++++- 4 files changed, 87 insertions(+), 92 deletions(-) diff --git a/resources/skins.minerva.scripts/pageIssues.js b/resources/skins.minerva.scripts/pageIssues.js index 284db03..23f7749 100644 --- a/resources/skins.minerva.scripts/pageIssues.js +++ b/resources/skins.minerva.scripts/pageIssues.js @@ -25,14 +25,6 @@ isUserRequestingNewTreatment = QUERY_STRING_FLAG === 'b', newTreatmentEnabled = abTest.isB() || isUserRequestingNewTreatment; - /** - * @typedef {Object} IssueSummary - * @prop {PageIssue} issue - * @prop {string} iconString a string representation of icon. - * This is kept for template compatibility (our views do not yet support composition). - * @prop {string} text HTML string. - */ - function isLoggingRequired( pageIssues ) { // No logging necessary when the A/B test is disabled (control group). return !isUserRequestingNewTreatment && abTest.isEnabled() && pageIssues.length; @@ -67,39 +59,6 @@ return formattedArr; } - /** - * Extract a summary message from a cleanup template generated element that is - * friendly for mobile display. - * @param {Object} $box element to extract the message from - * @return {IssueSummary} - */ - function extractMessage( $box ) { - var SELECTOR = '.mbox-text, .ambox-text', - $container = $( '
' ), - pageIssue; - - $box.find( SELECTOR ).each( function () { - var contents, - $this = $( this ); - // Clean up talk page boxes - $this.find( 'table, .noprint' ).remove(); - contents = $this.html(); - - if ( contents ) { - $( '

' ).html( contents ).appendTo( $container ); - } - } ); - - pageIssue = pageIssuesParser.parse( $box.get( 0 ) ); - - return { - issue: pageIssue, - // For template compatibility with PageIssuesOverlay - iconString: pageIssue.icon.toHtmlString(), - text: $container.html() - }; - } - /** * Create a link element that opens the issues overlay. * @@ -152,8 +111,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 ) { @@ -343,7 +302,6 @@ log: pageIssuesLogger.log, test: { formatPageIssuesSeverity: formatPageIssuesSeverity, - extractMessage: extractMessage, getAllIssuesSections: getAllIssuesSections, createBanner: createBanner } diff --git a/resources/skins.minerva.scripts/pageIssuesParser.js b/resources/skins.minerva.scripts/pageIssuesParser.js index 870c08d..cbf85bc 100644 --- a/resources/skins.minerva.scripts/pageIssuesParser.js +++ b/resources/skins.minerva.scripts/pageIssuesParser.js @@ -5,6 +5,13 @@ * @prop {boolean} grouped True if part of a group of multiple issues, false if singular. * @prop {Icon} icon */ + /** + * @typedef {Object} IssueSummary + * @prop {PageIssue} issue + * @prop {string} iconString a string representation of icon. + * This is kept for template compatibility (our views do not yet support composition). + * @prop {string} text HTML string. + */ var Icon = M.require( 'mobile.startup/Icon' ), // Icons are matching the type selector below use a TYPE_* icon. When unmatched, the icon is @@ -165,15 +172,44 @@ }; } + /** + * Extract a summary message from a cleanup template generated element that is + * friendly for mobile display. + * @param {Object} $box element to extract the message from + * @return {IssueSummary} + */ + function extract( $box ) { + var SELECTOR = '.mbox-text, .ambox-text', + $container = $( '

' ), + pageIssue; + + $box.find( SELECTOR ).each( function () { + var contents, + $this = $( this ); + // Clean up talk page boxes + $this.find( 'table, .noprint' ).remove(); + contents = $this.html(); + + if ( contents ) { + $( '

' ).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, diff --git a/tests/qunit/skins.minerva.scripts/pageIssues.test.js b/tests/qunit/skins.minerva.scripts/pageIssues.test.js index 0a3b38d..52aaf53 100644 --- a/tests/qunit/skins.minerva.scripts/pageIssues.test.js +++ b/tests/qunit/skins.minerva.scripts/pageIssues.test.js @@ -1,7 +1,6 @@ ( function ( M ) { var pageIssues = M.require( 'skins.minerva.scripts/pageIssues' ), util = M.require( 'mobile.startup/util' ), - extractMessage = pageIssues.test.extractMessage, createBanner = pageIssues.test.createBanner, icon = {}, formatPageIssuesSeverity = pageIssues.test.formatPageIssuesSeverity, @@ -124,47 +123,6 @@ } ); - QUnit.test( 'extractMessage', function () { - [ - [ - $( '

' ).html( - '
Smelly
' - ).appendTo( '
' ), - { - issue: { - severity: 'DEFAULT', - grouped: true, - icon: icon - }, - iconString: this.sandbox.match.typeOf( 'string' ), - text: '

Smelly

' - }, - 'When the box is a child of mw-collapsible-content it grouped' - ], - [ - $( '
' ).html( - '
Dirty
' - ), - { - issue: { - severity: 'DEFAULT', - grouped: false, - icon: icon - }, - iconString: this.sandbox.match.typeOf( 'string' ), - text: '

Dirty

' - }, - 'When the box is not child of mw-collapsible-content it !grouped' - ] - ].forEach( function ( test ) { - sinon.assert.match( // eslint-disable-line no-undef - extractMessage( test[ 0 ] ), - test[ 1 ], - test[ 2 ] - ); - } ); - } ); - QUnit.test( 'getAllIssuesSections', function ( assert ) { var multipleIssuesWithDeletion, multipleIssues, allIssuesOldTreatment, allIssuesNewTreatment; diff --git a/tests/qunit/skins.minerva.scripts/pageIssuesParser.test.js b/tests/qunit/skins.minerva.scripts/pageIssuesParser.test.js index 85ca9f6..367671f 100644 --- a/tests/qunit/skins.minerva.scripts/pageIssuesParser.test.js +++ b/tests/qunit/skins.minerva.scripts/pageIssuesParser.test.js @@ -1,5 +1,7 @@ ( function ( M ) { - var pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' ); + var icon = {}, + pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' ), + extractMessage = pageIssuesParser.extract; QUnit.module( 'Minerva pageIssuesParser' ); @@ -13,6 +15,47 @@ return box; } + QUnit.test( 'extractMessage', function () { + [ + [ + $( '
' ).html( + '
Smelly
' + ).appendTo( '
' ), + { + issue: { + severity: 'DEFAULT', + grouped: true, + icon: icon + }, + iconString: this.sandbox.match.typeOf( 'string' ), + text: '

Smelly

' + }, + 'When the box is a child of mw-collapsible-content it grouped' + ], + [ + $( '
' ).html( + '
Dirty
' + ), + { + issue: { + severity: 'DEFAULT', + grouped: false, + icon: icon + }, + iconString: this.sandbox.match.typeOf( 'string' ), + text: '

Dirty

' + }, + 'When the box is not child of mw-collapsible-content it !grouped' + ] + ].forEach( function ( test ) { + sinon.assert.match( // eslint-disable-line no-undef + extractMessage( test[ 0 ] ), + test[ 1 ], + test[ 2 ] + ); + } ); + } ); + QUnit.test( 'parseSeverity', function ( assert ) { var tests = [ [ '', 'DEFAULT', 'empty' ],