CON-206: Responsys access and erasure #4618
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Passing run #6423 ↗︎
Details:
Review all test suite changes for PR #4618 ↗︎ |
||||||||||||||||||||
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4618 +/- ##
==========================================
- Coverage 86.85% 86.67% -0.18%
==========================================
Files 334 336 +2
Lines 20013 20090 +77
Branches 2569 2581 +12
==========================================
+ Hits 17383 17414 +31
- Misses 2157 2203 +46
Partials 473 473 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This looks good so far! You're following all the connector conventions, I didn't have to call anything major out in the config, dataset, or tests. I only left one comment about testing the response transformation in the oracle_responsys_profile_list_recipients_read request override.
data/saas/icon/default.svg
Outdated
There was a problem hiding this comment.
This connector doesn't specify an icon so it falls back to the default ethyca.svg. This is how I noticed that the default icon has an unwanted fill. Just updating this so the entry looks good in the connector dropdown.
With that said, do you want to add an icon for Oracle Responsys?
There was a problem hiding this comment.
I added an SVG, but I couldn't change the viewbox to 0 0 32 32 without clipping some of the SVG. @galvana, you have a way to do this, right?
There was a problem hiding this comment.
I don't know about 'right' but typically we work with the icon in the figma page until we get it the way we like, then we can save it out to submit in the PR. We can walk through this pretty easy, but I can't lead just yet as I have requested access but don't have it yet =/
I found this icon that has both Oracle and Responsys in it but since I can't get into figma to verify if it will be easy to use or not =(
| response_data = pydash.get(members_response.json(), "recordData") | ||
| if response_data: | ||
| normalized_field_names = [ | ||
| field.lower().rstrip("_") for field in response_data["fieldNames"] | ||
| ] | ||
| serialized_data = [ | ||
| dict(zip(normalized_field_names, records)) | ||
| for records in response_data["records"] | ||
| ] | ||
|
|
||
| for record in serialized_data: | ||
| # Filter out the keys with falsy values and append it | ||
| filtered_records = { | ||
| key: value for key, value in record.items() if value | ||
| } | ||
| filtered_records["profile_list_id"] = list_id | ||
| results.append(filtered_records) |
There was a problem hiding this comment.
It might be a good idea to break out the response processing into its own function so that we can test out some different scenarios. It's also a good way for someone to understand what's happening here if some sample test cases are provided.
...ides/api/service/saas_request/override_implementations/oracle_responsys_request_overrides.py
Outdated
Show resolved
Hide resolved
...ides/api/service/saas_request/override_implementations/oracle_responsys_request_overrides.py
Show resolved
Hide resolved
|
I ran through the checklist and it looked like we may need docs for this, I took a stab at it and updated the description with the PR link and Issue link. Hope that's okay! |
currently the viewbox is 0 0 48 48, but I see the rest are 0 0 32 32, so we might need to adjust it.
…ne nums Co-authored-by: Adrian Galvan <[email protected]>
Co-authored-by: Adrian Galvan <[email protected]>
Closes CON-206
Description Of Changes
This PR adds access and erasure support for Oracle Responsys. The identities supported are email and phone number.
Code Changes
Steps to Confirm
Pre-Merge Checklist
https://github.com/ethyca/fidesdocs/pull/327
https://github.com/ethyca/fidesdocs/issues/326
CHANGELOG.md