diff --git a/resources/skins.minerva.scripts/PageIssuesOverlay.js b/resources/skins.minerva.scripts/PageIssuesOverlay.js index f4d6dbb..f85b43f 100644 --- a/resources/skins.minerva.scripts/PageIssuesOverlay.js +++ b/resources/skins.minerva.scripts/PageIssuesOverlay.js @@ -28,6 +28,7 @@ this.issues = issues; this.logger = logger; + this.section = section; options = {}; options.issues = issues; @@ -68,7 +69,8 @@ onExit: function () { this.logger.log( { action: 'modalClose', - issuesSeverity: this.issues.map( issueSummaryToSeverity ) + issuesSeverity: this.issues.map( issueSummaryToSeverity ), + sectionNumbers: [ this.section ] } ); }, @@ -85,7 +87,8 @@ var severity = parseSeverity( this.$( ev.target ) ); this.logger.log( { action: 'modalInternalClicked', - issuesSeverity: [ severity ] + issuesSeverity: [ severity ], + sectionNumbers: [ this.section ] } ); }, @@ -100,7 +103,8 @@ var severity = parseSeverity( this.$( ev.target ) ); this.logger.log( { action: 'modalEditClicked', - issuesSeverity: [ severity ] + issuesSeverity: [ severity ], + sectionNumbers: [ this.section ] } ); } } ); diff --git a/resources/skins.minerva.scripts/pageIssues.js b/resources/skins.minerva.scripts/pageIssues.js index bf41bab..d64d286 100644 --- a/resources/skins.minerva.scripts/pageIssues.js +++ b/resources/skins.minerva.scripts/pageIssues.js @@ -97,7 +97,7 @@ * 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 {number|string} section that the banner and its issues belong to. + * @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. @@ -161,20 +161,26 @@ var pageIssue = pageIssuesParser.parse( this ); pageIssuesLogger.log( { action: 'issueClicked', - issuesSeverity: [ pageIssue.severity ] + issuesSeverity: [ pageIssue.severity ], + sectionNumbers: [ section ] } ); overlayManager.router.navigate( issueUrl ); return false; } ); } else { $link = createLinkElement( labelText ); - // In group B, we link to all issues no matter where the banner is. + // In group A, we link to all issues no matter where the banner is. $link.attr( 'href', '#/issues/' + KEYWORD_ALL_SECTIONS ); $link.click( function () { pageIssuesLogger.log( { action: 'issueClicked', - // empty array is passed for 'old' treatment. - issuesSeverity: [] + issuesSeverity: [ + pageIssuesParser.maxSeverity( + getIssues( '0' ) + .map( function ( issue ) { return issue.severity; } ) + ) + ], + sectionNumbers: getAllIssuesSections() } ); } ); if ( $metadata.length ) { @@ -208,6 +214,16 @@ } } + /** + * Returns an array of all the page sections that have issues. + * @return {array} + */ + function getAllIssuesSections() { + return Object.keys( allIssues ).filter( function ( section ) { + return allIssues[ section ].length; + } ); + } + /** * Scan an element for any known cleanup templates and replace them with a button * that opens them in a mobile friendly overlay. @@ -234,15 +250,20 @@ createBanner( page, label, KEYWORD_ALL_SECTIONS, inline, overlayManager ); } else { // parse lead - createBanner( page, label, 0, inline, overlayManager ); + createBanner( page, label, '0', inline, overlayManager ); if ( newTreatmentEnabled ) { // parse other sections but only in group B. In treatment A no issues are shown for sections. page.$( Page.HEADING_SELECTOR ).each( function ( i, headingEl ) { var $headingEl = $( headingEl ), sectionNum = $headingEl.find( '.edit-page' ).data( 'section' ); - // Render banner for sectionNum associated with headingEl inside Page - createBanner( page, label, sectionNum, inline, overlayManager ); + // Note certain headings matched using Page.HEADING_SELECTOR may not be headings + // and will not have a edit link + // e.g. table of contents + if ( sectionNum ) { + // Render banner for sectionNum associated with headingEl inside Page + createBanner( page, label, sectionNum.toString(), inline, overlayManager ); + } } ); } } @@ -255,7 +276,8 @@ pageIssuesLogger.newPageIssueSchemaData( newTreatmentEnabled, CURRENT_NS, - getIssues( KEYWORD_ALL_SECTIONS ).map( formatPageIssuesSeverity ) + getIssues( KEYWORD_ALL_SECTIONS ).map( formatPageIssuesSeverity ), + getAllIssuesSections() ) ); diff --git a/resources/skins.minerva.scripts/pageIssuesLogger.js b/resources/skins.minerva.scripts/pageIssuesLogger.js index a5f141d..023725f 100644 --- a/resources/skins.minerva.scripts/pageIssuesLogger.js +++ b/resources/skins.minerva.scripts/pageIssuesLogger.js @@ -8,15 +8,18 @@ * @param {boolean} newTreatmentEnabled * @param {number} namespaceId The namespace for the page that has issues. * @param {string[]} pageIssueSeverities An array of PageIssue severities. + * @param {array} pageIssuesSections + * * @return {Object} A Partial Object meant to be mixed with track data. */ - function newPageIssueSchemaData( newTreatmentEnabled, namespaceId, pageIssueSeverities ) { + function newPageIssueSchemaData( newTreatmentEnabled, namespaceId, pageIssueSeverities, pageIssuesSections ) { return { pageTitle: mwConfig.get( 'wgTitle' ), namespaceId: namespaceId, pageIdSource: mwConfig.get( 'wgArticleId' ), issuesVersion: bucketToVersion( newTreatmentEnabled ), issuesSeverity: pageIssueSeverities, + sectionNumbers: pageIssuesSections, isAnon: mwUser.isAnon(), editCountBucket: getUserEditBuckets(), sessionToken: mwUser.sessionId() @@ -37,6 +40,13 @@ // intermediary event bus that extends the event data before being passed to event-logging. mwTrackSubscribe( EVENT_PAGE_ISSUE_LOG, function ( topic, data ) { var mixedData = util.extend( {}, pageIssueSchemaData, data ); + + // Although we use strings inside the issues code (due to the usage of the key word + // `all`) - these need to be numbers to be validated by the schema + mixedData.sectionNumbers = ( mixedData.sectionNumbers || [] ).map( function ( sectionStr ) { + return parseInt( sectionStr, 10 ); + } ); + // Log readingDepth schema.(ReadingDepth is guarded against multiple enables). // See https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/WikimediaEvents/+/437686/ mwTrack( 'wikimedia.event.ReadingDepthSchema.enable', bucketToGroup( newTreatmentEnabled ) ); @@ -83,9 +93,9 @@ /** * Log data to the PageIssuesAB test schema. It's safe to call this function prior to - * subscription. + * subscription. * @param {Object} data to log - * @return {void} + * @return {void} */ function log( data ) { mwTrack( EVENT_PAGE_ISSUE_LOG, data ); diff --git a/skin.json b/skin.json index 5234d80..cc9061c 100644 --- a/skin.json +++ b/skin.json @@ -152,7 +152,7 @@ } }, "EventLoggingSchemas": { - "PageIssues": 18255016 + "PageIssues": 18326688 }, "ResourceModules": { "skins.minerva.base.reset": { diff --git a/tests/qunit/skins.minerva.scripts/test_pageIssues.js b/tests/qunit/skins.minerva.scripts/test_pageIssues.js index 4f83341..878ecf4 100644 --- a/tests/qunit/skins.minerva.scripts/test_pageIssues.js +++ b/tests/qunit/skins.minerva.scripts/test_pageIssues.js @@ -13,11 +13,10 @@ '' ), labelText = 'label text', - section = 0, inline = true, processedAmbox = createBanner( new Page( { el: $mockContainer } ), - labelText, section, inline, overlayManager + labelText, '0', inline, overlayManager ); QUnit.module( 'Minerva cleanuptemplates' );