Clean up keys; Fix up GoalTimeline tests (#2721)

sillsdev · Oct 30, 2023 · bf5b4e9 · bf5b4e9
1 parent 42b599c
commit bf5b4e9
Show file tree

Hide file tree

Showing 17 changed files with 125 additions and 131 deletions.
diff --git a/src/components/DataEntry/DataEntryTable/index.tsx b/src/components/DataEntry/DataEntryTable/index.tsx
@@ -865,9 +865,12 @@ export default function DataEntryTable(
         </Grid>
 
         {state.recentWords.map((wordAccess, index) => (
-          <Grid item xs={12} key={index}>
+          <Grid
+            item
+            key={`${wordAccess.word.id}_${wordAccess.senseGuid}`}
+            xs={12}
+          >
             <RecentEntry
-              key={wordAccess.word.id + "_" + wordAccess.senseGuid}
               rowIndex={index}
               entry={wordAccess.word}
               senseGuid={wordAccess.senseGuid}

diff --git a/src/components/GoalTimeline/GoalList.tsx b/src/components/GoalTimeline/GoalList.tsx
@@ -55,24 +55,29 @@ export default function GoalList(props: GoalListProps): ReactElement {
   const [scrollVisible, setScrollVisible] = useState<boolean | undefined>();
   const tileSize = props.size / 3 - 1.25;
 
+  const id = (g: Goal): string =>
+    props.completed ? `completed-goal-${g.guid}` : `new-goal-${g.name}`;
+
   return (
     <ImageList
       style={gridStyle(props.orientation, props.size, scrollVisible)}
       cols={props.orientation === "horizontal" ? props.numPanes : 1}
       onMouseOver={() => setScrollVisible(props.scrollable)}
       onMouseLeave={() => setScrollVisible(false)}
     >
-      {props.data.length > 0
-        ? props.data.map((g, i) => {
-            const buttonProps = {
-              id: props.completed
-                ? `completed-goal-${i}`
-                : `new-goal-${g.name}`,
-              onClick: () => props.handleChange(g),
-            };
-            return makeGoalTile(tileSize, props.orientation, g, buttonProps);
-          })
-        : makeGoalTile(tileSize, props.orientation)}
+      {props.data.length > 0 ? (
+        props.data.map((g) => (
+          <GoalTile
+            buttonProps={{ id: id(g), onClick: () => props.handleChange(g) }}
+            goal={g}
+            key={g.guid || g.name}
+            orientation={props.orientation}
+            size={tileSize}
+          />
+        ))
+      ) : (
+        <GoalTile size={tileSize} orientation={props.orientation} />
+      )}
       <div
         ref={(element: HTMLDivElement) => {
           if (props.scrollToEnd && element) {
@@ -93,19 +98,22 @@ function buttonStyle(orientation: Orientation, size: number): CSSProperties {
   }
 }
 
-export function makeGoalTile(
-  size: number,
-  orientation: Orientation,
-  goal?: Goal,
-  buttonProps?: ButtonProps
-): ReactElement {
+interface GoalTileProps {
+  buttonProps?: ButtonProps;
+  goal?: Goal;
+  orientation: Orientation;
+  size: number;
+}
+
+function GoalTile(props: GoalTileProps): ReactElement {
+  const goal = props.goal;
   return (
-    <ImageListItem key={goal?.guid + orientation} cols={1}>
+    <ImageListItem cols={1}>
       <Button
-        {...buttonProps}
+        {...props.buttonProps}
         color="primary"
         variant={goal ? "outlined" : "contained"}
-        style={buttonStyle(orientation, size)}
+        style={buttonStyle(props.orientation, props.size)}
         disabled={
           /* Hide completed, except goalTypes for which the completed view is implemented. */
           !goal ||
@@ -125,6 +133,7 @@ export function makeGoalTile(
 interface GoalInfoProps {
   goal?: Goal;
 }
+
 function GoalInfo(props: GoalInfoProps): ReactElement {
   const { t } = useTranslation();
 

diff --git a/src/components/GoalTimeline/tests/GoalRedux.test.tsx b/src/components/GoalTimeline/tests/GoalRedux.test.tsx
@@ -2,7 +2,7 @@ import "@testing-library/jest-dom";
 import { act, cleanup } from "@testing-library/react";
 
 import "tests/reactI18nextMock";
-import { MergeUndoIds, Permission, User, UserEdit } from "api/models";
+import { Edit, MergeUndoIds, Permission, User, UserEdit } from "api/models";
 import * as LocalStorage from "backend/localStorage";
 import GoalTimeline from "components/GoalTimeline";
 import {
@@ -64,20 +64,21 @@ const mockUpdateUser = jest.fn();
 function setMockFunctions(): void {
   mockAddGoalToUserEdit.mockResolvedValue(0);
   mockAddStepToGoal.mockResolvedValue(0);
-  mockCreateUserEdit.mockResolvedValue(mockUser);
+  mockCreateUserEdit.mockResolvedValue(mockUser());
   mockGetCurrentPermissions.mockResolvedValue([
     Permission.CharacterInventory,
     Permission.MergeAndReviewEntries,
   ]);
   mockGetDuplicates.mockResolvedValue(goalDataMock.plannedWords);
   mockGetGraylistEntries.mockResolvedValue([]);
-  mockGetUser.mockResolvedValue(mockUser);
-  mockGetUserEditById.mockResolvedValue(mockUserEdit);
-  mockUpdateUser.mockResolvedValue(mockUser);
+  mockGetUser.mockResolvedValue(mockUser());
+  mockGetUserEditById.mockResolvedValue(mockUserEdit(true));
+  mockUpdateUser.mockResolvedValue(mockUser());
 }
 
 const mockProjectId = "123";
 const mockUserEditId = "456";
+const mockUserId = "789";
 const mockCompletedMerge: MergeUndoIds = {
   parentIds: ["1", "2"],
   childIds: ["3", "4"],
@@ -86,34 +87,31 @@ const mockCharInvChanges: CharacterChange[] = [
   ["'", CharacterStatus.Undecided, CharacterStatus.Accepted],
 ];
 
-const mockUserEdit: UserEdit = {
-  id: mockUserEditId,
-  edits: [
-    {
-      guid: "edit-guid",
-      goalType: 4,
-      stepData: [],
-      changes: JSON.stringify(mockCompletedMerge),
-    },
-  ],
-  projectId: "",
-};
-const mockNoUserEdits: UserEdit = {
+const mockEdit = (): Edit => ({
+  changes: JSON.stringify(mockCompletedMerge),
+  goalType: 4,
+  guid: "edit-guid",
+  stepData: [],
+});
+const mockUserEdit = (hasEdit: boolean): UserEdit => ({
+  edits: hasEdit ? [mockEdit()] : [],
   id: mockUserEditId,
-  edits: [],
-  projectId: "",
-};
-const mockUserId = "789";
-const mockUser = newUser("First Last", "username");
-mockUser.id = mockUserId;
-mockUser.workedProjects[mockProjectId] = mockUserEditId;
+  projectId: mockProjectId,
+});
+const mockUser = (): User => ({
+  ...newUser("First Last", "username"),
+  id: mockUserId,
+  workedProjects: { [mockProjectId]: mockUserEditId },
+});
 
 function setupLocalStorage(): void {
-  LocalStorage.setCurrentUser(mockUser);
+  LocalStorage.setCurrentUser(mockUser());
   LocalStorage.setProjectId(mockProjectId);
 }
 
 beforeEach(() => {
+  // Restore any spied functions before clearing all
+  jest.restoreAllMocks();
   jest.clearAllMocks();
   setMockFunctions();
   setupLocalStorage();
@@ -140,10 +138,12 @@ describe("asyncGetUserEdits", () => {
     await act(async () => {
       renderWithProviders(<GoalTimeline />, { store: store });
     });
-    const convertGoalToEditSpy = jest.spyOn(goalUtilities, "convertEditToGoal");
-    await store.dispatch(asyncGetUserEdits());
+    const convertEditToGoalSpy = jest.spyOn(goalUtilities, "convertEditToGoal");
+    await act(async () => {
+      await store.dispatch(asyncGetUserEdits());
+    });
     expect(store.getState().goalsState.history).toHaveLength(1);
-    expect(convertGoalToEditSpy).toBeCalledTimes(1);
+    expect(convertEditToGoalSpy).toBeCalledTimes(1);
   });
 
   it("backend returns no user edits", async () => {
@@ -155,23 +155,26 @@ describe("asyncGetUserEdits", () => {
 
     // setup mocks for testing the action/reducers
     jest.clearAllMocks();
-    const convertGoalToEditSpy = jest.spyOn(goalUtilities, "convertEditToGoal");
-    mockGetUserEditById.mockResolvedValueOnce(mockNoUserEdits);
+    const convertEditToGoalSpy = jest.spyOn(goalUtilities, "convertEditToGoal");
+    mockGetUserEditById.mockResolvedValueOnce(mockUserEdit(false));
 
     // dispatch the action
-    await store.dispatch(asyncGetUserEdits());
+    await act(async () => {
+      await store.dispatch(asyncGetUserEdits());
+    });
     expect(store.getState().goalsState.history).toHaveLength(0);
-    expect(convertGoalToEditSpy).toBeCalledTimes(0);
+    expect(convertEditToGoalSpy).toBeCalledTimes(0);
   });
 
   it("creates new user edits", async () => {
     const store = setupStore();
-
     await act(async () => {
       renderWithProviders(<GoalTimeline />, { store: store });
     });
-    LocalStorage.setCurrentUser(newUser("", ""));
-    await store.dispatch(asyncGetUserEdits());
+    LocalStorage.setCurrentUser(newUser());
+    await act(async () => {
+      await store.dispatch(asyncGetUserEdits());
+    });
     expect(store.getState().goalsState.history).toHaveLength(0);
     expect(mockCreateUserEdit).toBeCalledTimes(1);
   });
@@ -186,7 +189,7 @@ describe("asyncAddGoal", () => {
 
     const goal = new MergeDups();
     await act(async () => {
-      store.dispatch(asyncAddGoal(goal));
+      await store.dispatch(asyncAddGoal(goal));
     });
     // verify the new goal was loaded
     const currentGoal = store.getState().goalsState.currentGoal as MergeDups;
@@ -207,7 +210,7 @@ describe("asyncAddGoal", () => {
 
     const goal = new CreateCharInv();
     await act(async () => {
-      store.dispatch(asyncAddGoal(goal));
+      await store.dispatch(asyncAddGoal(goal));
     });
     // verify the new goal was loaded
     const currentGoal = store.getState().goalsState
@@ -233,7 +236,7 @@ describe("asyncAdvanceStep", () => {
     // create mergeDups goal
     const goal = new MergeDups();
     await act(async () => {
-      store.dispatch(asyncAddGoal(goal));
+      await store.dispatch(asyncAddGoal(goal));
     });
     let currentGoal = store.getState().goalsState.currentGoal as MergeDups;
     expect(currentGoal.currentStep).toBe(0);
@@ -243,7 +246,7 @@ describe("asyncAdvanceStep", () => {
     for (let i = 0; i < numSteps - 1; i++) {
       // dispatch asyncAdvanceStep
       await act(async () => {
-        store.dispatch(asyncAdvanceStep());
+        await store.dispatch(asyncAdvanceStep());
       });
       // verify current step is incremented each time
       currentGoal = store.getState().goalsState.currentGoal as MergeDups;
@@ -253,7 +256,7 @@ describe("asyncAdvanceStep", () => {
     }
     // iterate past the last step
     await act(async () => {
-      store.dispatch(asyncAdvanceStep());
+      await store.dispatch(asyncAdvanceStep());
     });
     expect(store.getState().goalsState.currentGoal.currentStep).toBe(7);
     expect(mockNavigate).toHaveBeenCalledWith(Path.GoalNext);
@@ -268,12 +271,12 @@ describe("asyncAdvanceStep", () => {
     // create character inventory goal
     const goal = new CreateCharInv();
     await act(async () => {
-      store.dispatch(asyncAddGoal(goal));
+      await store.dispatch(asyncAddGoal(goal));
     });
     expect(store.getState().goalsState.currentGoal.numSteps).toBe(1);
     // iterate past the last step
     await act(async () => {
-      store.dispatch(asyncAdvanceStep());
+      await store.dispatch(asyncAdvanceStep());
     });
     expect(store.getState().goalsState.currentGoal.currentStep).toBe(0);
     expect(mockNavigate).toHaveBeenCalledWith(Path.Goals);
@@ -290,9 +293,9 @@ describe("asyncUpdateGoal", () => {
     // create CreateCharInv goal
     const goal = new CreateCharInv();
     await act(async () => {
-      store.dispatch(asyncAddGoal(goal));
+      await store.dispatch(asyncAddGoal(goal));
+      store.dispatch(addCharInvChangesToGoal(mockCharInvChanges));
     });
-    store.dispatch(addCharInvChangesToGoal(mockCharInvChanges));
     const changes = store.getState().goalsState.currentGoal
       .changes as CharInvChanges;
     expect(changes!.charChanges).toBe(mockCharInvChanges);
@@ -314,7 +317,7 @@ describe("asyncUpdateGoal", () => {
     // create MergeDups goal
     const goal = new MergeDups();
     await act(async () => {
-      store.dispatch(asyncAddGoal(goal));
+      await store.dispatch(asyncAddGoal(goal));
     });
     // dispatch asyncUpdateGoal()
     await act(async () => {
@@ -339,7 +342,7 @@ describe("asyncUpdateGoal", () => {
     // create ReviewDeferredDups goal
     const goal = new ReviewDeferredDups();
     await act(async () => {
-      store.dispatch(asyncAddGoal(goal));
+      await store.dispatch(asyncAddGoal(goal));
     });
     // dispatch asyncUpdateGoal()
     await act(async () => {

diff --git a/src/components/ProjectSettings/ProjectSchedule/CalendarView.tsx b/src/components/ProjectSettings/ProjectSchedule/CalendarView.tsx
@@ -39,7 +39,7 @@ export default function CalendarView(props: CalendarViewProps): ReactElement {
 
     return monthToRender.map((tempDayjs) => (
       <CalendarPicker
-        key={"calendarPick" + tempDayjs.toString()}
+        key={tempDayjs.toISOString()}
         components={{ LeftArrowIcon: Icon, RightArrowIcon: Icon }}
         readOnly
         disabled

diff --git a/src/components/ProjectSettings/ProjectSelect.tsx b/src/components/ProjectSettings/ProjectSelect.tsx
@@ -41,8 +41,8 @@ export default function ProjectSelect(
       sx={{ maxWidth: "100%" }}
       value={props.project.name}
     >
-      {projectList.map((p, i) => (
-        <MenuItem key={i} value={p.name}>
+      {projectList.map((p) => (
+        <MenuItem key={p.id} value={p.name}>
           {p.name}
         </MenuItem>
       ))}

diff --git a/src/components/Statistics/DomainStatistics.tsx b/src/components/Statistics/DomainStatistics.tsx
@@ -68,9 +68,9 @@ function TableRow(props: {
   return (
     <ListItem style={{ minWidth: "600px" }}>
       <Grid container wrap="nowrap" justifyContent="space-around">
-        <TableCell key={"id_" + props.dom.id} text={props.dom.id} />
-        <TableCell key={"domain_" + props.dom.id} text={props.dom.name} />
-        <TableCell key={"count_" + props.dom.id} text={props.count} />
+        <TableCell text={props.dom.id} />
+        <TableCell text={props.dom.name} />
+        <TableCell text={props.count} />
       </Grid>
     </ListItem>
   );

diff --git a/src/components/Statistics/UserStatistics.tsx b/src/components/Statistics/UserStatistics.tsx
@@ -61,18 +61,9 @@ function TableRow(props: { counts: SemanticDomainUserCount }): ReactElement {
   return (
     <ListItem style={{ minWidth: "600px" }}>
       <Grid container wrap="nowrap" justifyContent="space-around">
-        <TableCell
-          key={"username_" + props.counts.id}
-          text={props.counts.username}
-        />
-        <TableCell
-          key={"domCount_" + props.counts.id}
-          text={props.counts.domainCount}
-        />
-        <TableCell
-          key={"wordCount_" + props.counts.id}
-          text={props.counts.wordCount}
-        />
+        <TableCell text={props.counts.username} />
+        <TableCell text={props.counts.domainCount} />
+        <TableCell text={props.counts.wordCount} />
       </Grid>
     </ListItem>
   );