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

support previous_vendor_id attribute on System model #182

Merged

Conversation

adamsachs
Copy link
Contributor

@adamsachs adamsachs commented Nov 6, 2023

part of https://ethyca.atlassian.net/browse/PROD-1296

Description Of Changes

Adds a new, optional previous_vendor_id attribute/field to the System model. This field will help us indicate whether a given vendor record (or a system based on that vendor) used to be represented by a different vendor record, e.g. if a google AC vendor transitions to a GVL vendor, as is expected to happen somewhat regularly.

Code Changes

  • add previous_vendor_id attribute to System model
  • some test cleanup, add a TODO comment since i realized some of our model tests aren't really effective...

Steps to Confirm

  • will test out by publishing an alpha tag and using this in Compass webservice code to ensure it can support this new column on Compass data. UPDATE: tested with 2.2.1a0 alpha package, works well 👍

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

@@ -419,7 +424,7 @@ def test_expanded_system(self):
cookie_refresh=True,
uses_non_cookie_access=True,
legitimate_interest_disclosure_url="http://www.example.com/legitimate_interest_disclosure",
flexible_legal_basis_for_processing=True,
Copy link
Contributor Author

@adamsachs adamsachs Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unrelated to the change in this PR, but saw this test mistake while i was updating the test. i moved this up to the PrivacyDeclaration where it should be, this was just a mindless mistake to have it on the System in the first place.

more importantly, the test has been passing even with this error in here, which means that the test is ineffective! i put in a TODO comment for now, i'll see if i can circle back to improving the test...

cookies=[
{"name": "ANON_ID", "path": "/", "domain": "tribalfusion.com"}
],
)
],
third_country_transfers=["ARM"],
vendor_id="1",
vendor_id="gvl.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't really matter but figured i'd update to make the value more realistic in our test case

@adamsachs adamsachs marked this pull request as ready for review November 6, 2023 17:41
@adamsachs adamsachs requested a review from pattisdr November 6, 2023 17:41
@adamsachs adamsachs merged commit 1bcbd0a into main Nov 6, 2023
31 checks passed
@adamsachs adamsachs deleted the asachs/PROD-1296-previous-vendor-id-property-fideslang branch November 6, 2023 19:15
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.

2 participants