[Backend] Fix Validation Error where Current Privacy Preferences might be None

[pattisdr]

Closes https://github.com/ethyca/fidesplus/issues/979

Description Of Changes

Under higher loads, saving Privacy Preferences could return Pydantic Validation Errors because the CurrentPrivacyPreferenceSchema is not supposed to be None.

2023-06-27 15:53:55.491 [ERROR] (logger:_log_exception:34): 1 validation error for CurrentPrivacyPreferenceSchema
response -> 0
  none is not an allowed value (type=type_error.none.not_allowed)

The root cause is that if a single user saves their preferences in rapid succession, the older PrivacyPreferenceHistory history record will no longer have a relationship with the CurrentPrivacyPreference record because a newer PrivacyPreferenceHistory had superseded it.

Code Changes

• Add a new method to PrivacyPreferenceHistory that returns both the created PrivacyPreferenceHistory and upserted CurrentPrivacyPreference record instead of relying on the relationship between the PrivacyPreferenceHistory and the CurrentPrivacyPreference to fetch the latter after the fact before returning a response. The response should return the CurrentPrivacyPreference upserted at that moment, even if the FK has since been replaced.

Steps to Confirm

• I would just validate that saving preferences still works - PATCH {{host}}/privacy-preferences

Example request:

{
   "browser_identity": {
       "fides_user_device_id": "75495a74-fc09-4e21-8250-0d53b854877d"
   },
   "preferences": [{
       "privacy_notice_history_id": "pri_d6bbc55d-bc06-4b7c-858b-016ed280e668",
       "preference": "opt_out"
   }],
   "user_geography": "us_ca",
   "privacy_experience_id": "pri_475641b6-75e4-4e77-8a30-d9640c3616e7",
   "method": "button"
}

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
• For API changes, the Postman collection has been updated

[pattisdr]

@adamsachs hoping you could review this small change - it was the source of the first error we were running into here, creating higher than expected failure rates.

[cypress]

Passing run #3050 ↗︎

0

4

0

0

0

Details:

Merge f7141e3 into f526d9f...
Project: fides	Commit: 1e2711f4fe ℹ️
Status: Passed	Duration: 00:56 💡
Started: Jul 5, 2023 6:41 PM	Ended: Jul 5, 2023 6:42 PM

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

[codecov]

Codecov Report

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

Comparison is base (f526d9f) 65.24% compared to head (e682154) 87.07%.

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

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3719       +/-   ##
===========================================
+ Coverage   65.24%   87.07%   +21.83%     
===========================================
  Files         310      310               
  Lines       19033    19019       -14     
  Branches     2456     2454        -2     
===========================================
+ Hits        12418    16561     +4143     
+ Misses       6178     2029     -4149     
+ Partials      437      429        -8     

Impacted Files	Coverage Δ
...i/api/v1/endpoints/privacy_preference_endpoints.py	98.03% <ø> (+65.57%)	⬆️
src/fides/api/models/privacy_preference.py	100.00% <100.00%> (+28.70%)	⬆️

... and 144 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.

[adamsachs]

nice simple fix! i'm sure it was tougher to track down the timing problem than it was to actually fix it once you found it 😄

manually verified no obvious regressions in the PATCH /api/v1/privacy-preferences endpoint per your steps.

have you confirmed this fixes the problem you'd seen in load testing? not a prerequisite for merging, just curious if that's something you'd been able to do before merging.

[pattisdr]

@adamsachs yes! you're able to hit this error locally if you send requests at a high enough rate, and I did verify that this particular error has gone away.

Thanks for reviewing this!

[pattisdr]

Failing test not related and ticketed here: #3736