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

Removes Hiding By Scope for Scopes not Issuable by the UI #2771

Merged
merged 4 commits into from
Mar 8, 2023

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Mar 8, 2023

Closes #2769
Reverts some changes introduced in #2682

Code Changes

  • Removes hiding by scope for navbar and tiles for scopes not issuable via the UI:
PRIVACY_REQUEST_READ,
PRIVACY_REQUEST_REVIEW,
PRIVACY_REQUEST_RESUME,
CONNECTION_READ,
CONNECTION_CREATE_OR_UPDATE,
CONNECTION_INSTANTIATE,
CONNECTION_TYPE_READ,
CONNECTION_DELETE,
CONSENT_READ,
DATASET_CREATE_OR_UPDATE,
DATASET_DELETE,
POLICY_READ,
POLICY_CREATE_OR_UPDATE,
USER_READ,
USER_CREATE,
USER_UPDATE,
USER_DELETE,
USER_PASSWORD_RESET,
USER_PERMISSION_CREATE,
USER_PERMISSION_UPDATE,
USER_PERMISSION_READ,
PRIVACY_REQUEST_UPLOAD_DATA,
PRIVACY_REQUEST_VIEW_DATA,
WEBHOOK_CREATE_OR_UPDATE,
WEBHOOK_READ,
WEBHOOK_DELETE,
  • Fixes corresponding unit tests

Steps to Confirm

  • list any manual steps for reviewers to confirm the changes

Pre-Merge Checklist

Description Of Changes

Write some things here about the changes and any potential caveats

@pattisdr pattisdr changed the title Removes hiding by scope in navbar and tiles for scopes not issuable v… Removes Hiding By Scope for Scopes not Issuable by the UI Mar 8, 2023
@pattisdr
Copy link
Contributor Author

pattisdr commented Mar 8, 2023

tile-config.test.ts and nav-config.test.ts unaddressed

Copy link
Contributor

@NevilleS NevilleS left a comment

Choose a reason for hiding this comment

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

LGTM so far. Let's get the tests disabled/skipped/passing enough for this to work and we can take our time to migrate to this system in 2.9.0

clients/admin-ui/src/features/common/nav/v2/nav-config.ts Outdated Show resolved Hide resolved
@pattisdr
Copy link
Contributor Author

pattisdr commented Mar 8, 2023

@NevilleS do you want me to do the tests as well?

@NevilleS
Copy link
Contributor

NevilleS commented Mar 8, 2023

I'm on it!

@cypress
Copy link

cypress bot commented Mar 8, 2023

Passing run #697 ↗︎

0 3 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge f43ce6f into 05e418d...
Project: fides Commit: 69039597b5 ℹ️
Status: Passed Duration: 00:38 💡
Started: Mar 8, 2023 2:48 PM Ended: Mar 8, 2023 2:49 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@NevilleS NevilleS marked this pull request as ready for review March 8, 2023 14:28
@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (05e418d) 86.55% compared to head (f43ce6f) 86.55%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2771   +/-   ##
=======================================
  Coverage   86.55%   86.55%           
=======================================
  Files         291      291           
  Lines       16488    16488           
  Branches     2117     2117           
=======================================
  Hits        14272    14272           
  Misses       1817     1817           
  Partials      399      399           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pattisdr
Copy link
Contributor Author

pattisdr commented Mar 8, 2023

Testing changes locally...

@NevilleS
Copy link
Contributor

NevilleS commented Mar 8, 2023

There was one failing Cypress test on that past run:

1) Taxonomy management page
       Can create data
         Can open a create form for each taxonomy entity:

I've re-run that one or twice locally and I think it's a flaky test - looks like a potential timing issue with how the create form opens/closes. I'm quite sure it's unrelated so don't want to hold this up further

@NevilleS NevilleS merged commit f5ae75a into main Mar 8, 2023
@NevilleS NevilleS deleted the fides_2769_revert_ui_disabling_by_scope branch March 8, 2023 15:00
@allisonking allisonking mentioned this pull request Mar 16, 2023
16 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.

New user with full permissions can not add new systems
2 participants