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

Edit taxonomy form #977

Merged
merged 15 commits into from
Aug 17, 2022
Merged

Edit taxonomy form #977

merged 15 commits into from
Aug 17, 2022

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Aug 10, 2022

Closes #857, closes #855

Code Changes

  • Rename DataCategoryTag component to TaxonomyEntityTag and refactor it a little to share more code
  • Hook up the AccordionTree edit button to trigger stuff
    • Also subtle design thing I didn't notice the first time around: when a field is being edited, it turns purple in the accordion tree
  • A few tweaks to the inputs in inputs.tsx, one to include a disabled state, and one to add a grid option to the CustomTextArea component
  • Add PUT call to each taxonomy slice
  • Add EditTaxonomyForm component for the edit form, reusable between each taxonomy type
  • Remove all the individual taxonomy tab components in favor of a more generalized approach
    • I'm not sure if this pattern is the best, as it involves passing custom hooks as props, but it works well and keeps thing pretty clean. Open to other suggestions if it's weird though!
    • hooks.ts file for getting info about each type of taxonomy, including labels, queries, mutations, and in the future tooltips can go here too.
  • Cypress tests for editing a taxonomy form (for each type of taxonomy)

Steps to Confirm

  • nox -s dev
  • Navigate to localhost:3000/taxonomy
  • Hover over one of the items and click "Edit"
  • Fill out the form and click "Update"

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

These taxonomy pages are all very similar but operate on slightly different data. However, they all have a few fields in common, which are the ones that are editable. This means we can mostly use the same component and pull out the parts that are unique:

  • Copy (i.e. "Modify data categories" vs. "Modify data subjects")
  • Tooltips (TODO, as there's no tooltip copy yet)
  • Endpoint calls (i.e. /api/v1/data_category vs /api/v1/data_subject)

The pattern I went for was that EditTaxonomyForm.tsx and TaxonomyTabContent.tsx stay agnostic of which taxonomy field they are operating on. Instead, custom hooks pull in all of the unique parts into an object with properties that are the same across the taxonomies. When the tabs are being created, we pass in each custom hook as a prop, and then everything renders nicely.

An alternative which seems more natural would be to call each hook and then pass its data into each TaxonomyTabContent.tsx, but this would remove the lazy loading the tabs provide us (only loading the data when a tab is clicked on).

Screen.Recording.2022-08-15.at.2.13.47.PM.mov

@allisonking
Copy link
Contributor Author

Not sure what's going on with the pytest external test 😕

@allisonking allisonking marked this pull request as ready for review August 11, 2022 23:05
@allisonking allisonking requested a review from a team August 11, 2022 23:05
@ThomasLaPiana
Copy link
Contributor

@allisonking good news, the test is passing again! bad news....the merge conflicts 🙃

@ThomasLaPiana
Copy link
Contributor

@allisonking do we need a follow-up issue for the tooltips TODO?

Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, LGTM!

@allisonking allisonking merged commit c9fdca0 into main Aug 17, 2022
@allisonking allisonking deleted the aking-857-edit-taxonomy branch August 17, 2022 13:54
@allisonking allisonking mentioned this pull request Aug 19, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add edit button for taxonomy fields Do not allow modifying fides keys from the UI
2 participants