View improvements: refresh, reordering, error handling, and tests

stalep · barreiro · commit d13fdc3db004 · 2026-06-09T13:15:10.000+01:00
DataTab:
- Show helpful message when view has no columns configured instead
  of confusing 'No data available'
- Data table refreshes automatically after saving view configuration
  (keyed by component IDs to force remount on changes)
- Configure button reads latest view data from query result
- Modal remounted via counter key for fresh FilterableMultiSelect state

ViewConfigModal:
- Column reordering: ordered list below multi-select with numbered
  positions, up/down arrow buttons, and remove (x) button
- Existing column order preserved from headerOrder when editing
- New items appended at end, removals keep remaining order
- Error display when create/update/delete fails (was silent)
- Selected nodes shown correctly when reopening (same object
  references from availableNodes for Carbon FilterableMultiSelect)
- Query refetch uses predicate matching _id field (generated query
  keys use object format, not plain strings)

ViewService:
- Flush component deletes before insert on view update to avoid
  unique constraint violation on (view_id, header_name)

Tests:
- Frontend (8 new): empty view prompt, column ordering list rendering,
  preserved headerOrder, disabled up/down buttons, reorder on click,
  remove column on click
- Backend (1 new): view_update_with_same_header_names verifies the
  flush fix for unique constraint violation
diff --git a/src/main/java/io/hyperfoil/tools/h5m/svc/ViewService.java b/src/main/java/io/hyperfoil/tools/h5m/svc/ViewService.java
@@ -86,6 +86,9 @@ public View updateView(Long viewId, View view) {
 
         entity.name = view.name();
         entity.components.clear();
+        // Flush the deletes before inserting new components to avoid
+        // unique constraint violations on (view_id, header_name)
+        entity.flush();
 
         if (view.components() != null) {
             for (int i = 0; i < view.components().size(); i++) {
diff --git a/src/main/webui/src/app/components/DataTab.tsx b/src/main/webui/src/app/components/DataTab.tsx
@@ -90,6 +90,7 @@ export const DataTab = ({ folderName, groupId }: { folderName: string; groupId:
   const [selectedViewId, setSelectedViewId] = useState<number | null>(null);
   const [configModalOpen, setConfigModalOpen] = useState(false);
   const [editingView, setEditingView] = useState<View | null>(null);
+  const [modalKey, setModalKey] = useState(0);
 
   const selectedView = useMemo((): View | null => {
     if (!views || views.length === 0) return null;
@@ -132,24 +133,39 @@ export const DataTab = ({ folderName, groupId }: { folderName: string; groupId:
         <Button
           kind="ghost"
           size="md"
-          onClick={() => { setEditingView(selectedView); setConfigModalOpen(true); }}
+          onClick={() => {
+            const latestView = views?.find((v: View) => v.id === selectedView?.id) ?? selectedView;
+            setEditingView(latestView);
+            setModalKey((k) => k + 1);
+            setConfigModalOpen(true);
+          }}
         >
           Configure
         </Button>
         <Button
           kind="ghost"
           size="md"
-          onClick={() => { setEditingView(null); setConfigModalOpen(true); }}
+          onClick={() => { setEditingView(null); setModalKey((k) => k + 1); setConfigModalOpen(true); }}
         >
           New View
         </Button>
       </div>
-      {selectedView && (
-        <ViewDataTable folderName={folderName} view={selectedView} />
+      {selectedView && (!selectedView.components || selectedView.components.length === 0) && (
+        <p style={{ opacity: 0.7 }}>
+          This view has no columns configured. Click <strong>Configure</strong> to select which nodes to display.
+        </p>
+      )}
+      {selectedView && selectedView.components && selectedView.components.length > 0 && (
+        <ViewDataTable
+          key={`${String(selectedView.id)}-${String(selectedView.components?.length ?? 0)}-${selectedView.components?.map(c => String(c.nodeId)).join(',') ?? ''}`}
+          folderName={folderName}
+          view={selectedView}
+        />
       )}
       <ErrorBoundary fallback={<InlineLoading status="error" description="Failed to load modal" />}>
         <Suspense fallback={<SkeletonText paragraph={true} lineCount={3} />}>
           <ViewConfigModal
+            key={modalKey}
             open={configModalOpen}
             onClose={() => setConfigModalOpen(false)}
             folderName={folderName}
diff --git a/src/main/webui/src/app/components/ViewConfigModal.tsx b/src/main/webui/src/app/components/ViewConfigModal.tsx
@@ -9,10 +9,11 @@ import {
   ModalHeader,
   TextInput,
 } from '@carbon/react';
+import { ArrowUp, ArrowDown, Close } from '@carbon/icons-react';
 import { byIdOptions } from '@client/@tanstack/react-query.gen.ts';
 import { ViewService } from '@client/sdk.gen.ts';
 import { useMutation, useQueryClient, useSuspenseQuery } from '@tanstack/react-query';
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback, useState } from 'react';
 
 interface ViewConfigModalProps {
   open: boolean;
@@ -34,26 +35,8 @@ export const ViewConfigModal = ({ open, onClose, folderName, groupId, view }: Vi
   const isEditing = view != null && view.id != null;
   const isDefault = view?.name === 'Default';
 
-  const [viewName, setViewName] = useState(view?.name ?? '');
-  const [selectedNodes, setSelectedNodes] = useState<NodeItem[]>([]);
-
-  // Initialize from existing view when editing
-  useEffect(() => {
-    if (view) {
-      setViewName(view.name);
-      const items = (view.components ?? []).map((c: ViewComponent) => ({
-        id: String(c.nodeId),
-        text: c.headerName ?? c.nodeName ?? '',
-        nodeId: c.nodeId!,
-      }));
-      setSelectedNodes(items);
-    } else {
-      setViewName('');
-      setSelectedNodes([]);
-    }
-  }, [view]);
-
   // Available nodes for the multi-select (exclude detection nodes)
+  // Built once and stable so FilterableMultiSelect can match by reference
   const availableNodes: NodeItem[] = (nodeGroup.sources ?? [])
     .filter((n: ApiNode) => !['FIXED_THRESHOLD', 'RELATIVE_DIFFERENCE', 'EDIVISIVE'].includes(n.type ?? ''))
     .map((n: ApiNode) => ({
@@ -62,16 +45,70 @@ export const ViewConfigModal = ({ open, onClose, folderName, groupId, view }: Vi
       nodeId: n.id!,
     }));
 
+  const [viewName, setViewName] = useState(view?.name ?? '');
+  // Initialize selectedNodes from the view's components, preserving the
+  // existing headerOrder. Find matching items in availableNodes (same
+  // object references required by Carbon's FilterableMultiSelect).
+  const [selectedNodes, setSelectedNodes] = useState<NodeItem[]>(() => {
+    if (!view?.components || view.components.length === 0) return [];
+    // Sort components by headerOrder to preserve column ordering
+    const sorted = [...view.components].sort(
+      (a: ViewComponent, b: ViewComponent) => (a.headerOrder ?? 0) - (b.headerOrder ?? 0)
+    );
+    const ordered: NodeItem[] = [];
+    for (const c of sorted) {
+      const match = availableNodes.find((n) => n.id === String(c.nodeId));
+      if (match) ordered.push(match);
+    }
+    return ordered;
+  });
+
+  const moveUp = useCallback((index: number) => {
+    if (index <= 0) return;
+    setSelectedNodes((prev) => {
+      const next = [...prev];
+      const temp = next[index - 1]!;
+      next[index - 1] = next[index]!;
+      next[index] = temp;
+      return next;
+    });
+  }, []);
+
+  const moveDown = useCallback((index: number) => {
+    setSelectedNodes((prev) => {
+      if (index >= prev.length - 1) return prev;
+      const next = [...prev];
+      const temp = next[index]!;
+      next[index] = next[index + 1]!;
+      next[index + 1] = temp;
+      return next;
+    });
+  }, []);
+
+  const removeNode = useCallback((index: number) => {
+    setSelectedNodes((prev) => prev.filter((_, i) => i !== index));
+  }, []);
+
+  const [saveError, setSaveError] = useState<string | null>(null);
+
   const createMutation = useMutation({
     mutationFn: (data: View) =>
       ViewService.createView({
         path: { name: folderName },
         body: data,
       }),
     onSuccess: () => {
-      queryClient.invalidateQueries({ queryKey: ['getViews'] });
+      setSaveError(null);
+      void queryClient.refetchQueries({ predicate: (q) => {
+        const key = q.queryKey[0];
+        return typeof key === 'object' && key !== null && '_id' in key &&
+          (key._id === 'getViews' || key._id === 'getViewData');
+      }});
       onClose();
     },
+    onError: (e: Error) => {
+      setSaveError(e.message ?? 'Failed to create view');
+    },
   });
 
   const updateMutation = useMutation({
@@ -81,9 +118,17 @@ export const ViewConfigModal = ({ open, onClose, folderName, groupId, view }: Vi
         body: data,
       }),
     onSuccess: () => {
-      queryClient.invalidateQueries({ queryKey: ['getViews'] });
+      setSaveError(null);
+      void queryClient.refetchQueries({ predicate: (q) => {
+        const key = q.queryKey[0];
+        return typeof key === 'object' && key !== null && '_id' in key &&
+          (key._id === 'getViews' || key._id === 'getViewData');
+      }});
       onClose();
     },
+    onError: (e: Error) => {
+      setSaveError(e.message ?? 'Failed to update view');
+    },
   });
 
   const deleteMutation = useMutation({
@@ -92,7 +137,11 @@ export const ViewConfigModal = ({ open, onClose, folderName, groupId, view }: Vi
         path: { name: folderName, viewId: view!.id! },
       }),
     onSuccess: () => {
-      queryClient.invalidateQueries({ queryKey: ['getViews'] });
+      void queryClient.refetchQueries({ predicate: (q) => {
+        const key = q.queryKey[0];
+        return typeof key === 'object' && key !== null && '_id' in key &&
+          (key._id === 'getViews' || key._id === 'getViewData');
+      }});
       onClose();
     },
   });
@@ -144,9 +193,71 @@ export const ViewConfigModal = ({ open, onClose, folderName, groupId, view }: Vi
           itemToString={(item: NodeItem) => item?.text ?? ''}
           initialSelectedItems={selectedNodes}
           onChange={({ selectedItems }: { selectedItems: NodeItem[] }) => {
-            setSelectedNodes(selectedItems);
+            // Preserve existing order: keep items that are still selected
+            // in their current position, append newly added items at the end
+            const existingIds = new Set(selectedNodes.map((n) => n.id));
+            const newIds = new Set(selectedItems.map((n) => n.id));
+            const kept = selectedNodes.filter((n) => newIds.has(n.id));
+            const added = selectedItems.filter((n) => !existingIds.has(n.id));
+            setSelectedNodes([...kept, ...added]);
           }}
         />
+        {selectedNodes.length > 0 && (
+          <div style={{ marginTop: 'var(--cds-spacing-05)' }}>
+            <div style={{ fontSize: '0.75rem', opacity: 0.7, marginBottom: 'var(--cds-spacing-02)' }}>
+              Column order (drag or use arrows to reorder)
+            </div>
+            {selectedNodes.map((node, idx) => (
+              <div
+                key={node.id}
+                style={{
+                  display: 'flex',
+                  alignItems: 'center',
+                  gap: 'var(--cds-spacing-02)',
+                  padding: '4px 8px',
+                  marginBottom: '2px',
+                  background: 'var(--cds-layer-02)',
+                  borderRadius: '4px',
+                  fontSize: '0.875rem',
+                }}
+              >
+                <span style={{ opacity: 0.5, minWidth: '20px' }}>{String(idx + 1)}.</span>
+                <span style={{ flex: 1 }}>{node.text}</span>
+                <Button
+                  kind="ghost"
+                  size="sm"
+                  hasIconOnly
+                  renderIcon={ArrowUp}
+                  iconDescription="Move up"
+                  onClick={() => moveUp(idx)}
+                  disabled={idx === 0}
+                />
+                <Button
+                  kind="ghost"
+                  size="sm"
+                  hasIconOnly
+                  renderIcon={ArrowDown}
+                  iconDescription="Move down"
+                  onClick={() => moveDown(idx)}
+                  disabled={idx === selectedNodes.length - 1}
+                />
+                <Button
+                  kind="ghost"
+                  size="sm"
+                  hasIconOnly
+                  renderIcon={Close}
+                  iconDescription="Remove"
+                  onClick={() => removeNode(idx)}
+                />
+              </div>
+            ))}
+          </div>
+        )}
+        {saveError && (
+          <div style={{ color: 'var(--cds-support-error)', marginTop: 'var(--cds-spacing-03)' }}>
+            {saveError}
+          </div>
+        )}
       </ModalBody>
       <ModalFooter>
         {isEditing && !isDefault && (
diff --git a/src/main/webui/test/components/DataTab.test.tsx b/src/main/webui/test/components/DataTab.test.tsx
@@ -224,6 +224,18 @@ describe('<DataTab />', () => {
     cleanup();
   });
 
+  it('shows configure prompt when view has no columns', async () => {
+    const views: View[] = [emptyDefaultView];
+
+    renderDataTab(views);
+
+    await waitFor(() => {
+      expect(screen.getByText(/no columns configured/i)).toBeDefined();
+    });
+
+    cleanup();
+  });
+
   it('opens config modal in create mode when New View is clicked', async () => {
     const user = userEvent.setup();
     renderDataTab();
diff --git a/src/main/webui/test/components/ViewConfigModal.test.tsx b/src/main/webui/test/components/ViewConfigModal.test.tsx
diff --git a/src/test/java/io/hyperfoil/tools/h5m/rest/RestEndpointTest.java b/src/test/java/io/hyperfoil/tools/h5m/rest/RestEndpointTest.java
