Skip to content

Conversation

@bokelley
Copy link
Contributor

@bokelley bokelley commented Nov 2, 2025

Summary

Implements real-time querying of GAM custom targeting values and introduces a reusable visual widget for product targeting configuration. Users can now see available targeting keys/values directly in the admin UI without manual JSON entry.

Key Changes

Backend

  • New endpoint: GET /api/tenant/<id>/targeting/values/<key_id>
    • Queries GAM in real-time (values are lazy-loaded, not pre-synced)
    • Validates tenant GAM configuration before querying
    • Returns formatted value metadata with status, match type, display name
    • Limits to 1000 values per key to prevent timeouts
    • Proper tenant isolation and authentication

Frontend

  • New reusable widget: templates/components/targeting_selector_widget.html

    • Self-contained JavaScript TargetingWidget class for DRY targeting selection
    • Tabbed interface: Key-Value Pairs, Geography, Device, Audiences
    • Dual-pane key/value selector with search and filtering
    • Builds JSON automatically - no manual text editing required
    • Can be included in any form with `{% include %}`
  • Updated templates:

    • `add_product.html`: Integrated targeting widget
    • `edit_product.html`: Integrated targeting widget with pre-population support
    • `targeting_browser.html`: Enhanced with async value loading

Tests

  • New integration tests (`tests/integration/test_targeting_values_endpoint.py`):
    • Validates real-time GAM queries with mock values
    • Tests empty results, tenant isolation, and authentication
    • Mocks GAM client to avoid real API calls (4 comprehensive tests)

Test Plan

  • Unit tests pass (861 passed)
  • Pre-commit hooks pass (all 32 checks)
  • New integration tests validate endpoint behavior with mocked GAM client
  • Tests verify tenant isolation and authentication requirements
  • Manual testing in UI to verify widget renders and builds JSON correctly
  • Verify backward compatibility with existing products

Why This Matters

  1. Better UX: Visual selector instead of manual JSON entry
  2. Real-time data: Always shows current GAM configuration
  3. Lazy loading: Values only queried when needed (performance)
  4. Code reuse: Widget can be used in multiple places
  5. Proper isolation: Full tenant isolation with GAM credential validation

Technical Notes

  • Values are not pre-synced (by design) - prevents database bloat
  • Real-time queries are faster than syncing 10k+ values per key
  • Widget updates hidden `targeting_template` field for form submission
  • Full backward compatibility - existing products unaffected

🤖 Generated with Claude Code

bokelley and others added 3 commits November 2, 2025 04:57
…tor widget

## Summary
Implements real-time querying of GAM custom targeting values and introduces a
reusable visual widget for product targeting configuration. Users can now see
available targeting keys/values directly in the admin UI without manual JSON entry.

## Changes

### Backend
- **New endpoint**: `GET /api/tenant/<id>/targeting/values/<key_id>`
  - Queries GAM in real-time (values are lazy-loaded, not pre-synced)
  - Validates tenant GAM configuration before querying
  - Returns formatted value metadata with status, match type, display name
  - Limits to 1000 values per key to prevent timeouts
  - Proper tenant isolation and authentication

### Frontend
- **New reusable widget**: `templates/components/targeting_selector_widget.html`
  - Self-contained JavaScript `TargetingWidget` class for DRY targeting selection
  - Tabbed interface: Key-Value Pairs, Geography, Device, Audiences
  - Dual-pane key/value selector with search and filtering
  - Builds JSON automatically - no manual text editing required
  - Can be included in any form with `{% include %}`

- **Updated templates**:
  - `add_product.html`: Integrated targeting widget
  - `edit_product.html`: Integrated targeting widget with pre-population support
  - `targeting_browser.html`: Enhanced with async value loading

### Tests
- **New integration tests** (`tests/integration/test_targeting_values_endpoint.py`):
  - Validates real-time GAM queries with mock values
  - Tests empty results, tenant isolation, and authentication
  - Mocks GAM client to avoid real API calls

## Why This Matters
1. **Better UX**: Visual selector instead of manual JSON entry
2. **Real-time data**: Always shows current GAM configuration
3. **Lazy loading**: Values only queried when needed (performance)
4. **Code reuse**: Widget can be used in multiple places
5. **Proper isolation**: Full tenant isolation with GAM credential validation

## Technical Notes
- Values are not pre-synced (by design) - prevents database bloat
- Real-time queries are faster than syncing 10k+ values per key
- Widget updates hidden `targeting_template` field for form submission
- Full backward compatibility - existing products unaffected

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The test was trying to patch 'src.admin.blueprints.inventory.GAMInventoryDiscovery'
but GAMInventoryDiscovery is imported inside the endpoint function, so it needs to be
patched at its definition location: 'src.adapters.gam_inventory_discovery.GAMInventoryDiscovery'.

This fixes the AttributeError in CI integration tests.
The targeting widget is causing a RecursionError during template rendering in CI
(but not locally). Temporarily commenting out the widget includes to unblock CI
while we investigate the root cause.

Affected tests:
- test_admin_ui_pages.py::test_create_product_page_renders
- test_admin_ui_routes_comprehensive.py::test_add_product_page

The targeting values endpoint and tests are working correctly. This is only
a template rendering issue in the CI environment.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@bokelley bokelley force-pushed the add-kv-values-endpoint branch from 1e386fb to 7482583 Compare November 2, 2025 11:37
bokelley and others added 4 commits November 2, 2025 06:40
Testing if the RecursionError is caused by the size/complexity of the full widget.
This minimal version has only basic HTML/JS to verify template rendering works in CI.

If this passes CI, we'll know the issue is widget size/complexity, not the include
mechanism itself.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Creates improved widget architecture to avoid CI RecursionError:

**New Architecture:**
- External JavaScript: static/js/targeting_widget.js (~300 lines)
  - Full TargetingWidget class with all functionality
  - Async data loading from API endpoints
  - Real-time value loading for selected keys
  - Tab-based UI (Key-Value, Geography, Device, Audiences)
  - Search and filtering
  - JSON generation for hidden field

- Simplified Template: targeting_selector_widget_v2.html (~280 lines)
  - CSS styles only
  - Minimal HTML structure
  - Script tag to load external JS

**Benefits:**
- Avoids template recursion by splitting large file
- Better separation of concerns (CSS/HTML vs JavaScript)
- Easier to maintain and debug
- Better browser caching of JavaScript
- Full visual targeting selector functionality

**Next Step:** Once CI confirms minimal widget works, switch to v2 widget.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Activates the complete visual targeting selector in product forms:

**Changes:**
- Updated add_product.html to use targeting_selector_widget_v2.html
- Updated edit_product.html to use targeting_selector_widget_v2.html
- Removed minimal test widget (no longer needed)

**Full Feature Set Restored:**
✅ Visual selector UI with tabbed interface
✅ Real-time GAM key/value loading via API
✅ Custom Key-Value Pairs tab with dual-pane selector
✅ Geography, Device, and Audiences tabs
✅ Search and filtering within keys/values
✅ Live preview of selected targeting
✅ Automatic JSON generation for form submission
✅ Pre-population of existing targeting data (edit mode)

**Architecture Benefits:**
- External JavaScript (static/js/targeting_widget.js)
- Smaller template file (no recursion issues)
- Better browser caching
- Cleaner separation of concerns
- Production-ready

This completes the visual targeting selector feature. Users can now easily
configure product targeting through an intuitive UI instead of manual JSON entry.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Cleanup widget file structure:
- Removed original 619-line targeting_selector_widget.html (caused CI recursion)
- Renamed targeting_selector_widget_v2.html to targeting_selector_widget.html
- Updated add_product.html and edit_product.html to use standard filename
- No 'v2' suffix needed - this is now the only version

Result: Single, clean widget implementation with external JavaScript architecture.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@bokelley bokelley merged commit ebd89b9 into main Nov 2, 2025
10 checks passed
danf-newton pushed a commit to Newton-Research-Inc/salesagent that referenced this pull request Nov 24, 2025
…tor widget (adcontextprotocol#678)

* feat: Add real-time custom targeting values endpoint and visual selector widget

## Summary
Implements real-time querying of GAM custom targeting values and introduces a
reusable visual widget for product targeting configuration. Users can now see
available targeting keys/values directly in the admin UI without manual JSON entry.

## Changes

### Backend
- **New endpoint**: `GET /api/tenant/<id>/targeting/values/<key_id>`
  - Queries GAM in real-time (values are lazy-loaded, not pre-synced)
  - Validates tenant GAM configuration before querying
  - Returns formatted value metadata with status, match type, display name
  - Limits to 1000 values per key to prevent timeouts
  - Proper tenant isolation and authentication

### Frontend
- **New reusable widget**: `templates/components/targeting_selector_widget.html`
  - Self-contained JavaScript `TargetingWidget` class for DRY targeting selection
  - Tabbed interface: Key-Value Pairs, Geography, Device, Audiences
  - Dual-pane key/value selector with search and filtering
  - Builds JSON automatically - no manual text editing required
  - Can be included in any form with `{% include %}`

- **Updated templates**:
  - `add_product.html`: Integrated targeting widget
  - `edit_product.html`: Integrated targeting widget with pre-population support
  - `targeting_browser.html`: Enhanced with async value loading

### Tests
- **New integration tests** (`tests/integration/test_targeting_values_endpoint.py`):
  - Validates real-time GAM queries with mock values
  - Tests empty results, tenant isolation, and authentication
  - Mocks GAM client to avoid real API calls

## Why This Matters
1. **Better UX**: Visual selector instead of manual JSON entry
2. **Real-time data**: Always shows current GAM configuration
3. **Lazy loading**: Values only queried when needed (performance)
4. **Code reuse**: Widget can be used in multiple places
5. **Proper isolation**: Full tenant isolation with GAM credential validation

## Technical Notes
- Values are not pre-synced (by design) - prevents database bloat
- Real-time queries are faster than syncing 10k+ values per key
- Widget updates hidden `targeting_template` field for form submission
- Full backward compatibility - existing products unaffected

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Correct GAMInventoryDiscovery import path in test mocking

The test was trying to patch 'src.admin.blueprints.inventory.GAMInventoryDiscovery'
but GAMInventoryDiscovery is imported inside the endpoint function, so it needs to be
patched at its definition location: 'src.adapters.gam_inventory_discovery.GAMInventoryDiscovery'.

This fixes the AttributeError in CI integration tests.

* fix: Temporarily disable targeting widget to resolve CI RecursionError

The targeting widget is causing a RecursionError during template rendering in CI
(but not locally). Temporarily commenting out the widget includes to unblock CI
while we investigate the root cause.

Affected tests:
- test_admin_ui_pages.py::test_create_product_page_renders
- test_admin_ui_routes_comprehensive.py::test_add_product_page

The targeting values endpoint and tests are working correctly. This is only
a template rendering issue in the CI environment.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: Use minimal targeting widget to isolate CI RecursionError

Testing if the RecursionError is caused by the size/complexity of the full widget.
This minimal version has only basic HTML/JS to verify template rendering works in CI.

If this passes CI, we'll know the issue is widget size/complexity, not the include
mechanism itself.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: Add v2 targeting widget with external JavaScript

Creates improved widget architecture to avoid CI RecursionError:

**New Architecture:**
- External JavaScript: static/js/targeting_widget.js (~300 lines)
  - Full TargetingWidget class with all functionality
  - Async data loading from API endpoints
  - Real-time value loading for selected keys
  - Tab-based UI (Key-Value, Geography, Device, Audiences)
  - Search and filtering
  - JSON generation for hidden field

- Simplified Template: targeting_selector_widget_v2.html (~280 lines)
  - CSS styles only
  - Minimal HTML structure
  - Script tag to load external JS

**Benefits:**
- Avoids template recursion by splitting large file
- Better separation of concerns (CSS/HTML vs JavaScript)
- Easier to maintain and debug
- Better browser caching of JavaScript
- Full visual targeting selector functionality

**Next Step:** Once CI confirms minimal widget works, switch to v2 widget.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: Enable full v2 targeting widget with visual selector

Activates the complete visual targeting selector in product forms:

**Changes:**
- Updated add_product.html to use targeting_selector_widget_v2.html
- Updated edit_product.html to use targeting_selector_widget_v2.html
- Removed minimal test widget (no longer needed)

**Full Feature Set Restored:**
✅ Visual selector UI with tabbed interface
✅ Real-time GAM key/value loading via API
✅ Custom Key-Value Pairs tab with dual-pane selector
✅ Geography, Device, and Audiences tabs
✅ Search and filtering within keys/values
✅ Live preview of selected targeting
✅ Automatic JSON generation for form submission
✅ Pre-population of existing targeting data (edit mode)

**Architecture Benefits:**
- External JavaScript (static/js/targeting_widget.js)
- Smaller template file (no recursion issues)
- Better browser caching
- Cleaner separation of concerns
- Production-ready

This completes the visual targeting selector feature. Users can now easily
configure product targeting through an intuitive UI instead of manual JSON entry.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: Consolidate targeting widget files and remove redundancy

Cleanup widget file structure:
- Removed original 619-line targeting_selector_widget.html (caused CI recursion)
- Renamed targeting_selector_widget_v2.html to targeting_selector_widget.html
- Updated add_product.html and edit_product.html to use standard filename
- No 'v2' suffix needed - this is now the only version

Result: Single, clean widget implementation with external JavaScript architecture.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
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