Merge pull request #86 from UgnisSoftware/UGN-318

Fix UGN-318 fix indexes messing up after delete

Author: masiulis

src/steps/ValidationStep/ValidationStep.tsx

@@ -1,9 +1,9 @@
 import React, { useCallback, useMemo, useState } from "react"
-import { Box, Button, Heading, ModalBody, Switch, Text, useStyleConfig } from "@chakra-ui/react"
+import { Box, Button, Heading, ModalBody, Switch, useStyleConfig } from "@chakra-ui/react"
 import { ContinueButton } from "../../components/ContinueButton"
 import { useRsi } from "../../hooks/useRsi"
 import type { Meta } from "./types"
-import { addErrorsAndRunHooks, addIndexes } from "./utils/dataMutations"
+import { addErrorsAndRunHooks } from "./utils/dataMutations"
 import { generateColumns } from "./components/columns"
 import { Table } from "../../components/Table"
 import { SubmitDataAlert } from "../../components/Alerts/SubmitDataAlert"
@@ -20,7 +20,7 @@ export const ValidationStep = <T extends string>({ initialData }: Props<T>) => {
   const styles = useStyleConfig("ValidationStep") as typeof themeOverrides["components"]["ValidationStep"]["baseStyle"]
 
   const [data, setData] = useState<(Data<T> & Meta)[]>(
-    useMemo(() => addErrorsAndRunHooks<T>(addIndexes<T>(initialHook(initialData)), fields, rowHook, tableHook), []),
+    useMemo(() => addErrorsAndRunHooks<T>(initialHook(initialData), fields, rowHook, tableHook), []),
   )
   const [selectedRows, setSelectedRows] = useState<ReadonlySet<number | string>>(new Set())
   const [filterByErrors, setFilterByErrors] = useState(false)
@@ -30,7 +30,7 @@ export const ValidationStep = <T extends string>({ initialData }: Props<T>) => {
     (rows: typeof data) => {
       setData(addErrorsAndRunHooks<T>(rows, fields, rowHook, tableHook))
     },
-    [setData, addErrorsAndRunHooks, rowHook, tableHook],
+    [setData, addErrorsAndRunHooks, rowHook, tableHook, fields],
   )
 
   const deleteSelectedRows = () => {
@@ -41,24 +41,25 @@ export const ValidationStep = <T extends string>({ initialData }: Props<T>) => {
     }
   }
 
-  const updateRowTable = useCallback(
+  const updateRow = useCallback(
     (rows: typeof data, changedData?: RowsChangeData<typeof data[number]>) => {
-      const changes = changedData?.indexes.reduce((acc, val) => {
-        const realIndex = rows[val].__index
-        acc[realIndex] = rows[val]
+      const changes = changedData?.indexes.reduce((acc, index) => {
+        // when data is filtered val !== actual index in data
+        const realIndex = data.findIndex((value) => value.__index === rows[index].__index)
+        acc[realIndex] = rows[index]
         return acc
       }, {} as Record<number, typeof data[number]>)
       const newData = Object.assign([], data, changes)
       updateData(newData)
     },
-    [data, setData, addErrorsAndRunHooks, rowHook, tableHook],
+    [data, updateData],
   )
 
   const columns = useMemo(() => generateColumns(fields), [fields, generateColumns])
 
   const tableData = useMemo(() => {
     if (filterByErrors) {
-      return data.filter((value, index) => {
+      return data.filter((value) => {
         if (value?.__errors) {
           return Object.values(value.__errors)?.filter((err) => err.level === "error").length
         }
@@ -127,7 +128,7 @@ export const ValidationStep = <T extends string>({ initialData }: Props<T>) => {
         <Table
           rowKeyGetter={rowKeyGetter}
           rows={tableData}
-          onRowsChange={updateRowTable}
+          onRowsChange={updateRow}
           columns={columns}
           selectedRows={selectedRows}
           onSelectedRowsChange={setSelectedRows}

src/steps/ValidationStep/tests/ValidationStep.test.tsx

@@ -154,7 +154,6 @@ describe("Validation step tests", () => {
 
     // don't really know another way to select an empty cell
     const emptyCell = screen.getAllByRole("gridcell", { name: undefined })[1]
-    console.log(emptyCell)
     userEvent.click(emptyCell)
 
     await userEvent.keyboard(FINAL_NAME + "{enter}")
@@ -328,6 +327,72 @@ describe("Validation step tests", () => {
     expect(validRow).toBeInTheDocument()
   })
 
+  test("Deletes selected rows, changes the last one", async () => {
+    const FIRST_DELETE = "first"
+    const SECOND_DELETE = "second"
+    const THIRD = "third"
+    const THIRD_CHANGED = "third_changed"
+
+    const initialData = [
+      {
+        name: FIRST_DELETE,
+      },
+      {
+        name: SECOND_DELETE,
+      },
+      {
+        name: THIRD,
+      },
+    ]
+    const fields = [
+      {
+        label: "Name",
+        key: "name",
+        fieldType: {
+          type: "input",
+        },
+      },
+    ] as const
+    render(
+      <Providers theme={defaultTheme} rsiValues={{ ...mockValues, fields }}>
+        <ModalWrapper isOpen={true} onClose={() => {}}>
+          <ValidationStep initialData={initialData} />
+        </ModalWrapper>
+      </Providers>,
+    )
+
+    const allRowsWithHeader = await screen.findAllByRole("row")
+    expect(allRowsWithHeader).toHaveLength(4)
+
+    const switchFilters = screen.getAllByRole("checkbox", {
+      name: "Select",
+    })
+
+    userEvent.click(switchFilters[0])
+    userEvent.click(switchFilters[1])
+
+    const discardButton = screen.getByRole("button", {
+      name: "Discard selected rows",
+    })
+
+    userEvent.click(discardButton)
+
+    const filteredRowsWithHeader = await screen.findAllByRole("row")
+    expect(filteredRowsWithHeader).toHaveLength(2)
+
+    const nameCell = screen.getByRole("gridcell", {
+      name: THIRD,
+    })
+
+    userEvent.click(nameCell)
+
+    screen.getByRole<HTMLInputElement>("textbox")
+    await userEvent.keyboard(THIRD_CHANGED + "{enter}")
+
+    const validRow = screen.getByText(THIRD_CHANGED)
+    expect(validRow).toBeInTheDocument()
+  })
+
   test("All inputs change values", async () => {
     const NAME = "John"
     const NEW_NAME = "Johnny"

src/steps/ValidationStep/types.ts

@@ -1,5 +1,5 @@
 import type { Info } from "../../types"
 
-export type Meta = { __index: number; __errors?: Error | null }
+export type Meta = { __index: string; __errors?: Error | null }
 export type Error = { [key: string]: Info }
-export type Errors = { [id: string]: Error }
+export type Errors = { [id: string]: Error }

src/steps/ValidationStep/utils/dataMutations.ts

@@ -2,7 +2,7 @@ import type { Data, Fields, Info, RowHook, TableHook } from "../../../types"
 import type { Meta, Errors } from "../types"
 
 export const addErrorsAndRunHooks = <T extends string>(
-  data: (Data<T> & Meta)[],
+  data: (Data<T> & Partial<Meta>)[],
   fields: Fields<T>,
   rowHook?: RowHook<T>,
   tableHook?: TableHook<T>,
@@ -17,11 +17,11 @@ export const addErrorsAndRunHooks = <T extends string>(
   }
 
   if (tableHook) {
-    data = addIndexes(tableHook(data, addHookError))
+    data = tableHook(data, addHookError)
   }
 
   if (rowHook) {
-    data = addIndexes(data.map((value, index) => rowHook(value, (...props) => addHookError(index, ...props), data)))
+    data = data.map((value, index) => rowHook(value, (...props) => addHookError(index, ...props), data))
   }
 
   fields.forEach((field) => {
@@ -78,20 +78,18 @@ export const addErrorsAndRunHooks = <T extends string>(
   })
 
   return data.map((value, index) => {
+    // This is required only for table. Mutates to prevent needless rerenders
+    if (!("__index" in value)) {
+      value.__index = crypto.randomUUID()
+    }
+    const newValue = value as Data<T> & Meta
+
     if (errors[index]) {
-      return { ...value, __errors: errors[index] }
+      return { ...newValue, __errors: errors[index] }
     }
     if (!errors[index] && value?.__errors) {
-      return { ...value, __errors: null }
+      return { ...newValue, __errors: null }
     }
-    return value
+    return newValue
   })
 }
-
-export const addIndexes = <T extends string>(arr: Data<T>[]): (Data<T> & { __index: number })[] =>
-  arr.map((value, index) => {
-    if ("__index" in value) {
-      return value as Data<T> & { __index: number }
-    }
-    return { ...value, __index: index }
-  })

src/tests/setup.ts

@@ -1,5 +1,11 @@
+const nodeCrypto = require("crypto")
+
 // Yeeted from https://github.com/adazzle/react-data-grid/blob/main/test/setup.ts
 if (typeof window !== "undefined") {
+  if (!window.crypto) {
+    ;(window as any).crypto = nodeCrypto
+  }
+
   window.ResizeObserver ??= class {
     callback: ResizeObserverCallback