Skip to content

Commit

Permalink
[Workplace Search] Remove isFederatedAuth checks to expose user fea…
Browse files Browse the repository at this point in the history
…tures (elastic#103278)

* Remove isFederated from main app and routes

* Expose all overview cards that were hidden for federated auth

* Expose all user features that were hidden for groups

* Remove remaining isFederatedAuth references

* Lint fixes

* Add modified test back for Workplace Search

* Remove extraCell

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

* Remove brackets

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

* Update test name

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

Co-authored-by: Constance <constancecchen@users.noreply.github.com>
  • Loading branch information
scottybollinger and Constance committed Jun 24, 2021
1 parent fb3e8f4 commit 0857e62
Show file tree
Hide file tree
Showing 24 changed files with 76 additions and 187 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
export const DEFAULT_INITIAL_APP_DATA = {
readOnlyMode: false,
ilmEnabled: true,
isFederatedAuth: false,
configuredLimits: {
appSearch: {
engine: {
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/enterprise_search/common/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
export interface InitialAppData {
readOnlyMode?: boolean;
ilmEnabled?: boolean;
isFederatedAuth?: boolean;
configuredLimits?: ConfiguredLimits;
access?: ProductAccess;
appSearch?: AppSearchAccount;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ describe('AppLogic', () => {
const DEFAULT_VALUES = {
account: {},
hasInitialized: false,
isFederatedAuth: true,
isOrganization: false,
organization: {},
};
Expand All @@ -36,7 +35,6 @@ describe('AppLogic', () => {
viewedOnboardingPage: true,
},
hasInitialized: true,
isFederatedAuth: false,
isOrganization: false,
organization: {
defaultOrgName: 'My Organization',
Expand All @@ -61,7 +59,6 @@ describe('AppLogic', () => {
expect(AppLogic.values).toEqual({
...DEFAULT_VALUES,
hasInitialized: true,
isFederatedAuth: false,
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {

interface AppValues extends WorkplaceSearchInitialData {
hasInitialized: boolean;
isFederatedAuth: boolean;
isOrganization: boolean;
}
interface AppActions {
Expand All @@ -32,10 +31,7 @@ const emptyAccount = {} as Account;
export const AppLogic = kea<MakeLogicType<AppValues, AppActions>>({
path: ['enterprise_search', 'workplace_search', 'app_logic'],
actions: {
initializeAppData: ({ workplaceSearch, isFederatedAuth }) => ({
workplaceSearch,
isFederatedAuth,
}),
initializeAppData: ({ workplaceSearch }) => ({ workplaceSearch }),
setContext: (isOrganization) => isOrganization,
setOrgName: (name: string) => name,
setSourceRestriction: (canCreatePersonalSources: boolean) => canCreatePersonalSources,
Expand All @@ -47,12 +43,6 @@ export const AppLogic = kea<MakeLogicType<AppValues, AppActions>>({
initializeAppData: () => true,
},
],
isFederatedAuth: [
true,
{
initializeAppData: (_, { isFederatedAuth }) => !!isFederatedAuth,
},
],
isOrganization: [
false,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import '../__mocks__/shallow_useeffect.mock';
import { DEFAULT_INITIAL_APP_DATA } from '../../../common/__mocks__';
import { setMockValues, setMockActions, mockKibanaValues } from '../__mocks__/kea_logic';
import { mockUseRouteMatch } from '../__mocks__/react_router';

Expand Down Expand Up @@ -75,9 +76,10 @@ describe('WorkplaceSearchConfigured', () => {
});

it('initializes app data with passed props', () => {
shallow(<WorkplaceSearchConfigured isFederatedAuth />);
const { workplaceSearch } = DEFAULT_INITIAL_APP_DATA;
shallow(<WorkplaceSearchConfigured workplaceSearch={workplaceSearch} />);

expect(initializeAppData).toHaveBeenCalledWith({ isFederatedAuth: true });
expect(initializeAppData).toHaveBeenCalledWith({ workplaceSearch });
});

it('does not re-initialize app data or re-render header actions', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const PERSONAL_PATH = '/p';

export const USERS_AND_ROLES_PATH = '/users_and_roles';

export const USERS_PATH = '/users';
export const SECURITY_PATH = '/security';

export const GROUPS_PATH = '/groups';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
import { i18n } from '@kbn/i18n';

import { TruncatedContent } from '../../../../shared/truncate';
import { AppLogic } from '../../../app_logic';
import noSharedSourcesIcon from '../../../assets/share_circle.svg';
import { WorkplaceSearchPageTemplate } from '../../../components/layout';
import { ContentSection } from '../../../components/shared/content_section';
Expand Down Expand Up @@ -124,8 +123,6 @@ export const GroupOverview: React.FC = () => {
confirmDeleteModalVisible,
} = useValues(GroupLogic);

const { isFederatedAuth } = useValues(AppLogic);

const truncatedName = name && (
<TruncatedContent tooltipType="title" content={name} length={MAX_NAME_LENGTH} />
);
Expand Down Expand Up @@ -167,7 +164,7 @@ export const GroupOverview: React.FC = () => {
{MANAGE_SOURCES_BUTTON_TEXT}
</EuiButton>
);
const manageUsersButton = !isFederatedAuth && (
const manageUsersButton = (
<EuiButton color="primary" onClick={showManageUsersModal}>
{MANAGE_USERS_BUTTON_TEXT}
</EuiButton>
Expand Down Expand Up @@ -199,7 +196,7 @@ export const GroupOverview: React.FC = () => {
</>
);

const usersSection = !isFederatedAuth && (
const usersSection = (
<ContentSection
title="Group users"
description={hasUsers ? GROUP_USERS_DESCRIPTION : EMPTY_USERS_DESCRIPTION}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { setMockValues } from '../../../../__mocks__/kea_logic';
import { groups } from '../../../__mocks__/groups.mock';

import React from 'react';
Expand All @@ -19,18 +18,13 @@ import { GroupRow, NO_USERS_MESSAGE, NO_SOURCES_MESSAGE } from './group_row';
import { GroupUsers } from './group_users';

describe('GroupRow', () => {
beforeEach(() => {
setMockValues({ isFederatedAuth: true });
});

it('renders', () => {
const wrapper = shallow(<GroupRow {...groups[0]} />);

expect(wrapper.find(EuiTableRow)).toHaveLength(1);
});

it('renders group users', () => {
setMockValues({ isFederatedAuth: false });
const wrapper = shallow(<GroupRow {...groups[0]} />);

expect(wrapper.find(GroupUsers)).toHaveLength(1);
Expand All @@ -51,7 +45,6 @@ describe('GroupRow', () => {
});

it('renders empty users message when no users present', () => {
setMockValues({ isFederatedAuth: false });
const wrapper = shallow(<GroupRow {...groups[0]} usersCount={0} />);

expect(wrapper.find('.user-group__accounts').text()).toEqual(NO_USERS_MESSAGE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@

import React from 'react';

import { useValues } from 'kea';
import moment from 'moment';

import { EuiTableRow, EuiTableRowCell, EuiIcon } from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { EuiLinkTo } from '../../../../shared/react_router_helpers';
import { TruncatedContent } from '../../../../shared/truncate';
import { AppLogic } from '../../../app_logic';
import { getGroupPath } from '../../../routes';
import { Group } from '../../../types';
import { MAX_NAME_LENGTH } from '../group_logic';
Expand Down Expand Up @@ -50,8 +48,6 @@ export const GroupRow: React.FC<Group> = ({
users,
usersCount,
}) => {
const { isFederatedAuth } = useValues(AppLogic);

const GROUP_UPDATED_TEXT = i18n.translate(
'xpack.enterpriseSearch.workplaceSearch.groups.groupUpdatedText',
{
Expand Down Expand Up @@ -80,17 +76,15 @@ export const GroupRow: React.FC<Group> = ({
)}
</div>
</EuiTableRowCell>
{!isFederatedAuth && (
<EuiTableRowCell>
<div className="user-group__accounts">
{usersCount > 0 ? (
<GroupUsers groupUsers={users} usersCount={usersCount} groupId={id} />
) : (
NO_USERS_MESSAGE
)}
</div>
</EuiTableRowCell>
)}
<EuiTableRowCell>
<div className="user-group__accounts">
{usersCount > 0 ? (
<GroupUsers groupUsers={users} usersCount={usersCount} groupId={id} />
) : (
NO_USERS_MESSAGE
)}
</div>
</EuiTableRowCell>
<EuiTableRowCell align="right">
<strong>
<EuiLinkTo to={getGroupPath(id)}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,10 @@ const group = groups[0];

describe('GroupUsersTable', () => {
it('renders', () => {
setMockValues({ isFederatedAuth: true, group });
setMockValues({ group });
const wrapper = shallow(<GroupUsersTable />);

expect(wrapper.find(EuiTable)).toHaveLength(1);
expect(wrapper.find(TableHeader).prop('headerItems')).toHaveLength(1);
});

it('adds header item for non-federated auth', () => {
setMockValues({ isFederatedAuth: false, group });
const wrapper = shallow(<GroupUsersTable />);

expect(wrapper.find(TableHeader).prop('headerItems')).toHaveLength(2);
});

Expand All @@ -48,7 +41,7 @@ describe('GroupUsersTable', () => {
});
});

setMockValues({ isFederatedAuth: true, group: { users } });
setMockValues({ group: { users } });
const wrapper = shallow(<GroupUsersTable />);
const pagination = wrapper.find(EuiTablePagination);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,17 @@ import { Pager } from '@elastic/eui';

import { USERNAME_LABEL, EMAIL_LABEL } from '../../../../shared/constants';
import { TableHeader } from '../../../../shared/table_header';
import { AppLogic } from '../../../app_logic';
import { UserRow } from '../../../components/shared/user_row';
import { User } from '../../../types';
import { GroupLogic } from '../group_logic';

const USERS_PER_PAGE = 10;

export const GroupUsersTable: React.FC = () => {
const { isFederatedAuth } = useValues(AppLogic);
const {
group: { users },
} = useValues(GroupLogic);
const headerItems = [USERNAME_LABEL];
if (!isFederatedAuth) {
headerItems.push(EMAIL_LABEL);
}
const headerItems = [USERNAME_LABEL, EMAIL_LABEL];

const [firstItem, setFirstItem] = useState(0);
const [lastItem, setLastItem] = useState(USERS_PER_PAGE - 1);
Expand Down Expand Up @@ -58,10 +53,10 @@ export const GroupUsersTable: React.FC = () => {
return (
<>
<EuiTable>
<TableHeader extraCell={isFederatedAuth} headerItems={headerItems} />
<TableHeader headerItems={headerItems} />
<EuiTableBody>
{users.slice(firstItem, lastItem + 1).map((user: User) => (
<UserRow key={user.id} showEmail={!isFederatedAuth} user={user} />
<UserRow key={user.id} showEmail user={user} />
))}
</EuiTableBody>
</EuiTable>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import React from 'react';

import { shallow } from 'enzyme';

import { EuiTable, EuiTableHeaderCell } from '@elastic/eui';
import { EuiTable } from '@elastic/eui';

import { DEFAULT_META } from '../../../../shared/constants';
import { TablePaginationBar } from '../../../components/shared/table_pagination_bar';
Expand All @@ -27,7 +27,6 @@ const mockValues = {
groupsMeta: DEFAULT_META,
groups,
hasFiltersSet: false,
isFederatedAuth: true,
};

describe('GroupsTable', () => {
Expand All @@ -43,13 +42,6 @@ describe('GroupsTable', () => {
expect(wrapper.find(GroupRow)).toHaveLength(1);
});

it('renders extra header for non-federated auth', () => {
setMockValues({ ...mockValues, isFederatedAuth: false });
const wrapper = shallow(<GroupsTable />);

expect(wrapper.find(EuiTableHeaderCell)).toHaveLength(4);
});

it('handles pagination', () => {
setMockValues({
...mockValues,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { AppLogic } from '../../../app_logic';
import { TablePaginationBar } from '../../../components/shared/table_pagination_bar';
import { GroupsLogic } from '../groups_logic';

Expand Down Expand Up @@ -53,7 +52,6 @@ export const GroupsTable: React.FC<{}> = () => {
groups,
hasFiltersSet,
} = useValues(GroupsLogic);
const { isFederatedAuth } = useValues(AppLogic);

const clearFiltersLink = hasFiltersSet ? <ClearFiltersLink /> : undefined;

Expand All @@ -79,7 +77,7 @@ export const GroupsTable: React.FC<{}> = () => {
<EuiTableHeader>
<EuiTableHeaderCell>{GROUP_TABLE_HEADER}</EuiTableHeaderCell>
<EuiTableHeaderCell>{SOURCES_TABLE_HEADER}</EuiTableHeaderCell>
{!isFederatedAuth && <EuiTableHeaderCell>{USERS_TABLE_HEADER}</EuiTableHeaderCell>}
<EuiTableHeaderCell>{USERS_TABLE_HEADER}</EuiTableHeaderCell>
<EuiTableHeaderCell />
</EuiTableHeader>
<EuiTableBody>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,14 @@ const setFilterValue = jest.fn();

describe('TableFilters', () => {
beforeEach(() => {
setMockValues({ filterValue: '', isFederatedAuth: true });
setMockValues({ filterValue: '' });
setMockActions({ setFilterValue });
});
it('renders', () => {
const wrapper = shallow(<TableFilters />);

expect(wrapper.find(EuiFieldSearch)).toHaveLength(1);
expect(wrapper.find(TableFilterSourcesDropdown)).toHaveLength(1);
expect(wrapper.find(TableFilterUsersDropdown)).toHaveLength(0);
});

it('renders for non-federated Auth', () => {
setMockValues({ filterValue: '', isFederatedAuth: false });
const wrapper = shallow(<TableFilters />);

expect(wrapper.find(TableFilterUsersDropdown)).toHaveLength(1);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { useActions, useValues } from 'kea';
import { EuiFieldSearch, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { AppLogic } from '../../../app_logic';
import { GroupsLogic } from '../groups_logic';

import { TableFilterSourcesDropdown } from './table_filter_sources_dropdown';
Expand All @@ -28,7 +27,6 @@ const FILTER_GROUPS_PLACEHOLDER = i18n.translate(
export const TableFilters: React.FC = () => {
const { setFilterValue } = useActions(GroupsLogic);
const { filterValue } = useValues(GroupsLogic);
const { isFederatedAuth } = useValues(AppLogic);

const handleSearchChange = (e: ChangeEvent<HTMLInputElement>) => setFilterValue(e.target.value);

Expand All @@ -47,11 +45,9 @@ export const TableFilters: React.FC = () => {
<EuiFlexItem className="user-groups-filters__filter-sources">
<TableFilterSourcesDropdown />
</EuiFlexItem>
{!isFederatedAuth && (
<EuiFlexItem>
<TableFilterUsersDropdown />
</EuiFlexItem>
)}
<EuiFlexItem>
<TableFilterUsersDropdown />
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Loading

0 comments on commit 0857e62

Please sign in to comment.