diff --git a/src/components/events/partials/ModalTabsAndPages/NewAccessPage.tsx b/src/components/events/partials/ModalTabsAndPages/NewAccessPage.tsx index b2a7d9961b..07ad6d0a24 100644 --- a/src/components/events/partials/ModalTabsAndPages/NewAccessPage.tsx +++ b/src/components/events/partials/ModalTabsAndPages/NewAccessPage.tsx @@ -2,14 +2,13 @@ import { useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; import Notifications from "../../../shared/Notifications"; import { - Role, checkAcls, fetchAclActions, fetchAclTemplates, fetchRolesWithTarget, } from "../../../../slices/aclSlice"; import { FormikProps } from "formik"; -import { policiesFiltered, rolesFiltered } from "../../../../utils/aclUtils"; +import { policiesFiltered } from "../../../../utils/aclUtils"; import { useAppDispatch, useAppSelector } from "../../../../store"; import { fetchSeriesDetailsAcls } from "../../../../slices/seriesDetailsSlice"; import { getSeriesDetailsAcl } from "../../../../selectors/seriesDetailsSelectors"; @@ -55,21 +54,23 @@ const NewAccessPage = ({ // States containing response from server concerning acl templates, actions and roles const [aclTemplates, setAclTemplates] = useState<{ id: string, value: string}[]>([]); const [aclActions, setAclActions] = useState<{ id: string, value: string}[]>([]); - const [roles, setRoles] = useState([]); + const [isSanitize, setIsSanitize] = useState(undefined); const [loading, setLoading] = useState(false); const seriesAcl = useAppSelector(state => getSeriesDetailsAcl(state)); const user = useAppSelector(state => getUserInformation(state)); useEffect(() => { - // fetch data about roles, acl templates and actions from backend + // fetch data about acl templates and actions from backend async function fetchData() { setLoading(true); const [responseTemplates, responseActions, responseRoles] = await Promise.all([ - fetchAclTemplates(), fetchAclActions(), fetchRolesWithTarget("ACL")]); + fetchAclTemplates(), fetchAclActions(), fetchRolesWithTarget("ACL", { limit: 1 })]); setAclTemplates(responseTemplates); setAclActions(responseActions); - setRoles(responseRoles); + if (responseRoles.length > 0) { + setIsSanitize(responseRoles[0].isSanitize); + } setLoading(false); } @@ -118,13 +119,13 @@ const NewAccessPage = ({ defaultUser={user} /> - {roles.length > 0 && !roles[0].isSanitize && + {isSanitize === false && <> {hasAccess(viewUsersAccessRole, user) && ({ hasActions={aclActions.length > 0} transactions={{ readOnly: false }} aclActions={aclActions} - roles={roles} editAccessRole={editAccessRole} /> } @@ -141,7 +141,7 @@ const NewAccessPage = ({ ({ hasActions={aclActions.length > 0} transactions={{ readOnly: false }} aclActions={aclActions} - roles={roles} editAccessRole={editAccessRole} /> } } - {roles.length > 0 && roles[0].isSanitize && + {isSanitize === true && <> 0} transactions={{ readOnly: false }} aclActions={aclActions} - roles={roles} editAccessRole={editAccessRole} />
diff --git a/src/components/shared/DropDown.tsx b/src/components/shared/DropDown.tsx index 03dd4a8078..5a2ed29700 100644 --- a/src/components/shared/DropDown.tsx +++ b/src/components/shared/DropDown.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from "react"; +import React, { useCallback, useEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { dropDownSpacingTheme, @@ -16,11 +16,25 @@ export type DropDownOption = { order?: number } +// How long to wait after the user stops typing before firing a fetchOptions() search request. +const SEARCH_DEBOUNCE_MS = 300; + +function MenuListRow({ + index, + names, + style, +}: RowComponentProps<{ + names: string[]; +}>) { + const name = names[index]; + return
{name}
; +} + /** * This component renders a dropdown menu using react-select */ const DropDown = ({ - ref = React.createRef, boolean, GroupBase>>>(), + ref, value, text, options, @@ -40,6 +54,7 @@ const DropDown = ({ optionHeight = 25, customCSS, fetchOptions, + loadOptionsOnMount = true, }: { ref?: React.RefObject, boolean, GroupBase>> | null> value: T @@ -66,10 +81,22 @@ const DropDown = ({ optionLineHeight?: string }, fetchOptions?: (inputValue: string) => Promise[]> + // Whether an async (fetchOptions-based) dropdown should eagerly fetch its default option list on + // mount, before the user has interacted with it. Set to false when many instances of this dropdown + // may be mounted at once (e.g. one per table row), to avoid firing one request per instance on + // render; the default option list is then fetched lazily, the first time this instance's menu is + // opened, instead of unconditionally on mount. + loadOptionsOnMount?: boolean }) => { const { t } = useTranslation(); - const selectRef = ref; + const internalRef = useRef, boolean, GroupBase>> | null>(null); + const selectRef = ref ?? internalRef; + + // Holds the result of the one-off default-option fetch below, once it has completed. + const [preloadedOptions, setPreloadedOptions] = useState[] | undefined>(undefined); + const [isPreloading, setIsPreloading] = useState(false); + const hasStartedPreload = useRef(false); const style = dropDownStyle(customCSS ?? {}); @@ -81,6 +108,14 @@ const DropDown = ({ }, [menuIsOpen, selectRef]); const openMenu = (open: boolean) => { + // If loadOptionsOnMount === false, fetch option list ourselves here + if (open && fetchOptions && !loadOptionsOnMount && !hasStartedPreload.current) { + hasStartedPreload.current = true; + setIsPreloading(true); + fetchOptions("") + .then(fetched => setPreloadedOptions(formatOptions(fetched, required))) + .finally(() => setIsPreloading(false)); + } if (handleMenuIsOpen !== undefined) { handleMenuIsOpen(open); } @@ -91,10 +126,11 @@ const DropDown = ({ required: boolean, ) => { // Translate - // Translating is expensive, skip it if it is not required - if (!skipTranslate) { - unformattedOptions = unformattedOptions.map(option => ({ ...option, label: t(option.label as ParseKeys) })); - } + // Translating is expensive, skip it if it is not required. + // Either way, copy the array so the input is not transmuted later. + unformattedOptions = skipTranslate + ? [...unformattedOptions] + : unformattedOptions.map(option => ({ ...option, label: t(option.label as ParseKeys) })); // Add "No value" option if (!required) { @@ -130,7 +166,7 @@ const DropDown = ({ /** * Custom component for list virtualization */ - const MenuList = (props: MenuListProps, false>) => { + const MenuList = useCallback((props: MenuListProps, false>) => { const { children, maxHeight } = props; return Array.isArray(children) ? ( @@ -148,19 +184,10 @@ const DropDown = ({ overscanCount={4} />
- ) : null; - }; - - function MenuListRow({ - index, - names, - style, - }: RowComponentProps<{ - names: string[]; - }>) { - const name = names[index]; - return
{name}
; - } + // react-select passes a single NoOptionsMessage/LoadingMessage node here (not an array) when + // there are no options to list, e.g. before the user has typed anything into an async dropdown. + ) : children; + }, [itemHeight]); const filterOptions = (inputValue: string) => { if (options) { @@ -171,14 +198,23 @@ const DropDown = ({ return []; }; + const debounceTimeoutRef = useRef>(undefined); + + useEffect(() => { + return () => clearTimeout(debounceTimeoutRef.current); + }, []); + const loadOptionsAsync = (inputValue: string, callback: (options: DropDownOption[]) => void) => { - const timeout = async () => { - callback(formatOptions( - fetchOptions ? await fetchOptions(inputValue) : filterOptions(inputValue), - required, - )); - }; - setTimeout(() => { timeout(); }, 1000); + clearTimeout(debounceTimeoutRef.current); + debounceTimeoutRef.current = setTimeout(() => { + const timeout = async () => { + callback(formatOptions( + fetchOptions ? await fetchOptions(inputValue) : filterOptions(inputValue), + required, + )); + }; + void timeout(); + }, SEARCH_DEBOUNCE_MS); }; const loadOptions = ( @@ -206,7 +242,8 @@ const DropDown = ({ options, required, ) - : true, + : loadOptionsOnMount || (preloadedOptions ?? []), + isLoading: isPreloading, cacheOptions: true, loadOptions: fetchOptions ? loadOptionsAsync : loadOptions, placeholder: placeholder, @@ -216,7 +253,7 @@ const DropDown = ({ onMenuClose: () => openMenu(false), isDisabled: disabled, openMenuOnFocus: openMenuOnFocus, - menuPlacement: menuPlacement ?? "auto", + menuPlacement: menuPlacement, components: { MenuList }, }; @@ -229,7 +266,6 @@ const DropDown = ({ t("SELECT_NO_MATCHING_RESULTS")} /> ); diff --git a/src/components/shared/modals/ResourceDetailsAccessPolicyTab.tsx b/src/components/shared/modals/ResourceDetailsAccessPolicyTab.tsx index b45978c763..8cc406e747 100644 --- a/src/components/shared/modals/ResourceDetailsAccessPolicyTab.tsx +++ b/src/components/shared/modals/ResourceDetailsAccessPolicyTab.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useMemo } from "react"; +import React, { useState, useEffect, useRef } from "react"; import RenderMultiField from "../wizard/RenderMultiField"; import { Acl, @@ -17,8 +17,8 @@ import { } from "../../../utils/resourceUtils"; import { getUserInformation } from "../../../selectors/userInfoSelectors"; import { hasAccess } from "../../../utils/utils"; -import DropDown from "../DropDown"; -import { getAclTemplateText, handleTemplateChange, policiesFiltered, rolesFiltered } from "../../../utils/aclUtils"; +import DropDown, { DropDownOption } from "../DropDown"; +import { getAclTemplateText, handleTemplateChange, policiesFiltered } from "../../../utils/aclUtils"; import { useAppDispatch, useAppSelector } from "../../../store"; import { removeNotificationWizardForm, addNotification } from "../../../slices/notificationSlice"; import { useTranslation } from "react-i18next"; @@ -101,8 +101,10 @@ const ResourceDetailsAccessPolicyTab = ({ // shows, whether a resource has additional actions on top of normal read and write rights const [hasActions, setHasActions] = useState(false); - // list of possible roles - const [roles, setRoles] = useState([]); + // Whether per-role user info is sanitized (hidden) by this Opencast instance. Determines whether the roles + // list is split into a "Users" table and a "Groups & other roles" table, or shown as a single combined table. + // undefined until the initial, minimal check below has resolved. + const [isSanitize, setIsSanitize] = useState(undefined); // this state is used, because the policies should be read-only, if a transaction is currently being performed on a resource const [transactions, setTransactions] = useState({ readOnly: false }); @@ -122,7 +124,12 @@ const ResourceDetailsAccessPolicyTab = ({ setAclTemplates(responseTemplates); setAclActions(responseActions); setHasActions(responseActions.length > 0); - fetchRolesWithTarget("ACL").then(roles => setRoles(roles)); + // Fetch a single role just to read the isSanitize flag off it, rather than fetching every role. + fetchRolesWithTarget("ACL", { limit: 1 }).then(roles => { + if (roles.length > 0) { + setIsSanitize(roles[0].isSanitize); + } + }); if (fetchHasActiveTransactions) { const fetchTransactionResult = await dispatch(fetchHasActiveTransactions(resourceId)).then(unwrapResult); if (fetchTransactionResult.active !== undefined) { @@ -306,13 +313,13 @@ const ResourceDetailsAccessPolicyTab = ({ defaultUser={user} /> - {roles.length > 0 && !roles[0].isSanitize && + {isSanitize === false && <> {hasAccess(viewUsersAccessRole, user) && } @@ -329,7 +335,7 @@ const ResourceDetailsAccessPolicyTab = ({ } } - {roles.length > 0 && roles[0].isSanitize && + {isSanitize === true && <>
@@ -400,7 +404,7 @@ type AccessPolicyTabFormikProps = { export const AccessPolicyTable = ({ isUserTable, policiesFiltered, - rolesFilteredbyPolicies, + hasUser, header, firstColumnHeader, createLabel, @@ -408,12 +412,13 @@ export const AccessPolicyTable = ({ hasActions, transactions, aclActions, - roles, editAccessRole, }: { isUserTable: boolean policiesFiltered: TransformedAcl[] - rolesFilteredbyPolicies: Role[] + // If set, only search roles that do (true) or don't (false) resolve to a user account. + // undefined means no filter (single combined table, sanitized instances). + hasUser: boolean | undefined header?: ParseKeys firstColumnHeader: ParseKeys createLabel: ParseKeys, @@ -421,7 +426,6 @@ export const AccessPolicyTable = ({ hasActions: boolean transactions: { readOnly: boolean } aclActions: { id: string, value: string }[] - roles: Role[] editAccessRole: string }) => { const { t } = useTranslation(); @@ -435,11 +439,26 @@ export const AccessPolicyTable = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - const dropdownOptions = useMemo(() => { - return roles.length > 0 - ? formatAclRolesForDropdown(rolesFilteredbyPolicies) - : []; - }, [roles, rolesFilteredbyPolicies]); + // Roles seen in the most recent search, keyed by name, so a selection can be enriched with its + // full Role (including .user) without having to hold or re-fetch the entire roles list. + const roleCacheRef = useRef>(new Map()); + + const fetchRoleOptions = async (inputValue: string): Promise[]> => { + let fetchedRoles = await fetchRolesWithTarget("ACL", { query: inputValue, limit: 50, hasUser }); + + if (aclDefaults && aclDefaults["display_role_filter_blacklist_prefixes"]) { + const prefixes = aclDefaults["display_role_filter_blacklist_prefixes"].split(","); + fetchedRoles = fetchedRoles.filter(role => + !prefixes.some(prefix => role.name.startsWith(prefix)), + ); + } + + for (const role of fetchedRoles) { + roleCacheRef.current.set(role.name, role); + } + + return formatAclRolesForDropdown(fetchedRoles); + }; const createPolicy = (role: string, withUser: boolean): TransformedAcl => { const user = withUser ? { username: "", name: "", email: "" } : undefined; @@ -472,16 +491,6 @@ export const AccessPolicyTable = ({ return newRole; }; - // Filter available options by custom prefixes from the config - if (aclDefaults) { - if (aclDefaults["display_role_filter_blacklist_prefixes"]) { - const prefixes = aclDefaults["display_role_filter_blacklist_prefixes"].split(","); - rolesFilteredbyPolicies = rolesFilteredbyPolicies.filter(role => - !prefixes.some(prefix => role.name.startsWith(prefix)), - ); - } - } - return ( <> {/* list of policy details and interface for changing them */} @@ -554,12 +563,13 @@ export const AccessPolicyTable = ({ { if (element) { - const matchingRole = roles.find(role => role.name === element.value); + const matchingRole = roleCacheRef.current.get(element.value); arrayHelpers.replace(formik.values.policies.findIndex(p => p === policy), { ...policy, role: element.value, diff --git a/src/components/shared/wizard/RenderField.tsx b/src/components/shared/wizard/RenderField.tsx index d2c72ea52f..eb6800cdae 100644 --- a/src/components/shared/wizard/RenderField.tsx +++ b/src/components/shared/wizard/RenderField.tsx @@ -422,7 +422,9 @@ const EditableSingleSelectDropDown = ({ } customCSS={{ isMetadataStyle: focused ? false : true, width: "100%" }} handleMenuIsOpen={(open: boolean) => setFocused(open)} - openMenuOnFocus + // Deliberately false: with the menu open, Tab breaks keyboard + // navigation through the metadata form fields (see f44c9b7). + openMenuOnFocus={false} autoFocus={isFirstField} skipTranslate={!metadataField.translatable} /> diff --git a/src/components/users/partials/wizard/AclAccessPage.tsx b/src/components/users/partials/wizard/AclAccessPage.tsx index c591b7ac82..35d1d9363d 100644 --- a/src/components/users/partials/wizard/AclAccessPage.tsx +++ b/src/components/users/partials/wizard/AclAccessPage.tsx @@ -3,13 +3,12 @@ import { useTranslation } from "react-i18next"; import { FormikProps } from "formik"; import Notifications from "../../../shared/Notifications"; import { - Role, checkAcls, fetchAclActions, fetchAclTemplates, fetchRolesWithTarget, } from "../../../../slices/aclSlice"; -import { policiesFiltered, rolesFiltered } from "../../../../utils/aclUtils"; +import { policiesFiltered } from "../../../../utils/aclUtils"; import { useAppDispatch } from "../../../../store"; import { TransformedAcl } from "../../../../slices/aclDetailsSlice"; import { AccessPolicyTable, TemplateSelector } from "../../../shared/modals/ResourceDetailsAccessPolicyTab"; @@ -40,20 +39,22 @@ const AclAccessPage = ({ const [aclTemplates, setAclTemplates] = useState<{ id: string, value: string }[]>([]); const [aclActions, setAclActions] = useState<{ id: string, value: string }[]>([]); - const [roles, setRoles] = useState([]); + const [isSanitize, setIsSanitize] = useState(undefined); const [loading, setLoading] = useState(false); const editAccessRole = "ROLE_UI_SERIES_DETAILS_ACL_EDIT"; useEffect(() => { - // fetch data about roles, acl templates and actions from backend + // fetch data about acl templates and actions from backend async function fetchData() { setLoading(true); const [responseTemplates, responseActions, responseRoles] = await Promise.all([ - fetchAclTemplates(), fetchAclActions(), fetchRolesWithTarget("ACL")]); + fetchAclTemplates(), fetchAclActions(), fetchRolesWithTarget("ACL", { limit: 1 })]); setAclTemplates(responseTemplates); setAclActions(responseActions); - setRoles(responseRoles); + if (responseRoles.length > 0) { + setIsSanitize(responseRoles[0].isSanitize); + } setLoading(false); } @@ -83,12 +84,12 @@ const AclAccessPage = ({ aclTemplates={aclTemplates} /> - {roles.length > 0 && !roles[0].isSanitize && + {isSanitize === false && <> ({ hasActions={aclActions.length > 0} transactions={{ readOnly: false }} aclActions={aclActions} - roles={roles} editAccessRole={editAccessRole} /> ({ hasActions={aclActions.length > 0} transactions={{ readOnly: false }} aclActions={aclActions} - roles={roles} editAccessRole={editAccessRole} /> } - {roles.length > 0 && roles[0].isSanitize && + {isSanitize === true && <> 0} transactions={{ readOnly: false }} aclActions={aclActions} - roles={roles} editAccessRole={editAccessRole} />
diff --git a/src/slices/aclSlice.ts b/src/slices/aclSlice.ts index 01f6e13052..4ab5a59a60 100644 --- a/src/slices/aclSlice.ts +++ b/src/slices/aclSlice.ts @@ -187,10 +187,18 @@ export const fetchAclTemplateByName = async (name: string) => { }; // fetch roles for select dialogs and access policy pages -export const fetchRolesWithTarget = async (target: string) => { +export const fetchRolesWithTarget = async (target: string, options?: { + query?: string, + limit?: number, + offset?: number, + hasUser?: boolean, // If set, only return roles that do (true) or don't (false) resolve to an actual user account. +}) => { const params = { - limit: -1, + limit: options?.limit ?? -1, + offset: options?.offset, target: target, + query: options?.query, + hasUser: options?.hasUser, }; const response = await axios.get("/admin-ng/acl/roles.json", { params: params });