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

Deprecate connection config fields in UI #3684

Merged
merged 14 commits into from
Jun 29, 2023

Conversation

TheAndrewJackson
Copy link
Contributor

@TheAndrewJackson TheAndrewJackson commented Jun 26, 2023

Closes fidesplus#947

Description Of Changes

This PR makes the ConnectionConfig.name field optional and removes both the ConnectionConfig.name and ConnectionConfig.description fields from the integration tab form.

Code Changes

  • Add migration that makes ConnectionConfig.name optional
  • Remove name and description fields from the integration form
  • Stop defaulting ConnectionConfig.key to ConnectionConfig.name for saas connectors if it's left empty. This is because the name isn't guaranteed to be there anymore. Now it defaults to the instance key.

Steps to Confirm

  • Run fides and the admin ui
  • Create and edit a database connector
  • Create and edit a saas connector

Pre-Merge Checklist

@cypress
Copy link

cypress bot commented Jun 26, 2023

Passing run #2985 ↗︎

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

Details:

Merge cb9c8ab into 028ee83...
Project: fides Commit: 85d6b102c4 ℹ️
Status: Passed Duration: 00:44 💡
Started: Jun 29, 2023 6:51 PM Ended: Jun 29, 2023 6:52 PM

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

@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +21.83 🎉

Comparison is base (028ee83) 65.27% compared to head (733c159) 87.10%.

❗ Current head 733c159 differs from pull request most recent head cb9c8ab. Consider uploading reports for the commit cb9c8ab to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3684       +/-   ##
===========================================
+ Coverage   65.27%   87.10%   +21.83%     
===========================================
  Files         310      310               
  Lines       18993    19028       +35     
  Branches     2447     2436       -11     
===========================================
+ Hits        12398    16575     +4177     
+ Misses       6158     2026     -4132     
+ Partials      437      427       -10     
Impacted Files Coverage Δ
src/fides/api/models/connectionconfig.py 97.00% <100.00%> (+17.00%) ⬆️
...emas/connection_configuration/connection_config.py 100.00% <100.00%> (+6.17%) ⬆️
...vice/connectors/saas/connector_registry_service.py 93.15% <100.00%> (+44.22%) ⬆️

... and 145 files with indirect coverage changes

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

@TheAndrewJackson TheAndrewJackson self-assigned this Jun 27, 2023
@TheAndrewJackson TheAndrewJackson marked this pull request as ready for review June 27, 2023 21:24
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.

This looks good, thanks for taking the time to go over the key/fides_key part

@TheAndrewJackson TheAndrewJackson merged commit cd579f7 into main Jun 29, 2023
@TheAndrewJackson TheAndrewJackson deleted the ajackson_947_deprecate_ui_fields branch June 29, 2023 19:07
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