fix: safe nested travesal in orderArrayBy

packages/utils/package.json

@@ -15,6 +15,7 @@
   "scripts": {
     "build": "tsdown",
     "dev": "tsdown --watch --no-clean",
+    "test": "vitest run",
     "check:lint": "oxlint --max-warnings=38 .",
     "check:types": "tsc --noEmit",
     "check:format": "oxfmt --check .",
@@ -53,6 +54,7 @@
     "@types/react": "catalog:",
     "@types/sanitize-html": "catalog:",
     "tsdown": "catalog:",
-    "typescript": "catalog:"
+    "typescript": "catalog:",
+    "vitest": "catalog:"
   }
 }

packages/utils/src/array.ts

@@ -19,8 +19,8 @@ import type { IIssueLabel, IIssueLabelTree } from "@plane/types";
 export const groupBy = (array: any[], key: string) => {
   const innerKey = key.split("."); // split the key by dot
   return array.reduce((result, currentValue) => {
-    const key = innerKey.reduce((obj, i) => obj?.[i], currentValue) ?? "None"; // get the value of the inner key
-    (result[key] = result[key] || []).push(currentValue);
+    const groupKey = innerKey.reduce((obj, i) => obj?.[i], currentValue) ?? "None"; // get the value of the inner key
+    (result[groupKey] = result[groupKey] || []).push(currentValue);
     return result;
   }, {});
 };
@@ -36,27 +36,40 @@ export const groupBy = (array: any[], key: string) => {
  * orderArrayBy(array, 'value', 'ascending') // returns [{value: 1}, {value: 2}, {value: 3}]
  */
 export const orderArrayBy = (orgArray: any[], key: string, ordering: "ascending" | "descending" = "ascending") => {
-  if (!orgArray || !Array.isArray(orgArray) || orgArray.length === 0) return [];
+  if (!orgArray || !Array.isArray(orgArray) || orgArray.length === 0 || !key) return [];
 
   const array = [...orgArray];
 
-  if (key[0] === "-") {
+  if (key.startsWith("-")) {
     ordering = "descending";
     key = key.slice(1);
   }
 
-  const innerKey = key.split("."); // split the key by dot
-
-  return array.sort((a, b) => {
-    const keyA = innerKey.reduce((obj, i) => obj[i], a); // get the value of the inner key
-    const keyB = innerKey.reduce((obj, i) => obj[i], b); // get the value of the inner key
-    if (keyA < keyB) {
-      return ordering === "ascending" ? -1 : 1;
+  const innerKey = key.split(".");
+  const isAscending = ordering === "ascending";
+
+  return array.toSorted((a, b) => {
+    //safe traversal with optional chaining
+    const keyA = innerKey.reduce((obj, i) => (obj != null ? obj[i] : undefined), a);
+    const keyB = innerKey.reduce((obj, i) => (obj != null ? obj[i] : undefined), b);
+
+    //both equal or both null/undefined
+    if (keyA === keyB) return 0;
+    // null/undefined at the end
+    if (keyA == null) return 1;
+    if (keyB == null) return -1;
+
+    //  Type-safe comparison
+    let comparison = 0;
+    if (typeof keyA === "number" && typeof keyB === "number") {
+      comparison = keyA - keyB;
+    } else if (typeof keyA === "string" && typeof keyB === "string") {
+      comparison = keyA.localeCompare(keyB, undefined, { numeric: true, sensitivity: "base" });
+    } else {
+      comparison = keyA < keyB ? -1 : keyA > keyB ? 1 : 0;
     }
-    if (keyA > keyB) {
-      return ordering === "ascending" ? 1 : -1;
-    }
-    return 0;
+
+    return isAscending ? comparison : -comparison;
   });
 };
 
@@ -141,7 +154,7 @@ export const sortByField = (array: any[], field: string): any[] =>
 export const orderGroupedDataByField = <T>(groupedData: GroupedItems<T>, orderBy: keyof T): GroupedItems<T> => {
   for (const key in groupedData) {
     if (groupedData.hasOwnProperty(key)) {
-      groupedData[key] = groupedData[key].sort((a, b) => {
+      groupedData[key] = groupedData[key].toSorted((a, b) => {
         if (a[orderBy] < b[orderBy]) return -1;
         if (a[orderBy] > b[orderBy]) return 1;
         return 0;
@@ -221,8 +234,7 @@ export const sortBySelectedFirst = <T extends { value: string | null }>(
 
   if (selectedSet.size === 0) return options;
 
-  // Create a shallow copy to avoid mutating the original array
-  return [...options].sort((a, b) => {
+  return options.toSorted((a, b) => {
     const aSelected = a.value !== null && selectedSet.has(a.value);
     const bSelected = b.value !== null && selectedSet.has(b.value);
 
@@ -255,8 +267,7 @@ export const sortByCurrentUserThenSelected = <T extends { value: string | null }
   // Normalize selectedValues to array for consistent handling
   const selectedSet = new Set(Array.isArray(selectedValues) ? selectedValues : selectedValues ? [selectedValues] : []);
 
-  // Create a shallow copy to avoid mutating the original array
-  return [...options].sort((a, b) => {
+  return options.toSorted((a, b) => {
     const aIsCurrent = currentUserId && a.value === currentUserId;
     const bIsCurrent = currentUserId && b.value === currentUserId;
 

packages/utils/tests/array.test.ts

@@ -0,0 +1,166 @@
+/**
+ * Copyright (c) 2023-present Plane Software, Inc. and contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ * See the LICENSE file for details.
+ */
+
+import { describe, expect, it } from "vitest";
+import {
+  checkDuplicates,
+  checkIfArraysHaveSameElements,
+  findStringWithMostCharacters,
+  groupBy,
+  orderArrayBy,
+} from "../src/array";
+
+describe("orderArrayBy", () => {
+  it("should sort numbers in ascending order by default", () => {
+    const input = [{ value: 3 }, { value: 1 }, { value: 2 }];
+    const result = orderArrayBy(input, "value");
+    expect(result).toEqual([{ value: 1 }, { value: 2 }, { value: 3 }]);
+  });
+
+  it("should sort numbers in descending order when specified", () => {
+    const input = [{ value: 3 }, { value: 1 }, { value: 2 }];
+    const result = orderArrayBy(input, "value", "descending");
+    expect(result).toEqual([{ value: 3 }, { value: 2 }, { value: 1 }]);
+  });
+
+  it("should sort in descending order when key starts with '-'", () => {
+    const input = [{ priority: 1 }, { priority: 5 }, { priority: 3 }];
+    const result = orderArrayBy(input, "-priority");
+    expect(result).toEqual([{ priority: 5 }, { priority: 3 }, { priority: 1 }]);
+  });
+
+  it("should sort strings alphabetically (case-insensitive / natural)", () => {
+    const input = [{ name: "banana" }, { name: "Apple" }, { name: "cherry" }];
+    const result = orderArrayBy(input, "name");
+    expect(result).toEqual([{ name: "Apple" }, { name: "banana" }, { name: "cherry" }]);
+  });
+
+  it("should safely sort nested properties using dot notation", () => {
+    const input = [
+      { user: { profile: { name: "Charlie" } } },
+      { user: { profile: { name: "Alice" } } },
+      { user: { profile: { name: "Bob" } } },
+    ];
+    const result = orderArrayBy(input, "user.profile.name");
+    expect(result).toEqual([
+      { user: { profile: { name: "Alice" } } },
+      { user: { profile: { name: "Bob" } } },
+      { user: { profile: { name: "Charlie" } } },
+    ]);
+  });
+
+  it("should NOT throw TypeError when nested parent properties are null or undefined", () => {
+    const input = [
+      { id: 1, user: { profile: { name: "Charlie" } } },
+      { id: 2, user: null },
+      { id: 3, user: { profile: null } },
+      { id: 4, user: { profile: { name: "Alice" } } },
+      { id: 5 },
+    ];
+
+    expect(() => orderArrayBy(input, "user.profile.name")).not.toThrow();
+
+    const result = orderArrayBy(input, "user.profile.name");
+    // Alice and Charlie first, then items with null/undefined values placed at end
+    expect(result[0].id).toBe(4); // Alice
+    expect(result[1].id).toBe(1); // Charlie
+    // remaining items (ids 2, 3, 5) are placed after defined items
+    expect(result.slice(2).map((x) => x.id)).toEqual(expect.arrayContaining([2, 3, 5]));
+  });
+
+  it("should deterministically place null and undefined values at the end in ascending order", () => {
+    const input = [
+      { id: 1, date: "2024-05-01" },
+      { id: 2, date: null },
+      { id: 3, date: "2024-01-01" },
+      { id: 4, date: undefined },
+      { id: 5, date: "2024-03-01" },
+    ];
+
+    const result = orderArrayBy(input, "date", "ascending");
+    expect(result[0].id).toBe(3); // 2024-01-01
+    expect(result[1].id).toBe(5); // 2024-03-01
+    expect(result[2].id).toBe(1); // 2024-05-01
+    expect([result[3].id, result[4].id]).toEqual(expect.arrayContaining([2, 4]));
+  });
+
+  it("should handle empty array, null input, or empty key gracefully", () => {
+    expect(orderArrayBy([], "key")).toEqual([]);
+    expect(orderArrayBy(null as any, "key")).toEqual([]);
+    expect(orderArrayBy(undefined as any, "key")).toEqual([]);
+    expect(orderArrayBy([{ val: 1 }], "")).toEqual([]);
+  });
+
+  it("should not mutate the original array", () => {
+    const input = [{ value: 3 }, { value: 1 }];
+    const copy = [...input];
+    orderArrayBy(input, "value");
+    expect(input).toEqual(copy);
+  });
+});
+
+describe("groupBy", () => {
+  it("should group objects by a specified key", () => {
+    const array = [
+      { type: "A", value: 1 },
+      { type: "B", value: 2 },
+      { type: "A", value: 3 },
+    ];
+    expect(groupBy(array, "type")).toEqual({
+      A: [
+        { type: "A", value: 1 },
+        { type: "A", value: 3 },
+      ],
+      B: [{ type: "B", value: 2 }],
+    });
+  });
+
+  it("should group objects by nested key with 'None' fallback for missing properties", () => {
+    const array = [
+      { state: { group: "started" }, id: 1 },
+      { state: null, id: 2 },
+      { state: { group: "backlog" }, id: 3 },
+    ];
+    expect(groupBy(array, "state.group")).toEqual({
+      started: [{ state: { group: "started" }, id: 1 }],
+      None: [{ state: null, id: 2 }],
+      backlog: [{ state: { group: "backlog" }, id: 3 }],
+    });
+  });
+});
+
+describe("checkDuplicates", () => {
+  it("should return true if array contains duplicates", () => {
+    expect(checkDuplicates([1, 2, 2, 3])).toBe(true);
+    expect(checkDuplicates(["a", "b", "a"])).toBe(true);
+  });
+
+  it("should return false if array has only unique elements", () => {
+    expect(checkDuplicates([1, 2, 3])).toBe(false);
+    expect(checkDuplicates(["a", "b", "c"])).toBe(false);
+  });
+});
+
+describe("findStringWithMostCharacters", () => {
+  it("should return the longest string in array", () => {
+    expect(findStringWithMostCharacters(["a", "bb", "ccc"])).toBe("ccc");
+  });
+
+  it("should return empty string for empty input", () => {
+    expect(findStringWithMostCharacters([])).toBe("");
+    expect(findStringWithMostCharacters(null as any)).toBe("");
+  });
+});
+
+describe("checkIfArraysHaveSameElements", () => {
+  it("should return true if arrays contain same elements in different order", () => {
+    expect(checkIfArraysHaveSameElements([1, 2], [2, 1])).toBe(true);
+  });
+
+  it("should return false if arrays have different elements", () => {
+    expect(checkIfArraysHaveSameElements([1, 2], [1, 3])).toBe(false);
+  });
+});