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

Read only consent management table and filters #4456

Merged
merged 30 commits into from
Dec 1, 2023

Conversation

TheAndrewJackson
Copy link
Contributor

@TheAndrewJackson TheAndrewJackson commented Nov 22, 2023

Closes PROD-1341

Description Of Changes

This adds a new consent management table view when TCF is enabled.

Code Changes

  • Add purposes API slice
  • Add new system report API function
  • Add filter modal component
  • Add client side and server side pagination hooks for the pagination bar
  • Update AddVendor modal so the fields can be disabled
  • Add new Filter modal that dynamically shows different options based on whether TCF is enabled
  • Add new consent management table that dynamically displays different columns based on whether TCF is enabled

Steps to Confirm

TCF Enabled

TCF-enabled.mov
  • Visit consent/configure
  • Make sure pagination works
  • open the filter modal and make sure filter works across all of the sections
  • check that the search bar works
  • check that the reset filter button works

TCF Disabled

Screenshot from 2023-11-29 19-48-45

  • Visit consent/configure
  • It should show the current configure consent behavior

Pre-Merge Checklist

@TheAndrewJackson TheAndrewJackson self-assigned this Nov 22, 2023
Copy link

vercel bot commented Nov 22, 2023

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

Name Status Preview Comments Updated (UTC)
fides-plus-nightly ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 1, 2023 4:37pm

Copy link

cypress bot commented Nov 27, 2023

Passing run #5429 ↗︎

0 4 0 0 Flakiness 0

Details:

Merge 981d86a into f008dcf...
Project: fides Commit: 45416ef1e4 ℹ️
Status: Passed Duration: 00:35 💡
Started: Dec 1, 2023 4:46 PM Ended: Dec 1, 2023 4:47 PM

Review all test suite changes for PR #4456 ↗︎

@TheAndrewJackson TheAndrewJackson marked this pull request as ready for review November 27, 2023 21:39
@galvana
Copy link
Contributor

galvana commented Dec 1, 2023

Some of the text needs to be changed to sentence-case (Data uses, Legal bases, 16 purposes, 16 data uses etc.)
image

Same for the text here (Reset filters)
image

@galvana
Copy link
Contributor

galvana commented Dec 1, 2023

I found a bug with the pagination, it seems like we might just need to reset the current page whenever the filter updates

Steps to reproduce:

  1. Add enough systems to be able to paginate (like adding all the vendors)
  2. Paginate in a few pages
  3. Type into the search box, the pagination details will be incorrect
  4. Paginate back and the results will be available in the earlier pages
    image

galvana
galvana previously requested changes Dec 1, 2023
Copy link
Contributor

@galvana galvana left a comment

Choose a reason for hiding this comment

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

I called out a few places that need sentence-casing but the biggest issue is the pagination bug 🐞. I also started commenting with some style suggestions but I decided to just create another PR so you can review that and pull in whatever you agree with (borrowed the approach from @adamsachs, thanks Adam!)

@galvana galvana mentioned this pull request Dec 1, 2023
Copy link
Contributor

@jpople jpople left a comment

Choose a reason for hiding this comment

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

Pagination bug is fixed, looks good to me!

@TheAndrewJackson TheAndrewJackson dismissed galvana’s stale review December 1, 2023 18:50

I fixed what was addressed and the PR is saying a didn't

@TheAndrewJackson TheAndrewJackson merged commit 19ff5fc into main Dec 1, 2023
13 checks passed
@TheAndrewJackson TheAndrewJackson deleted the ajackson/PROD-1341/readonly_table branch December 1, 2023 18:50
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.

3 participants