Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
fetchRolesWithTarget,
} from "../../../../slices/aclSlice";
import { FormikProps } from "formik";
import { filterRoles, policiesFiltered, rolesFilteredbyPolicies } from "../../../../utils/aclUtils";
import { policiesFiltered } from "../../../../utils/aclUtils";
import { useAppDispatch, useAppSelector } from "../../../../store";
import { fetchSeriesDetailsAcls } from "../../../../slices/seriesDetailsSlice";
import { getSeriesDetailsAcl } from "../../../../selectors/seriesDetailsSelectors";
Expand Down Expand Up @@ -122,7 +122,6 @@ const NewAccessPage = <T extends RequiredFormProps>({
<AccessPolicyTable
isUserTable={true}
policiesFiltered={policiesFiltered(formik.values.policies, true)}
rolesFilteredbyPolicies={rolesFilteredbyPolicies(roles, formik.values.policies, true)}
header={"EVENTS.EVENTS.DETAILS.ACCESS.ACCESS_POLICY.USERS"}
firstColumnHeader={"EVENTS.EVENTS.DETAILS.ACCESS.ACCESS_POLICY.USER"}
createLabel={"EVENTS.EVENTS.DETAILS.ACCESS.ACCESS_POLICY.NEW_USER"}
Expand All @@ -139,7 +138,6 @@ const NewAccessPage = <T extends RequiredFormProps>({
<AccessPolicyTable
isUserTable={false}
policiesFiltered={policiesFiltered(formik.values.policies, false)}
rolesFilteredbyPolicies={rolesFilteredbyPolicies(roles, formik.values.policies, false)}
header={"USERS.ACLS.NEW.ACCESS.ACCESS_POLICY.NON_USER_ROLES"}
firstColumnHeader={"EVENTS.EVENTS.DETAILS.ACCESS.ACCESS_POLICY.ROLE"}
createLabel={"EVENTS.EVENTS.DETAILS.ACCESS.ACCESS_POLICY.NEW"}
Expand All @@ -159,7 +157,6 @@ const NewAccessPage = <T extends RequiredFormProps>({
<AccessPolicyTable
isUserTable={false}
policiesFiltered={formik.values.policies}
rolesFilteredbyPolicies={filterRoles(roles, formik.values.policies)}
firstColumnHeader={"EVENTS.EVENTS.DETAILS.ACCESS.ACCESS_POLICY.ROLE"}
createLabel={"EVENTS.EVENTS.DETAILS.ACCESS.ACCESS_POLICY.NEW"}
formik={formik}
Expand Down
16 changes: 13 additions & 3 deletions src/components/shared/DropDown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
dropDownSpacingTheme,
dropDownStyle,
} from "../../utils/componentStyles";
import { GroupBase, MenuListProps, Props, SelectInstance } from "react-select";
import { GroupBase, MenuListProps, Props, SelectInstance, createFilter } from "react-select";
import { isJson } from "../../utils/utils";
import { ParseKeys } from "i18next";
import { FixedSizeList, ListChildComponentProps } from "react-window";
Expand Down Expand Up @@ -41,6 +41,7 @@ const DropDown = <T, >({
optionHeight = 25,
customCSS,
fetchOptions,
isAclDropDown = false,
}: {
ref?: React.RefObject<SelectInstance<any, boolean, GroupBase<any>> | null>
value: T
Expand All @@ -66,7 +67,8 @@ const DropDown = <T, >({
optionPaddingTop?: number,
optionLineHeight?: string
},
fetchOptions?: (inputValue: string) => Promise<{ label: string, value: string }[]>
fetchOptions?: (inputValue: string) => Promise<{ label: string, value: string }[]>,
isAclDropDown?: boolean;
}) => {
const { t } = useTranslation();

Expand Down Expand Up @@ -182,7 +184,6 @@ const DropDown = <T, >({
callback(formatOptions(filterOptions(_inputValue), required));
};


const commonProps: Props = {
tabIndex: tabIndex,
theme: theme => (dropDownSpacingTheme(theme)),
Expand Down Expand Up @@ -210,6 +211,15 @@ const DropDown = <T, >({

// @ts-expect-error: React-Select typing does not account for the typing of option it itself requires
components: { MenuList },
filterOption: createFilter({ ignoreAccents: false }), // To improve performance on filtering
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious, did you test if this actually performance on filtering and if so, by how much?


isOptionSelected: (option, _selectValue) => {
const typedOption = option as DropDownOption;
if (isAclDropDown) {
return false;
}
return typedOption.value === value;
},
Comment on lines +216 to +222
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So from what I understand, what this does is that if the Dropdown is defined as an AclDropdown, there is never an option selected. I don't understand why we wouldwant that, what is this supposed to achieve?

};

return creatable ? (
Expand Down
11 changes: 4 additions & 7 deletions src/components/shared/modals/ResourceDetailsAccessPolicyTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
import { getUserInformation } from "../../../selectors/userInfoSelectors";
import { hasAccess } from "../../../utils/utils";
import DropDown from "../DropDown";
import { filterRoles, getAclTemplateText, handleTemplateChange, policiesFiltered, rolesFilteredbyPolicies } from "../../../utils/aclUtils";
import { filterRoles, getAclTemplateText, handleTemplateChange, policiesFiltered } from "../../../utils/aclUtils";
import { useAppDispatch, useAppSelector } from "../../../store";
import { removeNotificationWizardForm, addNotification } from "../../../slices/notificationSlice";
import { useTranslation } from "react-i18next";
Expand Down Expand Up @@ -311,7 +311,6 @@ const ResourceDetailsAccessPolicyTab = ({
<AccessPolicyTable
isUserTable={true}
policiesFiltered={policiesFiltered(formik.values.policies, true)}
rolesFilteredbyPolicies={rolesFilteredbyPolicies(roles, formik.values.policies, true)}
header={userPolicyTableHeaderText}
firstColumnHeader={userPolicyTableRoleText}
createLabel={userPolicyTableNewText}
Expand All @@ -328,7 +327,6 @@ const ResourceDetailsAccessPolicyTab = ({
<AccessPolicyTable
isUserTable={false}
policiesFiltered={policiesFiltered(formik.values.policies, false)}
rolesFilteredbyPolicies={rolesFilteredbyPolicies(roles, formik.values.policies, false)}
header={policyTableHeaderText}
firstColumnHeader={policyTableRoleText}
createLabel={policyTableNewText}
Expand All @@ -348,7 +346,6 @@ const ResourceDetailsAccessPolicyTab = ({
<AccessPolicyTable
isUserTable={false}
policiesFiltered={formik.values.policies}
rolesFilteredbyPolicies={filterRoles(roles, formik.values.policies)}
header={policyTableHeaderText}
firstColumnHeader={policyTableRoleText}
createLabel={policyTableNewText}
Expand Down Expand Up @@ -398,7 +395,6 @@ type AccessPolicyTabFormikProps = {
export const AccessPolicyTable = <T extends AccessPolicyTabFormikProps>({
isUserTable,
policiesFiltered,
rolesFilteredbyPolicies,
header,
firstColumnHeader,
createLabel,
Expand All @@ -411,7 +407,6 @@ export const AccessPolicyTable = <T extends AccessPolicyTabFormikProps>({
}: {
isUserTable: boolean
policiesFiltered: TransformedAcl[]
rolesFilteredbyPolicies: Role[]
header?: ParseKeys
firstColumnHeader: ParseKeys
createLabel: ParseKeys,
Expand Down Expand Up @@ -538,7 +533,7 @@ export const AccessPolicyTable = <T extends AccessPolicyTabFormikProps>({
text={createPolicyLabel(policy)}
options={
roles.length > 0
? formatAclRolesForDropdown(rolesFilteredbyPolicies)
? formatAclRolesForDropdown(filterRoles(roles, formik.values.policies, policy.role))
: []
}
required={true}
Expand All @@ -565,6 +560,7 @@ export const AccessPolicyTable = <T extends AccessPolicyTabFormikProps>({
skipTranslate
optionHeight={35}
customCSS={{ width: "100%", optionPaddingTop: 5 }}
isAclDropDown
/>
) : (
<p>{policy.role}</p>
Expand Down Expand Up @@ -794,6 +790,7 @@ export const TemplateSelector = <T extends TemplateSelectorProps>({
}
}}
placeholder={t(buttonText)}
isAclDropDown
/>
)}
{!(aclTemplates.length > 0) &&
Expand Down
5 changes: 1 addition & 4 deletions src/components/users/partials/wizard/AclAccessPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
fetchAclTemplates,
fetchRolesWithTarget,
} from "../../../../slices/aclSlice";
import { filterRoles, policiesFiltered, rolesFilteredbyPolicies } from "../../../../utils/aclUtils";
import { policiesFiltered } from "../../../../utils/aclUtils";
import { useAppDispatch } from "../../../../store";
import { TransformedAcl } from "../../../../slices/aclDetailsSlice";
import { AccessPolicyTable, TemplateSelector } from "../../../shared/modals/ResourceDetailsAccessPolicyTab";
Expand Down Expand Up @@ -88,7 +88,6 @@ const AclAccessPage = <T extends RequiredFormProps>({
<AccessPolicyTable
isUserTable={true}
policiesFiltered={policiesFiltered(formik.values.policies, true)}
rolesFilteredbyPolicies={rolesFilteredbyPolicies(roles, formik.values.policies, true)}
header={"USERS.ACLS.NEW.ACCESS.ACCESS_POLICY.USERS"}
firstColumnHeader={"USERS.ACLS.NEW.ACCESS.ACCESS_POLICY.USER"}
createLabel={"USERS.ACLS.NEW.ACCESS.ACCESS_POLICY.NEW_USER"}
Expand All @@ -103,7 +102,6 @@ const AclAccessPage = <T extends RequiredFormProps>({
<AccessPolicyTable
isUserTable={false}
policiesFiltered={policiesFiltered(formik.values.policies, false)}
rolesFilteredbyPolicies={rolesFilteredbyPolicies(roles, formik.values.policies, false)}
header={"USERS.ACLS.NEW.ACCESS.ACCESS_POLICY.NON_USER_ROLES"}
firstColumnHeader={"USERS.ACLS.NEW.ACCESS.ACCESS_POLICY.ROLE"}
createLabel={"USERS.ACLS.NEW.ACCESS.ACCESS_POLICY.NEW"}
Expand All @@ -122,7 +120,6 @@ const AclAccessPage = <T extends RequiredFormProps>({
<AccessPolicyTable
isUserTable={false}
policiesFiltered={formik.values.policies}
rolesFilteredbyPolicies={filterRoles(roles, formik.values.policies)}
firstColumnHeader={"USERS.ACLS.NEW.ACCESS.ACCESS_POLICY.ROLE"}
createLabel={"USERS.ACLS.NEW.ACCESS.ACCESS_POLICY.NEW"}
formik={formik}
Expand Down
30 changes: 12 additions & 18 deletions src/utils/aclUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,18 @@ export const getAclTemplateText = (
}
};

export const filterRoles = (roles: Role[], policies: TransformedAcl[]) => {
return roles.filter(
role => !policies.find(policy => policy.role === role.name),
);
export const filterRoles = (roles: Role[], policies: TransformedAcl[], currentRole?: string) => {
// Collect all roles currently selected in policies
const selectedRoles = policies
.map(p => p.role)
.filter(Boolean);

return roles.filter(r =>
// keep it if it's not already selected
!selectedRoles.includes(r.name)
// OR it is the current role for this dropdown row
|| r.name === currentRole,
);
};

// Get all policies that have user information, or all policies that do not have user information
Expand All @@ -42,20 +50,6 @@ export const policiesFiltered = (
}
};

// Get all roles that have user information, or all policies that do not have user information
export const rolesFilteredbyPolicies = (
roles: Role[],
policies: TransformedAcl[],
byUser: boolean,
) => {
roles = filterRoles(roles, policies);
if (byUser) {
return roles.filter(role => role.user !== undefined);
} else {
return roles.filter(role => role.user === undefined);
}
};

/* fetches the policies for the chosen template and sets the policies in the formik form to those policies */
export const handleTemplateChange = async <T extends { policies: TransformedAcl[], aclTemplate: string }>(
templateId: string,
Expand Down