Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui/datasets: Refactor selectors #1087

Merged
merged 6 commits into from
Sep 21, 2022
Merged
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
11 changes: 6 additions & 5 deletions clients/ctl/admin-ui/cypress/e2e/datasets.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,9 @@ describe("Dataset", () => {
"putDataset"
);
});

it("Can render chosen data categories", () => {
cy.visit("/dataset/demo_users");
cy.visit("/dataset/demo_users_dataset");
cy.getByTestId("field-row-uuid").click();
cy.getByTestId("data-category-dropdown").click();
cy.get("[data-testid='checkbox-Unique ID'] > span").should(
Expand All @@ -440,7 +441,7 @@ describe("Dataset", () => {
});

it("Can deselect data categories", () => {
cy.visit("/dataset/demo_users");
cy.visit("/dataset/demo_users_dataset");
cy.getByTestId("field-row-uuid").click();
cy.getByTestId("data-category-dropdown").click();
cy.getByTestId("checkbox-Unique ID").click();
Expand All @@ -463,7 +464,7 @@ describe("Dataset", () => {
});

it("Can select more data categories", () => {
cy.visit("/dataset/demo_users");
cy.visit("/dataset/demo_users_dataset");
cy.getByTestId("field-row-uuid").click();
cy.getByTestId("data-category-dropdown").click();
cy.getByTestId("checkbox-Telemetry Data").click();
Expand All @@ -484,7 +485,7 @@ describe("Dataset", () => {
});

it("Can interact with the checkbox tree properly", () => {
cy.visit("/dataset/demo_users");
cy.visit("/dataset/demo_users_dataset");
cy.getByTestId("field-row-uuid").click();
cy.getByTestId("data-category-dropdown").click();
// expand system data
Expand Down Expand Up @@ -529,7 +530,7 @@ describe("Dataset", () => {
});

it("Should be able to clear selected", () => {
cy.visit("/dataset/demo_users");
cy.visit("/dataset/demo_users_dataset");
cy.getByTestId("field-row-uuid").click();
cy.getByTestId("data-category-dropdown").click();
cy.getByTestId("checkbox-System Data").click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { DEFAULT_ORGANIZATION_FIDES_KEY } from "~/features/organization";
import { Dataset, GenerateTypes, System, ValidTargets } from "~/types/api";

import {
setActiveDataset,
setActiveDatasetFidesKey,
useCreateDatasetMutation,
useGenerateDatasetMutation,
} from "./dataset.slice";
Expand Down Expand Up @@ -175,7 +175,7 @@ const DatabaseConnectForm = () => {
}

toast(successToastParams(`Generate and classify are now in progress`));
dispatch(setActiveDataset(createResult.dataset));
dispatch(setActiveDatasetFidesKey(createResult.dataset.fides_key));
router.push(`/dataset`);
};

Expand Down
70 changes: 34 additions & 36 deletions clients/ctl/admin-ui/src/features/dataset/DatasetCollectionView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,19 @@ import { useGetAllDataCategoriesQuery } from "~/features/taxonomy/taxonomy.slice

import ColumnDropdown from "./ColumnDropdown";
import {
selectActiveCollectionIndex,
selectActiveCollection,
selectActiveCollections,
selectActiveEditor,
setActiveCollectionIndex,
setActiveDataset,
setActiveDatasetFidesKey,
setActiveEditor,
useGetDatasetByKeyQuery,
} from "./dataset.slice";
import DatasetFieldsTable from "./DatasetFieldsTable";
import EditCollectionDrawer from "./EditCollectionDrawer";
import EditDatasetDrawer from "./EditDatasetDrawer";
import MoreActionsMenu from "./MoreActionsMenu";
import { ColumnMetadata } from "./types";
import { ColumnMetadata, EditableType } from "./types";

const ALL_COLUMNS: ColumnMetadata[] = [
{ name: "Field Name", attribute: "name" },
Expand All @@ -40,33 +43,35 @@ interface Props {
const DatasetCollectionView = ({ fidesKey }: Props) => {
const dispatch = useDispatch();
const { dataset, isLoading } = useDataset(fidesKey);
const activeCollectionIndex = useSelector(selectActiveCollectionIndex);
const activeCollections = useSelector(selectActiveCollections);
const activeCollection = useSelector(selectActiveCollection);
const activeEditor = useSelector(selectActiveEditor);

const [columns, setColumns] = useState<ColumnMetadata[]>(ALL_COLUMNS);
const [isModifyingCollection, setIsModifyingCollection] = useState(false);
const [isModifyingDataset, setIsModifyingDataset] = useState(false);

// Query subscriptions:
useGetAllDataCategoriesQuery();

useEffect(() => {
if (dataset) {
dispatch(setActiveDataset(dataset));
dispatch(setActiveDatasetFidesKey(dataset.fides_key));
dispatch(setActiveCollectionIndex(0));
}
}, [dispatch, dataset]);

useEffect(() => {
dispatch(setActiveCollectionIndex(0));
}, [dispatch]);

useEffect(
() => () => {
dispatch(setActiveDataset(null));
dispatch(setActiveDatasetFidesKey(undefined));
},
// This hook only runs on component un-mount to clear the active dataset.
// eslint-disable-next-line react-hooks/exhaustive-deps
[]
);

const handleChangeCollection = (event: ChangeEvent<HTMLSelectElement>) => {
dispatch(setActiveCollectionIndex(event.target.selectedIndex));
};

if (isLoading) {
return <Spinner />;
}
Expand All @@ -75,14 +80,6 @@ const DatasetCollectionView = ({ fidesKey }: Props) => {
return <div>Dataset not found</div>;
}

const { collections } = dataset;
const activeCollection =
activeCollectionIndex != null ? collections[activeCollectionIndex] : null;

const handleChangeCollection = (event: ChangeEvent<HTMLSelectElement>) => {
dispatch(setActiveCollectionIndex(event.target.selectedIndex));
};

return (
<Box>
<Box mb={4} display="flex" justifyContent="space-between">
Expand All @@ -92,7 +89,7 @@ const DatasetCollectionView = ({ fidesKey }: Props) => {
width="auto"
data-testid="collection-select"
>
{collections.map((collection) => (
{(activeCollections ?? []).map((collection) => (
<option key={collection.name} value={collection.name}>
{collection.name}
</option>
Expand All @@ -107,29 +104,30 @@ const DatasetCollectionView = ({ fidesKey }: Props) => {
/>
</Box>
<MoreActionsMenu
onModifyCollection={() => setIsModifyingCollection(true)}
onModifyDataset={() => setIsModifyingDataset(true)}
onModifyCollection={() =>
dispatch(setActiveEditor(EditableType.COLLECTION))
}
onModifyDataset={() =>
dispatch(setActiveEditor(EditableType.DATASET))
}
/>
</Box>
</Box>

<DatasetFieldsTable columns={columns} />

{activeCollection ? (
<>
<DatasetFieldsTable
fields={activeCollection.fields}
columns={columns}
/>
<EditCollectionDrawer
collection={activeCollection}
isOpen={isModifyingCollection}
onClose={() => setIsModifyingCollection(false)}
/>
</>
<EditCollectionDrawer
collection={activeCollection}
isOpen={activeEditor === EditableType.COLLECTION}
onClose={() => dispatch(setActiveEditor(undefined))}
/>
) : null}
{dataset ? (
<EditDatasetDrawer
dataset={dataset}
isOpen={isModifyingDataset}
onClose={() => setIsModifyingDataset(false)}
isOpen={activeEditor === EditableType.DATASET}
onClose={() => dispatch(setActiveEditor(undefined))}
/>
) : null}
</Box>
Expand Down
86 changes: 54 additions & 32 deletions clients/ctl/admin-ui/src/features/dataset/DatasetFieldsTable.tsx
Original file line number Diff line number Diff line change
@@ -1,38 +1,76 @@
import { Box, Table, Tbody, Td, Th, Thead, Tr } from "@fidesui/react";
import { useState } from "react";
import { useDispatch, useSelector } from "react-redux";

import { DatasetField } from "~/types/api";

import IdentifiabilityTag from "../taxonomy/IdentifiabilityTag";
import TaxonomyEntityTag from "../taxonomy/TaxonomyEntityTag";
import { selectActiveFieldIndex, setActiveFieldIndex } from "./dataset.slice";
import {
selectActiveEditor,
selectActiveField,
selectActiveFields,
setActiveEditor,
setActiveFieldIndex,
} from "./dataset.slice";
import EditFieldDrawer from "./EditFieldDrawer";
import { ColumnMetadata } from "./types";
import { ColumnMetadata, EditableType } from "./types";

// Chakra wants a JSX.Element, so all returns need to be wrapped in a fragment.
/* eslint-disable react/jsx-no-useless-fragment */
const Cell = ({
attribute,
field,
}: {
attribute: keyof DatasetField;
field: DatasetField;
}): JSX.Element => {
if (attribute === "data_qualifier") {
const dataQualifierName = field[attribute];
return (
<>
{dataQualifierName ? (
<IdentifiabilityTag dataQualifierName={dataQualifierName} />
) : null}
</>
);
}

if (attribute === "data_categories") {
return (
<>
{(field[attribute] ?? []).map((dc) => (
<Box key={`${field.name}-${dc}`} mr={2} mb={2}>
<TaxonomyEntityTag name={dc} />
</Box>
))}
</>
);
}

return <>{field[attribute]}</>;
};
/* eslint-disable react/jsx-no-useless-fragment */

interface Props {
fields: DatasetField[];
columns: ColumnMetadata[];
}

const DatasetFieldsTable = ({ fields, columns }: Props) => {
const DatasetFieldsTable = ({ columns }: Props) => {
const dispatch = useDispatch();
const [editDrawerIsOpen, setEditDrawerIsOpen] = useState(false);
const activeFieldIndex = useSelector(selectActiveFieldIndex);
const activeFields = useSelector(selectActiveFields);
const activeField = useSelector(selectActiveField);
const activeEditor = useSelector(selectActiveEditor);

const handleClose = () => {
setEditDrawerIsOpen(false);
dispatch(setActiveFieldIndex(null));
dispatch(setActiveFieldIndex(undefined));
dispatch(setActiveEditor(undefined));
};

const handleClick = (index: number) => {
dispatch(setActiveFieldIndex(index));
setEditDrawerIsOpen(true);
dispatch(setActiveEditor(EditableType.FIELD));
};

const activeField =
activeFieldIndex != null ? fields[activeFieldIndex] : null;

return (
<Box>
<Table size="sm" data-testid="dataset-fields-table">
Expand All @@ -46,7 +84,7 @@ const DatasetFieldsTable = ({ fields, columns }: Props) => {
</Tr>
</Thead>
<Tbody>
{fields.map((field, idx) => (
{(activeFields ?? []).map((field, idx) => (
<Tr
key={field.name}
_hover={{ bg: "gray.50" }}
Expand All @@ -62,23 +100,7 @@ const DatasetFieldsTable = ({ fields, columns }: Props) => {
>
{columns.map((c) => (
<Td key={`${c.name}-${field.name}`} pl={0}>
{(() => {
if (c.attribute === "data_qualifier") {
return (
<IdentifiabilityTag
dataQualifierName={field[c.attribute] ?? ""}
/>
);
}
if (c.attribute === "data_categories") {
return field[c.attribute]?.map((dc) => (
<Box key={`${field.name}-${dc}`} mr={2} mb={2}>
<TaxonomyEntityTag name={dc} />
</Box>
));
}
return field[c.attribute];
})()}
<Cell field={field} attribute={c.attribute} />
</Td>
))}
</Tr>
Expand All @@ -87,7 +109,7 @@ const DatasetFieldsTable = ({ fields, columns }: Props) => {
</Table>
{activeField ? (
<EditFieldDrawer
isOpen={editDrawerIsOpen}
isOpen={activeEditor === EditableType.FIELD}
onClose={handleClose}
field={activeField}
/>
Expand Down
15 changes: 8 additions & 7 deletions clients/ctl/admin-ui/src/features/dataset/DatasetTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,24 @@ import { Dataset } from "~/types/api";

import { STATUS_DISPLAY } from "./constants";
import {
selectActiveDataset,
setActiveDataset,
selectActiveDatasetFidesKey,
setActiveDatasetFidesKey,
useGetAllDatasetsQuery,
} from "./dataset.slice";

const DatasetsTable = () => {
const dispatch = useDispatch();
const activeDataset = useSelector(selectActiveDataset);
const activeDatasetFidesKey = useSelector(selectActiveDatasetFidesKey);

const { data: datasets } = useGetAllDatasetsQuery();
const classificationsMap = useClassificationsMap();

const handleRowClick = (dataset: Dataset) => {
// toggle the active dataset
if (dataset.fides_key === activeDataset?.fides_key) {
dispatch(setActiveDataset(null));
if (dataset.fides_key === activeDatasetFidesKey) {
dispatch(setActiveDatasetFidesKey(undefined));
} else {
dispatch(setActiveDataset(dataset));
dispatch(setActiveDatasetFidesKey(dataset.fides_key));
}
};

Expand All @@ -52,7 +52,8 @@ const DatasetsTable = () => {
<Tbody>
{datasets.map((dataset) => {
const isActive =
activeDataset && activeDataset.fides_key === dataset.fides_key;
activeDatasetFidesKey &&
activeDatasetFidesKey === dataset.fides_key;

const classification = classificationsMap.get(dataset.fides_key);
const statusDisplay =
Expand Down
7 changes: 5 additions & 2 deletions clients/ctl/admin-ui/src/features/dataset/DatasetYamlForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import { Dataset } from "~/types/api";

import { successToastParams } from "../common/toast";
import YamlForm from "../YamlForm";
import { setActiveDataset, useCreateDatasetMutation } from "./dataset.slice";
import {
setActiveDatasetFidesKey,
useCreateDatasetMutation,
} from "./dataset.slice";

// handle the common case where everything is nested under a `dataset` key
interface NestedDataset {
Expand Down Expand Up @@ -41,7 +44,7 @@ const DatasetYamlForm = () => {

const handleSuccess = (newDataset: Dataset) => {
toast(successToastParams("Successfully loaded new dataset YAML"));
setActiveDataset(newDataset);
setActiveDatasetFidesKey(newDataset.fides_key);
router.push(`/dataset/${newDataset.fides_key}`);
};

Expand Down
Loading