Sovrn connector UI #2579

allisonking · 2023-02-13T16:19:16Z

Code Changes

There was unfortunately quite a bit of tech debt in this section of code with typescript being ignored and other eslint workarounds. I couldn't resist fixing it up (also because things were confusing me). These were:

Extracting filling in defaults to a helper function (with tests). Making copies of parameters instead of reassigning them
Refactoring skip logic on querying for connection schema
Fix bug that made connector types with only one tab not render properly

Actual feature changes

Update openapi typescript types to include latest email type endpoints
Add a new Email type ConnectorParameters.tsx (following the pattern of the other connector types)
- This uses the same handleSubmit code as the Database type connector parameter, so I did some refactoring so they could both use the same logic
Cypress tests

Steps to Confirm

This is probably best QA'd alongside #2582

If you make a separate branch out of this one plus #2582, then you should be able to go through the whole flow:

Go through the same steps as in Sovrn Email Consent Connector #2543 except:
- Instead of Because we can't pass in the ljt_readerID from the frontend yet, cache a fake one in the request runner service..., do the following:
  - Add a cookie to your page by opening up the inspector in your browser (opt+cmd+i on osx), entering the console, and setting your cookie via `document.cookie = "ljt_readerID=fides_test_reader_id;"
  - Issue a consent request via the UI to opt out of Email Marketing. In the network tab, you should see ljt_readerID show up in the payload for PATCH preferences under browser_identity
- Instead of Add a sovrn consent email connector via the API:
  - Go through the admin UI and add a sovrn connector! You should be able to add it and also send a test email

Pre-Merge Checklist

All CI Pipelines Succeeded
Documentation:
- documentation complete, PR opened in fidesdocs
- documentation issue created in fidesdocs
Issue Requirements are Met
Relevant Follow-Up Issues Created
Update CHANGELOG.md

Description Of Changes

Write some things here about the changes and any potential caveats

allisonking · 2023-02-13T19:34:53Z

clients/admin-ui/src/features/datastore-connections/add-connection/ConfigureConnector.tsx

-    if (step) {
-      setSelectedItem(
-        connector?.options[Number(step.stepId) - connector.options.length]
-      );
-    }


I really could not figure out the purpose of this line, and it caused a bug when there's only one tab available, so I got rid of it

Bravo tracking this down!

allisonking · 2023-02-13T19:35:55Z

clients/admin-ui/src/features/datastore-connections/add-connection/ConnectorParameters.tsx

-  useEffect(() => {
-    mounted.current = true;
-    if (connectionOption?.type !== SystemType.MANUAL) {
-      setSkip(false);
-    }
-    return () => {
-      mounted.current = false;
-    };
-  }, [connectionOption?.type]);


this useEffect + skip pattern was hard to follow, so I refactored it and it seems to still work well

allisonking · 2023-02-13T19:36:58Z

.../admin-ui/src/features/datastore-connections/add-connection/database/ConnectorParameters.tsx

@@ -36,19 +39,14 @@ type ConnectorParametersProps = {
  onTestConnectionClick: (value: any) => void;
 };

-export const ConnectorParameters: React.FC<ConnectorParametersProps> = ({
-  data,
+export const useDatabaseConnector = ({


I extracted this to a hook because the email connector uses the same API logic

allisonking · 2023-02-13T19:39:10Z

...admin-ui/src/features/datastore-connections/add-connection/forms/ConnectorParametersForm.tsx

    }
-    return defaultValues;
+    return fillInDefaults(initialValues, data);


basically an effort to remove duplicate code (note the two Object.entries()... that are the same above) and also to extract to a function that is tested

codecov · 2023-02-13T19:42:39Z

Codecov Report

❗ No coverage uploaded for pull request base (fides_2394_sovrn_connector@e56bfdf). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files

@@                      Coverage Diff                      @@
##             fides_2394_sovrn_connector    #2579   +/-   ##
=============================================================
  Coverage                              ?   86.05%           
=============================================================
  Files                                 ?      289           
  Lines                                 ?    15760           
  Branches                              ?     1983           
=============================================================
  Hits                                  ?    13562           
  Misses                                ?     1818           
  Partials                              ?      380

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.

allisonking · 2023-02-13T23:18:31Z

Oh actually I forgot I still need to fix the Awaiting Email Send

allisonking · 2023-02-13T23:31:43Z

Another option was:

I thought the first one looked slightly better, but it's easy enough to change (the second option uses whiteSpace="break-spaces" and removes the fixed width)

pattisdr

@allisonking could you actually change this so it makes a request that looks something like this? My latest commit on the backend branch yesterday where I change the expected format expects the advanced settings block.

{
    "recipient_email_address": "dawn@ethyca.com",
    "test_email_address": "dawn@ethyca.com",
    "advanced_settings": {
        "identity_types": {
            "email": false,
            "phone_number": false,
            "cookie_ids":  []
        }
    }
}

clients/admin-ui/cypress/fixtures/connectors/sovrn_secret.json

clients/admin-ui/src/features/connection-type/types.ts

…ctor-ui

cypress · 2023-02-15T16:26:14Z

Passing run #108 ↗︎

0	3	0	0	0

Details:

Merge `9e26755` into `e56bfdf`...
Project: fides	Commit: `826feceff7 ℹ️`
Status: Passed	Duration: 00:38 💡
Started: Feb 15, 2023 4:25 PM	Ended: Feb 15, 2023 4:26 PM

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

pattisdr

While I can't easily review the code, I QA'd this and it looks great to me. Nice with the one tab so DatasetConfigs aren't created. Thanks again for stubbing out the API response for now.

You're welcome to merge this into my backend base branch when ready!

allisonking · 2023-02-15T18:12:29Z

Thanks @pattisdr ! requesting @TheAndrewJackson to review the code part. I think you've done the hard part of QA-ing it, so hopefully Andrew can just look through the code (not that that is easy either, sorry for all the changes... it took me a long time to wrap my head around how the connectors stuff worked)

allisonking added 13 commits February 10, 2023 11:58

Fix bug with selecting tab when there is only one tab

8a7e10c

Add initial email connector form

c72c544

Try to add advanced field

5ef34aa

Update ts types for sovrn work

bb20357

wip refactor initial values

166fae5

wip fix handleSubmit

c742a7f

Add tests for fillInDefault

371487d

cleanup

706fa0a

Refactor database connector parameters to be reusable

4dd68a4

Add test for email connector flow

c51d382

Clean up import

3ac88dd

Reroute after configuring sovrn connector

8ce0a8f

Fix param reassigning

fd99d71

allisonking commented Feb 13, 2023

View reviewed changes

allisonking requested a review from pattisdr February 13, 2023 23:17

allisonking marked this pull request as ready for review February 13, 2023 23:17

Fix long label being cut off

4adb411

pattisdr reviewed Feb 14, 2023

View reviewed changes

clients/admin-ui/cypress/fixtures/connectors/sovrn_secret.json Outdated Show resolved Hide resolved

clients/admin-ui/cypress/fixtures/connectors/sovrn_secret.json Show resolved Hide resolved

clients/admin-ui/src/features/connection-type/types.ts Outdated Show resolved Hide resolved

allisonking added 4 commits February 15, 2023 10:44

Merge branch 'fides_2394_sovrn_connector' into aking/2458/sovrn-conne…

ef0802c

…ctor-ui

Fix updated payload for sovrn

6437a87

Update types

2cff663

Update cypress tests

9e26755

allisonking requested a review from pattisdr February 15, 2023 16:44

pattisdr approved these changes Feb 15, 2023

View reviewed changes

allisonking requested a review from TheAndrewJackson February 15, 2023 18:11

allisonking merged commit 08d3393 into fides_2394_sovrn_connector Feb 16, 2023

allisonking deleted the aking/2458/sovrn-connector-ui branch February 16, 2023 16:20

allisonking mentioned this pull request Feb 16, 2023

[Frontend] Sovrn Email Connector Setup #2458

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sovrn connector UI #2579

Sovrn connector UI #2579

allisonking commented Feb 13, 2023 •

edited

Loading

allisonking Feb 13, 2023

pattisdr Feb 14, 2023

allisonking Feb 13, 2023

allisonking Feb 13, 2023

allisonking Feb 13, 2023

codecov bot commented Feb 13, 2023 •

edited

Loading

allisonking commented Feb 13, 2023

allisonking commented Feb 13, 2023

pattisdr left a comment •

edited

Loading

cypress bot commented Feb 15, 2023

pattisdr left a comment •

edited

Loading

allisonking commented Feb 15, 2023

Sovrn connector UI #2579

Sovrn connector UI #2579

Conversation

allisonking commented Feb 13, 2023 • edited Loading

Code Changes

Steps to Confirm

Pre-Merge Checklist

Description Of Changes

allisonking Feb 13, 2023

Choose a reason for hiding this comment

pattisdr Feb 14, 2023

Choose a reason for hiding this comment

allisonking Feb 13, 2023

Choose a reason for hiding this comment

allisonking Feb 13, 2023

Choose a reason for hiding this comment

allisonking Feb 13, 2023

Choose a reason for hiding this comment

codecov bot commented Feb 13, 2023 • edited Loading

Codecov Report

allisonking commented Feb 13, 2023

allisonking commented Feb 13, 2023

pattisdr left a comment • edited Loading

Choose a reason for hiding this comment

cypress bot commented Feb 15, 2023

Passing run #108 ↗︎

pattisdr left a comment • edited Loading

Choose a reason for hiding this comment

allisonking commented Feb 15, 2023

allisonking commented Feb 13, 2023 •

edited

Loading

codecov bot commented Feb 13, 2023 •

edited

Loading

pattisdr left a comment •

edited

Loading

pattisdr left a comment •

edited

Loading