Skip to content

Commit

Permalink
722 tie user role and system save states together (#2913)
Browse files Browse the repository at this point in the history
  • Loading branch information
eastandwestwind authored Mar 27, 2023
1 parent 4241fb0 commit ff0e747
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 114 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 9 additions & 11 deletions clients/admin-ui/cypress/e2e/user-management.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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"]);
});
});
});
Expand All @@ -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([
Expand Down Expand Up @@ -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([]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ModalProps, "isOpen" | "onClose">) => {
const activeUserId = useAppSelector(selectActiveUserId);
assignedSystems,
onAssignedSystemChange,
}: Pick<ModalProps, "isOpen" | "onClose"> & {
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<System[]>(
initialManagedSystems
);
const toast = useToast();

useEffect(() => {
setAssignedSystems(initialManagedSystems);
}, [initialManagedSystems]);
const [selectedSystems, setSelectedSystems] =
useState<System[]>(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();
};

Expand All @@ -88,19 +58,19 @@ const AssignSystemsModal = ({
const handleToggleAllSystems = (event: ChangeEvent<HTMLInputElement>) => {
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 (
<Modal isOpen={isOpen} onClose={onClose} size="2xl">
Expand All @@ -114,7 +84,7 @@ const AssignSystemsModal = ({
>
<Text>Assign systems</Text>
<Badge bg="green.500" color="white" px={1}>
Assigned to {initialManagedSystems.length} systems
Assigned to {assignedSystems.length} systems
</Badge>
</ModalHeader>
<ModalBody data-testid="assign-systems-modal-body">
Expand Down Expand Up @@ -154,8 +124,8 @@ const AssignSystemsModal = ({
/>
<AssignSystemsTable
allSystems={filteredSystems}
assignedSystems={assignedSystems}
onChange={setAssignedSystems}
assignedSystems={selectedSystems}
onChange={setSelectedSystems}
/>
</Stack>
)}
Expand All @@ -173,9 +143,8 @@ const AssignSystemsModal = ({
{!emptySystems ? (
<Button
colorScheme="primary"
onClick={handleAssign}
onClick={handleConfirm}
data-testid="confirm-btn"
isLoading={isSubmitting}
>
Confirm
</Button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Table size="sm" data-testid="assign-systems-delete-table">
<Thead>
Expand All @@ -85,20 +67,9 @@ export const AssignSystemsDeleteTable = () => {
icon={<TrashCanSolidIcon />}
variant="outline"
size="sm"
onClick={onDeleteOpen}
onClick={() => onDelete(system)}
data-testid="unassign-btn"
/>
<ConfirmationModal
isOpen={deleteIsOpen}
onClose={onDeleteClose}
onConfirm={() => handleDelete(system)}
title="Remove System"
testId={`remove-${system.fides_key}-confirmation-modal`}
continueButtonText="Yes, Remove System"
message={
<Text>Are you sure you want to remove this system?</Text>
}
/>
</Td>
</Tr>
))}
Expand Down
38 changes: 33 additions & 5 deletions clients/admin-ui/src/features/user-management/PermissionsForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,23 @@ 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";
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";

Expand All @@ -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<System[]>(
initialManagedSystems
);
const [updateUserManagedSystemsTrigger] =
useUpdateUserManagedSystemsMutation();

useEffect(() => {
setAssignedSystems(initialManagedSystems);
}, [initialManagedSystems]);

const { data: userPermissions, isLoading } = useGetUserPermissionsQuery(
activeUserId ?? "",
Expand All @@ -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"));
Expand Down Expand Up @@ -113,6 +139,8 @@ const PermissionsForm = () => {
isDisabled={
role.roleKey === RoleRegistryEnum.OWNER ? !isOwner : false
}
assignedSystems={assignedSystems}
onAssignedSystemChange={setAssignedSystems}
{...role}
/>
);
Expand All @@ -126,7 +154,7 @@ const PermissionsForm = () => {
colorScheme="primary"
type="submit"
isLoading={isSubmitting}
disabled={!dirty}
disabled={!dirty && assignedSystems === initialManagedSystems}
data-testid="save-btn"
>
Save
Expand Down
Loading

0 comments on commit ff0e747

Please sign in to comment.