-
Notifications
You must be signed in to change notification settings - Fork 85
Display saas integrations in the integrations page #6480
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
Display saas integrations in the integrations page #6480
Conversation
…aas-integrations-in-the-integrations-page
…aas-integrations-in-the-integrations-page
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment @cursor review
or bugbot run
to trigger another review on this PR
…aas-integrations-in-the-integrations-page
…aas-integrations-in-the-integrations-page
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6480 +/- ##
==========================================
+ Coverage 87.64% 87.66% +0.01%
==========================================
Files 477 480 +3
Lines 30609 30657 +48
Branches 3445 3446 +1
==========================================
+ Hits 26828 26876 +48
Misses 3040 3040
Partials 741 741 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me!
The only comment I had would be to make sure there were clear enum tests to verify how things would pipe through to the front end. It looks like they are in a lot of the integration tests though and - since you have been using them though - my guess is they are ok!
…aas-integrations-in-the-integrations-page
fides
|
Project |
fides
|
Branch Review |
main
|
Run status |
|
Run duration | 00m 59s |
Commit |
|
Committer | Lucano Vera |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
1
|
|
0
|
|
0
|
|
0
|
|
4
|
View all changes introduced in this branch ↗︎ |
Tests for review
cypress/e2e/smoke_test.cy.ts • 1 failed test
Test | Artifacts | |
---|---|---|
Smoke test > can submit an access request from the Privacy Center |
Screenshots
Video
|
Description Of Changes
Implements a new beta flag called "New integration management". When the flag is on, all existing SaaS integrations will show in the Settings > Integrations screen and you'll be able to add new ones from that screen as well. This is the first step towards unifying all integration management things in this page.
The Integrations page UI requires some extra details about integrations, like tags, category, enabled features, etc. To keep a single source of truth, these extra information was added as an optional display_info field on the saas config files.
Code Changes
Steps to Confirm
Prerequisite: Having a Saas integration setup locally. Skip if you have one already
Checking saas integrations don't show up without the flag on:
Checking saas integration do show up with the flag on:
Checking you can add a new saas integration:
Checking you can still upload a custom integration without display_info and it works:
Pre-Merge Checklist
nox -s demo -- dev
CHANGELOG.md