From 2cbd57c2f3ad4e5cbe0a6c7f6022b23021bf94ec Mon Sep 17 00:00:00 2001
From: Jan Drewniak <jan.drewniak@gmail.com>
Date: Wed, 29 Aug 2018 11:45:04 +0200
Subject: [PATCH] Consistent sectionNumbers and issuesSeverity for page-issues.

The value of `sectionNumbers` should be the section number of each issue
Not the number of the sections that have issues.

Bug: T203050
Change-Id: I6fd55c35b9e2ce35894259f36d1a50fb5dca5e43
---
 resources/skins.minerva.scripts/pageIssues.js | 21 +++++---
 .../skins.minerva.scripts/test_pageIssues.js  | 50 ++++++++++++++++++-
 2 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/resources/skins.minerva.scripts/pageIssues.js b/resources/skins.minerva.scripts/pageIssues.js
index d64d286..0ea0a18 100644
--- a/resources/skins.minerva.scripts/pageIssues.js
+++ b/resources/skins.minerva.scripts/pageIssues.js
@@ -180,7 +180,7 @@
 								.map( function ( issue ) { return issue.severity; } )
 						)
 					],
-					sectionNumbers: getAllIssuesSections()
+					sectionNumbers: getAllIssuesSections( allIssues )
 				} );
 			} );
 			if ( $metadata.length ) {
@@ -215,13 +215,19 @@
 	}
 
 	/**
-	 * Returns an array of all the page sections that have issues.
+	 * Returns an array containing the section of each page issue.
+	 * @param {Object} allIssues mapping section {Number} to {IssueSummary}
 	 * @return {array}
 	 */
-	function getAllIssuesSections() {
-		return Object.keys( allIssues ).filter( function ( section ) {
-			return allIssues[ section ].length;
-		} );
+	function getAllIssuesSections( allIssues ) {
+		return Object.keys( allIssues ).reduce( function ( acc, section ) {
+			if ( allIssues[ section ].length ) {
+				allIssues[ section ].forEach( function () {
+					acc.push( section );
+				} );
+			}
+			return acc;
+		}, [] );
 	}
 
 	/**
@@ -277,7 +283,7 @@
 					newTreatmentEnabled,
 					CURRENT_NS,
 					getIssues( KEYWORD_ALL_SECTIONS ).map( formatPageIssuesSeverity ),
-					getAllIssuesSections()
+					getAllIssuesSections( allIssues )
 				)
 			);
 
@@ -300,6 +306,7 @@
 		// the subscription call in cleanuptemplates.
 		log: pageIssuesLogger.log,
 		test: {
+			getAllIssuesSections: getAllIssuesSections,
 			createBanner: createBanner
 		}
 	} );
diff --git a/tests/qunit/skins.minerva.scripts/test_pageIssues.js b/tests/qunit/skins.minerva.scripts/test_pageIssues.js
index 878ecf4..a7ed350 100644
--- a/tests/qunit/skins.minerva.scripts/test_pageIssues.js
+++ b/tests/qunit/skins.minerva.scripts/test_pageIssues.js
@@ -1,5 +1,22 @@
 ( function ( M ) {
-	var createBanner = M.require( 'skins.minerva.scripts/pageIssues' ).test.createBanner,
+	var pageIssues = M.require( 'skins.minerva.scripts/pageIssues' ),
+		createBanner = pageIssues.test.createBanner,
+		MEDIUM_ISSUE = {
+			severity: 'MEDIUM',
+			icon: 'i',
+			text: 't'
+		},
+		LOW_ISSUE = {
+			severity: 'LOW',
+			icon: 'i',
+			text: 't'
+		},
+		HIGH_ISSUE = {
+			severity: 'HIGH',
+			icon: 'i',
+			text: 't'
+		},
+		getAllIssuesSections = pageIssues.test.getAllIssuesSections,
 		OverlayManager = M.require( 'mobile.startup/OverlayManager' ),
 		Page = M.require( 'mobile.startup/Page' ),
 		overlayManager = new OverlayManager( require( 'mediawiki.router' ) ),
@@ -43,4 +60,35 @@
 			assert.equal( JSON.toString( mockAction ), JSON.toString( data ) );
 		} );
 	} );
+
+	QUnit.test( 'getAllIssuesSections', function ( assert ) {
+		var allIssuesOldTreatment, allIssuesNewTreatment;
+		allIssuesOldTreatment = {
+			0: [
+				MEDIUM_ISSUE,
+				LOW_ISSUE,
+				MEDIUM_ISSUE
+			]
+		};
+		allIssuesNewTreatment = {
+			0: [
+				HIGH_ISSUE,
+				LOW_ISSUE,
+				MEDIUM_ISSUE
+			],
+			1: [
+				MEDIUM_ISSUE
+			]
+		};
+		assert.deepEqual(
+			getAllIssuesSections( allIssuesOldTreatment ),
+			[ '0', '0', '0' ],
+			'section numbers correctly extracted from old treatment'
+		);
+		assert.deepEqual(
+			getAllIssuesSections( allIssuesNewTreatment ),
+			[ '0', '0', '0', '1' ],
+			'section numbers correctly extracted from new treatment'
+		);
+	} );
 }( mw.mobileFrontend ) );