Don't send NaN as a value for sectionNumbers

The keyword "all" was being parsed as an integer incorrectly. To avoid this

Bug: T202940
Change-Id: I5553a4bb50cd7639c879f2c6e812fba25a216175

includes/MinervaHooks.php

@@ -94,6 +94,7 @@ class MinervaHooks {
 				'tests/qunit/skins.minerva.scripts/test_DownloadIcon.js',
 				'tests/qunit/skins.minerva.scripts/test_pageIssuesParser.js',
 				'tests/qunit/skins.minerva.scripts/test_AB.js',
+				'tests/qunit/skins.minerva.scripts/test_PageIssuesOverlay.js',
 				'tests/qunit/skins.minerva.scripts/test_pageIssues.js',
 				'tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js'
 			],

resources/skins.minerva.scripts/PageIssuesOverlay.js

@@ -32,6 +32,14 @@
 
 		options = {};
 		options.issues = issues;
+
+		// Set default logging data
+		this.defaultLoggerData = {};
+		// In the case of KEYWORD_ALL_SECTIONS all issues are in the overlay and the sectionNumbers
+		// field should be no different from the default behaviour.
+		if ( this.section !== KEYWORD_ALL_SECTIONS ) {
+			this.defaultLoggerData.sectionNumbers = [ this.section ];
+		}
 		options.heading = '<strong>' + headingText + '</strong>';
 		Overlay.call( this, options );
 
@@ -62,15 +70,24 @@
 			content: mw.template.get( 'skins.minerva.scripts', 'PageIssuesOverlayContent.hogan' )
 		} ),
 
+		/**
+		 * Log data via the associated logger, adding sectionNumbers to override the event default
+		 * if applicable.
+		 * @param {Object} data
+		 * @instance
+		 */
+		log: function ( data ) {
+			this.logger.log( util.extend( {}, this.defaultLoggerData, data ) );
+		},
+
 		/**
 		 * Note: an "on enter" state is tracked by the issueClicked log event.
 		 * @return {void}
 		 */
 		onExit: function () {
-			this.logger.log( {
+			this.log( {
 				action: 'modalClose',
-				issuesSeverity: this.issues.map( issueSummaryToSeverity ),
-				sectionNumbers: [ this.section ]
+				issuesSeverity: this.issues.map( issueSummaryToSeverity )
 			} );
 		},
 
@@ -85,10 +102,9 @@
 		 */
 		onInternalClick: function ( ev ) {
 			var severity = parseSeverity( this.$( ev.target ) );
-			this.logger.log( {
+			this.log( {
 				action: 'modalInternalClicked',
-				issuesSeverity: [ severity ],
-				sectionNumbers: [ this.section ]
+				issuesSeverity: [ severity ]
 			} );
 		},
 
@@ -101,10 +117,9 @@
 		 */
 		onEditClick: function ( ev ) {
 			var severity = parseSeverity( this.$( ev.target ) );
-			this.logger.log( {
+			this.log( {
 				action: 'modalEditClicked',
-				issuesSeverity: [ severity ],
-				sectionNumbers: [ this.section ]
+				issuesSeverity: [ severity ]
 			} );
 		}
 	} );

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

@@ -0,0 +1,36 @@
+( function ( M ) {
+	var PageIssuesOverlay = M.require( 'skins.minerva.scripts/PageIssuesOverlay' );
+
+	QUnit.module( 'Minerva PageIssuesOverlay', {
+		setup: function () {
+			this.logger = {
+				log: this.sandbox.spy()
+			};
+		}
+	} );
+
+	QUnit.test( '#log (section=all)', function ( assert ) {
+		var overlay = new PageIssuesOverlay( [], this.logger, 'all', 0 );
+		overlay.onExit();
+		assert.strictEqual( this.logger.log.calledOnce, true, 'Logger called once' );
+		assert.strictEqual(
+			this.logger.log.calledWith( {
+				action: 'modalClose',
+				issuesSeverity: []
+			} ), true, 'sectionNumbers is not set (T202940)'
+		);
+	} );
+
+	QUnit.test( '#log (section=1)', function ( assert ) {
+		var overlay = new PageIssuesOverlay( [], this.logger, '1', 0 );
+		overlay.onExit();
+		assert.strictEqual(
+			this.logger.log.calledWith( {
+				action: 'modalClose',
+				issuesSeverity: [],
+				sectionNumbers: [ '1' ]
+			} ), true, 'sectionNumbers is set'
+		);
+	} );
+
+}( mw.mobileFrontend ) );