Log sectionNumbers in PageIssues schema

Adds logging for the sectionNumbers field in the PageIssues schema.

Additional changes:
* createBanner now requires section number to be a string - this ensures
consistency with how these are used.
* fix a bug which meant createBanner was being called with undefined
section number (due to table of contents)
* Fix some indents in some JSDoc blocks
* Change parameter in function signature from mixed type (int or string)
to explicit string
* update schema number

Depends-On: Ia2696b86c6855d7b46a3f668585377d106d7af23
Bug: T202098
Change-Id: I20511a77258ea245f3d6fe93ade238e5df397a71

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 ]
 			} );
 		}
 	} );

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()
 				)
 			);
 

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<Schema:PageIssues> 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 );

skin.json

@@ -152,7 +152,7 @@
 		}
 	},
 	"EventLoggingSchemas": {
-		"PageIssues": 18255016
+		"PageIssues": 18326688
 	},
 	"ResourceModules": {
 		"skins.minerva.base.reset": {

tests/qunit/skins.minerva.scripts/test_pageIssues.js

@@ -13,11 +13,10 @@
 			'</div>'
 		),
 		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' );