fix(#10504): Assigning owners doesn't work for non-admin users (#10551)

* fix(#10504): Assigning owners doesn't work for non-admin users

* chore: improve naming

* chore: remove admin checks

* chore: fix type

* chore: use owner widget in glossary page for updating/adding/removing owner

* chore: minor fix

* fix: unit test

* test: add unit test

* test: add more test

openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryHeader/GlossaryHeader.component.tsx

@@ -13,27 +13,20 @@
 import { CheckOutlined, CloseOutlined } from '@ant-design/icons';
 import { Button, Col, Input, Row, Space, Tooltip, Typography } from 'antd';
 import Description from 'components/common/description/Description';
+import OwnerWidgetWrapper from 'components/common/OwnerWidget/OwnerWidgetWrapper.component';
 import ProfilePicture from 'components/common/ProfilePicture/ProfilePicture';
-import DropDownList from 'components/dropdown/DropDownList';
 import ReviewerModal from 'components/Modals/ReviewerModal/ReviewerModal.component';
 import { OperationPermission } from 'components/PermissionProvider/PermissionProvider.interface';
-import { WILD_CARD_CHAR } from 'constants/char.constants';
 import { getUserPath } from 'constants/constants';
 import { NO_PERMISSION_FOR_ACTION } from 'constants/HelperTextUtil';
 import { EntityReference, Glossary } from 'generated/entity/data/glossary';
 import { GlossaryTerm } from 'generated/entity/data/glossaryTerm';
-import { cloneDeep, debounce, includes, isEqual } from 'lodash';
+import { cloneDeep, includes, isEqual } from 'lodash';
-import React, { useCallback, useEffect, useMemo, useState } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import { useTranslation } from 'react-i18next';
 import { Link } from 'react-router-dom';
 import { getEntityName } from 'utils/EntityUtils';
-import { getOwnerList } from 'utils/ManageUtils';
 import SVGIcons, { Icons } from 'utils/SvgUtils';
-import {
-  isCurrentUserAdmin,
-  searchFormattedUsersAndTeams,
-  suggestFormattedUsersAndTeams,
-} from 'utils/UserDataUtils';
 
 export interface GlossaryHeaderProps {
   supportAddOwner?: boolean;
@@ -53,10 +46,7 @@ const GlossaryHeader = ({
   const [isNameEditing, setIsNameEditing] = useState<boolean>(false);
   const [isDescriptionEditable, setIsDescriptionEditable] =
     useState<boolean>(false);
-  const [listVisible, setListVisible] = useState<boolean>(false);
+  const [isEditOwner, setIsEditOwner] = useState<boolean>(false);
-  const [isUserLoading, setIsUserLoading] = useState<boolean>(false);
-  const [listOwners, setListOwners] = useState(getOwnerList());
-  const [searchText, setSearchText] = useState<string>('');
   const [showReviewerModal, setShowReviewerModal] = useState<boolean>(false);
 
   const editDisplayNamePermission = useMemo(() => {
@@ -95,71 +85,6 @@ const GlossaryHeader = ({
       setIsDescriptionEditable(false);
     }
   };
-  const getOwnerSearch = useCallback(
-    (searchQuery = WILD_CARD_CHAR, from = 1) => {
-      setIsUserLoading(true);
-      searchFormattedUsersAndTeams(searchQuery, from)
-        .then((res) => {
-          const { users, teams } = res;
-          setListOwners(getOwnerList(users, teams, false, searchQuery));
-        })
-        .catch(() => {
-          setListOwners([]);
-        })
-        .finally(() => {
-          setIsUserLoading(false);
-        });
-    },
-    [setListOwners, setIsUserLoading]
-  );
-  const handleSelectOwnerDropdown = () => {
-    setListVisible((visible) => {
-      const newState = !visible;
-
-      if (newState) {
-        getOwnerSearch();
-      }
-
-      return newState;
-    });
-  };
-  const getOwnerSuggestion = useCallback(
-    (qSearchText = '') => {
-      setIsUserLoading(true);
-      suggestFormattedUsersAndTeams(qSearchText)
-        .then((res) => {
-          const { users, teams } = res;
-          setListOwners(getOwnerList(users, teams, false, qSearchText));
-        })
-        .catch(() => {
-          setListOwners([]);
-        })
-        .finally(() => {
-          setIsUserLoading(false);
-        });
-    },
-    [setListOwners, setIsUserLoading]
-  );
-
-  const debouncedOnChange = useCallback(
-    (text: string): void => {
-      if (text) {
-        getOwnerSuggestion(text);
-      } else {
-        getOwnerSearch();
-      }
-    },
-    [getOwnerSuggestion, getOwnerSearch]
-  );
-
-  const debounceOnSearch = useCallback(debounce(debouncedOnChange, 400), [
-    debouncedOnChange,
-  ]);
-
-  const handleOwnerSearch = (text: string) => {
-    setSearchText(text);
-    debounceOnSearch(text);
-  };
 
   const handleRemoveReviewer = (id: string) => {
     let updatedGlossary = cloneDeep(selectedData);
@@ -174,22 +99,7 @@ const GlossaryHeader = ({
     onUpdate(updatedGlossary);
   };
 
-  const prepareOwner = (updatedOwner?: EntityReference) => {
+  const handleUpdatedOwner = (newOwner: Glossary['owner']) => {
-    return !isEqual(updatedOwner, selectedData.owner)
-      ? updatedOwner
-      : undefined;
-  };
-  const handleOwnerSelection = (
-    _e: React.MouseEvent<HTMLElement, MouseEvent>,
-    value = ''
-  ) => {
-    const owner = listOwners.find((item) => item.value === value);
-
-    if (owner) {
-      const newOwner = prepareOwner({
-        type: owner.type,
-        id: owner.value || '',
-      });
     if (newOwner) {
       const updatedData = {
         ...selectedData,
@@ -197,16 +107,15 @@ const GlossaryHeader = ({
       };
       onUpdate(updatedData);
     }
-    }
-    setListVisible(false);
   };
-  const onRemoveOwner = () => {
+
+  const handleRemoveOwner = () => {
     const updatedData = {
       ...selectedData,
       owner: undefined,
     };
     onUpdate(updatedData);
-    setListVisible(false);
+    setIsEditOwner(false);
   };
 
   const handleReviewerSave = (data: Array<EntityReference>) => {
@@ -330,23 +239,16 @@ const GlossaryHeader = ({
                   }
                   size="small"
                   type="text"
-                  onClick={handleSelectOwnerDropdown}
+                  onClick={() => setIsEditOwner(true)}
                 />
               </Tooltip>
-              {listVisible && (
+              {isEditOwner && (
-                <DropDownList
+                <OwnerWidgetWrapper
-                  showEmptyList
+                  currentUser={selectedData.owner}
-                  controlledSearchStr={searchText}
+                  hideWidget={() => setIsEditOwner(false)}
-                  dropDownList={listOwners}
+                  removeOwner={handleRemoveOwner}
-                  groupType="tab"
+                  updateUser={handleUpdatedOwner}
-                  horzPosRight={false}
+                  visible={isEditOwner}
-                  isLoading={isUserLoading}
-                  listGroups={['Teams', 'Users']}
-                  removeOwner={onRemoveOwner}
-                  showSearchBar={isCurrentUserAdmin()}
-                  value={selectedData.owner?.id || ''}
-                  onSearchTextChange={handleOwnerSearch}
-                  onSelect={handleOwnerSelection}
                 />
               )}
             </div>

openmetadata-ui/src/main/resources/ui/src/components/common/OwnerWidget/OwnerWidgetWrapper.component.tsx

@@ -11,31 +11,20 @@
  *  limitations under the License.
  */
 
-import { AxiosError } from 'axios';
+import DropDownList from 'components/dropdown/DropDownList';
+import { WILD_CARD_CHAR } from 'constants/char.constants';
+import { Table } from 'generated/entity/data/table';
+import { EntityReference } from 'generated/type/entityReference';
 import { debounce, isEqual, lowerCase } from 'lodash';
 import { LoadingState } from 'Models';
 import React, { useCallback, useEffect, useMemo, useState } from 'react';
-import { getGroupTypeTeams } from 'rest/userAPI';
+import { getOwnerList, OwnerItem } from 'utils/ManageUtils';
-import { getEntityName } from 'utils/EntityUtils';
+import { searchFormattedUsersAndTeams } from 'utils/UserDataUtils';
-import { default as AppState, default as appState } from '../../../AppState';
-import { WILD_CARD_CHAR } from '../../../constants/char.constants';
-import { Table } from '../../../generated/entity/data/table';
-import { EntityReference } from '../../../generated/type/entityReference';
-import { useAuth } from '../../../hooks/authHooks';
-import { getOwnerList } from '../../../utils/ManageUtils';
-import { showErrorToast } from '../../../utils/ToastUtils';
-import {
-  isCurrentUserAdmin,
-  searchFormattedUsersAndTeams,
-} from '../../../utils/UserDataUtils';
-import { useAuthContext } from '../../authentication/auth-provider/AuthProvider';
-import DropDownList from '../../dropdown/DropDownList';
 import './OwnerWidgetWrapper.style.less';
 
 interface OwnerWidgetWrapperProps {
   currentOwner?: Table['owner'];
   updateUser?: (value: Table['owner']) => void;
-  isListLoading?: boolean;
   visible: boolean;
   currentUser?: EntityReference;
   allowTeamOwner?: boolean;
@@ -52,78 +41,35 @@ const OwnerWidgetWrapper = ({
   hideWidget,
   removeOwner,
 }: OwnerWidgetWrapperProps) => {
-  const { isAuthDisabled } = useAuthContext();
-  const { isAdminUser } = useAuth();
   const [statusOwner, setStatusOwner] = useState<LoadingState>('initial');
 
-  const [listOwners, setListOwners] = useState<
+  const [ownersList, setOwnersList] = useState<OwnerItem[]>([]);
-    {
-      name: string;
-      value: string | undefined;
-      group: string;
-      type: string;
-    }[]
-  >([]);
   const [isUserLoading, setIsUserLoading] = useState<boolean>(true);
   const [owner, setOwner] = useState(currentUser);
 
   const [searchText, setSearchText] = useState<string>('');
-  const userDetails = useMemo(() => {
-    const userData = AppState.getCurrentUserDetails();
-
-    return [
-      {
-        name: getEntityName(userData),
-        value: userData?.id,
-        group: 'Users',
-        type: 'user',
-      },
-    ];
-  }, [appState.users, appState.userDetails]);
 
   const [totalUsersCount, setTotalUsersCount] = useState<number>(0);
   const [totalTeamsCount, setTotalTeamsCount] = useState<number>(0);
 
-  const fetchGroupTypeTeams = async () => {
-    try {
-      if (listOwners.length === 0) {
-        const data = await getGroupTypeTeams();
-        const updatedData = data.map((team) => ({
-          name: getEntityName(team),
-          value: team.id,
-          group: 'Teams',
-          type: 'team',
-        }));
-        // set team count for logged in user
-        setTotalTeamsCount(data.length);
-        setListOwners([...updatedData, ...userDetails]);
-      }
-    } catch (error) {
-      showErrorToast(error as AxiosError);
-    } finally {
-      setIsUserLoading(false);
-    }
-  };
-
   const getOwnerSearch = useCallback(
     (searchQuery = WILD_CARD_CHAR, from = 1) => {
       setIsUserLoading(true);
       searchFormattedUsersAndTeams(searchQuery, from)
         .then((res) => {
           const { users, teams, teamsTotal, usersTotal } = res;
-          // set team and user count for admin user
           setTotalTeamsCount(teamsTotal ?? 0);
           setTotalUsersCount(usersTotal ?? 0);
-          setListOwners(getOwnerList(users, teams, false, searchQuery));
+          setOwnersList(getOwnerList(users, teams, false));
         })
         .catch(() => {
-          setListOwners([]);
+          setOwnersList([]);
         })
         .finally(() => {
           setIsUserLoading(false);
         });
     },
-    [setListOwners, setIsUserLoading]
+    [setOwnersList, setIsUserLoading]
   );
 
   const debouncedOnChange = useCallback(
@@ -145,7 +91,7 @@ const OwnerWidgetWrapper = ({
     _e: React.MouseEvent<HTMLElement, MouseEvent>,
     value = ''
   ) => {
-    const owner = listOwners.find((item) => item.value === value);
+    const owner = ownersList.find((item) => item.value === value);
 
     if (owner) {
       const newOwner = prepareOwner({
@@ -179,8 +125,7 @@ const OwnerWidgetWrapper = ({
    */
   const handleTotalCountForGroup = (groupName: string) => {
     if (lowerCase(groupName) === 'users') {
-      // if user is admin return total user count otherwise return 1
+      return totalUsersCount;
-      return isAdminUser ? totalUsersCount : 1;
     } else if (lowerCase(groupName) === 'teams') {
       return totalTeamsCount;
     } else {
@@ -190,19 +135,9 @@ const OwnerWidgetWrapper = ({
 
   useEffect(() => {
     if (visible) {
-      if (isAuthDisabled || !isAdminUser) {
+      handleOwnerSearch(searchText ?? '');
-        fetchGroupTypeTeams();
-      } else {
-        handleOwnerSearch('');
     }
-    }
+  }, [visible, searchText]);
-  }, [visible]);
-
-  useEffect(() => {
-    if (visible) {
-      debounceOnSearch(searchText);
-    }
-  }, [searchText]);
 
   const ownerGroupList = useMemo(() => {
     return allowTeamOwner ? ['Teams', 'Users'] : ['Users'];
@@ -233,15 +168,15 @@ const OwnerWidgetWrapper = ({
   return visible ? (
     <DropDownList
       showEmptyList
+      showSearchBar
       className="edit-owner-dropdown"
       controlledSearchStr={searchText}
-      dropDownList={listOwners}
+      dropDownList={ownersList}
       getTotalCountForGroup={handleTotalCountForGroup}
       groupType={ownerGroupList.length > 1 ? 'tab' : 'label'}
       isLoading={isUserLoading}
       listGroups={ownerGroupList}
       removeOwner={handleRemoveOwner}
-      showSearchBar={isCurrentUserAdmin()}
       value={owner?.id || ''}
       onSearchTextChange={handleSearchOwnerDropdown}
       onSelect={handleOwnerSelection}

openmetadata-ui/src/main/resources/ui/src/components/common/OwnerWidget/OwnerWidgetWrapper.test.tsx

@@ -0,0 +1,77 @@
+/*
+ *  Copyright 2023 Collate.
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+import { act, fireEvent, render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import React from 'react';
+
+import OwnerWidgetWrapper from './OwnerWidgetWrapper.component';
+
+const mockSearchAPI = jest.fn();
+const mockHideWidget = jest.fn();
+
+jest.mock('utils/UserDataUtils', () => ({
+  searchFormattedUsersAndTeams: mockSearchAPI,
+}));
+
+const mockProps = {
+  visible: true,
+  currentUser: { id: '1', type: 'User' },
+  hideWidget: mockHideWidget,
+};
+
+describe('OwnerWidgetWrapper', () => {
+  it('Should renders the component when visible is true', () => {
+    render(<OwnerWidgetWrapper {...mockProps} />);
+    const dropDownList = screen.getByTestId('dropdown-list');
+    const searchBox = screen.getByTestId('searchInputText');
+
+    expect(dropDownList).toBeInTheDocument();
+    expect(searchBox).toBeInTheDocument();
+  });
+
+  it('Should not render the component when visible is false', () => {
+    render(<OwnerWidgetWrapper {...mockProps} visible={false} />);
+
+    const component = screen.queryByTestId('dropdown-list');
+
+    expect(component).toBeNull();
+  });
+
+  it('Search Should work', async () => {
+    render(<OwnerWidgetWrapper {...mockProps} />);
+
+    const searchInput = screen.getByTestId('searchInputText');
+
+    await act(async () => {
+      fireEvent.change(searchInput, {
+        target: {
+          value: 'user1',
+        },
+      });
+    });
+
+    expect(searchInput).toHaveValue('user1');
+  });
+
+  it('Hide Widget should work', async () => {
+    render(<OwnerWidgetWrapper {...mockProps} />);
+
+    const backdropButton = screen.getByTestId('backdrop-button');
+
+    await act(async () => {
+      userEvent.click(backdropButton);
+    });
+
+    expect(mockHideWidget).toHaveBeenCalled();
+  });
+});

openmetadata-ui/src/main/resources/ui/src/utils/ManageUtils.ts

@@ -11,53 +11,55 @@
  *  limitations under the License.
  */
 
-import { toString } from 'lodash';
+import { ReactNode } from 'react';
 import AppState from '../AppState';
-import { WILD_CARD_CHAR } from '../constants/char.constants';
 import { Team } from '../generated/entity/teams/team';
 import { User } from '../generated/entity/teams/user';
-import { EntityReference } from '../generated/type/entityUsage';
 import { getEntityName } from './EntityUtils';
 
+export type OwnerItem = {
+  name: string;
+  value: string;
+  group: string;
+  type: string;
+} & Record<string, ReactNode>;
+
 /**
  * @param listUsers - List of users
  * @param listTeams - List of teams
- * @param excludeCurrentUser - Wether to exclude current user to be on list. Needed when calls from searching
+ * @param excludeCurrentUser - Whether to exclude current user from the list
- * @param searchQuery - search query for user or team
+ * @param searchQuery - Search query for user or team
- * @returns List of user or team
+ * @returns List of users or teams
  */
 export const getOwnerList = (
-  listUsers?: User[],
+  listUsers: User[] = [],
-  listTeams?: Team[],
+  listTeams: Team[] = [],
-  excludeCurrentUser?: boolean,
+  excludeCurrentUser = false
-  searchQuery?: string
+): OwnerItem[] => {
-) => {
   const userDetails = AppState.getCurrentUserDetails();
 
-  const isAdminIncludeInQuery =
+  const users = listUsers.flatMap((user) =>
-    getEntityName(userDetails).includes(toString(searchQuery)) ||
+    user.id !== userDetails?.id
-    searchQuery === WILD_CARD_CHAR
+      ? [
-      ? true
+          {
-      : false;
+            name: getEntityName(user),
-
-  if (userDetails?.isAdmin) {
-    const users = (listUsers || [])
-      .map((user) => ({
-        name: getEntityName(user as unknown as EntityReference),
             value: user.id,
             group: 'Users',
             type: 'user',
-      }))
+          },
-      .filter((u) => u.value !== userDetails.id);
+        ]
-    const teams = (listTeams || []).map((team) => ({
+      : []
+  );
+
+  const teams = listTeams.map((team) => ({
     name: getEntityName(team),
     value: team.id,
     group: 'Teams',
     type: 'team',
   }));
 
-    return [
+  const currentUser =
-      ...(!excludeCurrentUser && isAdminIncludeInQuery
+    !excludeCurrentUser && userDetails
       ? [
           {
             name: getEntityName(userDetails),
@@ -66,18 +68,7 @@ export const getOwnerList = (
             type: 'user',
           },
         ]
-        : []),
+      : [];
-      ...users,
+
-      ...teams,
+  return [...currentUser, ...users, ...teams];
-    ];
-  } else {
-    return [
-      {
-        name: getEntityName(userDetails),
-        value: userDetails?.id,
-        group: 'Users',
-        type: 'user',
-      },
-    ];
-  }
 };