From a9a0c4d477231384c4d702071fc51ac4c519149e Mon Sep 17 00:00:00 2001
From: Anders Olsson <anders.e.olsson@gmail.com>
Date: Mon, 8 Jun 2026 13:58:12 +0200
Subject: [PATCH] refactor(web): extract groupDefinitions helper; object-detail
 uses it (#45)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 web/src/lib/group-fields.test.ts  | 40 +++++++++++++++++++++++++++++++
 web/src/lib/group-fields.ts       | 30 +++++++++++++++++++++++
 web/src/objects/object-detail.tsx | 17 ++++---------
 3 files changed, 74 insertions(+), 13 deletions(-)
 create mode 100644 web/src/lib/group-fields.test.ts
 create mode 100644 web/src/lib/group-fields.ts

diff --git a/web/src/lib/group-fields.test.ts b/web/src/lib/group-fields.test.ts
new file mode 100644
index 0000000..e0c59f6
--- /dev/null
+++ b/web/src/lib/group-fields.test.ts
@@ -0,0 +1,40 @@
+import { expect, test } from "vitest";
+import { groupDefinitions } from "./group-fields";
+import type { components } from "../api/schema";
+
+type FieldDefinitionView = components["schemas"]["FieldDefinitionView"];
+type MinDef = Pick<FieldDefinitionView, "key" | "group">;
+
+const def = (key: string, group: string | null): MinDef => ({
+  key,
+  group: group as FieldDefinitionView["group"],
+});
+
+function keysByGroup(defs: MinDef[]) {
+  return groupDefinitions(defs as FieldDefinitionView[], "Other").map((g) => ({
+    group: g.group,
+    keys: g.defs.map((d) => d.key),
+  }));
+}
+
+test("preserves definition order within and across groups; Other is last", () => {
+  const result = keysByGroup([
+    def("a", "Description"),
+    def("b", null),
+    def("c", "Description"),
+    def("d", "Provenance"),
+    def("e", "  "),
+  ]);
+
+  expect(result).toEqual([
+    { group: "Description", keys: ["a", "c"] },
+    { group: "Provenance", keys: ["d"] },
+    { group: "Other", keys: ["b", "e"] },
+  ]);
+});
+
+test("all-ungrouped → a single trailing Other group", () => {
+  expect(keysByGroup([def("x", null), def("y", null)])).toEqual([
+    { group: "Other", keys: ["x", "y"] },
+  ]);
+});
diff --git a/web/src/lib/group-fields.ts b/web/src/lib/group-fields.ts
new file mode 100644
index 0000000..d7fb9f7
--- /dev/null
+++ b/web/src/lib/group-fields.ts
@@ -0,0 +1,30 @@
+import type { components } from "../api/schema";
+
+type FieldDefinitionView = components["schemas"]["FieldDefinitionView"];
+
+export type FieldGroup = { group: string; defs: FieldDefinitionView[] };
+
+/** Group field definitions by `def.group` (trimmed), preserving definition order
+ *  within and across groups; ungrouped defs fall into a trailing `otherLabel` bucket. */
+export function groupDefinitions(
+  definitions: FieldDefinitionView[],
+  otherLabel: string,
+): FieldGroup[] {
+  const groups: FieldGroup[] = [];
+
+  for (const def of definitions) {
+    const group = def.group?.trim() ? def.group : otherLabel;
+    let bucket = groups.find((g) => g.group === group);
+
+    if (!bucket) {
+      bucket = { group, defs: [] };
+      groups.push(bucket);
+    }
+
+    bucket.defs.push(def);
+  }
+
+  groups.sort((a, b) => Number(a.group === otherLabel) - Number(b.group === otherLabel));
+
+  return groups;
+}
diff --git a/web/src/objects/object-detail.tsx b/web/src/objects/object-detail.tsx
index 326d48f..de1aef6 100644
--- a/web/src/objects/object-detail.tsx
+++ b/web/src/objects/object-detail.tsx
@@ -4,6 +4,7 @@ import { useTranslation } from "react-i18next";
 
 import type { components } from "../api/schema";
 import { useObject, useFieldDefinitions } from "../api/queries";
+import { groupDefinitions } from "../lib/group-fields";
 import { formatDate } from "../lib/format-date";
 import { useDocumentTitle } from "../lib/use-document-title";
 import { useBreadcrumb } from "../shell/use-breadcrumb";
@@ -14,7 +15,6 @@ import { VisibilityBadge } from "./visibility-badge";
 import { buttonVariants } from "@/components/ui/button";
 import { Skeleton } from "@/components/ui/skeleton";
 
-type FieldDefinitionView = components["schemas"]["FieldDefinitionView"];
 type AdminObjectView = components["schemas"]["AdminObjectView"];
 
 function Field({ label, value }: { label: string; value: ReactNode }) {
@@ -70,17 +70,8 @@ function ObjectDetailLoaded({ object }: { object: AdminObjectView }) {
   // into a trailing "Other" group.
   const other = t("fields.other");
   const present = (definitions ?? []).filter((d) => object.fields[d.key] != null);
-  const groups: { group: string; defs: FieldDefinitionView[] }[] = [];
-  for (const def of present) {
-    const isOther = !def.group?.trim();
-    const group = isOther ? other : def.group!;
-    let bucket = groups.find((x) => x.group === group);
-    if (!bucket) {
-      bucket = { group, defs: [] };
-      groups.push(bucket);
-    }
-    bucket.defs.push(def);
-  }
+  const groups = groupDefinitions(present, other);
+
   // Defensive: a key present in object.fields with no matching definition (e.g. a
   // definition removed after the value was set). Render it muted under "Other"
   // rather than silently dropping data; the raw key is the label.
@@ -88,10 +79,10 @@ function ObjectDetailLoaded({ object }: { object: AdminObjectView }) {
   const orphans = Object.entries(object.fields).filter(
     ([key, value]) => !definedKeys.has(key) && value != null,
   );
+
   if (orphans.length > 0 && !groups.some((g) => g.group === other)) {
     groups.push({ group: other, defs: [] });
   }
-  groups.sort((a, b) => Number(a.group === other) - Number(b.group === other));
 
   return (
     <div className="overflow-auto p-4">