Repository: Redocly/redocly-cli
Pull request #972 (2022-12-28 23:59:34)

chore: change sortOrder logic
## What/Why/How?
Fix an issue with asserts not accepting the List nodes properly (e.g TagList).
Allow sortOrder to sort string values in a case-insensitive manner. 
Throw a message if the user wants to sort objects without defining a property to sort by.

## Reference

## Testing
Locally.

## Screenshots (optional)
N/a

## Check yourself

- [x] Code is linted
- [ ] Tested with redoc/reference-docs/workflows
- [x] All new/updated code is covered with tests

## Security

- [x] Security impact of change has been considered
- [x] Code follows company security practices and guidelines


github-actions[bot]: <!-- jest coverage report action for options with hash 3339ae53a38c44382c0fb4491bdb282d -->

## Coverage report 



| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | Category   | Percentage | Covered / Total |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :--------- | :--------- | :-------------: |
|                                                                           🟡                                                                           | Statements | 73.92%     |    3416/4621    |
|                                                                           🟡                                                                           | Branches   | 65.82%     |    1935/2940    |
|                                                                           🟡                                                                           | Functions  | 66.48%     |     579/871     |
|                                                                           🟡                                                                           | Lines      | 73.95%     |    3176/4295    |


## Test suite run success
535 tests passing in 85 suites.

<p align="right">Report generated by <a href="https://github.com/ArtiomTr/jest-coverage-report-action">🧪jest coverage report action</a> from 1a21eff9251b80e34d3f5805bda4cf14a6aa58f6</p>


Patch:
diff --git a/packages/core/src/rules/common/assertions/asserts.ts b/packages/core/src/rules/common/assertions/asserts.ts
index 1596d688..712489ab 100644
--- a/packages/core/src/rules/common/assertions/asserts.ts
+++ b/packages/core/src/rules/common/assertions/asserts.ts
@@ -239,9 +239,18 @@ export const asserts: Asserts = {
     condition: OrderOptions | OrderDirection,
     baseLocation: Location
   ) => {
-    if (typeof value === 'undefined' || isOrdered(value, condition)) return [];
     const direction = (condition as OrderOptions).direction || (condition as OrderDirection);
     const property = (condition as OrderOptions).property;
+    if (Array.isArray(value) && value.length > 0 && typeof value[0] === 'object' && !property) {
+      return [
+        {
+          message: `Please define a property to sort objects by`,
+          location: baseLocation,
+        },
+      ];
+    }
+    if (typeof value === 'undefined' || isOrdered(value, condition)) return [];
+
     return [
       {
         message: `Should be sorted in ${
diff --git a/packages/core/src/rules/common/assertions/utils.ts b/packages/core/src/rules/common/assertions/utils.ts
index 2195027d..40b54a00 100644
--- a/packages/core/src/rules/common/assertions/utils.ts
+++ b/packages/core/src/rules/common/assertions/utils.ts
@@ -118,11 +118,11 @@ function applyAssertions(
         );
       }
     } else {
-      const value = assert.name === 'ref' ? rawNode : Object.keys(node);
+      const value = Array.isArray(node) ? node : Object.keys(node);
       assertResults.push(
         runAssertion({
-          values: Object.keys(node),
-          rawValues: value,
+          values: value,
+          rawValues: rawNode,
           assert,
           location: currentLocation,
         })
@@ -288,11 +288,20 @@ export function isOrdered(value: any[], options: OrderOptions | OrderDirection):
     let prevVal = value[i - 1];
 
     if (property) {
-      if (!value[i][property] || !value[i - 1][property]) {
+      const currPropValue = value[i][property];
+      const prevPropValue = value[i - 1][property];
+
+      if (!currPropValue || !prevPropValue) {
         return false; // property doesn't exist, so collection is not ordered
       }
-      currValue = value[i][property];
-      prevVal = value[i - 1][property];
+
+      currValue = currPropValue;
+      prevVal = prevPropValue;
+    }
+
+    if (typeof currValue === 'string' && typeof prevVal === 'string') {
+      currValue = currValue.toLowerCase();
+      prevVal = prevVal.toLowerCase();
     }
 
     const result = direction === 'asc' ? currValue >= prevVal : currValue <= prevVal;


Test patch:
diff --git a/packages/core/src/rules/common/assertions/__tests__/asserts.test.ts b/packages/core/src/rules/common/assertions/__tests__/asserts.test.ts
index 7a879317..4edb01c8 100644
--- a/packages/core/src/rules/common/assertions/__tests__/asserts.test.ts
+++ b/packages/core/src/rules/common/assertions/__tests__/asserts.test.ts
@@ -549,21 +549,25 @@ describe('oas3 assertions', () => {
       });
     });
 
-    describe.skip('sortOrder', () => {
+    describe('sortOrder', () => {
       it('value should be ordered in ASC direction', () => {
         expect(asserts.sortOrder(['example', 'foo', 'test'], 'asc', baseLocation)).toEqual([]);
         expect(
           asserts.sortOrder(['example', 'foo', 'test'], { direction: 'asc' }, baseLocation)
         ).toEqual([]);
         expect(asserts.sortOrder(['example'], 'asc', baseLocation)).toEqual([]);
-        expect(asserts.sortOrder(['example', 'test', 'foo'], 'asc', baseLocation)).toEqual({
-          isValid: false,
-          location: baseLocation,
-        });
-        expect(asserts.sortOrder(['example', 'foo', 'test'], 'desc', baseLocation)).toEqual({
-          isValid: false,
-          location: baseLocation,
-        });
+        expect(asserts.sortOrder(['example', 'test', 'foo'], 'asc', baseLocation)).toEqual([
+          {
+            message: 'Should be sorted in an ascending order',
+            location: baseLocation,
+          },
+        ]);
+        expect(asserts.sortOrder(['example', 'foo', 'test'], 'desc', baseLocation)).toEqual([
+          {
+            message: 'Should be sorted in a descending order',
+            location: baseLocation,
+          },
+        ]);
         expect(
           asserts.sortOrder(
             [{ name: 'bar' }, { name: 'baz' }, { name: 'foo' }],
@@ -577,7 +581,12 @@ describe('oas3 assertions', () => {
             { direction: 'desc', property: 'name' },
             baseLocation
           )
-        ).toEqual({ isValid: false, location: baseLocation });
+        ).toEqual([
+          {
+            message: 'Should be sorted in a descending order by property name',
+            location: baseLocation,
+          },
+        ]);
       });
       it('value should be ordered in DESC direction', () => {
         expect(asserts.sortOrder(['test', 'foo', 'example'], 'desc', baseLocation)).toEqual([]);
@@ -585,14 +594,18 @@ describe('oas3 assertions', () => {
           asserts.sortOrder(['test', 'foo', 'example'], { direction: 'desc' }, baseLocation)
         ).toEqual([]);
         expect(asserts.sortOrder(['example'], 'desc', baseLocation)).toEqual([]);
-        expect(asserts.sortOrder(['example', 'test', 'foo'], 'desc', baseLocation)).toEqual({
-          isValid: false,
-          location: baseLocation,
-        });
-        expect(asserts.sortOrder(['test', 'foo', 'example'], 'asc', baseLocation)).toEqual({
-          isValid: false,
-          location: baseLocation,
-        });
+        expect(asserts.sortOrder(['example', 'test', 'foo'], 'desc', baseLocation)).toEqual([
+          {
+            message: 'Should be sorted in a descending order',
+            location: baseLocation,
+          },
+        ]);
+        expect(asserts.sortOrder(['test', 'foo', 'example'], 'asc', baseLocation)).toEqual([
+          {
+            message: 'Should be sorted in an ascending order',
+            location: baseLocation,
+          },
+        ]);
         expect(
           asserts.sortOrder(
             [{ name: 'foo' }, { name: 'baz' }, { name: 'bar' }],
@@ -606,7 +619,64 @@ describe('oas3 assertions', () => {
             { direction: 'asc', property: 'name' },
             baseLocation
           )
-        ).toEqual({ isValid: false, location: baseLocation });
+        ).toEqual([
+          {
+            message: 'Should be sorted in an ascending order by property name',
+            location: baseLocation,
+          },
+        ]);
+      });
+      it('should not order objects without property defined', () => {
+        expect(
+          asserts.sortOrder(
+            [
+              { name: 'bar', id: 1 },
+              { name: 'baz', id: 2 },
+              { name: 'foo', id: 3 },
+            ],
+            { direction: 'desc' },
+            baseLocation
+          )
+        ).toEqual([
+          {
+            message: 'Please define a property to sort objects by',
+            location: baseLocation,
+          },
+        ]);
+        expect(
+          asserts.sortOrder(
+            [
+              { name: 'bar', id: 1 },
+              { name: 'baz', id: 2 },
+              { name: 'foo', id: 3 },
+            ],
+            { direction: 'asc' },
+            baseLocation
+          )
+        ).toEqual([
+          {
+            message: 'Please define a property to sort objects by',
+            location: baseLocation,
+          },
+        ]);
+      });
+      it('should ignore string value casing while ordering', () => {
+        expect(asserts.sortOrder(['Example', 'foo', 'Test'], 'asc', baseLocation)).toEqual([]);
+        expect(asserts.sortOrder(['Test', 'foo', 'Example'], 'desc', baseLocation)).toEqual([]);
+        expect(
+          asserts.sortOrder(
+            [{ name: 'bar' }, { name: 'Baz' }, { name: 'Foo' }],
+            { direction: 'asc', property: 'name' },
+            baseLocation
+          )
+        ).toEqual([]);
+        expect(
+          asserts.sortOrder(
+            [{ name: 'Foo' }, { name: 'baz' }, { name: 'Bar' }],
+            { direction: 'desc', property: 'name' },
+            baseLocation
+          )
+        ).toEqual([]);
       });
     });