From 1a09676159f662bd3f82231456bb6d0a7b4da1a9 Mon Sep 17 00:00:00 2001
From: thiemowmde <thiemo.kreuz@wikimedia.de>
Date: Fri, 21 Apr 2023 16:22:33 +0200
Subject: [PATCH] Add test for MWTemplateSpecModel.getDocumentedParameterOrder

It appears like this was never tested.

Now that it is covered it's much easier to play around with the
implementation and compact it a bit.

Change-Id: Ie9cc14082f69e7240380d352fb362d0a3fa4d341
---
 .../dm/models/ve.dm.MWTemplateSpecModel.js    | 22 +++++--------------
 .../models/ve.dm.MWTemplateSpecModel.test.js  |  9 +++++++-
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/modules/ve-mw/dm/models/ve.dm.MWTemplateSpecModel.js b/modules/ve-mw/dm/models/ve.dm.MWTemplateSpecModel.js
index b20562aa07..4847fb1049 100644
--- a/modules/ve-mw/dm/models/ve.dm.MWTemplateSpecModel.js
+++ b/modules/ve-mw/dm/models/ve.dm.MWTemplateSpecModel.js
@@ -248,23 +248,13 @@ ve.dm.MWTemplateSpecModel.prototype.getCanonicalParameterOrder = function () {
 	var undocumentedParameters = this.getUndocumentedParameterNames();
 
 	undocumentedParameters.sort( function ( a, b ) {
-		var aIsNaN = isNaN( a ),
-			bIsNaN = isNaN( b );
-
-		if ( aIsNaN && bIsNaN ) {
-			// Two strings
-			return a.localeCompare( b );
+		if ( isNaN( a ) ) {
+			// If a and b are string, order alphabetically, otherwise numbers before strings
+			return isNaN( b ) ? a.localeCompare( b ) : 1;
+		} else {
+			// If a and b are numeric, order incrementally, otherwise numbers before strings
+			return !isNaN( b ) ? a - b : -1;
 		}
-		if ( aIsNaN ) {
-			// A is a string
-			return 1;
-		}
-		if ( bIsNaN ) {
-			// B is a string
-			return -1;
-		}
-		// Two numbers
-		return a - b;
 	} );
 
 	return this.getDocumentedParameterOrder().concat( undocumentedParameters );
diff --git a/modules/ve-mw/tests/dm/models/ve.dm.MWTemplateSpecModel.test.js b/modules/ve-mw/tests/dm/models/ve.dm.MWTemplateSpecModel.test.js
index 395dc4bdd5..e403dc0ca2 100644
--- a/modules/ve-mw/tests/dm/models/ve.dm.MWTemplateSpecModel.test.js
+++ b/modules/ve-mw/tests/dm/models/ve.dm.MWTemplateSpecModel.test.js
@@ -5,7 +5,7 @@
 	 * @param {string[]} [parameterNames]
 	 * @return {ve.dm.MWTemplateModel} but it's a mock
 	 */
-	const createTemplateMock = function ( parameterNames ) {
+	const createTemplateMock = ( parameterNames ) => {
 		const params = {};
 		( parameterNames || [] ).forEach( ( name ) => {
 			params[ name ] = {};
@@ -311,6 +311,13 @@
 		} )
 	);
 
+	QUnit.test( 'getCanonicalParameterOrder sorting undocumented parameters alphabetically', ( assert ) => {
+		const template = createTemplateMock( [ 'ö', '3', 'x', 9, '', 1, 'a' ] ),
+			spec = new ve.dm.MWTemplateSpecModel( template );
+
+		assert.deepEqual( spec.getCanonicalParameterOrder(), [ '1', '3', '9', 'a', 'ö', 'x' ] );
+	} );
+
 	QUnit.test( 'getDocumentedParameterOrder() should not return a reference', ( assert ) => {
 		const template = createTemplateMock(),
 			spec = new ve.dm.MWTemplateSpecModel( template );