diff --git a/e2e/test/scenarios/admin-2/people/people.cy.spec.js b/e2e/test/scenarios/admin-2/people/people.cy.spec.js index 2e28104ce25d0ae90ba35ce0d102f6fc02cee08c..c6c62f983768d1d7a7fa491f232573a25daf38d3 100644 --- a/e2e/test/scenarios/admin-2/people/people.cy.spec.js +++ b/e2e/test/scenarios/admin-2/people/people.cy.spec.js @@ -396,13 +396,16 @@ describe("scenarios > admin > people", () => { assertTableRowsCount(PAGE_SIZE); cy.findByLabelText("Previous page").should("be.disabled"); - cy.findByLabelText("Next page").click(); + // cy.findByLabelText("Next page").click(); + cy.findByTestId("next-page-btn").click(); waitForUserRequests(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Loading...").should("not.exist"); // Page 2 - cy.findByTextEnsureVisible(`${PAGE_SIZE + 1} - ${NEW_TOTAL_USERS}`); + cy.findByTestId("people-list-footer") + .findByText(`${PAGE_SIZE + 1} - ${NEW_TOTAL_USERS}`) + .should("be.visible"); assertTableRowsCount(NEW_TOTAL_USERS % PAGE_SIZE); cy.findByLabelText("Next page").should("be.disabled"); diff --git a/frontend/src/metabase-types/api/user.ts b/frontend/src/metabase-types/api/user.ts index 760a8faf8ff93fce14ed70c0ab4b0caf55ca25d9..61f7640efbc66e90cbb119441787277dce307d8a 100644 --- a/frontend/src/metabase-types/api/user.ts +++ b/frontend/src/metabase-types/api/user.ts @@ -54,3 +54,7 @@ export type UserInfo = Pick< | "is_superuser" | "is_qbnewb" >; + +export type UserListQuery = { + recipients: boolean; +}; diff --git a/frontend/src/metabase/admin/people/components/PeopleList.jsx b/frontend/src/metabase/admin/people/components/PeopleList.jsx index 5c597978c4c54eb2d439c3ef1304becc68a1e3ae..e14fa8796080f3c097b604b21bc5f34699382a21 100644 --- a/frontend/src/metabase/admin/people/components/PeopleList.jsx +++ b/frontend/src/metabase/admin/people/components/PeopleList.jsx @@ -219,7 +219,10 @@ const PeopleList = ({ </table> {hasUsers && ( - <div className="flex align-center justify-between p2"> + <div + className="flex align-center justify-between p2" + data-testid="people-list-footer" + > <div className="text-medium text-bold"> {ngettext( msgid`${total} person found`, diff --git a/frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.ts b/frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.ts index ba5d526e4f91f0edc5e16e40b69665b84453de2e..80f455373db459736efad7324ad6fecec72560ae 100644 --- a/frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.ts +++ b/frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.ts @@ -4,10 +4,10 @@ import type { UseEntityListQueryResult, } from "metabase/common/hooks/use-entity-list-query"; import { useEntityListQuery } from "metabase/common/hooks/use-entity-list-query"; -import type { UserListResult } from "metabase-types/api"; +import type { UserListQuery, UserListResult } from "metabase-types/api"; export const useUserListQuery = ( - props: UseEntityListQueryProps<Record<string, never>> = {}, + props: UseEntityListQueryProps<UserListQuery> = {}, ): UseEntityListQueryResult<UserListResult> => { return useEntityListQuery(props, { fetchList: Users.actions.fetchList, diff --git a/frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.unit.spec.tsx b/frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.unit.spec.tsx index 8c15abfa09055030a71efa635f3ead381d977110..a7a785d77182e44486cad555cc8120ff7baae49c 100644 --- a/frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.unit.spec.tsx +++ b/frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.unit.spec.tsx @@ -1,4 +1,8 @@ -import { setupUsersEndpoints } from "__support__/server-mocks"; +import fetchMock from "fetch-mock"; +import { + setupUserRecipientsEndpoint, + setupUsersEndpoints, +} from "__support__/server-mocks"; import { renderWithProviders, screen, @@ -12,8 +16,19 @@ import { useUserListQuery } from "./use-user-list-query"; const TEST_USER = createMockUserInfo(); -function TestComponent() { - const { data = [], metadata, isLoading, error } = useUserListQuery(); +type TestComponentProps = { getRecipients?: boolean }; + +function TestComponent({ getRecipients = false }: TestComponentProps) { + const { + data = [], + metadata, + isLoading, + error, + } = useUserListQuery({ + query: { + recipients: getRecipients, + }, + }); if (isLoading || error) { return <LoadingAndErrorWrapper loading={isLoading} error={error} />; @@ -32,9 +47,12 @@ function TestComponent() { ); } -function setup() { +function setup({ getRecipients = false }: TestComponentProps = {}) { setupUsersEndpoints([TEST_USER]); - renderWithProviders(<TestComponent />); + setupUserRecipientsEndpoint({ + users: [TEST_USER], + }); + renderWithProviders(<TestComponent getRecipients={getRecipients} />); } describe("useUserListQuery", () => { @@ -56,4 +74,18 @@ describe("useUserListQuery", () => { within(screen.getByTestId("metadata")).getByText("No metadata"), ).toBeInTheDocument(); }); + + it("should call /api/user when recipient isn't passed or is false", () => { + setup(); + + expect(fetchMock.calls("path:/api/user")).toHaveLength(1); + expect(fetchMock.calls("path:/api/user/recipients")).toHaveLength(0); + }); + + it("should call /api/user/recipients when the `recipient` is passed", () => { + setup({ getRecipients: true }); + + expect(fetchMock.calls("path:/api/user")).toHaveLength(0); + expect(fetchMock.calls("path:/api/user/recipients")).toHaveLength(1); + }); }); diff --git a/frontend/src/metabase/entities/users.js b/frontend/src/metabase/entities/users.js index 5b9619057cf9f6e3e6d3376fad14bad603597f42..39e1ca86ca3486152af55778a519ea091b7e48ae 100644 --- a/frontend/src/metabase/entities/users.js +++ b/frontend/src/metabase/entities/users.js @@ -9,6 +9,7 @@ import { createEntity } from "metabase/lib/entities"; import { UserApi, SessionApi } from "metabase/services"; import { generatePassword } from "metabase/lib/security"; +import { GET } from "metabase/lib/api"; import forms from "./users/forms"; export const DEACTIVATE = "metabase/entities/users/DEACTIVATE"; @@ -24,6 +25,9 @@ function loadMemberships() { return require("metabase/admin/people/people").loadMemberships(); } +const getUserList = GET("/api/user"); +const getRecipientsList = GET("/api/user/recipients"); + const Users = createEntity({ name: "users", nameOne: "user", @@ -31,6 +35,11 @@ const Users = createEntity({ path: "/api/user", + api: { + list: ({ recipients = false, ...args }) => + recipients ? getRecipientsList() : getUserList(args), + }, + objectSelectors: { getName: user => user.common_name, }, diff --git a/frontend/src/metabase/nav/components/search/SearchResults/SearchResults.unit.spec.tsx b/frontend/src/metabase/nav/components/search/SearchResults/SearchResults.unit.spec.tsx index 3de190d89d66644b9a2b07e87fa4952487b8355e..f21f0d885b440e4f6a12cfbefabb45523f8894e6 100644 --- a/frontend/src/metabase/nav/components/search/SearchResults/SearchResults.unit.spec.tsx +++ b/frontend/src/metabase/nav/components/search/SearchResults/SearchResults.unit.spec.tsx @@ -1,5 +1,6 @@ /* eslint-disable react/prop-types */ import userEvent from "@testing-library/user-event"; +import fetchMock from "fetch-mock"; import { Route } from "react-router"; import { renderWithProviders, @@ -47,8 +48,8 @@ const setup = async ({ searchText = "test", footer = null, }: SearchResultsSetupProps = {}) => { - setupSearchEndpoints(searchResults); setupUserRecipientsEndpoint({ users: [createMockUser()] }); + setupSearchEndpoints(searchResults); setupCollectionByIdEndpoint({ collections: [createMockCollection()], }); @@ -177,4 +178,9 @@ describe("SearchResults", () => { TEST_SEARCH_RESULTS.length.toString(), ); }); + + it("should only call the /api/user/recipients endpoint once even if there are multiple search results", async () => { + await setup(); + expect(fetchMock.calls("path:/api/user/recipients").length).toBe(1); + }); }); diff --git a/frontend/src/metabase/search/components/InfoText/InfoTextEditedInfo.tsx b/frontend/src/metabase/search/components/InfoText/InfoTextEditedInfo.tsx index c4f373dea75dd7b6ff6f994fe33167b70d86c64b..ac0733a7c224db170b5d45d467ed1bbe63ff0dd2 100644 --- a/frontend/src/metabase/search/components/InfoText/InfoTextEditedInfo.tsx +++ b/frontend/src/metabase/search/components/InfoText/InfoTextEditedInfo.tsx @@ -1,8 +1,7 @@ import dayjs from "dayjs"; -import { useAsync } from "react-use"; import { t } from "ttag"; import { isNull } from "underscore"; -import { UserApi } from "metabase/services"; +import { useUserListQuery } from "metabase/common/hooks/use-user-list-query"; import type { UserListResult } from "metabase-types/api"; import Tooltip from "metabase/core/components/Tooltip"; import { isNotNull } from "metabase/lib/types"; @@ -37,12 +36,11 @@ export const InfoTextEditedInfo = ({ result: WrappedResult; isCompact?: boolean; }) => { - const { - loading: isLoading, - value, - error, - } = useAsync<() => Promise<{ data: UserListResult[] }>>(UserApi.list); - const users = value?.data ?? []; + const { isLoading, data, error } = useUserListQuery({ + query: { recipients: true }, + }); + + const users = data ?? []; if (isLoading) { return ( diff --git a/frontend/src/metabase/search/components/SearchUserPicker/SearchUserPicker.tsx b/frontend/src/metabase/search/components/SearchUserPicker/SearchUserPicker.tsx index db9b56ade906eabad32fec3eaaabb4ebfb151710..e9812d1ac08ac51b742cb3fbdd500288037425d0 100644 --- a/frontend/src/metabase/search/components/SearchUserPicker/SearchUserPicker.tsx +++ b/frontend/src/metabase/search/components/SearchUserPicker/SearchUserPicker.tsx @@ -1,8 +1,7 @@ -import { useAsync } from "react-use"; import { without } from "underscore"; import { useState } from "react"; import { t } from "ttag"; -import { UserApi } from "metabase/services"; +import { useUserListQuery } from "metabase/common/hooks/use-user-list-query"; import type { UserId, UserListResult } from "metabase-types/api"; import { Center, Text } from "metabase/ui"; import { SearchFilterPopoverWrapper } from "metabase/search/components/SearchFilterPopoverWrapper"; @@ -24,11 +23,11 @@ export const SearchUserPicker = ({ value: UserId[]; onChange: (value: UserId[]) => void; }) => { - const { loading: isLoading, value: userApiReturnValue } = useAsync< - () => Promise<{ data: UserListResult[] }> - >(UserApi.list); + const { isLoading, data } = useUserListQuery({ + query: { recipients: true }, + }); - const users = userApiReturnValue?.data ?? []; + const users = data ?? []; const [userFilter, setUserFilter] = useState(""); const [selectedUserIds, setSelectedUserIds] = useState(value); diff --git a/frontend/test/__support__/server-mocks/user.ts b/frontend/test/__support__/server-mocks/user.ts index 68cfdc52bc1ddd5ab5f604add05e8dfda7714b31..ce083382aa2a681178fccedbc38b59656940836d 100644 --- a/frontend/test/__support__/server-mocks/user.ts +++ b/frontend/test/__support__/server-mocks/user.ts @@ -6,8 +6,8 @@ export function setupUserEndpoints(user: UserListResult) { } export function setupUsersEndpoints(users: UserListResult[]) { - fetchMock.get("path:/api/user", users); users.forEach(user => setupUserEndpoints(user)); + return fetchMock.get("path:/api/user", users); } export function setupCurrentUserEndpoint(user: User) {