Skip to content

Commit

Permalink
fix: unnecessary group related api calls during the initial group pag…
Browse files Browse the repository at this point in the history 
…e loading (#8882)

* fix: unnecessary group related api call during inital group page load

* fix: minor fix
  • Loading branch information
@keita-determined
keita-determined authored Feb 26, 2024
1 parent f37bc3e commit be1ab85
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 103 deletions.
8 changes: 4 additions & 4 deletions webui/react/package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion webui/react/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"fp-ts": "^2.16.1",
"fuse.js": "^7.0.0",
"hermes-parallel-coordinates": "^0.6.17",
"hew": "npm:@hpe.com/hew@^0.6.30",
"hew": "npm:@hpe.com/hew@^0.6.31",
"humanize-duration": "^3.28.0",
"immutable": "^4.3.0",
"io-ts": "^2.2.20",
Expand Down
139 changes: 46 additions & 93 deletions webui/react/src/components/CreateGroupModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,18 @@ import Form from 'hew/Form';
import Input from 'hew/Input';
import { Modal } from 'hew/Modal';
import Select, { Option } from 'hew/Select';
import Spinner from 'hew/Spinner';
import { useToast } from 'hew/Toast';
import { Body } from 'hew/Typography';
import { Loadable } from 'hew/utils/loadable';
import _ from 'lodash';
import { useObservable } from 'micro-observables';
import React, { useCallback, useEffect, useId, useState } from 'react';
import React, { useId } from 'react';

import Link from 'components/Link';
import usePermissions from 'hooks/usePermissions';
import { paths } from 'routes/utils';
import {
assignRolesToGroup,
createGroup,
getGroup,
getGroupRoles,
removeRolesFromGroup,
updateGroup,
} from 'services/api';
import { V1GroupDetails, V1GroupSearchResult } from 'services/api-ts-sdk';
import { assignRolesToGroup, createGroup, removeRolesFromGroup, updateGroup } from 'services/api';
import { V1GroupSearchResult } from 'services/api-ts-sdk';
import determinedStore from 'stores/determinedInfo';
import roleStore from 'stores/roles';
import { UserRole } from 'types';
Expand Down Expand Up @@ -60,17 +52,15 @@ const EDIT_VALUES: Messages = {

interface Props {
group?: V1GroupSearchResult;
groupRoles?: UserRole[];
onClose?: () => void;
}

const CreateGroupModalComponent: React.FC<Props> = ({ onClose, group }: Props) => {
const CreateGroupModalComponent: React.FC<Props> = ({ onClose, group, groupRoles }: Props) => {
const idPrefix = useId();
const [form] = Form.useForm();
const { rbacEnabled } = useObservable(determinedStore.info);
const { canModifyPermissions } = usePermissions();
const [groupRoles, setGroupRoles] = useState<UserRole[]>([]);
const [groupDetail, setGroupDetail] = useState<V1GroupDetails>();
const [isLoading, setIsLoading] = useState(true);
const isCreateModal = !group;
const messages = isCreateModal ? CREATE_VALUES : EDIT_VALUES;

Expand All @@ -79,53 +69,15 @@ const CreateGroupModalComponent: React.FC<Props> = ({ onClose, group }: Props) =
const roles = useObservable(roleStore.roles);
const groupName = Form.useWatch(GROUP_NAME_NAME, form);

const fetchGroupDetail = useCallback(async () => {
if (group?.group.groupId) {
try {
const response = await getGroup({ groupId: group?.group.groupId });
const groupDetail = response.group;
setGroupDetail(groupDetail);
form.setFieldsValue({
[GROUP_NAME_NAME]: groupDetail.name,
});
} catch (e) {
handleError(e, { publicSubject: 'Unable to fetch group data.' });
}
}
}, [group, form]);

const fetchGroupRoles = useCallback(async () => {
if (group?.group.groupId && rbacEnabled) {
try {
const roles = await getGroupRoles({ groupId: group.group.groupId });
const groupRoles = roles.filter((r) => r.scopeCluster);
setGroupRoles(groupRoles);
form.setFieldValue(GROUP_ROLE_NAME, groupRoles?.map((r) => r.id));
} catch (e) {
handleError(e, { publicSubject: "Unable to fetch this group's roles." });
}
}
}, [form, group, rbacEnabled]);

const fetchData = useCallback(async () => {
await fetchGroupDetail();
await fetchGroupRoles();
setIsLoading(false);
}, [fetchGroupDetail, fetchGroupRoles]);

useEffect(() => {
fetchData();
}, [fetchData]);

const handleSubmit = async () => {
try {
const formData = await form.validateFields();

if (group) {
const nameUpdated = !_.isEqual(formData.name, groupDetail?.name);
const nameUpdated = !_.isEqual(formData.name, group.group?.name);
const rolesUpdated = !_.isEqual(
formData.roles,
groupRoles.map((r) => r.id),
(groupRoles ?? []).map((r) => r.id),
);
if (!nameUpdated && !rolesUpdated) {
openToast({ title: 'No changes to save.' });
Expand Down Expand Up @@ -154,7 +106,6 @@ const CreateGroupModalComponent: React.FC<Props> = ({ onClose, group }: Props) =
roleIds: Array.from(rolesToRemove),
});
}
await fetchGroupRoles();
}
} else {
const newGroup = await createGroup(formData);
Expand Down Expand Up @@ -196,43 +147,45 @@ const CreateGroupModalComponent: React.FC<Props> = ({ onClose, group }: Props) =
}}
title={messages.MODAL_HEADER_LABEL}
onClose={form.resetFields}>
<Spinner spinning={isLoading}>
<Form form={form} id={idPrefix + FORM_ID}>
<Form.Item
label={GROUP_NAME_LABEL}
name={GROUP_NAME_NAME}
required
rules={[{ whitespace: true }]}
validateTrigger={['onSubmit', 'onChange']}>
<Input autoComplete="off" autoFocus maxLength={128} placeholder={GROUP_NAME_LABEL} />
</Form.Item>
{rbacEnabled && canModifyPermissions && (
<>
<Form.Item label={GROUP_ROLE_LABEL} name={GROUP_ROLE_NAME}>
<Select
loading={Loadable.isNotLoaded(roles)}
mode="multiple"
placeholder={'Add Roles'}>
{roles
.getOrElse([])
.sort((r1, r2) => r1.id - r2.id)
.map((r) => (
<Option key={r.id} value={r.id}>
{r.name}
</Option>
))}
</Select>
</Form.Item>
<Body inactive>
Groups may have additional inherited workspace roles not reflected here. &nbsp;
<Link external path={paths.docs('/cluster-setup-guide/security/rbac.html')} popout>
Learn more
</Link>
</Body>
</>
)}
</Form>
</Spinner>
<Form form={form} id={idPrefix + FORM_ID}>
<Form.Item
initialValue={group?.group.name}
label={GROUP_NAME_LABEL}
name={GROUP_NAME_NAME}
required
rules={[{ whitespace: true }]}
validateTrigger={['onSubmit', 'onChange']}>
<Input autoComplete="off" autoFocus maxLength={128} placeholder={GROUP_NAME_LABEL} />
</Form.Item>
{rbacEnabled && canModifyPermissions && (
<>
<Form.Item
initialValue={(groupRoles ?? []).map((r) => r.id)}
label={GROUP_ROLE_LABEL}
name={GROUP_ROLE_NAME}>
<Select
loading={Loadable.isNotLoaded(roles)}
mode="multiple"
placeholder={'Add Roles'}>
{roles
.getOrElse([])
.sort((r1, r2) => r1.id - r2.id)
.map((r) => (
<Option key={r.id} value={r.id}>
{r.name}
</Option>
))}
</Select>
</Form.Item>
<Body inactive>
Groups may have additional inherited workspace roles not reflected here. &nbsp;
<Link external path={paths.docs('/cluster-setup-guide/security/rbac.html')} popout>
Learn more
</Link>
</Body>
</>
)}
</Form>
</Modal>
);
};
Expand Down
26 changes: 21 additions & 5 deletions webui/react/src/pages/Admin/GroupManagement.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import { defaultRowClassName, getFullPaginationConfig } from 'components/Table/T
import UserBadge from 'components/UserBadge';
import usePermissions from 'hooks/usePermissions';
import { useSettings } from 'hooks/useSettings';
import { getGroup, getGroups } from 'services/api';
import { getGroup, getGroupRoles, getGroups } from 'services/api';
import { V1GroupDetails, V1GroupSearchResult, V1User } from 'services/api-ts-sdk';
import determinedStore from 'stores/determinedInfo';
import roleStore from 'stores/roles';
import userStore from 'stores/users';
import { DetailedUser, User } from 'types';
import { DetailedUser, User, UserRole } from 'types';
import handleError from 'utils/error';
import { useObservable } from 'utils/observable';
import { alphaNumericSorter } from 'utils/sort';
Expand Down Expand Up @@ -66,12 +66,27 @@ const GroupActionDropdown = ({
fetchGroups();
expanded && group.group.groupId && fetchGroup(group.group.groupId);
};
const [groupRoles, setGroupRoles] = useState<UserRole[]>([]);
const { rbacEnabled } = useObservable(determinedStore.info);

const EditGroupModal = useModal(CreateGroupModalComponent);
const AddUsersToGroupModal = useModal(AddUsersToGroupModalComponent);
const DeleteGroupModal = useModal(DeleteGroupModalComponent);

const fetchGroupRoles = useCallback(async () => {
if (group?.group.groupId && rbacEnabled) {
try {
const roles = await getGroupRoles({ groupId: group.group.groupId });
const groupRoles = roles.filter((r) => r.scopeCluster);
setGroupRoles(groupRoles);
} catch (e) {
handleError(e, { publicSubject: "Unable to fetch this group's roles." });
}
}
}, [group, rbacEnabled]);

const handleDropdown = useCallback(
(key: string) => {
async (key: string) => {
switch (key) {
case MenuKey.AddMembers:
AddUsersToGroupModal.open();
Expand All @@ -80,11 +95,12 @@ const GroupActionDropdown = ({
DeleteGroupModal.open();
break;
case MenuKey.Edit:
await fetchGroupRoles();
EditGroupModal.open();
break;
}
},
[AddUsersToGroupModal, DeleteGroupModal, EditGroupModal],
[AddUsersToGroupModal, DeleteGroupModal, EditGroupModal, fetchGroupRoles],
);

return (
Expand All @@ -93,7 +109,7 @@ const GroupActionDropdown = ({
<Button icon={<Icon name="overflow-vertical" size="small" title="Action menu" />} />
</Dropdown>
<AddUsersToGroupModal.Component group={group} users={availabeUsers} onClose={onFinishEdit} />
<EditGroupModal.Component group={group} onClose={onFinishEdit} />
<EditGroupModal.Component group={group} groupRoles={groupRoles} onClose={onFinishEdit} />
<DeleteGroupModal.Component group={group} onClose={fetchGroups} />
</div>
);
Expand Down

0 comments on commit be1ab85

Please sign in to comment.