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 for Braze and Domo overrides #4196

Merged
merged 4 commits into from
Oct 2, 2023

Conversation

SteveDMurphy
Copy link
Contributor

@SteveDMurphy SteveDMurphy commented Sep 29, 2023

Closes #4179

Description Of Changes

The if was blocking the formatting of the email. This change simplifies it to block it if the key is present instead of relying on user.contact.email (user.contact is included in the policy instead)

We may also want to replicate this change to Domo overrides

Code Changes

  • Modify the if to format email correctly if present

Steps to Confirm

  • Implemented live today with the email being formatted

Pre-Merge Checklist

@cypress
Copy link

cypress bot commented Sep 29, 2023

Passing run #4453 ↗︎

0 4 0 0 Flakiness 0

Details:

Merge 17fed22 into b1c0f4f...
Project: fides Commit: 309b895477 ℹ️
Status: Passed Duration: 01:19 💡
Started: Oct 2, 2023 10:41 PM Ended: Oct 2, 2023 10:42 PM

Review all test suite changes for PR #4196 ↗︎

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (7932483) 87.67% compared to head (17fed22) 87.67%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4196   +/-   ##
=======================================
  Coverage   87.67%   87.67%           
=======================================
  Files         333      333           
  Lines       20743    20743           
  Branches     2690     2690           
=======================================
  Hits        18187    18187           
  Misses       2093     2093           
  Partials      463      463           
Files Coverage Δ
...verride_implementations/braze_request_overrides.py 50.00% <0.00%> (ø)
...override_implementations/domo_request_overrides.py 45.45% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SteveDMurphy SteveDMurphy marked this pull request as ready for review October 2, 2023 14:19
@SteveDMurphy SteveDMurphy changed the title Further update for Braze override Update for Braze and Domo overrides Oct 2, 2023
@Kelsey-Ethyca Kelsey-Ethyca merged commit 84d1d84 into main Oct 2, 2023
38 of 39 checks passed
@Kelsey-Ethyca Kelsey-Ethyca deleted the SteveDMurphy-4179-braze-part-two branch October 2, 2023 23:36
Kelsey-Ethyca added a commit that referenced this pull request Oct 2, 2023
Co-authored-by: Neville Samuell <[email protected]>
Co-authored-by: Kelsey Thomas <[email protected]>
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.

Braze requires email to formatted correctly, connector fails on erasure
2 participants