fix: GAM product placements not saving when line_item_type absent #691
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Root cause: The GAM configuration save logic was nested inside an 'if line_item_type:' check. Since the form doesn't include line_item_type (it's automatic now), this entire block never executed, causing placements and other GAM config to not be saved. Changes: - Move GAM config save logic outside the line_item_type check - Check 'if adapter_type == "google_ad_manager"' instead - Preserve existing implementation_config by copying it first - Only regenerate default config when line_item_type is present - This allows placement/ad unit updates without full config regeneration Now placements, ad units, targeting, and other GAM config will save correctly even in automatic mode (no line_item_type field).
- Log product object and implementation config on page load - Log placement IDs type and value - Handle both string and array formats for targeted_placement_ids - Add logging when setting hidden field value - Log final populated placement IDs array This will help diagnose placement persistence issues in the future.
bokelley
force-pushed
the
fix-placement-save-bug
branch
from
3d2ab3a to
83c6ad5
Compare
danf-newton
pushed a commit
to Newton-Research-Inc/salesagent
that referenced
this pull request
Nov 24, 2025
…contextprotocol#691) * fix: GAM product edit not saving placements when line_item_type absent Root cause: The GAM configuration save logic was nested inside an 'if line_item_type:' check. Since the form doesn't include line_item_type (it's automatic now), this entire block never executed, causing placements and other GAM config to not be saved. Changes: - Move GAM config save logic outside the line_item_type check - Check 'if adapter_type == "google_ad_manager"' instead - Preserve existing implementation_config by copying it first - Only regenerate default config when line_item_type is present - This allows placement/ad unit updates without full config regeneration Now placements, ad units, targeting, and other GAM config will save correctly even in automatic mode (no line_item_type field). * chore: add diagnostic logging for placement field population - Log product object and implementation config on page load - Log placement IDs type and value - Handle both string and array formats for targeted_placement_ids - Add logging when setting hidden field value - Log final populated placement IDs array This will help diagnose placement persistence issues in the future.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixed a critical bug where placements weren't being saved when editing GAM products because the save logic was nested inside an
if line_item_type:check that never executed in automatic mode.Root Cause
The GAM product configuration save logic (lines 1147-1209 in
src/admin/blueprints/products.py) was inside anif line_item_type:conditional. Since the edit form uses automatic line item type selection (based on pricing model), the form doesn't submit aline_item_typefield. This caused the entire block to be skipped, preventing placements, ad units, targeting, and other GAM config from being saved.Changes
File:
src/admin/blueprints/products.pyline_item_typecheckif adapter_type == "google_ad_manager"insteadline_item_typeis explicitly providedFile:
templates/add_product_gam.htmltargeted_placement_ids(handles both string and array)Testing
Manual Testing Steps
Impact