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

Update Admin UI to support new dataset config changes #1766

Closed
TheAndrewJackson opened this issue Nov 14, 2022 · 3 comments · Fixed by #2113
Closed

Update Admin UI to support new dataset config changes #1766

TheAndrewJackson opened this issue Nov 14, 2022 · 3 comments · Fixed by #2113

Comments

@TheAndrewJackson
Copy link
Contributor

TheAndrewJackson commented Nov 14, 2022

❗ Depends on #1762

Now that the backend no longer supports storing dataset yaml files on the datasetconfig model the admin ui will need to be updated.

When configuring connectors users are able to upload yaml configs like so
Screen Shot 2022-11-14 at 10 08 02
Whenever edits are made they're sent to /connection/{connection_key}/dataset/.

This form needs to be updated to send yaml configs to /dataset and use the returned ID to link the ctl_datasets model to the datasetconfig model using the new ctl_dataset_id foreign key. Now only that ID will be sent in to /connection/{connection_key}/dataset/

So the new form flow will be:

Creating new config

  • Send config yaml to /dataset and save the return id in memory for the next request
  • Send nearly the same data to /connection/{connection_key}/dataset/ but leave out the dataset field and instead include the stored in in ctl_datasets_id

Editing config

  • Post updates to /dataset
    • This assumes that the ctl_datasets model is already linked datasetconfigs

This should get current form to feature parity with how it currently works. It still needs some additional updates.

Now that all the datasets are managed in the same place it's possible that a user will configure a connector and want to link it with a dataset that has already been created from the datasets UI. They will need a way to select from datasets that already exist.

This means that users will be able to either create datasets from this page or reuse one from the list of datasets that already exist.

Extra considerations

  • There are two separate yaml forms in the admin ui. This might be a good time to unify them
  • It looks like the admin ui may not call the dataset validation endpoints. It would be good to double check and make sure they're called when creating and editing datasets
@pattisdr
Copy link
Contributor

pattisdr commented Dec 21, 2022

@allisonking The unified-fides-resources branch is ready to work with this new UI. A few changes from above.

Creating new config:

  • Save dataset: Send config json to /dataset and save the fides key in memory for the next request (POST {{host}}/dataset/upsert)
  • Link dataset to datasetconfig: "PATCH /connection/{connection_key}/datasetconfig"

request body:

[
        {
            "fides_key": "<dataset config fides key>",
            "ctl_dataset_fides_key": "<fides key from previous step - for the CTL Dataset>",
        }
    ]

Update new config:

  • Get the Dataset Config: `GET "/connection/{connection_key}/dataset/{dataset config fides key}". The response is the ctl dataset attached to that dataset config, which has the ctl dataset fides key
  • POST {{host}}/dataset/upsert - fides key should be ctl dataset fides key.
  • Also you could link a new CTL Dataset to the existing dataset config with the link call above

@allisonking
Copy link
Contributor

allisonking commented Dec 21, 2022

The changes needed here are in this file: https://github.com/ethyca/fides/blob/main/clients/admin-ui/src/features/datastore-connections/add-connection/DatasetConfiguration.tsx

Current state

  1. GET /api/v1/connection/{connection_key}/dataset to populate the yaml editor with datasets associated with this connection
  2. PATCH /api/v1/connection/{connection_key}/dataset to update the dataset after the user has made changes via the editor

Backend changes

@pattisdr made some changes in 0f872f3 which adds:

  1. GET /api/v1/connection/{connection_key}/datasetconfig - returns a list of all dataset configs attached to this connection config. There's a fides_key in the request body which is the DatasetConfig fides key, and then a ctl_dataset key under which is the entirety of the linked CTL Dataset
  2. GET /api/v1/connection/{connection_key}/datasetconfig/<datasetconfig fides key> returns the single specified Dataset Config with the given fides key with the same format as above.

And GET /api/v1/connection/{connection_key}/dataset and GET /api/v1/connection{connection_key}/dataset/fideskey will be deprecated.

Next state for the UI

  • Swap out the GET /api/v1/connection/{connection_key}/dataset with:
    • GET /api/v1/connection/{connection_key}/datasetconfig which will return the ctl datasets which we can use to populate the yaml form. There will be a dataset config fides key associated with each one, which we will have to hold on to internally. The key for the dataset config and the dataset will likely be the same, but the user could also change the dataset one.
  • To submit:
    • POST /api/v1//dataset/upsert the updated dataset, which will update the ctl dataset
    • Then link it via PATCH /api/v1/connection/{connection_key}/datasetconfig with a body of
[
    {
        "fides_key": "<dataset config fides key, from the original GET>",
        "ctl_dataset_fides_key": "<fides key from previous step - for the CTL Dataset>",
    },
    {
        "fides_key": "<dataset config fides key, from the original GET>",
        "ctl_dataset_fides_key": "<fides key from previous step if more than one dataset was submitted in the form>"
   }
]

And I guess we'll just have to rely on the order of the yaml to keep the dataset config associated with the right dataset

@pattisdr
Copy link
Contributor

Merged into unified-fides-resources

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants