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

PROD-1899 Move-admin-UI-away-from-using-unpaginated-system-and-dataset-APIs-where-possible #5135

Conversation

lucanovera
Copy link
Contributor

@lucanovera lucanovera commented Jul 29, 2024

Description Of Changes

Remove uses of unpaginated systems and dataset APIs where possible.

Code Changes

  • Remove fetch of all systems into redux store, replace it with individual calls to api whenever the system info is needed
  • Keep system count in redux store, but now obtain that information from the paginated endpoint by fetching just 1 system and getting the total count from the response
  • Removed currently unused /system-classify page, it's tests and it's redux states
  • Changed FE validation for "already taken" system names to use the paginated systems endpoint with search param

Steps to Confirm

  • Open browser dev tools & go to network tab

  • Log in to admin-ui

    • Confirm in the network tab that we no longer have a call to /systems that fetches all of the systems. Instead we should have a call to systems that just retrieves 1 item.
    • Confirm that the homepage still displays the correct number of systems
  • Go do Data Lineage and Click on a system, the sidepanel should display as usual

  • Go to Add System > Manual. Type a name for an existing system and check that the validation tells you the system already exists.

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Copy link

vercel bot commented Jul 29, 2024

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 Jul 31, 2024 4:21pm

Copy link

cypress bot commented Jul 30, 2024



Test summary

4 0 0 0


Run details

Project fides
Status Passed
Commit 18bd39865b ℹ️
Started Jul 31, 2024 4:27 PM
Ended Jul 31, 2024 4:28 PM
Duration 00:36 💡
OS Linux Ubuntu -
Browser Electron 106

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

Lucano Vera added 2 commits July 30, 2024 15:05
…min-UI-away-from-using-unpaginated-system-and-dataset-APIs-where-possible
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.

Code looks good!

✅ Doesn't query all systems on loading homepage
✅ Shows side panel in data lineage view
✅ Correctly checks duplicate system names

Aside from one text fix, looks all good to me.

lucanovera and others added 3 commits July 31, 2024 13:14
…min-UI-away-from-using-unpaginated-system-and-dataset-APIs-where-possible
…tem-and-dataset-APIs-where-possible' of github.com:ethyca/fides into PROD-1899-Move-admin-UI-away-from-using-unpaginated-system-and-dataset-APIs-where-possible
@lucanovera lucanovera merged commit 2de7500 into main Jul 31, 2024
10 of 11 checks passed
@lucanovera lucanovera deleted the PROD-1899-Move-admin-UI-away-from-using-unpaginated-system-and-dataset-APIs-where-possible branch July 31, 2024 16:16
Copy link

cypress bot commented Jul 31, 2024



Test summary

4 0 0 0


Run details

Project fides
Status Passed
Commit 2de7500
Started Jul 31, 2024 4:30 PM
Ended Jul 31, 2024 4:31 PM
Duration 00:36 💡
OS Linux Ubuntu -
Browser Electron 106

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

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.

2 participants