From 18405b746729ee800e8cc3f2a338f86c0bc0aaa1 Mon Sep 17 00:00:00 2001 From: Aapeli Date: Mon, 25 Nov 2024 23:05:17 -0500 Subject: [PATCH] Clean up user lists --- app/web/components/UsersList.test.tsx | 226 ++++++++++++++++++ app/web/components/UsersList.tsx | 82 +++++++ .../communities/CommunityModeratorsDialog.tsx | 29 +-- .../CommunityModeratorsSection.tsx | 37 +-- .../communities/events/EventAttendees.tsx | 15 +- .../events/EventAttendeesDialog.tsx | 25 +- .../communities/events/EventOrganizers.tsx | 6 - .../events/EventOrganizersDialog.tsx | 27 +-- .../communities/events/EventUsers.stories.tsx | 10 - .../communities/events/EventUsers.test.tsx | 85 ++----- .../communities/events/EventUsers.tsx | 53 ++-- app/web/features/communities/events/hooks.ts | 33 +-- app/web/features/communities/hooks.ts | 12 +- .../connections/friends/useFriendList.ts | 2 +- app/web/features/userQueries/useLiteUsers.ts | 7 +- 15 files changed, 384 insertions(+), 265 deletions(-) create mode 100644 app/web/components/UsersList.test.tsx create mode 100644 app/web/components/UsersList.tsx diff --git a/app/web/components/UsersList.test.tsx b/app/web/components/UsersList.test.tsx new file mode 100644 index 0000000000..5af7e4996c --- /dev/null +++ b/app/web/components/UsersList.test.tsx @@ -0,0 +1,226 @@ +import { + render, + screen, + waitForElementToBeRemoved, +} from "@testing-library/react"; +import { getProfileLinkA11yLabel } from "components/Avatar/constants"; +import { USER_TITLE_SKELETON_TEST_ID } from "components/UserSummary"; +import { service } from "service"; +import users from "test/fixtures/liteUsers.json"; +import wrapper from "test/hookWrapper"; +import { getLiteUsers } from "test/serviceMockDefaults"; +import { assertErrorAlert } from "test/utils"; + +import UsersList from "./UsersList"; + +const getLiteUsersMock = service.user.getLiteUsers as jest.MockedFunction< + typeof service.user.getLiteUsers +>; + +describe("UsersList", () => { + beforeEach(() => { + getLiteUsersMock.mockImplementation(getLiteUsers); + }); + + it("shows the users in a list if the user IDs and users map have loaded", async () => { + render(, { wrapper }); + + await waitForElementToBeRemoved( + screen.queryAllByTestId(USER_TITLE_SKELETON_TEST_ID) + ); + + // User 1 + expect(screen.getByRole("img", { name: users[0].name })).toBeVisible(); + expect( + screen.getByRole("heading", { name: `${users[0].name}, ${users[0].age}` }) + ).toBeVisible(); + + // User 2 + expect( + screen.getByRole("link", { name: getProfileLinkA11yLabel(users[1].name) }) + ).toBeVisible(); + expect( + screen.getByRole("heading", { name: `${users[1].name}, ${users[1].age}` }) + ).toBeVisible(); + }); + + it("shows a loading spinner when userIds are undefined", () => { + render( + I'm at the end!} + emptyListChildren={<>I show up when the map is empty!} + />, + { wrapper } + ); + + expect(screen.queryByRole("progressbar")).toBeInTheDocument(); + + expect( + screen.queryByTestId(USER_TITLE_SKELETON_TEST_ID) + ).not.toBeInTheDocument(); + + expect(screen.queryByText("I'm at the end!")).not.toBeInTheDocument(); + expect( + screen.queryByText("I show up when the map is empty!") + ).not.toBeInTheDocument(); + }); + + it("shows a loading skeleton while map is fetching", () => { + render( + I'm at the end!} + emptyListChildren={<>I show up when the map is empty!} + />, + { wrapper } + ); + + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + + expect(screen.getByTestId(USER_TITLE_SKELETON_TEST_ID)).toBeVisible(); + + expect(screen.queryByText("I'm at the end!")).not.toBeInTheDocument(); + expect( + screen.queryByText("I show up when the map is empty!") + ).not.toBeInTheDocument(); + }); + + it("hides skeleton when map is done fetching", () => { + render(, { wrapper }); + + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + + expect(screen.getByTestId(USER_TITLE_SKELETON_TEST_ID)).toBeVisible(); + }); + + it("shows only found users when map is done fetching", async () => { + render(, { wrapper }); + + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + + await waitForElementToBeRemoved( + screen.queryAllByTestId(USER_TITLE_SKELETON_TEST_ID) + ); + + // have user 2 + expect( + screen.getByRole("link", { name: getProfileLinkA11yLabel(users[1].name) }) + ).toBeVisible(); + expect( + screen.getByRole("heading", { name: `${users[1].name}, ${users[1].age}` }) + ).toBeVisible(); + + // don't have non-existent user 99 + expect( + screen.queryByTestId(USER_TITLE_SKELETON_TEST_ID) + ).not.toBeInTheDocument(); + }); + + it("shows endChildren but not emptyListChildren when map is not empty", async () => { + render( + I'm at the end!} + emptyListChildren={<>I show up when the map is empty!} + />, + { wrapper } + ); + + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + + await waitForElementToBeRemoved( + screen.queryAllByTestId(USER_TITLE_SKELETON_TEST_ID) + ); + + // have user 2 + expect( + screen.getByRole("link", { name: getProfileLinkA11yLabel(users[1].name) }) + ).toBeVisible(); + expect( + screen.getByRole("heading", { name: `${users[1].name}, ${users[1].age}` }) + ).toBeVisible(); + + // don't have non-existent user 99 + expect( + screen.queryByTestId(USER_TITLE_SKELETON_TEST_ID) + ).not.toBeInTheDocument(); + + // have end children + expect(await screen.findByText("I'm at the end!")).toBeVisible(); + // don't have empty children + expect( + screen.queryByText("I show up when the map is empty!") + ).not.toBeInTheDocument(); + }); + + it("shows emptyListChildren but not endChildren when map is empty", async () => { + render( + I'm at the end!} + emptyListChildren={<>I show up when the map is empty!} + />, + { wrapper } + ); + + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + + expect( + screen.queryByTestId(USER_TITLE_SKELETON_TEST_ID) + ).not.toBeInTheDocument(); + + // don't have end children + expect(screen.queryByText("I'm at the end!")).not.toBeInTheDocument(); + // have empty children + expect( + await screen.findByText("I show up when the map is empty!") + ).toBeVisible(); + }); + + it("shows emptyListChildren but not endChildren when map is not empty but no users were found", async () => { + render( + I'm at the end!} + emptyListChildren={<>I show up when the map is empty!} + />, + { wrapper } + ); + + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + + await waitForElementToBeRemoved( + screen.queryAllByTestId(USER_TITLE_SKELETON_TEST_ID) + ); + + // don't have end children + expect(screen.queryByText("I'm at the end!")).not.toBeInTheDocument(); + // have empty children + expect( + await screen.findByText("I show up when the map is empty!") + ).toBeVisible(); + }); + + it("shows an error alert if the event user IDs failed to load", async () => { + const errorMessage = "Error loading event users"; + render( + , + { wrapper } + ); + + await assertErrorAlert(errorMessage); + // Empty state should not be shown if there is an error + expect( + screen.queryByText("There aren't any users for this event yet!") + ).not.toBeInTheDocument(); + }); +}); diff --git a/app/web/components/UsersList.tsx b/app/web/components/UsersList.tsx new file mode 100644 index 0000000000..4d384b7366 --- /dev/null +++ b/app/web/components/UsersList.tsx @@ -0,0 +1,82 @@ +import { CircularProgress, styled } from "@mui/material"; +import UserSummary from "components/UserSummary"; +import { useLiteUsers } from "features/userQueries/useLiteUsers"; +import { RpcError } from "grpc-web"; +import { ReactNode } from "react"; + +import Alert from "./Alert"; + +const ContainingDiv = styled("div")(({ theme }) => ({ + padding: theme.spacing(2), +})); + +const StyledUsersDiv = styled("div")(({ theme }) => ({ + display: "grid", + marginBlockStart: theme.spacing(2), + rowGap: theme.spacing(1), +})); + +export interface UsersListProps { + userIds: number[] | undefined; + emptyListChildren?: ReactNode; + endChildren?: ReactNode; + error?: RpcError | null; +} + +/** + * A cute list of components for each userId. Automatically fetches the user info. + * + * A spinner shows up while `userIds` is `undefined`. When this component is fetching the lite users, it will show skeletons (the right number). + * + * If any users are not found or userIds is an empty list, this will show `emptyListChildren`. + * + * The end of the list will show `endChildren` if the list is not empty (this is a good place to add a "load more" button) + */ +export default function UsersList({ + userIds, + emptyListChildren, + endChildren, + error, +}: UsersListProps) { + const { + data: users, + isLoading: isLoadingLiteUsers, + error: usersError, + } = useLiteUsers(userIds || []); + + // this is undefined if userIds is undefined or users hasn't loaded, otherwise it's an actual list + const foundUserIds = + userIds && + (userIds.length > 0 ? userIds?.filter((userId) => users?.has(userId)) : []); + + return ( + + {error ? ( + {error.message} + ) : usersError ? ( + {usersError.message} + ) : !userIds ? ( + + ) : isLoadingLiteUsers ? ( + + {userIds.map((userId) => ( + + ))} + + ) : foundUserIds && foundUserIds.length > 0 ? ( + + {foundUserIds.map((userId) => ( + + ))} + <>{endChildren} + + ) : ( + <>{emptyListChildren} + )} + + ); +} diff --git a/app/web/features/communities/CommunityModeratorsDialog.tsx b/app/web/features/communities/CommunityModeratorsDialog.tsx index c1a2af1033..da631d25e0 100644 --- a/app/web/features/communities/CommunityModeratorsDialog.tsx +++ b/app/web/features/communities/CommunityModeratorsDialog.tsx @@ -1,5 +1,3 @@ -import { CircularProgress } from "@mui/material"; -import Alert from "components/Alert"; import Button from "components/Button"; import { AccessibleDialogProps, @@ -8,7 +6,7 @@ import { DialogContent, DialogTitle, } from "components/Dialog"; -import UserSummary from "components/UserSummary"; +import UsersList from "components/UsersList"; import { useTranslation } from "i18n"; import { COMMUNITIES } from "i18n/namespaces"; import { Community } from "proto/communities_pb"; @@ -29,15 +27,8 @@ export default function CommunityModeratorsDialog({ open = false, }: CommunityModeratorsDialogProps) { const { t } = useTranslation([COMMUNITIES]); - const { - adminIds, - adminUsers, - error, - fetchNextPage, - isFetchingNextPage, - isLoading, - hasNextPage, - } = useListAdmins(community.communityId, "all"); + const { adminIds, error, fetchNextPage, isFetchingNextPage, hasNextPage } = + useListAdmins(community.communityId, "all"); return ( @@ -45,19 +36,7 @@ export default function CommunityModeratorsDialog({ {t("communities:community_moderators")} - {error && {error.message}} - {isLoading ? ( - - ) : adminIds && adminIds.length > 0 && adminUsers ? ( - adminIds.map((id) => ( - - )) - ) : null} + {hasNextPage && ( diff --git a/app/web/features/communities/CommunityModeratorsSection.tsx b/app/web/features/communities/CommunityModeratorsSection.tsx index 6e38388196..0aa31f31d9 100644 --- a/app/web/features/communities/CommunityModeratorsSection.tsx +++ b/app/web/features/communities/CommunityModeratorsSection.tsx @@ -1,9 +1,7 @@ import { Typography } from "@mui/material"; -import Alert from "components/Alert"; import Button from "components/Button"; -import CircularProgress from "components/CircularProgress"; import { CommunityLeadersIcon } from "components/Icons"; -import UserSummary from "components/UserSummary"; +import UsersList from "components/UsersList"; import { useTranslation } from "i18n"; import { COMMUNITIES } from "i18n/namespaces"; import { Community } from "proto/communities_pb"; @@ -38,7 +36,7 @@ export default function CommunityModeratorsSection({ }: CommunityModeratorsSectionProps) { const { t } = useTranslation([COMMUNITIES]); const classes = useStyles(); - const { adminIds, adminUsers, error, isLoading, hasNextPage } = useListAdmins( + const { adminIds, error, hasNextPage } = useListAdmins( community.communityId, "summary" ); @@ -49,28 +47,15 @@ export default function CommunityModeratorsSection({ } variant="h2"> {t("communities:community_moderators")} - {error ? ( - {error.message} - ) : isLoading ? ( - - ) : adminIds && adminIds.length > 0 ? ( - adminUsers && ( -
- {adminIds.map((id) => ( - - ))} -
- ) - ) : ( - - {t("communities:no_moderators")} - - )} + + {t("communities:no_moderators")} + + } + /> {hasNextPage && ( <> - )} - - ) : ( - userIds.length === 0 && - !error && {emptyState} - )} + ) + } + emptyListChildren={ + !error && {emptyState} + } + /> ); } diff --git a/app/web/features/communities/events/hooks.ts b/app/web/features/communities/events/hooks.ts index fcc4746516..66a817592c 100644 --- a/app/web/features/communities/events/hooks.ts +++ b/app/web/features/communities/events/hooks.ts @@ -6,7 +6,6 @@ import { myEventsKey, QueryType, } from "features/queryKeys"; -import { useLiteUsers } from "features/userQueries/useLiteUsers"; import { RpcError } from "grpc-web"; import { Event, @@ -45,21 +44,11 @@ export function useEventOrganizers({ getNextPageParam: (lastPage) => lastPage.nextPageToken || undefined, enabled, }); - const organizerIds = - query.data?.pages.flatMap((res) => res.organizerUserIdsList) ?? []; - const { - data: organizers, - isLoading: isOrganizersLoading, - isRefetching: isOrganizersRefetching, - } = useLiteUsers(organizerIds); + const organizerIds = query.data?.pages.flatMap( + (res) => res.organizerUserIdsList + ); - return { - ...query, - organizerIds, - organizers, - isOrganizersLoading, - isOrganizersRefetching, - }; + return { ...query, organizerIds }; } export function useEventAttendees({ @@ -78,20 +67,12 @@ export function useEventAttendees({ getNextPageParam: (lastPage) => lastPage.nextPageToken || undefined, enabled, }); - const attendeesIds = - query.data?.pages.flatMap((data) => data.attendeeUserIdsList) ?? []; - const { - data: attendees, - isLoading: isAttendeesLoading, - isRefetching: isAttendeesRefetching, - } = useLiteUsers(attendeesIds); - + const attendeesIds = query.data?.pages.flatMap( + (data) => data.attendeeUserIdsList + ); return { ...query, attendeesIds, - attendees, - isAttendeesLoading, - isAttendeesRefetching, }; } diff --git a/app/web/features/communities/hooks.ts b/app/web/features/communities/hooks.ts index 0bea6a94c2..71d17b0f9c 100644 --- a/app/web/features/communities/hooks.ts +++ b/app/web/features/communities/hooks.ts @@ -12,7 +12,6 @@ import { subCommunitiesKey, threadKey, } from "features/queryKeys"; -import { useLiteUsers } from "features/userQueries/useLiteUsers"; import { RpcError } from "grpc-web"; import { useRouter } from "next/router"; import { @@ -152,16 +151,7 @@ export const useListAdmins = (communityId: number, type: QueryType) => { } ); const adminIds = query.data?.pages.flatMap((page) => page.adminUserIdsList); - const { data: adminUsers, isLoading: isAdminUsersLoading } = useLiteUsers( - adminIds ?? [] - ); - - return { - ...query, - adminIds, - adminUsers, - isLoading: query.isLoading || isAdminUsersLoading, - }; + return { ...query, adminIds }; }; export const useListMembers = (communityId?: number) => diff --git a/app/web/features/connections/friends/useFriendList.ts b/app/web/features/connections/friends/useFriendList.ts index c233f7553a..5d6de7b96f 100644 --- a/app/web/features/connections/friends/useFriendList.ts +++ b/app/web/features/connections/friends/useFriendList.ts @@ -20,7 +20,7 @@ function useFriendList() { isLoading: isLiteUsersLoading, isError: isLiteUserError, error: liteUserError, - } = useLiteUsersList(friendIds || []); + } = useLiteUsersList(friendIds); if (liteUserError) { errors.push(liteUserError.message); diff --git a/app/web/features/userQueries/useLiteUsers.ts b/app/web/features/userQueries/useLiteUsers.ts index eacbe7feac..b8e46862fd 100644 --- a/app/web/features/userQueries/useLiteUsers.ts +++ b/app/web/features/userQueries/useLiteUsers.ts @@ -8,7 +8,8 @@ import { service } from "service"; import { userStaleTime } from "./constants"; // React Query typically retains the last successful data until the next successful fetch -function useLiteUsers(ids: (number | undefined)[]) { +// if ids is `[]`, then `data` is `undefined` +function useLiteUsers(ids: (number | undefined)[] | undefined) { const nonFalseyIds = ids?.filter((id): id is number => !!id); // remove duplicate IDs from this list const uniqueIds = Array.from(new Set(nonFalseyIds)); @@ -40,9 +41,9 @@ function useLiteUsers(ids: (number | undefined)[]) { } // Like above, but returns users in a list of the same size in same order -function useLiteUsersList(ids: (number | undefined)[]) { +function useLiteUsersList(ids: (number | undefined)[] | undefined) { const liteUsersMap = useLiteUsers(ids); - const usersList = ids.map((id) => liteUsersMap.data?.get(id)); + const usersList = ids?.map((id) => liteUsersMap.data?.get(id)); return { ...liteUsersMap, usersById: liteUsersMap.data, data: usersList }; }