pageIssues.js has too many responsibilities, including some parsing. This task will remove one of those concerns by excising the parsing code.
At time of writing, pageIssues.js includes a hodgepodge of parsing logic. For example:
/** * Render a banner in a containing element. * if in group B, a learn more link will be append to any amboxes inside $container * if in group A or control, any amboxes in container will be removed and a link "page issues" * will be rendered above the heading. * This function comes with side effects. It will populate a global "allIssues" object which * will link section numbers to issues. * @param {Page} page to search for page issues inside * @param {string} labelText what the label of the page issues banner should say * @param {string} section that the banner and its issues belong to. * If string KEYWORD_ALL_SECTIONS banner should apply to entire page. * @param {boolean} inline - if true the first ambox in the section will become the entry point * for the issues overlay * and if false, a link will be rendered under the heading. * @param {OverlayManager} overlayManager * @ignore * * @return {JQuery.Object} */ function createBanner( page, labelText, section, inline, overlayManager ) { var $metadata, issueUrl = section === KEYWORD_ALL_SECTIONS ? '#/issues/' + KEYWORD_ALL_SECTIONS : '#/issues/' + section, selector = 'table.ambox, table.tmbox, table.cmbox, table.fmbox', issueSummaries = []; if ( section === KEYWORD_ALL_SECTIONS ) { $metadata = page.$( selector ); } else { // find heading associated with the section $metadata = page.findChildInSectionLead( parseInt( section, 10 ), selector ); } // clean it up a little $metadata.find( '.NavFrame' ).remove(); $metadata.each( function () { var issueSummary, $this = $( this ); if ( $this.find( selector ).length === 0 ) { issueSummary = 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 ( issueSummary.text ) { issueSummaries.push( issueSummary ); } } } ); // store it for later allIssues[section] = issueSummaries; if ( inline ) { issueSummaries.forEach( function ( issueSummary, i ) { var isGrouped = issueSummary.issue.grouped, lastIssueIsGrouped = issueSummaries[ i - 1] && issueSummaries[ i - 1].issue.grouped; // only render the first grouped issue of each group if ( isGrouped && !lastIssueIsGrouped ) { createPageIssueBannerMultiple( issueSummary, mw.msg( 'skin-minerva-issue-learn-more' ), issueUrl, overlayManager ); } else { createPageIssueBanner( issueSummary, mw.msg( 'skin-minerva-issue-learn-more' ), issueUrl, overlayManager ); } } ); } else if ( issueSummaries.length ) { createPageIssueNotice( labelText, section ); } return $metadata; } /** * Obtains the list of issues for the current page and provided section * @param {number|string} section either KEYWORD_ALL_SECTIONS or a number relating to the * section the issues belong to * @return {jQuery.Object[]} array of all issues. */ function getIssues( section ) { if ( section !== KEYWORD_ALL_SECTIONS ) { return allIssues[section] || []; } // Note section.all may not exist, depending on the structure of the HTML page. // It will only exist when Minerva has been run in desktop mode. // If it's absent, we'll reduce all the other lists into one. return allIssues.all || Object.keys( allIssues ).reduce( function ( all, key ) { return all.concat( allIssues[key] ); }, [] ); } /** * 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 ( 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.grouped && issue.grouped ) { acc[ acc.length - 1 ] = section; } else { acc.push( section ); } } ); } return acc; }, [] ); }
All that weird code that's massaging data must go... Somewhere else. Maybe some or all of it can be moved to pageIssuesParser.js and / or a new parser focused on issue groups.
The new implementation should be easy to test too.
Additional discussion in this patch.
Acceptance criteria
- All parsing code is removed from pageIssues.js
- Some tests exist for the refactored code