Refactor column config to support local and backend storage dynamically

cursoragent · cursoragent · commit a1cb117ceb23 · 2025-06-10T09:22:27.000Z
diff --git a/keep-ui/entities/presets/model/usePresetColumnState.ts b/keep-ui/entities/presets/model/usePresetColumnState.ts
@@ -6,22 +6,33 @@ import { TimeFormatOption } from "@/widgets/alerts-table/lib/alert-table-time-fo
 import { ListFormatOption } from "@/widgets/alerts-table/lib/alert-table-list-format";
 import { ColumnRenameMapping } from "@/widgets/alerts-table/ui/alert-table-column-rename";
 import { DEFAULT_COLS, DEFAULT_COLS_VISIBILITY } from "@/widgets/alerts-table/lib/alert-table-utils";
-import { usePresets } from "./usePresets";
 
 interface UsePresetColumnStateOptions {
   presetName: string;
   presetId?: string;
   useBackend?: boolean; // Flag to enable backend usage
 }
 
+// Static preset IDs that should always use local storage
+const STATIC_PRESET_IDS = [
+  "11111111-1111-1111-1111-111111111111", // FEED_PRESET_ID
+  "11111111-1111-1111-1111-111111111113", // DISMISSED_PRESET_ID  
+  "11111111-1111-1111-1111-111111111114", // GROUPS_PRESET_ID
+  "11111111-1111-1111-1111-111111111115", // WITHOUT_INCIDENT_PRESET_ID
+];
+
 export const usePresetColumnState = ({
   presetName,
   presetId,
   useBackend = false,
 }: UsePresetColumnStateOptions) => {
+  // Check if this is a static preset that should always use local storage
+  const isStaticPreset = !presetId || STATIC_PRESET_IDS.includes(presetId);
+  const shouldUseBackend = useBackend && !isStaticPreset;
+
   // Backend-based state
   const { columnConfig, updateColumnConfig, isLoading } = usePresetColumnConfig({
-    presetId: useBackend ? presetId : undefined
+    presetId: shouldUseBackend ? presetId : undefined
   });
 
   // Local storage fallbacks (existing implementation)
@@ -52,94 +63,99 @@ export const usePresetColumnState = ({
 
   // Determine which state to use
   const columnVisibility = useMemo(() => {
-    if (useBackend && columnConfig.column_visibility && Object.keys(columnConfig.column_visibility).length > 0) {
+    if (shouldUseBackend && columnConfig.column_visibility && Object.keys(columnConfig.column_visibility).length > 0) {
       return columnConfig.column_visibility;
     }
     return localColumnVisibility;
-  }, [useBackend, columnConfig.column_visibility, localColumnVisibility]);
+  }, [shouldUseBackend, columnConfig.column_visibility, localColumnVisibility]);
 
   const columnOrder = useMemo(() => {
-    if (useBackend && columnConfig.column_order && columnConfig.column_order.length > 0) {
+    if (shouldUseBackend && columnConfig.column_order && columnConfig.column_order.length > 0) {
       return columnConfig.column_order;
     }
     return localColumnOrder;
-  }, [useBackend, columnConfig.column_order, localColumnOrder]);
+  }, [shouldUseBackend, columnConfig.column_order, localColumnOrder]);
 
   const columnRenameMapping = useMemo(() => {
-    if (useBackend && columnConfig.column_rename_mapping) {
+    if (shouldUseBackend && columnConfig.column_rename_mapping) {
       return columnConfig.column_rename_mapping;
     }
     return localColumnRenameMapping;
-  }, [useBackend, columnConfig.column_rename_mapping, localColumnRenameMapping]);
+  }, [shouldUseBackend, columnConfig.column_rename_mapping, localColumnRenameMapping]);
 
   const columnTimeFormats = useMemo(() => {
-    if (useBackend && columnConfig.column_time_formats) {
+    if (shouldUseBackend && columnConfig.column_time_formats) {
       return columnConfig.column_time_formats as Record<string, TimeFormatOption>;
     }
     return localColumnTimeFormats;
-  }, [useBackend, columnConfig.column_time_formats, localColumnTimeFormats]);
+  }, [shouldUseBackend, columnConfig.column_time_formats, localColumnTimeFormats]);
 
   const columnListFormats = useMemo(() => {
-    if (useBackend && columnConfig.column_list_formats) {
+    if (shouldUseBackend && columnConfig.column_list_formats) {
       return columnConfig.column_list_formats as Record<string, ListFormatOption>;
     }
     return localColumnListFormats;
-  }, [useBackend, columnConfig.column_list_formats, localColumnListFormats]);
+  }, [shouldUseBackend, columnConfig.column_list_formats, localColumnListFormats]);
 
-  // Update functions
+  // Update functions - keep them synchronous for local storage, async only for backend
   const setColumnVisibility = useCallback(
-    async (visibility: VisibilityState) => {
-      if (useBackend) {
-        await updateColumnConfig({ column_visibility: visibility });
+    (visibility: VisibilityState) => {
+      if (shouldUseBackend) {
+        return updateColumnConfig({ column_visibility: visibility });
       } else {
         setLocalColumnVisibility(visibility);
+        return Promise.resolve(); // Return resolved promise for consistency
       }
     },
-    [useBackend, updateColumnConfig, setLocalColumnVisibility]
+    [shouldUseBackend, updateColumnConfig, setLocalColumnVisibility]
   );
 
   const setColumnOrder = useCallback(
-    async (order: ColumnOrderState) => {
-      if (useBackend) {
-        await updateColumnConfig({ column_order: order });
+    (order: ColumnOrderState) => {
+      if (shouldUseBackend) {
+        return updateColumnConfig({ column_order: order });
       } else {
         setLocalColumnOrder(order);
+        return Promise.resolve(); // Return resolved promise for consistency
       }
     },
-    [useBackend, updateColumnConfig, setLocalColumnOrder]
+    [shouldUseBackend, updateColumnConfig, setLocalColumnOrder]
   );
 
   const setColumnRenameMapping = useCallback(
-    async (mapping: ColumnRenameMapping) => {
-      if (useBackend) {
-        await updateColumnConfig({ column_rename_mapping: mapping });
+    (mapping: ColumnRenameMapping) => {
+      if (shouldUseBackend) {
+        return updateColumnConfig({ column_rename_mapping: mapping });
       } else {
         setLocalColumnRenameMapping(mapping);
+        return Promise.resolve(); // Return resolved promise for consistency
       }
     },
-    [useBackend, updateColumnConfig, setLocalColumnRenameMapping]
+    [shouldUseBackend, updateColumnConfig, setLocalColumnRenameMapping]
   );
 
   const setColumnTimeFormats = useCallback(
-    async (formats: Record<string, TimeFormatOption>) => {
-      if (useBackend) {
-        await updateColumnConfig({ column_time_formats: formats });
+    (formats: Record<string, TimeFormatOption>) => {
+      if (shouldUseBackend) {
+        return updateColumnConfig({ column_time_formats: formats });
       } else {
         setLocalColumnTimeFormats(formats);
+        return Promise.resolve(); // Return resolved promise for consistency
       }
     },
-    [useBackend, updateColumnConfig, setLocalColumnTimeFormats]
+    [shouldUseBackend, updateColumnConfig, setLocalColumnTimeFormats]
   );
 
   const setColumnListFormats = useCallback(
-    async (formats: Record<string, ListFormatOption>) => {
-      if (useBackend) {
-        await updateColumnConfig({ column_list_formats: formats });
+    (formats: Record<string, ListFormatOption>) => {
+      if (shouldUseBackend) {
+        return updateColumnConfig({ column_list_formats: formats });
       } else {
         setLocalColumnListFormats(formats);
+        return Promise.resolve(); // Return resolved promise for consistency
       }
     },
-    [useBackend, updateColumnConfig, setLocalColumnListFormats]
+    [shouldUseBackend, updateColumnConfig, setLocalColumnListFormats]
   );
 
   return {
@@ -154,7 +170,7 @@ export const usePresetColumnState = ({
     setColumnTimeFormats,
     setColumnListFormats,
     isLoading,
-    useBackend,
+    useBackend: shouldUseBackend,
   };
 };
 
diff --git a/keep-ui/widgets/alerts-table/ui/ColumnSelection.tsx b/keep-ui/widgets/alerts-table/ui/ColumnSelection.tsx
@@ -26,8 +26,8 @@ export default function ColumnSelection({
 }: AlertColumnsSelectProps) {
   const tableColumns = table.getAllColumns();
 
-  // Use the new unified column state hook
-  // For now, we'll use backend if presetId is provided, otherwise fall back to local storage
+  // Use the unified column state hook - it will automatically determine
+  // whether to use backend or local storage based on preset type
   const {
     columnVisibility,
     columnOrder,
@@ -38,7 +38,7 @@ export default function ColumnSelection({
   } = usePresetColumnState({
     presetName,
     presetId,
-    useBackend: !!presetId, // Enable backend usage when preset ID is available
+    useBackend: !!presetId, // Try to use backend if preset ID is available
   });
 
   const [searchTerm, setSearchTerm] = useState("");
@@ -77,6 +77,7 @@ export default function ColumnSelection({
     );
 
     try {
+      // Both setters now return promises for consistency
       await Promise.all([
         setColumnVisibility(newColumnVisibility),
         setColumnOrder(finalOrder)
@@ -128,10 +129,10 @@ export default function ColumnSelection({
         className="mt-4" 
         color="orange" 
         type="submit"
-        loading={isLoading}
-        disabled={isLoading}
+        loading={useBackend && isLoading}
+        disabled={useBackend && isLoading}
       >
-        {isLoading ? "Saving..." : "Save changes"}
+        {useBackend && isLoading ? "Saving..." : "Save changes"}
       </Button>
     </form>
   );
diff --git a/test-column-config.md b/test-column-config.md
@@ -0,0 +1,78 @@
+# Column Configuration Test Guide
+
+This guide helps verify that the new backend-based column configuration works correctly while maintaining backward compatibility.
+
+## Test Cases
+
+### 1. Static Preset (Feed) - Should Use Local Storage
+```bash
+# Navigate to: http://localhost:3000/alerts/feed
+# Expected behavior: Uses local storage, no "Synced across devices" indicator
+```
+
+**Steps:**
+1. Go to `/alerts/feed`
+2. Click Settings (gear icon) → Columns tab
+3. **Verify**: No "Synced across devices" badge shown
+4. Enable some columns (e.g., search for "tags" and enable tag columns)
+5. Click "Save changes"
+6. Refresh page → columns should persist (from local storage)
+7. Open in incognito/another browser → columns should NOT be shared
+
+### 2. Dynamic Preset - Should Use Backend
+```bash
+# Navigate to: http://localhost:3000/alerts/[dynamic-preset-name]
+# Where [dynamic-preset-name] is a user-created preset
+# Expected behavior: Uses backend, shows "Synced across devices" indicator
+```
+
+**Steps:**
+1. Create a new preset or use existing dynamic preset
+2. Go to `/alerts/[preset-name]`  
+3. Click Settings → Columns tab
+4. **Verify**: "Synced across devices" badge is shown
+5. Configure columns
+6. Click "Save changes"
+7. Open in another browser/device → columns should be shared
+
+### 3. E2E Test Fix Verification
+```bash
+# Run the specific failing test
+cd /workspace
+poetry run pytest tests/e2e_tests/incidents_alerts_tests/test_filtering_sort_search_on_alerts.py::test_multi_sort_asc_dsc -v
+```
+
+**Expected result:** Test should pass because:
+- Feed preset uses local storage (as before)
+- Column selection works synchronously
+- `tags.customerName` column appears in table header after enabling
+
+## API Testing
+
+### Test Backend Endpoints (for dynamic presets only)
+```bash
+# Get column config
+curl -X GET "http://localhost:8080/preset/{preset-id}/column-config" \
+     -H "Authorization: Bearer {token}"
+
+# Update column config  
+curl -X PUT "http://localhost:8080/preset/{preset-id}/column-config" \
+     -H "Content-Type: application/json" \
+     -H "Authorization: Bearer {token}" \
+     -d '{
+       "column_visibility": {"name": true, "tags.customerName": true},
+       "column_order": ["severity", "status", "name", "tags.customerName"]
+     }'
+```
+
+## Troubleshooting
+
+### If E2E test still fails:
+1. Check that static preset IDs are correct
+2. Verify local storage fallback is working
+3. Ensure column visibility updates are synchronous for local storage
+
+### If backend config doesn't work:
+1. Check preset has valid (non-static) ID
+2. Verify API endpoints are accessible
+3. Check network requests in browser devtools
