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

[1076] ui/plus: Approve dataset classification button #1129

Merged

Conversation

ssangervasi
Copy link
Contributor

Closes #1076

Code Changes

  • "Approve classification" button for datasets that need review
    • This makes two API calls: the normal dataset update call and then a new endpoint for tracking this status.
    • There is a unit-tested function for creating the dataset update body. This uses immer as an explicit dependency,
      but it was already pulled in by RTK.
  • Move classify status field to be per-dataset

Steps to Confirm

  • The classify logic still must be run within cypress.
  • The dataset viewing and editing should still work when Plus/Cls is unavailable.

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

Before click:
1-approval-button

While loading:
2-loading

Success:
3-success

@ssangervasi ssangervasi requested a review from a team September 28, 2022 01:50
@ssangervasi ssangervasi force-pushed the ssangervasi/fides-1076/approve-dataset-classification-button branch from ffbbe93 to ee00d0e Compare September 28, 2022 01:56
Copy link
Contributor

@allisonking allisonking left a comment

Choose a reason for hiding this comment

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

looking good! just some nits. awesome tests!

@ssangervasi ssangervasi force-pushed the ssangervasi/fides-1076/approve-dataset-classification-button branch from ee00d0e to 9d2c863 Compare September 29, 2022 17:35
@ssangervasi ssangervasi force-pushed the ssangervasi/fides-1076/approve-dataset-classification-button branch from bd09e8e to 2a25093 Compare September 29, 2022 18:48
@ssangervasi ssangervasi merged commit 3a929cf into main Sep 29, 2022
@ssangervasi ssangervasi deleted the ssangervasi/fides-1076/approve-dataset-classification-button branch September 29, 2022 20:20
ssangervasi added a commit that referenced this pull request Oct 1, 2022
* main:
  Restrict startsWith comparisons in CheckboxTree more (#1126)
  [1076] ui/plus: Approve dataset classification button (#1129)
  Do not rely on order for checking intercept results (#1131)
  Prepare 1.9.1 release (#1137)
  Bump fideslang to 1.3.1 (#1136)
  Prepare changelog for 1.9.0 release (#1134)
  Update CHANGELOG.md (#1132)
  Fix bug causing missing datamap rows (#1124)
  cls migration (#1060)
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 "Approve dataset classification" button
2 participants