Remove redundant /user/recipient API calls (#35644)

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");
 

frontend/src/metabase-types/api/user.ts

@@ -54,3 +54,7 @@ export type UserInfo = Pick<
   | "is_superuser"
   | "is_qbnewb"
 >;
+
+export type UserListQuery = {
+  recipients: boolean;
+};

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`,

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,

frontend/src/metabase/common/hooks/use-user-list-query/use-user-list-query.unit.spec.tsx

-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);
+  });
 });

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,
   },

frontend/src/metabase/nav/components/search/SearchResults/SearchResults.unit.spec.tsx

 /* 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);
+  });
 });

frontend/src/metabase/search/components/InfoText/InfoTextEditedInfo.tsx

 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 (

frontend/src/metabase/search/components/SearchUserPicker/SearchUserPicker.tsx

-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);

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) {