diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b7e663e77..97b8b3e387 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ The types of changes are: * Changed About Fides page to say "Fides Core Version:" over "Version". [#2899](https://github.com/ethyca/fides/pull/2899) * Polish Admin UI header & navigation [#2897](https://github.com/ethyca/fides/pull/2897) * Give new users a "viewer" role by default [#2900](https://github.com/ethyca/fides/pull/2900) +* Tie together save states for user permissions and systems [2913](https://github.com/ethyca/fides/pull/2913) ### Fixed diff --git a/clients/admin-ui/cypress/e2e/user-management.cy.ts b/clients/admin-ui/cypress/e2e/user-management.cy.ts index 3f305e90a6..ed9b67e934 100644 --- a/clients/admin-ui/cypress/e2e/user-management.cy.ts +++ b/clients/admin-ui/cypress/e2e/user-management.cy.ts @@ -232,9 +232,6 @@ describe("User management", () => { cy.intercept("PUT", "/api/v1/user/*/system-manager", { fixture: "systems/systems.json", }).as("updateUserManagedSystems"); - cy.intercept("DELETE", "/api/v1/user/*/system-manager/*", { - body: {}, - }).as("removeUserManagedSystem"); cy.intercept("GET", "/api/v1/system", { fixture: "systems/systems.json", }).as("getSystems"); @@ -258,14 +255,11 @@ describe("User management", () => { cy.getByTestId("row-fidesctl_system").within(() => { cy.getByTestId("unassign-btn").click(); }); - cy.getByTestId("remove-fidesctl_system-confirmation-modal").within( - () => { - cy.getByTestId("continue-btn").click({ force: true }); - } - ); - cy.wait("@removeUserManagedSystem").then((interception) => { - const { url } = interception.request; - expect(url).contains("fidesctl_system"); + cy.getByTestId("save-btn").click(); + + cy.wait("@updateUserManagedSystems").then((interception) => { + const { body } = interception.request; + expect(body).to.eql(["demo_analytics_system", "demo_marketing_system"]); }); }); }); @@ -292,7 +286,9 @@ describe("User management", () => { }); }); cy.getByTestId("confirm-btn").click(); + cy.getByTestId("save-btn").click(); + cy.wait("@updatePermission") cy.wait("@updateUserManagedSystems").then((interception) => { const { body } = interception.request; expect(body).to.eql([ @@ -331,6 +327,8 @@ describe("User management", () => { }); cy.getByTestId("confirm-btn").click(); + cy.getByTestId("save-btn").click(); + cy.wait("@updatePermission") cy.wait("@updateUserManagedSystems").then((interception) => { const { body } = interception.request; expect(body).to.eql([]); diff --git a/clients/admin-ui/src/features/user-management/AssignSystemsModal.tsx b/clients/admin-ui/src/features/user-management/AssignSystemsModal.tsx index 1a25dd3180..4c8ad6c21f 100644 --- a/clients/admin-ui/src/features/user-management/AssignSystemsModal.tsx +++ b/clients/admin-ui/src/features/user-management/AssignSystemsModal.tsx @@ -16,62 +16,32 @@ import { Stack, Switch, Text, - useToast, } from "@fidesui/react"; -import { ChangeEvent, useEffect, useMemo, useState } from "react"; +import { ChangeEvent, useMemo, useState } from "react"; -import { useAppSelector } from "~/app/hooks"; -import { getErrorMessage, isErrorResult } from "~/features/common/helpers"; import SearchBar from "~/features/common/SearchBar"; -import { errorToastParams, successToastParams } from "~/features/common/toast"; import { useGetAllSystemsQuery } from "~/features/system"; import { SEARCH_FILTER } from "~/features/system/SystemsManagement"; import { System } from "~/types/api"; import AssignSystemsTable from "./AssignSystemsTable"; -import { - selectActiveUserId, - selectActiveUsersManagedSystems, - useGetUserManagedSystemsQuery, - useUpdateUserManagedSystemsMutation, -} from "./user-management.slice"; const AssignSystemsModal = ({ isOpen, onClose, -}: Pick) => { - const activeUserId = useAppSelector(selectActiveUserId); + assignedSystems, + onAssignedSystemChange, +}: Pick & { + assignedSystems: System[]; + onAssignedSystemChange: (systems: System[]) => void; +}) => { const { data: allSystems } = useGetAllSystemsQuery(); - useGetUserManagedSystemsQuery(activeUserId as string, { - skip: !activeUserId, - }); - const [updateUserManagedSystemsTrigger, { isLoading: isSubmitting }] = - useUpdateUserManagedSystemsMutation(); const [searchFilter, setSearchFilter] = useState(""); - const initialManagedSystems = useAppSelector(selectActiveUsersManagedSystems); - const [assignedSystems, setAssignedSystems] = useState( - initialManagedSystems - ); - const toast = useToast(); - - useEffect(() => { - setAssignedSystems(initialManagedSystems); - }, [initialManagedSystems]); + const [selectedSystems, setSelectedSystems] = + useState(assignedSystems); - const handleAssign = async () => { - if (!activeUserId) { - return; - } - const fidesKeys = assignedSystems.map((s) => s.fides_key); - const result = await updateUserManagedSystemsTrigger({ - userId: activeUserId, - fidesKeys, - }); - if (isErrorResult(result)) { - toast(errorToastParams(getErrorMessage(result.error))); - return; - } - toast(successToastParams("Updated users permissions")); + const handleConfirm = async () => { + onAssignedSystemChange(selectedSystems); onClose(); }; @@ -88,19 +58,19 @@ const AssignSystemsModal = ({ const handleToggleAllSystems = (event: ChangeEvent) => { const { checked } = event.target; if (checked && allSystems) { - setAssignedSystems(filteredSystems); + setSelectedSystems(filteredSystems); } else { const notFilteredSystems = allSystems ? allSystems.filter((system) => !filteredSystems.includes(system)) : []; - setAssignedSystems(notFilteredSystems); + setSelectedSystems(notFilteredSystems); } }; const allSystemsAssigned = useMemo(() => { - const assignedSet = new Set(assignedSystems.map((s) => s.fides_key)); + const assignedSet = new Set(selectedSystems.map((s) => s.fides_key)); return filteredSystems.every((item) => assignedSet.has(item.fides_key)); - }, [filteredSystems, assignedSystems]); + }, [filteredSystems, selectedSystems]); return ( @@ -114,7 +84,7 @@ const AssignSystemsModal = ({ > Assign systems - Assigned to {initialManagedSystems.length} systems + Assigned to {assignedSystems.length} systems @@ -154,8 +124,8 @@ const AssignSystemsModal = ({ /> )} @@ -173,9 +143,8 @@ const AssignSystemsModal = ({ {!emptySystems ? ( diff --git a/clients/admin-ui/src/features/user-management/AssignSystemsTable.tsx b/clients/admin-ui/src/features/user-management/AssignSystemsTable.tsx index 24e38f9c65..327720d601 100644 --- a/clients/admin-ui/src/features/user-management/AssignSystemsTable.tsx +++ b/clients/admin-ui/src/features/user-management/AssignSystemsTable.tsx @@ -5,63 +5,45 @@ import { Table, Tbody, Td, - Text, Th, Thead, Tr, - useDisclosure, - useToast, } from "@fidesui/react"; -import ConfirmationModal from "common/ConfirmationModal"; -import { getErrorMessage } from "common/helpers"; -import { errorToastParams, successToastParams } from "common/toast"; import React from "react"; import { useAppSelector } from "~/app/hooks"; import { TrashCanSolidIcon } from "~/features/common/Icon/TrashCanSolidIcon"; import { System } from "~/types/api"; -import { isErrorResult } from "~/types/errors"; import { selectActiveUserId, - selectActiveUsersManagedSystems, useGetUserManagedSystemsQuery, - useRemoveUserManagedSystemMutation, } from "./user-management.slice"; -export const AssignSystemsDeleteTable = () => { +export const AssignSystemsDeleteTable = ({ + assignedSystems, + onAssignedSystemChange, +}: { + assignedSystems: System[]; + onAssignedSystemChange: (systems: System[]) => void; +}) => { const activeUserId = useAppSelector(selectActiveUserId); - const { - isOpen: deleteIsOpen, - onOpen: onDeleteOpen, - onClose: onDeleteClose, - } = useDisclosure(); useGetUserManagedSystemsQuery(activeUserId as string, { skip: !activeUserId, }); - const assignedSystems = useAppSelector(selectActiveUsersManagedSystems); - const toast = useToast(); - const [removeUserManagedSystemTrigger] = useRemoveUserManagedSystemMutation(); - const handleDelete = async (system: System) => { - if (!activeUserId) { - return; - } - const result = await removeUserManagedSystemTrigger({ - userId: activeUserId, - systemKey: system.fides_key, - }); - if (isErrorResult(result)) { - toast(errorToastParams(getErrorMessage(result.error))); - } else { - toast(successToastParams("Successfully removed system")); - onDeleteClose(); - } - }; if (assignedSystems.length === 0) { return null; } + const onDelete = (system: System) => { + onAssignedSystemChange( + assignedSystems.filter( + (assignedSystem) => assignedSystem.fides_key !== system.fides_key + ) + ); + }; + return ( @@ -85,20 +67,9 @@ export const AssignSystemsDeleteTable = () => { icon={} variant="outline" size="sm" - onClick={onDeleteOpen} + onClick={() => onDelete(system)} data-testid="unassign-btn" /> - handleDelete(system)} - title="Remove System" - testId={`remove-${system.fides_key}-confirmation-modal`} - continueButtonText="Yes, Remove System" - message={ - Are you sure you want to remove this system? - } - /> ))} diff --git a/clients/admin-ui/src/features/user-management/PermissionsForm.tsx b/clients/admin-ui/src/features/user-management/PermissionsForm.tsx index db3b750de2..1f740db8db 100644 --- a/clients/admin-ui/src/features/user-management/PermissionsForm.tsx +++ b/clients/admin-ui/src/features/user-management/PermissionsForm.tsx @@ -10,6 +10,7 @@ import { import { useHasRole } from "common/Restrict"; import { Form, Formik } from "formik"; import NextLink from "next/link"; +import { useEffect, useState } from "react"; import { useAppSelector } from "~/app/hooks"; import { USER_MANAGEMENT_ROUTE } from "~/constants"; @@ -17,12 +18,15 @@ import { getErrorMessage, isErrorResult } from "~/features/common/helpers"; import QuestionTooltip from "~/features/common/QuestionTooltip"; import { errorToastParams, successToastParams } from "~/features/common/toast"; import { ROLES } from "~/features/user-management/constants"; -import { RoleRegistryEnum } from "~/types/api"; +import { RoleRegistryEnum, System } from "~/types/api"; import RoleOption from "./RoleOption"; import { selectActiveUserId, + selectActiveUsersManagedSystems, + useGetUserManagedSystemsQuery, useGetUserPermissionsQuery, + useUpdateUserManagedSystemsMutation, useUpdateUserPermissionsMutation, } from "./user-management.slice"; @@ -35,6 +39,19 @@ export type FormValues = typeof defaultInitialValues; const PermissionsForm = () => { const toast = useToast(); const activeUserId = useAppSelector(selectActiveUserId); + useGetUserManagedSystemsQuery(activeUserId as string, { + skip: !activeUserId, + }); + const initialManagedSystems = useAppSelector(selectActiveUsersManagedSystems); + const [assignedSystems, setAssignedSystems] = useState( + initialManagedSystems + ); + const [updateUserManagedSystemsTrigger] = + useUpdateUserManagedSystemsMutation(); + + useEffect(() => { + setAssignedSystems(initialManagedSystems); + }, [initialManagedSystems]); const { data: userPermissions, isLoading } = useGetUserPermissionsQuery( activeUserId ?? "", @@ -50,15 +67,24 @@ const PermissionsForm = () => { if (!activeUserId) { return; } - const result = await updateUserPermissionMutationTrigger({ + const userPermissionsResult = await updateUserPermissionMutationTrigger({ user_id: activeUserId, // Scopes are not editable in the UI, but make sure we retain whatever scopes // the user might've already had payload: { scopes: userPermissions?.scopes, roles: values.roles }, }); - if (isErrorResult(result)) { - toast(errorToastParams(getErrorMessage(result.error))); + if (isErrorResult(userPermissionsResult)) { + toast(errorToastParams(getErrorMessage(userPermissionsResult.error))); + return; + } + const fidesKeys = assignedSystems.map((s) => s.fides_key); + const userSystemsResult = await updateUserManagedSystemsTrigger({ + userId: activeUserId, + fidesKeys, + }); + if (isErrorResult(userSystemsResult)) { + toast(errorToastParams(getErrorMessage(userSystemsResult.error))); return; } toast(successToastParams("Permissions updated")); @@ -113,6 +139,8 @@ const PermissionsForm = () => { isDisabled={ role.roleKey === RoleRegistryEnum.OWNER ? !isOwner : false } + assignedSystems={assignedSystems} + onAssignedSystemChange={setAssignedSystems} {...role} /> ); @@ -126,7 +154,7 @@ const PermissionsForm = () => { colorScheme="primary" type="submit" isLoading={isSubmitting} - disabled={!dirty} + disabled={!dirty && assignedSystems === initialManagedSystems} data-testid="save-btn" > Save diff --git a/clients/admin-ui/src/features/user-management/RoleOption.tsx b/clients/admin-ui/src/features/user-management/RoleOption.tsx index 49a3c73c92..8923bc5917 100644 --- a/clients/admin-ui/src/features/user-management/RoleOption.tsx +++ b/clients/admin-ui/src/features/user-management/RoleOption.tsx @@ -11,7 +11,7 @@ import { } from "@fidesui/react"; import { useFormikContext } from "formik"; -import { RoleRegistryEnum } from "~/types/api"; +import { RoleRegistryEnum, System } from "~/types/api"; import QuestionTooltip from "../common/QuestionTooltip"; import AssignSystemsModal from "./AssignSystemsModal"; @@ -23,9 +23,18 @@ interface Props { roleKey: RoleRegistryEnum; isSelected: boolean; isDisabled: boolean; + assignedSystems: System[]; + onAssignedSystemChange: (systems: System[]) => void; } -const RoleOption = ({ label, roleKey, isSelected, isDisabled }: Props) => { +const RoleOption = ({ + label, + roleKey, + isSelected, + isDisabled, + assignedSystems, + onAssignedSystemChange, +}: Props) => { const { setFieldValue } = useFormikContext(); const assignSystemsModal = useDisclosure(); @@ -75,13 +84,18 @@ const RoleOption = ({ label, roleKey, isSelected, isDisabled }: Props) => { > Assign systems + - + {/* By conditionally rendering the modal, we force it to reset its state whenever it opens */} {assignSystemsModal.isOpen ? ( ) : null}