Repository: inovex/scrumlr.io
Pull request #3170 (2023-07-03 15:55:52)

refactor: move ParticipantExtendedInfo logic
## Description
<!-- Explain the changes you’ve made. It doesn’t need to be fancy and you don’t have to get too technical. -->
this PR moves the logic of changing the `Participant[]` to `ParticipantExtendedInfo[]` array used in `NoteAuthorList`.  
semantically nothing should change, the reason I did this is so the component can be used elsewhere, and we don't have to repeat the logic from the `Note` component, but instead just input the Participants.

**Note**: I might also change the name of the component since it will also be used in #3058 to display who made a reaction

## Changelog
<!-- Please provide a detailed list of the changes you have made to the codebase. -->
* move logic from `Note` component to `NoteAuthorList` component
* adjust tests because the type is now different in `Note`

## Checklist

- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas

github-actions[bot]: The deployment to the dev cluster was successful. You can find the deployment here: https://3170.development.scrumlr.fra.ics.inovex.io
This deployment is only for testing purposes and will be deleted after 1 week.
To redeploy rerun the workflow.
<strong>DO NOT STORE IMPORTANT DATA ON THIS DEPLOYMENT</strong>

<details>
<summary>Deployed Images</summary>

- ghcr.io/inovex/scrumlr.io/scrumlr-frontend:sha-0bd89e1

- ghcr.io/inovex/scrumlr.io/scrumlr-server:sha-0bd89e1

</details>
octomind-dev[bot]: 
# 🐙 Octomind

⏳ Testing in progress 

| description | execution state | status | details |
|--|--|--|--|

github-actions[bot]: The deployment to the dev cluster was successful. You can find the deployment here: https://3170.development.scrumlr.fra.ics.inovex.io
This deployment is only for testing purposes and will be deleted after 1 week.
To redeploy rerun the workflow.
<strong>DO NOT STORE IMPORTANT DATA ON THIS DEPLOYMENT</strong>

<details>
<summary>Deployed Images</summary>

- ghcr.io/inovex/scrumlr.io/scrumlr-frontend:sha-a9abaee

- ghcr.io/inovex/scrumlr.io/scrumlr-server:sha-a9abaee

</details>
octomind-dev[bot]: 
# 🐙 Octomind

⏳ Testing in progress 

| description | execution state | status | details |
|--|--|--|--|

github-actions[bot]: The deployment to the dev cluster was successful. You can find the deployment here: https://3170.development.scrumlr.fra.ics.inovex.io
This deployment is only for testing purposes and will be deleted after 1 week.
To redeploy rerun the workflow.
<strong>DO NOT STORE IMPORTANT DATA ON THIS DEPLOYMENT</strong>

<details>
<summary>Deployed Images</summary>

- ghcr.io/inovex/scrumlr.io/scrumlr-frontend:sha-1dda32d

- ghcr.io/inovex/scrumlr.io/scrumlr-server:sha-1dda32d

</details>
octomind-dev[bot]: 
# 🐙 Octomind

⏳ Testing in progress 

| description | execution state | status | details |
|--|--|--|--|


Patch:
diff --git a/src/components/Note/Note.tsx b/src/components/Note/Note.tsx
index 0d74b6fa..44ce587d 100644
--- a/src/components/Note/Note.tsx
+++ b/src/components/Note/Note.tsx
@@ -1,5 +1,5 @@
 import classNames from "classnames";
-import {useRef, useEffect, KeyboardEvent} from "react";
+import {KeyboardEvent, useEffect, useRef} from "react";
 import {useDispatch} from "react-redux";
 import {useNavigate} from "react-router";
 import {useTranslation} from "react-i18next";
@@ -7,12 +7,12 @@ import {isEqual} from "underscore";
 import {Votes} from "components/Votes";
 import {useAppSelector} from "store";
 import {Actions} from "store/action";
-import {Participant, ParticipantExtendedInfo} from "types/participant";
-import "./Note.scss";
+import {Participant} from "types/participant";
 import {addProtocol} from "utils/images";
 import {useImageChecker} from "utils/hooks/useImageChecker";
 import {Sortable} from "components/DragAndDrop/Sortable";
 import {NoteAuthorList} from "./NoteAuthorList/NoteAuthorList";
+import "./Note.scss";
 
 interface NoteProps {
   noteId: string;
@@ -32,47 +32,19 @@ export const Note = (props: NoteProps) => {
   const isShared = useAppSelector((state) => state.board.data?.sharedNote === props.noteId);
   const allowStacking = useAppSelector((state) => state.board.data?.allowStacking ?? true);
   const showAuthors = useAppSelector((state) => !!state.board.data?.showAuthors);
+  const me = useAppSelector((state) => state.participants?.self);
   const moderating = useAppSelector((state) => state.view.moderating);
   const isModerator = props.viewer.role === "MODERATOR" || props.viewer.role === "OWNER";
 
   // all authors of a note, including its children if it's a stack.
-  // next to the Participant object there's also helper properties (displayName, isSelf) for easier identification.
-  const authors: ParticipantExtendedInfo[] = useAppSelector((state) => {
+  const authors = useAppSelector((state) => {
     const noteAuthor = state.participants?.others.find((p) => p.user.id === note?.author) ?? state.participants?.self;
     const childrenNoteAuthors = state.notes
       // get all notes which are in the same stack as the main note
       .filter((n) => n.position.stack === props.noteId)
       // find the corresponding author for the respective note in the list of other participants. if none is found, the author is therefore yourself
       .map((c) => state.participants?.others.find((p) => p.user.id === c.author) ?? state.participants?.self);
-    const allAuthorsRaw = [noteAuthor, ...childrenNoteAuthors];
-    // process and filter
-    const allAuthors = allAuthorsRaw
-      .map((a) => {
-        const isSelf = a?.user.id === state.participants?.self.user.id;
-        const displayName = isSelf ? t("Note.me") : a!.user.name;
-        return {
-          ...a,
-          displayName,
-          isSelf,
-        } as ParticipantExtendedInfo;
-      })
-      // remove duplicates (because notes can have multiple children by the same authors)
-      .filter((v, i, self) => self.findIndex((a) => a.user?.id === v.user?.id) === i);
-
-    // if self is part of the authors, we always want it to be visible
-    const selfIndex = allAuthors.findIndex((a) => a.isSelf);
-    if (selfIndex > 1) {
-      // in-place swap with second author
-      [allAuthors[selfIndex], allAuthors[1]] = [allAuthors[1], allAuthors[selfIndex]];
-    }
-
-    // if showAuthors is disabled, we still want to see cards written by yourself if you're the stack author.
-    // the other authors are excluded as we only require the stack author
-    if (!showAuthors && props.viewer.user.id === noteAuthor!.user!.id) {
-      return [allAuthors[0]]; // stack author is always first element
-    }
-
-    return allAuthors;
+    return [noteAuthor, ...childrenNoteAuthors].filter(Boolean) as Participant[]; // remove undefined values (which shouldn't exists, only for TS type assertion)
   }, isEqual);
 
   /* eslint-disable */
@@ -128,7 +100,7 @@ export const Note = (props: NoteProps) => {
             <img
               src={addProtocol(note.text)}
               className="note__image"
-              alt={t("Note.userImageAlt", {user: authors[0].isSelf ? t("Note.you") : authors[0].displayName})}
+              alt={t("Note.userImageAlt", {user: authors[0].user.id === me?.user.id ? t("Note.you") : authors[0].user.name})}
               draggable={false} // safari bugfix
             />
           </div>
diff --git a/src/components/Note/NoteAuthorList/NoteAuthorList.tsx b/src/components/Note/NoteAuthorList/NoteAuthorList.tsx
index 3fa7133e..5f59a09b 100644
--- a/src/components/Note/NoteAuthorList/NoteAuthorList.tsx
+++ b/src/components/Note/NoteAuthorList/NoteAuthorList.tsx
@@ -1,14 +1,48 @@
 import classNames from "classnames";
 import {Participant, ParticipantExtendedInfo} from "types/participant";
+import {useTranslation} from "react-i18next";
 import {UserAvatar} from "../../BoardUsers";
 import "./NoteAuthorList.scss";
 
-type Props = {
-  authors: ParticipantExtendedInfo[];
+type NoteAuthorListProps = {
+  authors: Participant[];
   showAuthors: boolean;
   viewer: Participant;
 };
-export const NoteAuthorList = (props: Props) => {
+export const NoteAuthorList = (props: NoteAuthorListProps) => {
+  const {t} = useTranslation();
+
+  // next to the Participant object there's also helper properties (displayName, isSelf) for easier identification.
+  const prepareAuthors = (authors: Participant[]): ParticipantExtendedInfo[] => {
+    const allAuthors = authors
+      .map((a) => {
+        const isSelf = a?.user.id === props.viewer.user.id; // assertion: viewer is self
+        const displayName = isSelf ? t("Note.me") : a!.user.name;
+        return {
+          ...a,
+          displayName,
+          isSelf,
+        } as ParticipantExtendedInfo;
+      })
+      // remove duplicates (because notes can have multiple children by the same authors)
+      .filter((v, i, self) => self.findIndex((a) => a.user?.id === v.user?.id) === i);
+
+    // if self is part of the authors, we always want it to be visible
+    const selfIndex = allAuthors.findIndex((a) => a.isSelf);
+    if (selfIndex > 1) {
+      // in-place swap with second author
+      [allAuthors[selfIndex], allAuthors[1]] = [allAuthors[1], allAuthors[selfIndex]];
+    }
+
+    // if showAuthors is disabled, we still want to see cards written by yourself if you're the stack author.
+    // the other authors are excluded as we only require the stack author
+    if (!props.showAuthors && props.viewer.user.id === props.authors[0].user!.id) {
+      return [allAuthors[0]]; // stack author is always first element
+    }
+
+    return allAuthors;
+  };
+  const authorExtendedInfo = prepareAuthors(props.authors);
   // expected behaviour:
   // 1 => p
   // 2 => p p
@@ -17,9 +51,9 @@ export const NoteAuthorList = (props: Props) => {
   // n where n >= 4 => p p (n - 2)
   // where (n) displays a number and p displays an avatar
   const SHOW_MAX_AUTHORS = 3;
-  const stackAuthor = props.authors[0];
-  const restAuthors = props.authors.slice();
-  const slicedAuthors = restAuthors.splice(0, props.authors.length > SHOW_MAX_AUTHORS ? SHOW_MAX_AUTHORS - 1 : SHOW_MAX_AUTHORS); // max first n authors
+  const stackAuthor = authorExtendedInfo[0];
+  const restAuthors = authorExtendedInfo.slice();
+  const slicedAuthors = restAuthors.splice(0, authorExtendedInfo.length > SHOW_MAX_AUTHORS ? SHOW_MAX_AUTHORS - 1 : SHOW_MAX_AUTHORS); // max first n authors
   const restUsersExist = restAuthors.length > 0;
   const restUsersTitle = restAuthors.map((a) => a.displayName).join("\x0A"); // join names with line breaks
   return props.showAuthors || props.viewer.user.id === stackAuthor.user!.id ? (


Test patch:
diff --git a/src/components/Note/NoteAuthorList/__tests__/NoteAuthorList.test.tsx b/src/components/Note/NoteAuthorList/__tests__/NoteAuthorList.test.tsx
index ba12a0d6..fc767470 100644
--- a/src/components/Note/NoteAuthorList/__tests__/NoteAuthorList.test.tsx
+++ b/src/components/Note/NoteAuthorList/__tests__/NoteAuthorList.test.tsx
@@ -2,17 +2,17 @@ import {ApplicationState} from "types";
 import {Provider} from "react-redux";
 import getTestStore from "utils/test/getTestStore";
 import {NoteAuthorList} from "../NoteAuthorList";
-import {Participant, ParticipantExtendedInfo} from "types/participant";
+import {Participant} from "types/participant";
 import {render} from "testUtils";
 import getTestParticipant from "utils/test/getTestParticipant";
 
-const AUTHOR1: ParticipantExtendedInfo = {...getTestParticipant({user: {id: "test-participant-id-1", name: "test-participant-name-1"}}), isSelf: true, displayName: "Me"};
-const AUTHOR2: ParticipantExtendedInfo = {...getTestParticipant({user: {id: "test-participant-id-2", name: "test-participant-name-2"}}), isSelf: false, displayName: "test-user-2"};
-const AUTHOR3: ParticipantExtendedInfo = {...getTestParticipant({user: {id: "test-participant-id-3", name: "test-participant-name-3"}}), isSelf: false, displayName: "test-user-3"};
-const AUTHOR4: ParticipantExtendedInfo = {...getTestParticipant({user: {id: "test-participant-id-4", name: "test-participant-name-4"}}), isSelf: false, displayName: "test-user-4"};
+const AUTHOR1: Participant = getTestParticipant({user: {id: "test-participant-id-1", name: "test-participant-name-1"}});
+const AUTHOR2: Participant = getTestParticipant({user: {id: "test-participant-id-2", name: "test-participant-name-2"}});
+const AUTHOR3: Participant = getTestParticipant({user: {id: "test-participant-id-3", name: "test-participant-name-3"}});
+const AUTHOR4: Participant = getTestParticipant({user: {id: "test-participant-id-4", name: "test-participant-name-4"}});
 const VIEWER1: Participant = getTestParticipant({user: {id: "test-participant-id-1", name: "test-participant-name-1"}});
 
-const createNoteAuthorList = (authors: ParticipantExtendedInfo[], showAuthors: boolean, overwrite?: Partial<ApplicationState>) => {
+const createNoteAuthorList = (authors: Participant[], showAuthors: boolean, overwrite?: Partial<ApplicationState>) => {
   return (
     <Provider store={getTestStore(overwrite)}>
       <NoteAuthorList authors={authors} showAuthors={showAuthors} viewer={VIEWER1} />
@@ -44,6 +44,17 @@ describe("NoteAuthorList", () => {
       expect(container.firstChild).toHaveClass("note-author-list--self");
     });
 
+    it("should contain Me as name", () => {
+      const {container} = render(createNoteAuthorList([AUTHOR1], true));
+      expect(container.firstChild).toHaveTextContent("Me");
+    });
+
+    it("should contain author 2s name", () => {
+      const {container} = render(createNoteAuthorList([AUTHOR2], true));
+      expect(container.firstChild).not.toHaveTextContent(AUTHOR1.user.name);
+      expect(container.firstChild).toHaveTextContent(AUTHOR2.user.name);
+    });
+
     it("should not have a rest bubble", () => {
       const {container} = render(createNoteAuthorList([AUTHOR1, AUTHOR2, AUTHOR3], true));
       expect(container.querySelector(".note-author-rest")).toBeNull();
diff --git a/src/components/Note/__tests__/Note.test.tsx b/src/components/Note/__tests__/Note.test.tsx
index dc9c5d62..b6b0ab89 100644
--- a/src/components/Note/__tests__/Note.test.tsx
+++ b/src/components/Note/__tests__/Note.test.tsx
@@ -8,7 +8,6 @@ import * as reactRouter from "react-router";
 import {ApplicationState} from "types";
 import {BoardState} from "types/board";
 import getTestApplicationState from "utils/test/getTestApplicationState";
-import getTestNote from "utils/test/getTestNote";
 import {CustomDndContext} from "components/DragAndDrop/CustomDndContext";
 
 const NOTE_ID = "test-notes-id-1";
@@ -55,11 +54,6 @@ describe("Note", () => {
       const {container} = render(createNote(false));
       expect(container.firstChild).toHaveTextContent(note!.text);
     });
-
-    test("own note author name is me", () => {
-      const {container} = render(createNote(false, {notes: [getTestNote({id: "test-notes-id-1", author: getTestParticipant().user.id})]}));
-      expect(container.firstChild).toHaveTextContent("Me");
-    });
   });
 
   describe("author information", () => {