fix: Remove unnecessary trafficker_id requirement from GAM creatives_manager #975
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Erin found a bug in the sales agent,
When approving a media buy with GAM adapter, creatives were not being uploaded to GAM even though:
GAM would show "Needs creatives (1)" on the line item.
Ticket: https://linear.app/scope3-projects/issue/PRO-88/hosted-sales-agent-creative-assignment-not-making-it-into-gam
Root Cause
The
GAMCreativesManagerinitialization ingoogle_ad_manager.pywas checking for bothadvertiser_idANDtrafficker_id:However,
GAMCreativesManagerdoes not actually usetrafficker_id- onlyGAMOrdersManagerdoes.When
gam_trafficker_idisNULLin theadapter_configtable, it becomes an empty string""which is falsy in Python. This causedcreatives_managerto be set toNone, and during approval the creative upload was silently skipped with the warning:Fix
Remove the unnecessary
and self.trafficker_idcheck sinceGAMCreativesManageronly needsadvertiser_id.The comment on that line already said "Only initialize creative manager if we have advertiser_id" - the code just didn't match the comment.
Testing
tests/unit/test_gam_creatives_manager.py- 5 tests pass