Skip to content

Simplify tenant setup flow with tab completion indicators#16

Merged
bokelley merged 6 commits into
mainfrom
tenant-setup-wizard
Aug 1, 2025
Merged

Simplify tenant setup flow with tab completion indicators#16
bokelley merged 6 commits into
mainfrom
tenant-setup-wizard

Conversation

@bokelley
Copy link
Copy Markdown
Collaborator

@bokelley bokelley commented Aug 1, 2025

Summary

  • Simplified tenant setup by removing the separate wizard and using existing tabs
  • Replaced asterisks with warning emojis (⚠️) for clearer incomplete status indicators
  • Improved separation between super admin tenant creation and publisher configuration

Changes

Removed Setup Wizard

  • Removed /tenant/<tenant_id>/setup route and associated templates
  • Simplified flow to use existing Ad Server Setup tab for initial configuration

UI Improvements

  • Changed completion indicators from asterisks (*) to warning emojis (⚠️)
  • Updated tenant creation success message to guide publishers to Ad Server Setup tab
  • Removed "billing plan" field as it's not relevant for open-source project
  • Clarified subdomain field to indicate both subdomain and path-based routing options

Field Updates

  • "Max Daily Budget" now clarifies it's a safety limit per media buy
  • "Enable AEE" renamed to "AEE integration is complete" with better help text

Test plan

  • Create new tenant as super admin
  • Log in as publisher and see setup incomplete warning
  • Complete Ad Server Setup tab
  • Verify warning emojis appear on incomplete tabs
  • Complete all required tabs and verify warnings disappear
  • Test both subdomain and path-based routing options

🤖 Generated with Claude Code

bokelley and others added 6 commits August 1, 2025 13:10
- Created multi-step wizard UI for tenant configuration
- Step 1: Choose ad server (GAM, Kevel, Triton, Mock)
- Step 2: Select standard IAB ad formats
- Step 3: Add custom ad formats
- Step 4: Configure initial products with pricing
- Step 5: Add initial principals (advertisers)
- Added wizard route and handler in admin_ui.py
- Fixed admin UI startup issues with Flask debug mode
- Fixed PostgreSQL JSON DISTINCT query issue
- Fixed Alembic migration reference (002 -> initial_schema)
- Added separate entrypoint for admin UI with migrations
- Updated main tenant list page with wizard button

The wizard provides a much more intuitive onboarding experience
compared to the previous single complex form, guiding users
through each configuration step with helpful context.

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Merged all features from /create_tenant into the wizard:
  - Added subdomain and billing plan fields to wizard start page
  - Added authorization (emails/domains) to wizard start page
  - Added GAM company ID field to Step 1
  - Added Platform Features section to Step 1 (max daily budget, human review, AEE)
  - Pass all new fields through wizard steps as hidden inputs
  - Updated POST handler to use all new fields

- Replaced old /create_tenant route with redirect to wizard
- Removed duplicate create_tenant.html template
- Updated main tenant list to have single "Create New Tenant" button

This provides a single, unified tenant creation flow that includes
all configuration options in a guided step-by-step process.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Split the combined wizard into two distinct workflows:

1. **Super Admin: Create Tenant** (/create_tenant)
   - Simple form with just essential fields
   - Tenant name, subdomain, billing plan
   - Authorization (emails/domains)
   - Creates tenant with setup_complete=false flag

2. **Publisher: Setup Wizard** (/tenant/<tenant_id>/setup)
   - Multi-step wizard for publishers to configure their tenant
   - Connect ad server (GAM, Kevel, etc.)
   - Configure ad formats and custom formats
   - Set up initial products with pricing
   - Add advertisers (principals)
   - Only accessible by tenant admins and super admins
   - Updates existing tenant configuration

Added setup detection:
- Tenant detail page shows warning if setup not complete
- Setup wizard redirects if already completed
- Clean separation of concerns between platform admin and publisher

This correctly models the real-world flow where platform operators
create tenant accounts, then publishers complete their own setup.

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

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove billing_plan field from create_tenant form (not relevant for open source)
- Update subdomain field label to clarify both subdomain and path-based routing are supported
- Remove billing_plan from database queries and admin UI
- Update index page to show 'Subdomain / Path' header

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

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

- Replace complex multi-step wizard with simple initial setup (ad server + platform settings)
- Add completion indicators (*) to tabs that need configuration
- Show setup incomplete warning when required tabs are not configured
- Separate super admin tenant creation from publisher setup flow
- Remove billing plan field (not relevant for open source)
- Update subdomain label to clarify both subdomain and path-based routing
- Clarify platform settings (max daily budget per media buy, AEE integration status)

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

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

- Remove separate tenant setup wizard and templates
- Use existing Ad Server Setup tab for initial configuration
- Replace asterisks with warning emojis (⚠️) for incomplete tabs
- Update tenant creation flow to guide users to Ad Server Setup tab
- Simplify setup process based on user feedback
@bokelley bokelley merged commit 5162085 into main Aug 1, 2025
3 checks passed
@bokelley bokelley deleted the tenant-setup-wizard branch August 1, 2025 15:56
bokelley added a commit that referenced this pull request Sep 15, 2025
Simplify tenant setup flow with tab completion indicators
ChrisHuie added a commit that referenced this pull request Apr 19, 2026
…uards (L0-01)

Per .claude/notes/flask-to-fastapi/L0-implementation-plan-v2.md §L0-01,
landing rows #1-#16 of the §5.5 Structural Guards Inventory plus guard #34
(no_werkzeug_imports, reassigned to L0-01 per v2 plan §7.1 RATIFIED).

Guards (each with AST scanner + main test + planted-violation meta-fixture):
- test_architecture_no_flask_imports.py                (#1, Captured→shrink)
- test_architecture_handlers_use_sync_def.py           (#2, frozen carve-out)
- test_architecture_no_async_db_access.py              (#3)
- test_architecture_middleware_order.py                (#4, DORMANT)
- test_architecture_exception_handlers_complete.py     (#5, DORMANT)
- test_architecture_csrf_exempt_covers_adcp.py         (#6, DORMANT)
- test_architecture_approximated_middleware_path_gated.py (#7, DORMANT)
- test_architecture_admin_routes_excluded_from_openapi.py (#8, DORMANT)
- test_architecture_admin_routes_named.py              (#9, DORMANT)
- test_architecture_admin_route_names_unique.py        (#10, DORMANT)
- test_architecture_no_module_scope_create_app.py      (#11, Captured→shrink)
- test_architecture_scheduler_lifespan_composition.py  (#12)
- test_architecture_a2a_routes_grafted.py              (#13)
- test_architecture_form_getlist_parity.py             (#14, Captured→shrink)
- test_architecture_no_module_level_engine.py          (#15, Captured→shrink)
- test_architecture_no_direct_env_access.py            (#16, Captured→shrink)
- test_architecture_no_werkzeug_imports.py             (#34, Captured→shrink)

Shared helpers in tests/unit/architecture/_ast_helpers.py (walk_py_files,
read_allowlist, relpath) per the DRY invariant in CLAUDE.md.

Captured→shrink baselines seeded:
- allowlists/no_flask_imports.txt           (40 files importing flask)
- allowlists/no_werkzeug_imports.txt        (6 files importing werkzeug)
- allowlists/module_scope_create_app.txt    (7 tests/*.py files)
- allowlists/no_form_getlist.txt            (3 admin routers)
- allowlists/no_module_level_engine.txt     (2 legacy gam services)
- allowlists/no_direct_env_access.txt       (121 path:line call sites)

Meta-guard `test_structural_guard_allowlist_monotonic.py` (§5.5 row #28)
enforcing allowlist monotonicity will land in a later L0 work item.

DORMANT guards (scanner present; repo state trivially compliant until
L0-04..L0-18 foundation modules land): #4, #5, #6, #7, #8, #9, #10. Each has
a planted-violation fixture that DOES fire the scanner; dormancy is about
the repo state, not scanner correctness.

Guard #15 landed as a new guard (the sibling test_architecture_no_module_level_get_engine.py
referenced in the plan does not exist in the pre-L0 state, so #15 is
not redundant with it).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley referenced this pull request in bokelley/salesagent May 5, 2026
…surfaces

5 of the 15 round-1 SDK_FEEDBACK asks already shipped in adcp main since
the original feedback (commit 68699763):

  ✅ #1  Dimensions/Renders on adcp.types public surface
  ✅ #2  MediaBuyFeatures, AiTool on adcp.types public surface
  ✅ #10 adcp.testing.make_request_context() helper

salesagent code refactored to use the new public surfaces:
- src/core/tools/capabilities.py: from adcp.types import MediaBuyFeatures
- src/core/schemas/creative.py:    from adcp.types import AiTool
- tests/unit/test_creative_formats_behavioral.py:    Dimensions, Renders
- tests/integration/test_creative_formats_*.py: same
- tests/factories/format.py: same
- core/tests/test_mock_platform_get_products.py: make_request_context()

5 core/ tests still green after refactor.

Round 2 of asks added (8 new items, framed as "what should the SDK own
so adopters write less"):

  #16 IdempotencyBackend.PgBackend is a scaffold — finish it, salesagent
      ships-ready to test. Real blocker for multi-worker dedup.
  #17 Capability-vs-store-wired silent mismatch — boot-time validator
      that catches "advertised X, didn't wire X store" before adopters
      lie to buyers in production.
  #18 First-class TokenAuthMiddleware — every multi-tenant token-auth
      adopter writes the same 50 LOC bridge from x-adcp-auth header
      to caller_identity. salesagent has it; would happily PR.
  #19 DefaultWebhookSender — hello_seller.py opts out of webhooks
      because there's no default sender. Sender should derive from
      supervisor by default.
  #20 DbBackedSubdomainTenantRouter — every multi-tenant adopter writes
      this glue (see core/main.py::_load_tenant_subdomain_map).
      Working impl available to port.
  #21 LazyPlatformRouter — eager construction at boot doesn't scale to
      N tenants × per-tenant SDK auth handshakes.
  #22 (stretch) adcp.upstream.gam helper for the SA-creds + cached-client
      pattern. ~30 LOC universal across salesagent-shaped GAM adopters.
  #23 (stretch) Placement→Product wire-shape projection helper.
  #24 build_asgi_app (carried from round 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley referenced this pull request in bokelley/salesagent May 5, 2026
SDK adcp-client-python#555 (complete PgBackend impl) merged. Wires it
into core/platforms/{mock,gam}.py via a process-singleton store factory
in core/idempotency.py — auto-detects DATABASE_URL, opens an
AsyncConnectionPool, calls create_schema() on boot to ensure the
adcp_idempotency table exists.

Selection logic (CORE_IDEMPOTENCY_BACKEND env var, default 'auto'):
- 'memory' → always MemoryBackend (test default for storyboard runner)
- 'auto' + no DATABASE_URL → MemoryBackend (tooling/import path:
  OpenAPI export, alembic autogen, unit-test collection)
- 'auto' + DATABASE_URL set → PgBackend (production default)
- 'pg' → force PgBackend (errors if DATABASE_URL missing)

Required psycopg3 + psycopg-pool (PgBackend wants AsyncConnectionPool;
salesagent's existing psycopg2-binary stays for SQLAlchemy). Both pinned
in pyproject.toml.

Verified end-to-end on the live core docker stack:
  Idempotency: PgBackend ready (pool open, adcp_idempotency table ensured)
  Table "public.adcp_idempotency"
    scope_key text C not null
    key text C not null
    payload_hash text not null
    response jsonb not null
    expires_at timestamp with time zone not null
    PRIMARY KEY (scope_key, key)

4 focused tests cover the matrix:
- memory backend when env says memory
- memory backend when no DATABASE_URL (tooling fallback)
- PgBackend when DATABASE_URL set + table bootstrapped
- store is process singleton (repeated calls return same instance)

**Atomicity caveat (SDK #555 docstring).** PgBackend.put commits on a
fresh pool connection — separate from the handler's business transaction.
Handlers that mutate state without a unique constraint on their
idempotency_key may double-execute on a crash between handler success
and cache commit. _create_media_buy_impl is non-idempotent today (no
unique constraint on tenant_id + idempotency_key). Tracked for follow-up
in sprint 1.6 piece C — a unique constraint on media_buys would close
the gap, OR adopt the SDK's planned co-tx variant once it ships.

SDK_FEEDBACK item #16 marked closed.
ChrisHuie added a commit that referenced this pull request May 22, 2026
…llow-up

Addresses items #6, #12, #16, #17, #19, #20, #23, #24 from the
2026-05-20 review (small mechanical fixes; no architectural change):

- #6 + #17: ``extract_error_info`` envelope branch now coerces
  ``recovery`` through ``_coerce_recovery()``, which validates the value
  against ``typing.get_args(RecoveryHint)`` instead of a hard-coded tuple
  duplicating the Literal. Both the envelope and legacy ToolError branches
  go through the same helper, so a future RecoveryHint extension is picked
  up automatically.

- #12: ``_body_contains_builder_call`` docstring now correctly says
  "N-level transitive call analysis with cycle detection via ``seen``"
  (was "1-level transitive", but the implementation actually walks the
  full call graph with cycle detection).

- #16: dropped the duplicate ``logger.warning`` in ``on_message_send``'s
  AdCPError handler — ``_handle_explicit_skill`` already logs the same
  error (with audit log + activity feed) before raising. Two log lines
  for the same failure was noise.

- #19: ``_translate_to_tool_error`` now uses ``raise error`` (not bare
  ``raise``) on its passthrough branches so the ``NoReturn`` contract
  holds even when called outside an active ``except`` block.

- #20: ``_handle_tool_error`` defensively copies ``e.envelope`` before
  returning to ``JSONResponse`` (preserves the envelope-builder's
  immutability contract — the dict is owned by the AdCPToolError instance
  and may be referenced elsewhere).

- #23: ``_handle_tool_exception`` Context extraction switched from
  ``hasattr(arg, "tenant_id")`` to explicit ``isinstance(arg, (FastMCPContext,
  ToolContext))`` — the hasattr check matched any Pydantic model with a
  ``tenant_id`` field, treating request bodies as Contexts.

- #24: hoisted ``from fastmcp.exceptions import ToolError`` to the top
  imports in ``src/app.py``; dropped the mid-file ``# noqa: E402``.

Local quality: 4682 passed, 1 skipped, 20 xfailed.

Deferred (separate scope):
- #5 byte-identical test, #7 delete per-boundary wrappers (30+ call
  sites), #9 decorator extraction, #10/14/25 move AdCPToolError to
  neutral module, #11 fail_step caller (#1311 coordination), #13
  CWD-relative paths, #15 dead legacy fallback, #18 already softened
  in b5c4356, #21 status_code keyword-only, #22 synthetic mutation.
ChrisHuie added a commit that referenced this pull request May 22, 2026
…llow-up

Addresses items #6, #12, #16, #17, #19, #20, #23, #24 from the
2026-05-20 review (small mechanical fixes; no architectural change):

- #6 + #17: ``extract_error_info`` envelope branch now coerces
  ``recovery`` through ``_coerce_recovery()``, which validates the value
  against ``typing.get_args(RecoveryHint)`` instead of a hard-coded tuple
  duplicating the Literal. Both the envelope and legacy ToolError branches
  go through the same helper, so a future RecoveryHint extension is picked
  up automatically.

- #12: ``_body_contains_builder_call`` docstring now correctly says
  "N-level transitive call analysis with cycle detection via ``seen``"
  (was "1-level transitive", but the implementation actually walks the
  full call graph with cycle detection).

- #16: dropped the duplicate ``logger.warning`` in ``on_message_send``'s
  AdCPError handler — ``_handle_explicit_skill`` already logs the same
  error (with audit log + activity feed) before raising. Two log lines
  for the same failure was noise.

- #19: ``_translate_to_tool_error`` now uses ``raise error`` (not bare
  ``raise``) on its passthrough branches so the ``NoReturn`` contract
  holds even when called outside an active ``except`` block.

- #20: ``_handle_tool_error`` defensively copies ``e.envelope`` before
  returning to ``JSONResponse`` (preserves the envelope-builder's
  immutability contract — the dict is owned by the AdCPToolError instance
  and may be referenced elsewhere).

- #23: ``_handle_tool_exception`` Context extraction switched from
  ``hasattr(arg, "tenant_id")`` to explicit ``isinstance(arg, (FastMCPContext,
  ToolContext))`` — the hasattr check matched any Pydantic model with a
  ``tenant_id`` field, treating request bodies as Contexts.

- #24: hoisted ``from fastmcp.exceptions import ToolError`` to the top
  imports in ``src/app.py``; dropped the mid-file ``# noqa: E402``.

Local quality: 4682 passed, 1 skipped, 20 xfailed.

Deferred (separate scope):
- #5 byte-identical test, #7 delete per-boundary wrappers (30+ call
  sites), #9 decorator extraction, #10/14/25 move AdCPToolError to
  neutral module, #11 fail_step caller (#1311 coordination), #13
  CWD-relative paths, #15 dead legacy fallback, #18 already softened
  in b5c4356, #21 status_code keyword-only, #22 synthetic mutation.
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.

1 participant