[fidesctl-plus #78] cross app navigation #1037

ssangervasi · 2022-08-31T23:48:10Z

Closes https://github.com/ethyca/fidesctl-plus/issues/78

Code Changes

The final commit is the crux of what we wanted:
- Query /plus/health API to determine if Plus features should be shown
- I put this in a "features" slice+hook instead of a "plus" slice because it's more of a feature flagging mechanism than an actual use of the Plus API. The Plus API slice itself can be extracted when we start using it, while the feature toggling could grow to have other kinds of checks.
To get that point, I had to solve Next Zone linking in a generic way. Now we can configure those links in one place.

Steps to Confirm

list any manual steps taken to confirm the changes

Pre-Merge Checklist

All CI Pipelines Succeeded
Documentation Updated:
- documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
- documentation issue created (tag docs-team to complete issue separately)
Issue Requirements are Met
Relevant Follow-Up Issues Created
Update CHANGELOG.md

Description Of Changes

Navigating between the two zones in dev:
https://user-images.githubusercontent.com/2236777/187803440-1603bee3-4562-4712-b072-bc390ec5a144.mp4

When the regular server is running the 404 indicates Plus is unavailable.

allisonking

code looks great, thanks for doing all that refactoring! going to try making a bundled fidesctl+ to make sure it all works as a static site too, brb

clients/ctl/admin-ui/src/features/common/nav/zone-config.ts

allisonking

built fidesctl as a python package and made fidesctl+ use it, seems to work great! here's what I did, for reference:

In fides:

$ cd clients/ctl/admin-ui
$ npm run prod-export
$ cd -
$ python setup.py bdist_wheel
$ cp dist/fidesctl-1.8.2.post0.dev20-py2.py3-none-any.whl ../fidesctl-plus/.
$ cd ../fidesctl-plus

And then in the fidesctl-plus repo, edit the Dockerfile to add these two lines:

COPY fidesctl-1.8.2*.whl .
RUN pip install fidesctl-1.8.2.*.whl

Then

make build

and copy the built docker image name into the docker-compose and comment out the volume mount, then docker-compose up fidesctl-plus. navigate to localhost:8080 and enjoy! is there an easier way? ¯\_(ツ)_/¯

ssangervasi · 2022-09-02T19:14:07Z

@allisonking

copy the built docker image name into the docker-compose and comment out the volume mount, then docker-compose up fidesctl-plus. navigate to localhost:8080 and enjoy! is there an easier way

Yeah I did a similar hack when experimenting with building env-vars, except I only copied the the UI files. I bet we could conjure a Dockerfile/docker-compose.yml that would automate your steps to build fctl+ with a locally-built fctl python wheel. Would be handy for testing.

This change brings in some zone handling code from fidesctl-plus and makes it more generic so that we can use the same pattern in both apps.

This change switches moves our nav routing tests into Cypress instead of Jest. The React test renderer was having trouble integrating with Next's dynamic import, which was going to require a complex solution. Instead, by moving the nav tests into Cypress we can test the real router state without relying on mocks. This will prove even more useful in a later commit when we update what links are available based on whether the API says we are in fidesctl-plus.

…hould be shown

* fctl/nav: Move NavBar into nav directory * fctl/nav: Decouple Header and NavBar * fctl/nav: Extract NavButton presentational component * fctl/nav: Extract NavLink component * fctl/zones: Generic utility for configuring app zones This change brings in some zone handling code from fidesctl-plus and makes it more generic so that we can use the same pattern in both apps. * fctl/nav: Use zone-aware nav links * fctl/nav: Replace Jest nav tests with Cypress tests This change switches moves our nav routing tests into Cypress instead of Jest. The React test renderer was having trouble integrating with Next's dynamic import, which was going to require a complex solution. Instead, by moving the nav tests into Cypress we can test the real router state without relying on mocks. This will prove even more useful in a later commit when we update what links are available based on whether the API says we are in fidesctl-plus. * fctl/features: Query /plus/health API to determine if Plus features should be shown * Update changelog

* Prevent modifying default taxonomy fields (#990) * Add is_default field to taxonomy sql models (#976) * Add is_default field to taxonomy sql models * Update changelog * Autoformat alembic migration file * Update dataset.yml with is_default * Bump fideslang version Update changelog Autoformat alembic migration file Update dataset.yml with is_default Bump fideslang version * Prevent modifying default taxonomy on update Add docstring * Move is_default check to routes/crud.py Revert database/crud.py changes * Handle forbidding when given a list of fides keys * Add test * Handle attempting to modify is_default field * Update changelog * Debug failing test * Add another print debugging stmt * Revert "Add another print debugging stmt" This reverts commit 527bd03. * Revert "Debug failing test" This reverts commit daf3ca8. * Scope resources_dict to function * Clean up changelog * Allow modifying defaults but not is_default * Handle case where checking for new upsert * Refactor to put tests in a class * Handle upserting is_default * Delete taxonomy UI (#1006) * Add delete call to slices * Hook up delete button * Add result handling * Only show delete on nodes without children * Add tests for delete * Update changelog * Render action buttons as a prop to AccordionTree * Fix import consistency * Rename onEdit and onDelete * Clear edit entity on delete * Use TreeNode type * Update cypress fixtures (#1022) * Update fixtures * Update tests based on updated fixtures * Add taxonomy entity form (#1019) * Rename data-categories.slice --> taxonomy.slice * Add active taxonomy type to store * Add create mutation to slices * Hook up create to form * Fixup form UX * Derive parent key from fides key * Conditionally render parent key field * Add tests for adding taxonomy entities * Clean up * Update changelog * Simplify setting add state * Add test for showing either add or create form * Derive isCreate from status of fides key * Add check for is_default before rendering delete button (#1023) * Add check for is_default before rendering delete button * Update changelog * Update test data to include is_default * Add cypress tests * Fix mypy error (#1030) * Fix mypy error * Update CHANGELOG * Remove unused import Co-authored-by: Paul Sanders <[email protected]> * Custom label for user defined taxonomy fields (#1027) * Add renderTag prop * Add cypress tests * Update changelog * Add boolean fields and use them in taxonomy forms (#1028) * Add CustomRadioGroup as a form input * Use new radio group input for taxonomy forms * Update extra form fields prop to be a function * Cast string boolean back to real boolean * Tests * Update changelog * Clean up some type comparisons * Revert === undefined since the fields can actually be null * remove exclude_unset=True to return clean diff (#1026) * remove exclude_unset=True to return clean diff * changelog * don't stop the test suite when there is a failure Co-authored-by: Thomas <[email protected]> * Set pydantic < 1.10.0 to fix CI issues with fideslang functions (#1045) Should be able to revert after fideslang figures out why the latest pydantic causes failures * [fidesctl-plus #78] cross app navigation (#1037) * fctl/nav: Move NavBar into nav directory * fctl/nav: Decouple Header and NavBar * fctl/nav: Extract NavButton presentational component * fctl/nav: Extract NavLink component * fctl/zones: Generic utility for configuring app zones This change brings in some zone handling code from fidesctl-plus and makes it more generic so that we can use the same pattern in both apps. * fctl/nav: Use zone-aware nav links * fctl/nav: Replace Jest nav tests with Cypress tests This change switches moves our nav routing tests into Cypress instead of Jest. The React test renderer was having trouble integrating with Next's dynamic import, which was going to require a complex solution. Instead, by moving the nav tests into Cypress we can test the real router state without relying on mocks. This will prove even more useful in a later commit when we update what links are available based on whether the API says we are in fidesctl-plus. * fctl/features: Query /plus/health API to determine if Plus features should be shown * Update changelog * 1.8.3 (#1050) * Prepare changelog for 1.8.3 release * Fixup misattributed changelog items * Add fix for pydantic version * noxfiles: Session for building fidesctl python package (#1047) * noxfiles: Session for building fctl python package * fix a pylint error Co-authored-by: Thomas <[email protected]> * [942] fctl/api: Serve static files using route maps (#1046) * fctl/api: Adapt generate_route_file_map from Ops and unit test it * fctl/api: Serve static files using route maps This includes both packaged (pip installed) and local build files, depending on what is available. * Update changelog * Fix truncated evaluation error messages (#1053) * Fix truncated evaluation error messages * add a test for the evaluations output * fix mypy error * update changelog * Bump next-auth from 4.9.0 to 4.10.3 in /clients/ctl/admin-ui (#1025) Bumps [next-auth](https://github.com/nextauthjs/next-auth) from 4.9.0 to 4.10.3. - [Release notes](https://github.com/nextauthjs/next-auth/releases) - [Changelog](https://github.com/nextauthjs/next-auth/blob/main/CHANGELOG.md) - [Commits](https://github.com/nextauthjs/next-auth/compare/[email protected]@v4.10.3) --- updated-dependencies: - dependency-name: next-auth dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Allison King <[email protected]> * Cascade delete taxonomy children (#1042) * Configure pytest for async functions * Refactor FixtureRequest so it can be shared across tests * Add test for deleting children taxonomy * Add logic to cascade delete * Update changelog * Update UI to allow deleting parent taxonomy fields with a warning * Clean up * Update cypress tests * Initial systems management page (#1054) * Rename SystemsTable to SystemsGrid * Add BorderGrid component to abstract out grid * Build out SystemCard * Add search feature * Pull SystemCard out into its own component * Add cypress tests for system management page * Update changelog * Fix nav bar test * 1.8.4 (#1061) * update changelog * remove phantom bullet point * Alter taxonomy upsert behavior (#1040) * Configure pytest for async functions * Add tests for changing updating default taxonomy * Change upsert behavior to append * Update changelog * Address PR comments * Replace upsert with create * Remove unused import * Remove disabled Nav Buttons (#1067) * Update NavBar.tsx * remove "more" from the navbar * fix a linting error * remove cypress tests for disabled links * Form to add a system via yaml (#1062) * Add new system page * Refactor YamlForm so it can be reused * Fixup some types in existing system slice * Add SystemYamlForm * Add cypress test for system * Update changelog * Fix empty state and remove ellipsis for now * Remove tests on more actions button * Move changelog items to Unreleased * ui/dataset: "Classify" toggle for fidesctl-plus (#1057) * ui/dataset: Spacing improvements for generate form * ui/inputs: CustomSwitch component for switches * ui/dataset: "Classify" toggle for fidesctl-plus * ui/dataset: Cypress test when classify is available * Update changelog * [1058] ui/dataset: Confirmation modal to kick off classify (#1069) * ui/featuers: Extract Plus API into its own slice * ui/dataset: Refactor dataset creation chain of mutations This should make it easier to extend the sequence of API calls to add Classify. Before, the structure made it hard to identify the orders in which mutations were called, and which object was the generated (temporary) vs persisted dataset. Now the generate and create functions return their results or an error string. I also made a change to how the error message is shown: instead of always assigning the message to the form's URL field, it's shown in a error toast. Now that we have multiple fields on this form, it wasn't clear the URL is going to be responsible for any errors. * ui/plus: Mock implementation of the classify API * ui/dataset: Request classify if toggled * ui/dataset: Confirmation modal to kick off classify * ui/dataset: Cypress test for starting classify * ui/dataset: Clear active dataset on un-mount instead of mount This change makes it possible for the classify flow to highlight the newly-created dataset when we navigate back to the table. Previously, the active dataset was cleared by index page, which made preserving the active set between routes impossible. Now we only clear when we leave a datset's view, which gives us the same experience and the new feature. * Update changelog * Scaffold manual system flow (#1068) * Refactor generate type to boolean * Add ManualSystemFlow * Add ConfigureSteps * Make button bigger * Update changelog * Remove UI features for WIP elements * [1073] ui/dataset: Show status badge for datasets using classification results (#1074) * ui/plus: Classifications grouped by dataset fides key * ui/dataset: Show status badge for datasets using classification results * ui/dataset: Cypress tests for classified table * Update changelog * ui/state: Deduplicate query results that were being stored in state slices (#1083) * ui/dedup-state: Systems unused * ui/dedup-state: Datasets unused * ui/dedup-state: Organization hydrate unused * ui/dedup-state: Taxonomy DataCategories query * ui/dedup-state: DataSubjects query * ui/dedup-state: DataQualifiers query * ui/dedup-state: DataUses query * Fix header help link (#1078) * Fix header help link * update changelog * Reuse config wizard forms for adding a system (#1072) * Consolidate tooltips in DescribeSystemsForm * Add form to manual system flow * Add validation schema for DescribeSystemsForm * Fix onBlur handlers of select fields * Refactor DescribeSystemsForm * Refactor PrivacyDeclarationForm * Continue refactoring PrivacyDeclarationForm * Refactor ReviewSystemForm * Refactor SuccessPage * Update changelog * Refactor ReviewSystemForm to use a grid ReviewItem * Add cypress test for flow * Try to fix flaky test * Ensure we stub taxonomy items * Refactor PrivacyDeclarationAccordion to match designs more * Fix adding another declaration when name field is blank * Rename SuccessPage --> SystemRegisterSuccess * Pass system object through props * Update tests * UI to delete a system (#1085) * Implement delete system feature * Add NotFoundError * Add cypress tests * Update changelog * Refactor errors to ErrorDetails * Separate 'next' and 'add' logic in PrivacyDeclarationForm (#1086) * Separate continue and add logic * Update tests * Update changelog * New fields on system forms (#1082) * Extend DescribeSystemsForm * Extend PrivacyDeclarationForm * Add joint controller and data protection impact assessment * Move system dependency field to abridged form * Prepare data protection impact assessment for payload * Add cypress tests * Extend ReviewSystemForm * Add tests for extended review form * Fix adding another declaration when name field is blank * Rename SuccessPage --> SystemRegisterSuccess * Pass system object through props * Update tests * Update changelog * Refactor ReviewSystemForm * Move config wizard system forms to system directory (#1097) * Move system forms to system directory * Refactor form layout components into its own file * Rename files from form --> step * Update changelog * ui/datasets: Refactor selectors (#1087) * ui/dataset: Track dataset by fides key instead of object copy and nulls Previously we've stored a copy of the dataset returned by the get-by-key query, but only every use its key property. With this change we only store the key and use the object from the query directly. As part of this, I also converted a lot of `null` to `undefined`. There different opinions on this, but undefined is generally more useful in a TS codebase because: 1. It plays nicely with optional function arguments. 2. It can be represented by a single question mark in interfaces. (See the State changes in this commit.) 3. It doesn't have null's `typeof null === "object"` confusion. * ui/dataset: Single state for edit drawer * ui/dataset: Extract collection lookups to selector * ui/dataset: Extract field lookups to selector * ui/dataset: Extract field types into Cell component * ui/cypress: Consistent test fides key * Bump fideslang to 1.3.0 (#1103) * Bump fideslang to 1.3.0 * Add `egress` and `ingress` to `ctl_systems` * Update `CHANGELOG.md` * Prepare 1.8.5 release (#1107) * Bump fideslang to 1.3.0 * Add `egress` and `ingress` to `ctl_systems` * Update `CHANGELOG.md` * Prepare `CHANGELOG.md` for v1.8.5 * upgrade pymysql to version 1.0.2 (#1094) * Edit system UI (#1096) * Set activeSystem in system.slice * Allow form fields to take undefined * Extend DescribeSystemForm to be able to edit * Allow editing system from its card * Update changelog * Pull the form part of privacy declaration into its own component * Further refactor PrivacyDeclarationForm to support editing * Pull common intercepts out into a stubs file * Add cypress test for editing * Reserve dataset_references for unabridged forms * Make sure not to override privacy declaration * Explain allowing undefined for form inputs * Change routing behavior to go back on cancel * Fix error message * Update CHANGELOG.md * included more changes from fidesctl * fix static checks * manual updates from ctl UI * formatting * change line endings to fix prettier error * fix docker build issue * merge alembic heads for fidesctl merge and get tests parsing * don't include the worker as a dependency for docker-compose fides * remove worker flag from default fides service * fix static checks * get webserver running again * reenable the worker for the docker-compose service * enable redis for the compose service * clean command is absolute in its destruction Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Allison King <[email protected]> Co-authored-by: Paul Sanders <[email protected]> Co-authored-by: Paul Sanders <[email protected]> Co-authored-by: Steve Murphy <[email protected]> Co-authored-by: Sebastian Sangervasi <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Phil Salant <[email protected]>

ssangervasi requested a review from a team August 31, 2022 23:48

ssangervasi force-pushed the ssangervasi/fidesctl-plus-78/cross-app-navigation branch from 3da1502 to 4adc629 Compare September 1, 2022 18:39

ssangervasi requested a review from allisonking September 1, 2022 18:51

allisonking reviewed Sep 2, 2022

View reviewed changes

clients/ctl/admin-ui/src/features/common/nav/zone-config.ts Outdated Show resolved Hide resolved

allisonking approved these changes Sep 2, 2022

View reviewed changes

ssangervasi added 5 commits September 2, 2022 15:20

fctl/nav: Move NavBar into nav directory

d458e6e

fctl/nav: Decouple Header and NavBar

7146c84

fctl/nav: Extract NavButton presentational component

7139ca1

fctl/nav: Extract NavLink component

d7b8508

fctl/zones: Generic utility for configuring app zones

2ac6128

This change brings in some zone handling code from fidesctl-plus and makes it more generic so that we can use the same pattern in both apps.

ssangervasi force-pushed the ssangervasi/fidesctl-plus-78/cross-app-navigation branch from f24b710 to 6a262de Compare September 2, 2022 22:20

ssangervasi added 4 commits September 2, 2022 15:24

fctl/nav: Use zone-aware nav links

0fdf892

fctl/features: Query /plus/health API to determine if Plus features s…

5c257e9

…hould be shown

Update changelog

c3b52f0

ssangervasi force-pushed the ssangervasi/fidesctl-plus-78/cross-app-navigation branch from 6a262de to c3b52f0 Compare September 2, 2022 22:24

ssangervasi merged commit 1eca516 into main Sep 2, 2022

ssangervasi deleted the ssangervasi/fidesctl-plus-78/cross-app-navigation branch September 2, 2022 22:38

ssangervasi mentioned this pull request Sep 6, 2022

Merge ops/ctl admin UIs #1009

Closed

ssangervasi mentioned this pull request Sep 12, 2022

Unified admin UI #1063

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fidesctl-plus #78] cross app navigation #1037

[fidesctl-plus #78] cross app navigation #1037

ssangervasi commented Aug 31, 2022 •

edited

Loading

allisonking left a comment

allisonking left a comment

ssangervasi commented Sep 2, 2022

[fidesctl-plus #78] cross app navigation #1037

[fidesctl-plus #78] cross app navigation #1037

Conversation

ssangervasi commented Aug 31, 2022 • edited Loading

Code Changes

Steps to Confirm

Pre-Merge Checklist

Description Of Changes

allisonking left a comment

Choose a reason for hiding this comment

allisonking left a comment

Choose a reason for hiding this comment

ssangervasi commented Sep 2, 2022

ssangervasi commented Aug 31, 2022 •

edited

Loading