Data catalog UI: MVP #5628

jpople · 2024-12-20T04:33:48Z

Closes HJ-333

Description Of Changes

First pass at data catalog (new D&D view).

"Systems" table which displays:
- Any system with an integration that supports D&D
- Any system with a non-empty "dataset reference"
On applicable systems, "projects" table which displays "database-level" resources
Streamlined combined view for resources in detection & discovery

Code Changes

Add new pages and tables under "data catalog"
Add new handling for D&D resource diff statuses
Refactor taxonomy cells and add new taxonomy cell for adding data uses to systems
Refactor data use CRUD
Regenerate OpenAPI types

Steps to Confirm

To test, run a Plus backend (using the fidesplus image) on this Plus branch. Pin requirements.txt to use the latest commit from this Fides branch, e.g.:

// or the most recent commit hash 
ethyca-fides[all] @ git+https://github.com/ethyca/fides.git@5e0afc137fbf857700209cb3bf9caa0f64324922#egg=ethyca-fides

Make systems:

with an integration with a database layer (e.g. BigQuery); add a monitor to the integration and scan it
with an integration without a database layer (e.g. DynamoDB); add a monitor to the integration and scan it
with no integration, but a dataset in the system's "dataset reference" field
with none of these
In nav, click on "data catalog" view
In systems table:
- Should see your systems with integrations and with dataset; should not see the system without
- Should be able to click into results under the systems with integrations, but not the system with the dataset reference
- Should be able to click "view details" in the overflow menu for each system to view the system details page
On clicking through to the system, should see databases/projects for the system with databases/projects and datasets for the other
Should be able to click through to staged resources under the dataset level
Should be able to classify a resource in the table, then optionally change data categories and confirm classification

Pre-Merge Checklist

…taset-lifecycle-top-tables

vercel · 2024-12-20T04:33:53Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	❌ Failed (Inspect)			Jan 13, 2025 11:21pm

cypress · 2025-01-13T16:55:08Z

fides Run #11726

Run Properties: Failed #11726 • 182a18b265 ℹ️: Merge f1ad82e5e54dd93e57a16ca87fe9f51b5a0507ee into 7ea4ad58d49dfcd525344fdf1de8...

Project	`fides`
Branch Review	`refs/pull/5628/merge`
Run status	`Failed #11726`
Run duration	`01m 03s`
Commit	`182a18b265 ℹ️: Merge f1ad82e5e54dd93e57a16ca87fe9f51b5a0507ee into 7ea4ad58d49dfcd525344fdf1de8...`
Committer	`jpople`
View all properties for this run ↗︎

Test results
Failures	`1`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`3`
View all changes introduced in this branch ↗︎

Tests for review

cypress/e2e/smoke_test.cy.ts • 1 failed test

View Output Video

Test		Artifacts
Smoke test > can access Mongo and Postgres connectors from the Admin UI		`Screenshots` `Video`

gilluminate

Overall looking good. Loving a lot of the abstractions for clean and more reusable code. Left a few comments, questions, and nit-picks for potential improvement.

gilluminate · 2025-01-13T17:19:02Z

clients/admin-ui/src/features/common/ResultStatusBadge.tsx

+import { Badge, BadgeProps } from "fidesui";
+import React from "react";
+
+const ResultStatusBadge = React.forwardRef<HTMLSpanElement, BadgeProps>(


What about this makes it specific to Result Status? Would this be better as a more generically named component, or does it need specific properties required with more strict type? (eg. colorScheme which has to be of STATUS_COLOR_MAP enum type). You might consider exporting the STATUS_COLOR_MAP enum from this component.

Good callout-- deleted this component and changed all of its uses to use the more generic BadgeCell.

CHANGELOG.md

gilluminate · 2025-01-13T17:25:55Z

clients/admin-ui/src/features/common/TaxonomiesPicker.tsx

-      gap={2}
-      overflowX="auto"
-    >
+    <TaxonomyCellContainer>


I wonder if this file should be renamed/moved since it's specific to TableV2 cells

clients/admin-ui/src/features/common/dropdown/TaxonomySelect.tsx

gilluminate · 2025-01-13T18:01:56Z

clients/admin-ui/src/features/common/form/AdvancedSettings.tsx

+import { ChevronDownIcon, Collapse, Flex, Text, useDisclosure } from "fidesui";
+import { ReactNode } from "react";
+
+const AdvancedSettings = ({ children }: { children: ReactNode }) => {


this component just seems superfluous. It's either too specifically named (where the label could be another prop), or not specific enough about the children it accepts. Does it always need to be inside a form or can it be moved out to the "common" parent directory?

clients/admin-ui/src/features/data-catalog/systems/EditMinimalDataUseModal.tsx

gilluminate · 2025-01-13T18:52:03Z

clients/admin-ui/src/features/data-catalog/systems/EditMinimalDataUseModal.tsx

+                </Stack>
+              </AdvancedSettings>
+            </Flex>
+            <div className="flex w-full justify-between">


This file also uses Chakra's Box quite a bit. What do you think about updating those to use tailwind like this for consistency? This could potentially use Ant's Flex component as well.

gilluminate · 2025-01-13T19:04:06Z

clients/admin-ui/src/features/data-discovery-and-detection/ClassificationCategoryBadge.tsx

+      <Flex
+        alignItems="center"
+        gap={1.5}
+        cursor={onClick ? "pointer" : "default"}


I know you are just moving this down, but this method is better for a11y and automatically gets the correct cursor so we might as well do that while we're here :)

Suggested change

cursor={onClick ? "pointer" : "default"}

as={onClick ? "button" : undefined}

gilluminate · 2025-01-13T19:40:45Z

clients/admin-ui/src/features/data-discovery-and-detection/tables/cells/TaxonomyAddButton.tsx

+  <Button
+    size="small"
+    icon={<SmallAddIcon mb="1px" />}
+    className=" max-h-[20px] max-w-[20px] rounded-sm border-gray-200 bg-white hover:!bg-gray-100"


We should only be using tailwind for layout and never for colors as we don't plan to maintain it with the palette. Isn't this just a variation of one of our defined button types anyway? That's by far the more preferred method for this so that the theme gets applied. Review the Order of priority for styling if needed.

gilluminate · 2025-01-13T19:44:23Z

clients/admin-ui/src/features/data-discovery-and-detection/tables/cells/TaxonomyAddButton.tsx

+const TaxonomyAddButton = (props: ButtonProps) => (
+  <Button
+    size="small"
+    icon={<SmallAddIcon mb="1px" />}


can we use the Carbon version of the icon in this new component?

gilluminate · 2025-01-13T21:09:19Z

clients/admin-ui/src/features/data-discovery-and-detection/ClassificationCategoryBadge.tsx

+        alignItems="center"
+        gap={1.5}


the alignment of the + button is off during testing, I think this alignItems and gap still belongs in the parent (probably both?). Probably don't need a margin on the button with gap on parent.

gilluminate · 2025-01-13T21:14:51Z

clients/admin-ui/src/pages/data-catalog/[systemId]/projects/[projectId].tsx

+  if (isLoading || isFetching || systemIsLoading) {
+    return <TableSkeletonLoader rowHeight={36} numRows={36} />;
+  }


Since this falls outside of the Layout the skeleton is taking up the entire page (bleeding edge to edge) which I don't think is the desired effect. This should probably be moved in lower.

gilluminate

None of the breadcrumbs seem to be working properly. Clicking down in to a child level and then clicking a breadcrumb to go back just results in an empty table.

jpople added 29 commits October 21, 2024 10:14

first pass at system/project tables

c2a27bc

Merge branch 'main' into jpople/hj-29/dataset-lifecycle-top-tables

fc71e17

fix subfield naming

6489e64

apply const

13f9cca

update changelog

f256f5b

add data use dropdown UI to system table

8d6a25b

refactor taxonomy selects

58df678

extract data use CRUD into hook

2b3e67e

code cleanup on data use form

72a4e87

TEMP: test PrivacyDeclarationResponse on BasicSystemResponse

7eaabcb

add minimal declaration form

dd40ca0

fix deleting opening edit modal

a279c2e

Merge branch 'main' into jpople/hj-29/dataset-lifecycle-top-tables

cfedd93

Merge branch 'main' into jpople/hj-29/dataset-lifecycle-top-tables

b0ca9bb

merge taxonomy select changes

f0d3ca1

refactor TaxonomySelect again

ecafa9d

WIP: new system endpoint hookup

c0900c7

Merge branch 'main' into jpople/hj-29/dataset-lifecycle-top-tables

056b3ab

Merge branch 'jpople/hj-13/data-use-picker-cell' into jpople/hj-29/da…

d068f50

…taset-lifecycle-top-tables

WIP: tables

0297e41

regenerate types

8453d8b

WIP: changes for updated backend

670e206

type update

482d662

tables; rework nav

4e53afa

type update

fcf6b21

result & status cells

81789af

Merge branch 'main' into jpople/hj-29/dataset-lifecycle-top-tables

4162b70

breadcrumb update

da3c337

resource actions & columns

92f1a40

vercel bot had a problem deploying to Preview December 20, 2024 04:35 Failure

Merge branch 'main' into jpople/hj-29/dataset-lifecycle-top-tables

41d69c3

vercel bot had a problem deploying to Preview January 10, 2025 20:20 Failure

cleanup

4b5ed9b

vercel bot deployed to Preview January 13, 2025 08:47 View deployment

cleanup II

f1ad82e

vercel bot deployed to Preview January 13, 2025 16:42 View deployment

jpople marked this pull request as ready for review January 13, 2025 17:04

jpople requested a review from gilluminate January 13, 2025 17:04

gilluminate requested changes Jan 13, 2025

View reviewed changes

gilluminate reviewed Jan 13, 2025

View reviewed changes

jpople added 2 commits January 13, 2025 16:06

Merge branch 'main' into jpople/hj-29/dataset-lifecycle-top-tables

a155e57

PR feedback

eb9f3be

vercel bot had a problem deploying to Preview January 13, 2025 23:18 Failure

jpople added 2 commits January 13, 2025 17:18

update changelog

cbe1901

add new feature flag

934de8b

vercel bot had a problem deploying to Preview January 13, 2025 23:21 Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Data catalog UI: MVP #5628

Data catalog UI: MVP #5628

jpople commented Dec 20, 2024 •

edited

Loading

vercel bot commented Dec 20, 2024 •

edited

Loading

cypress bot commented Jan 13, 2025

gilluminate left a comment

gilluminate Jan 13, 2025

jpople Jan 13, 2025

gilluminate Jan 13, 2025

gilluminate Jan 13, 2025

gilluminate Jan 13, 2025 •

edited

Loading

gilluminate Jan 13, 2025

gilluminate Jan 13, 2025

gilluminate Jan 13, 2025

gilluminate Jan 13, 2025 •

edited

Loading

gilluminate Jan 13, 2025

gilluminate left a comment

	cursor={onClick ? "pointer" : "default"}
	as={onClick ? "button" : undefined}

Data catalog UI: MVP #5628

Are you sure you want to change the base?

Data catalog UI: MVP #5628

Conversation

jpople commented Dec 20, 2024 • edited Loading

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Dec 20, 2024 • edited Loading

cypress bot commented Jan 13, 2025

Tests for review

gilluminate left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gilluminate Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gilluminate Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gilluminate left a comment

Choose a reason for hiding this comment

jpople commented Dec 20, 2024 •

edited

Loading

vercel bot commented Dec 20, 2024 •

edited

Loading

gilluminate Jan 13, 2025 •

edited

Loading

gilluminate Jan 13, 2025 •

edited

Loading