fix: fix sortByRequired (stabilise sort) (#1136)

fixes #1104
fixes #1121
fixes #1061

src/services/models/Operation.ts

@@ -184,12 +184,11 @@ export class OperationModel implements IMenuItem {
     ).map(paramOrRef => new FieldModel(this.parser, paramOrRef, this.pointer, this.options));
 
     if (this.options.sortPropsAlphabetically) {
-      sortByField(_parameters, 'name');
+      return sortByField(_parameters, 'name');
     }
     if (this.options.requiredPropsFirst) {
-      sortByRequired(_parameters);
+      return sortByRequired(_parameters);
     }
-    return _parameters;
   }
 
   @memoize

src/services/models/Schema.ts

@@ -275,7 +275,7 @@ function buildFields(
   const props = schema.properties || {};
   const additionalProps = schema.additionalProperties;
   const defaults = schema.default || {};
-  const fields = Object.keys(props || []).map(fieldName => {
+  let fields = Object.keys(props || []).map(fieldName => {
     let field = props[fieldName];
 
     if (!field) {
@@ -304,11 +304,11 @@ function buildFields(
   });
 
   if (options.sortPropsAlphabetically) {
-    sortByField(fields, 'name');
+    fields = sortByField(fields, 'name');
   }
   if (options.requiredPropsFirst) {
     // if not sort alphabetically sort in the order from required keyword
-    sortByRequired(fields, !options.sortPropsAlphabetically ? schema.required : undefined);
+    fields = sortByRequired(fields, !options.sortPropsAlphabetically ? schema.required : undefined);
   }
 
   if (typeof additionalProps === 'object' || additionalProps === true) {

src/utils/__tests__/openapi.test.ts

@@ -9,6 +9,7 @@ import {
   normalizeServers,
   pluralizeType,
   serializeParameterValue,
+  sortByRequired,
 } from '../';
 
 import { FieldModel, OpenAPIParser, RedocNormalizedOptions } from '../../services';
@@ -636,4 +637,370 @@ describe('Utils', () => {
       });
     });
   });
+
+  describe('OpenAPI sortByRequired', () => {
+    it('should equal to the old data when all items have no required props', () => {
+      let fields = [
+        {
+          name: 'loginName',
+          required: false,
+        },
+        {
+          name: 'displayName',
+          required: false,
+        },
+        {
+          name: 'email',
+          required: false,
+        },
+        {
+          name: 'space',
+          required: false,
+        },
+        {
+          name: 'type',
+          required: false,
+        },
+        {
+          name: 'depIds',
+          required: false,
+        },
+        {
+          name: 'depNames',
+          required: false,
+        },
+        {
+          name: 'password',
+          required: false,
+        },
+        {
+          name: 'pwdControl',
+          required: false,
+        },
+        {
+          name: 'csfLevel',
+          required: false,
+        },
+        {
+          name: 'priority',
+          required: false,
+        },
+        {
+          name: 'siteId',
+          required: false,
+        },
+      ];
+      expect(sortByRequired(fields as FieldModel[])).toEqual(fields);
+    });
+
+    it('other item should be the same order when some of items are required', () => {
+      let fields = [
+        {
+          name: 'loginName',
+          required: true,
+        },
+        {
+          name: 'displayName',
+          required: false,
+        },
+        {
+          name: 'email',
+          required: true,
+        },
+        {
+          name: 'space',
+          required: false,
+        },
+        {
+          name: 'type',
+          required: false,
+        },
+        {
+          name: 'depIds',
+          required: false,
+        },
+        {
+          name: 'depNames',
+          required: false,
+        },
+        {
+          name: 'password',
+          required: false,
+        },
+        {
+          name: 'pwdControl',
+          required: false,
+        },
+        {
+          name: 'csfLevel',
+          required: false,
+        },
+        {
+          name: 'priority',
+          required: false,
+        },
+        {
+          name: 'siteId',
+          required: false,
+        },
+      ];
+      let sortedFields = [
+        {
+          name: 'loginName',
+          required: true,
+        },
+        {
+          name: 'email',
+          required: true,
+        },
+        {
+          name: 'displayName',
+          required: false,
+        },
+        {
+          name: 'space',
+          required: false,
+        },
+        {
+          name: 'type',
+          required: false,
+        },
+        {
+          name: 'depIds',
+          required: false,
+        },
+        {
+          name: 'depNames',
+          required: false,
+        },
+        {
+          name: 'password',
+          required: false,
+        },
+        {
+          name: 'pwdControl',
+          required: false,
+        },
+        {
+          name: 'csfLevel',
+          required: false,
+        },
+        {
+          name: 'priority',
+          required: false,
+        },
+        {
+          name: 'siteId',
+          required: false,
+        },
+      ];
+      expect(sortByRequired(fields as FieldModel[])).toEqual(sortedFields);
+    });
+
+    it('should the order of required items is as same as the order parameter ', () => {
+      let fields = [
+        {
+          name: 'loginName',
+          required: true,
+        },
+        {
+          name: 'displayName',
+          required: true,
+        },
+        {
+          name: 'email',
+          required: true,
+        },
+        {
+          name: 'space',
+          required: false,
+        },
+        {
+          name: 'type',
+          required: false,
+        },
+        {
+          name: 'depIds',
+          required: false,
+        },
+        {
+          name: 'depNames',
+          required: false,
+        },
+        {
+          name: 'password',
+          required: false,
+        },
+        {
+          name: 'pwdControl',
+          required: false,
+        },
+        {
+          name: 'csfLevel',
+          required: false,
+        },
+        {
+          name: 'priority',
+          required: false,
+        },
+        {
+          name: 'siteId',
+          required: false,
+        },
+      ];
+      expect(
+        sortByRequired(fields as FieldModel[], ['siteId', 'displayName', 'loginName', 'email']),
+      ).toEqual([
+        {
+          name: 'displayName',
+          required: true,
+        },
+        {
+          name: 'loginName',
+          required: true,
+        },
+        {
+          name: 'email',
+          required: true,
+        },
+        {
+          name: 'space',
+          required: false,
+        },
+        {
+          name: 'type',
+          required: false,
+        },
+        {
+          name: 'depIds',
+          required: false,
+        },
+        {
+          name: 'depNames',
+          required: false,
+        },
+        {
+          name: 'password',
+          required: false,
+        },
+        {
+          name: 'pwdControl',
+          required: false,
+        },
+        {
+          name: 'csfLevel',
+          required: false,
+        },
+        {
+          name: 'priority',
+          required: false,
+        },
+        {
+          name: 'siteId',
+          required: false,
+        },
+      ]);
+      expect(sortByRequired(fields as FieldModel[], ['email', 'displayName'])).toEqual([
+        {
+          name: 'email',
+          required: true,
+        },
+        {
+          name: 'displayName',
+          required: true,
+        },
+        {
+          name: 'loginName',
+          required: true,
+        },
+        {
+          name: 'space',
+          required: false,
+        },
+        {
+          name: 'type',
+          required: false,
+        },
+        {
+          name: 'depIds',
+          required: false,
+        },
+        {
+          name: 'depNames',
+          required: false,
+        },
+        {
+          name: 'password',
+          required: false,
+        },
+        {
+          name: 'pwdControl',
+          required: false,
+        },
+        {
+          name: 'csfLevel',
+          required: false,
+        },
+        {
+          name: 'priority',
+          required: false,
+        },
+        {
+          name: 'siteId',
+          required: false,
+        },
+      ]);
+
+      expect(sortByRequired(fields as FieldModel[], ['displayName'])).toEqual([
+        {
+          name: 'displayName',
+          required: true,
+        },
+        {
+          name: 'loginName',
+          required: true,
+        },
+        {
+          name: 'email',
+          required: true,
+        },
+        {
+          name: 'space',
+          required: false,
+        },
+        {
+          name: 'type',
+          required: false,
+        },
+        {
+          name: 'depIds',
+          required: false,
+        },
+        {
+          name: 'depNames',
+          required: false,
+        },
+        {
+          name: 'password',
+          required: false,
+        },
+        {
+          name: 'pwdControl',
+          required: false,
+        },
+        {
+          name: 'csfLevel',
+          required: false,
+        },
+        {
+          name: 'priority',
+          required: false,
+        },
+        {
+          name: 'siteId',
+          required: false,
+        },
+      ]);
+    });
+  });
 });

src/utils/openapi.ts

@@ -1,6 +1,7 @@
 import { dirname } from 'path';
 const URLtemplate = require('url-template');
 
+import { FieldModel } from '../services/models';
 import { OpenAPIParser } from '../services/OpenAPIParser';
 import {
   OpenAPIEncoding,
@@ -444,25 +445,29 @@ export function humanizeConstraints(schema: OpenAPISchema): string[] {
   return res;
 }
 
-export function sortByRequired(
+export function sortByRequired(fields: FieldModel[], order: string[] = []) {
-  fields: Array<{ required: boolean; name: string }>,
+  const unrequiredFields: FieldModel[] = [];
-  order: string[] = [],
+  const orderedFields: FieldModel[] = [];
-) {
+  const unorderedFields: FieldModel[] = [];
-  fields.sort((a, b) => {
+
-    if (!a.required && b.required) {
+  fields.forEach(field => {
-      return 1;
+    if (field.required) {
-    } else if (a.required && !b.required) {
+      order.includes(field.name) ? orderedFields.push(field) : unorderedFields.push(field);
-      return -1;
-    } else if (a.required && b.required) {
-      return order.indexOf(a.name) - order.indexOf(b.name);
     } else {
-      return 0;
+      unrequiredFields.push(field);
     }
   });
+
+  orderedFields.sort((a, b) => order.indexOf(a.name) - order.indexOf(b.name));
+
+  return [...orderedFields, ...unorderedFields, ...unrequiredFields];
 }
 
-export function sortByField<T extends string>(fields: Array<{ [P in T]: string }>, param: T) {
+export function sortByField(
-  fields.sort((a, b) => {
+  fields: FieldModel[],
+  param: keyof Pick<FieldModel, 'name' | 'description' | 'kind'>,
+) {
+  return [...fields].sort((a, b) => {
     return a[param].localeCompare(b[param]);
   });
 }