fix: universal sponsor pitch template (#361) #362

Merged

forgejo_admin merged 2 commits from 361-universal-sponsor-pitch into main 2026-04-06 23:48:03 +00:00

Conversation 2 Commits 2 Files changed 2 +76 -106

forgejo_admin commented 2026-04-06 23:38:43 +00:00

Owner

Copy link

Summary

Replace 9 category-specific sponsor pitches with Marcus's universal template that leads with nonprofit/EIN status, national platform reach, and 150K+ monthly Instagram views. Category no longer affects pitch selection; custom_pitch override preserved.

Changes

• src/basketball_api/services/sponsor_service.py — removed CATEGORY_PITCHES dict (9 entries), added single DEFAULT_PITCH string from Marcus's 2026-04-06 email. Simplified get_pitch() to return custom_pitch if set, else DEFAULT_PITCH. SponsorCategory import retained for use in list_sponsors() and blast_sponsors() signatures.
• tests/test_sponsor_blast.py — replaced CATEGORY_PITCHES import with DEFAULT_PITCH. Updated test_returns_category_default_when_no_custom to assert universal pitch content. Replaced test_all_nine_categories_have_pitches with test_pitch_ignores_category that verifies all 9 categories return the same DEFAULT_PITCH.

Test Plan

• pytest tests/test_sponsor_blast.py -v — 9/9 passed
• ruff check and ruff format --check — clean

Review Checklist

• CATEGORY_PITCHES dict removed, replaced with single DEFAULT_PITCH
• get_pitch(sponsor) returns custom_pitch if set, else DEFAULT_PITCH
• Category parameter no longer affects pitch selection
• Existing tests updated to reflect universal pitch
• custom_pitch override still works
• No unrelated changes

Related Notes

None.

Related

Closes #361

• Parent epic: #316
• Blast endpoint: #325

## Summary Replace 9 category-specific sponsor pitches with Marcus's universal template that leads with nonprofit/EIN status, national platform reach, and 150K+ monthly Instagram views. Category no longer affects pitch selection; custom_pitch override preserved. ## Changes - `src/basketball_api/services/sponsor_service.py` — removed `CATEGORY_PITCHES` dict (9 entries), added single `DEFAULT_PITCH` string from Marcus's 2026-04-06 email. Simplified `get_pitch()` to return `custom_pitch` if set, else `DEFAULT_PITCH`. `SponsorCategory` import retained for use in `list_sponsors()` and `blast_sponsors()` signatures. - `tests/test_sponsor_blast.py` — replaced `CATEGORY_PITCHES` import with `DEFAULT_PITCH`. Updated `test_returns_category_default_when_no_custom` to assert universal pitch content. Replaced `test_all_nine_categories_have_pitches` with `test_pitch_ignores_category` that verifies all 9 categories return the same `DEFAULT_PITCH`. ## Test Plan - `pytest tests/test_sponsor_blast.py -v` — 9/9 passed - `ruff check` and `ruff format --check` — clean ## Review Checklist - [x] CATEGORY_PITCHES dict removed, replaced with single DEFAULT_PITCH - [x] get_pitch(sponsor) returns custom_pitch if set, else DEFAULT_PITCH - [x] Category parameter no longer affects pitch selection - [x] Existing tests updated to reflect universal pitch - [x] custom_pitch override still works - [x] No unrelated changes ## Related Notes None. ## Related Closes #361 - Parent epic: #316 - Blast endpoint: #325

forgejo_admin added 1 commit 2026-04-06 23:38:43 +00:00

fix: replace category pitches with universal sponsor template (#361) ... ee7e05e89e

Replace 9 category-specific CATEGORY_PITCHES with a single DEFAULT_PITCH
derived from Marcus's latest outreach template. The universal pitch leads
with nonprofit/EIN status, national platform, and 150K+ IG views. Category
no longer affects pitch selection; custom_pitch override preserved.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-04-06 23:41:07 +00:00

Author

Owner

Copy link

PR #362 Review

DOMAIN REVIEW

Tech stack: Python / FastAPI / SQLAlchemy -- applying Python/FastAPI domain checks.

Simplification is sound. Removing 9 category-specific pitches in favor of one universal template is a clean refactor. Net -41 lines. get_pitch() logic is now trivially correct: return custom_pitch if set, else DEFAULT_PITCH. The SponsorCategory import is retained for signatures that still need it -- no dead import.

Tests are well-structured. Three unit tests cover:

1. Custom pitch override (happy path)
2. Default pitch return with content assertions (edge case)
3. Category-agnostic behavior across all 9 enum values (exhaustive)

The test_pitch_ignores_category test properly creates distinct sponsors per category (unique business_name and email) to avoid DB constraint collisions. Good.

Ruff compliance: PR body states ruff check and ruff format --check passed clean.

BLOCKERS

{{business_name}} placeholders will render as literal text in sent emails.

DEFAULT_PITCH contains two occurrences of {{business_name}}. The blast_sponsors() function (from PR #330) passes the pitch to load_email_template() in this dict:

{
    "business_name": sponsor.business_name,
    "pitch": pitch,
    ...
}

load_email_template() iterates over the dict in insertion order, calling html.replace(f"{{{{{key}}}}}", str(value)) for each key. Since business_name is processed BEFORE pitch, the replacement sequence is:

1. {{business_name}} in the original HTML template gets replaced (correct)
2. {{pitch}} gets replaced, injecting the pitch text that contains {{business_name}} into the HTML
3. The {{business_name}} strings now embedded in the HTML from step 2 are NEVER replaced -- the business_name key was already iterated past

Result: emails will contain literal {{business_name}} instead of the sponsor's actual business name.

Additionally, the plain-text fallback (body=pitch) sends raw DEFAULT_PITCH with no substitution at all -- {{business_name}} will always appear as literal text there too.

Fix options (pick one):

• (A) In get_pitch(), do return DEFAULT_PITCH.replace("{{business_name}}", sponsor.business_name) before returning.
• (B) Reorder the template data dict so pitch comes before business_name -- but this is fragile and does not fix the plain-text fallback.
• (C) Apply .replace() in blast_sponsors() after calling get_pitch().

Option (A) is cleanest -- get_pitch() already receives the sponsor object.

This is a BLOCKER because it causes user-visible broken email content in a production outreach system. The old CATEGORY_PITCHES did not contain {{business_name}} placeholders, so this is a regression introduced by this PR.

NITS

1. The test test_pitch_ignores_category could add an assertion that {{business_name}} is present in DEFAULT_PITCH to document the template variable expectation, or better yet, assert that get_pitch() output for a given sponsor does NOT contain {{business_name}} (once the fix above is applied).

2. Minor: the escaped newlines in DEFAULT_PITCH use \\n (literal backslash-n in Python string). This is correct for the plain-text body rendering, just noting it is intentional.

SOP COMPLIANCE

• Branch named after issue: 361-universal-sponsor-pitch matches issue #361
• PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related all present
• Related references parent epic: #316 (sponsor outreach system) and #325 (blast endpoint)
• Related references plan slug -- no plan slug referenced (Related Notes says "None"), acceptable for a fix ticket
• No secrets committed
• No unrelated changes -- only sponsor_service.py and test_sponsor_blast.py modified
• Tests exist and updated

PROCESS OBSERVATIONS

• Change failure risk: HIGH. The {{business_name}} substitution bug would cause broken emails to real sponsors. This is a production outreach system with blast capability. The fix is straightforward but must be applied before merge.
• Scope is tight. Two files changed, clear purpose, tests updated. Good ticket discipline.

VERDICT: NOT APPROVED

One blocker: {{business_name}} template placeholders in DEFAULT_PITCH will not be substituted by the current load_email_template() flow due to dict iteration order. Emails will contain literal {{business_name}} text. Fix get_pitch() to perform the substitution, update tests to verify, and re-submit.

## PR #362 Review ### DOMAIN REVIEW **Tech stack:** Python / FastAPI / SQLAlchemy -- applying Python/FastAPI domain checks. **Simplification is sound.** Removing 9 category-specific pitches in favor of one universal template is a clean refactor. Net -41 lines. `get_pitch()` logic is now trivially correct: return `custom_pitch` if set, else `DEFAULT_PITCH`. The `SponsorCategory` import is retained for signatures that still need it -- no dead import. **Tests are well-structured.** Three unit tests cover: 1. Custom pitch override (happy path) 2. Default pitch return with content assertions (edge case) 3. Category-agnostic behavior across all 9 enum values (exhaustive) The `test_pitch_ignores_category` test properly creates distinct sponsors per category (unique `business_name` and `email`) to avoid DB constraint collisions. Good. **Ruff compliance:** PR body states `ruff check` and `ruff format --check` passed clean. ### BLOCKERS **`{{business_name}}` placeholders will render as literal text in sent emails.** `DEFAULT_PITCH` contains two occurrences of `{{business_name}}`. The `blast_sponsors()` function (from PR #330) passes the pitch to `load_email_template()` in this dict: ```python { "business_name": sponsor.business_name, "pitch": pitch, ... } ``` `load_email_template()` iterates over the dict in insertion order, calling `html.replace(f"{{{{{key}}}}}", str(value))` for each key. Since `business_name` is processed BEFORE `pitch`, the replacement sequence is: 1. `{{business_name}}` in the original HTML template gets replaced (correct) 2. `{{pitch}}` gets replaced, injecting the pitch text that contains `{{business_name}}` into the HTML 3. The `{{business_name}}` strings now embedded in the HTML from step 2 are NEVER replaced -- the `business_name` key was already iterated past Result: emails will contain literal `{{business_name}}` instead of the sponsor's actual business name. Additionally, the plain-text fallback (`body=pitch`) sends raw `DEFAULT_PITCH` with no substitution at all -- `{{business_name}}` will always appear as literal text there too. **Fix options (pick one):** - (A) In `get_pitch()`, do `return DEFAULT_PITCH.replace("{{business_name}}", sponsor.business_name)` before returning. - (B) Reorder the template data dict so `pitch` comes before `business_name` -- but this is fragile and does not fix the plain-text fallback. - (C) Apply `.replace()` in `blast_sponsors()` after calling `get_pitch()`. Option (A) is cleanest -- `get_pitch()` already receives the sponsor object. **This is a BLOCKER** because it causes user-visible broken email content in a production outreach system. The old `CATEGORY_PITCHES` did not contain `{{business_name}}` placeholders, so this is a regression introduced by this PR. ### NITS 1. The test `test_pitch_ignores_category` could add an assertion that `{{business_name}}` is present in `DEFAULT_PITCH` to document the template variable expectation, or better yet, assert that `get_pitch()` output for a given sponsor does NOT contain `{{business_name}}` (once the fix above is applied). 2. Minor: the escaped newlines in `DEFAULT_PITCH` use `\\n` (literal backslash-n in Python string). This is correct for the plain-text body rendering, just noting it is intentional. ### SOP COMPLIANCE - [x] Branch named after issue: `361-universal-sponsor-pitch` matches issue #361 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related all present - [x] Related references parent epic: `#316` (sponsor outreach system) and `#325` (blast endpoint) - [ ] Related references plan slug -- no plan slug referenced (Related Notes says "None"), acceptable for a fix ticket - [x] No secrets committed - [x] No unrelated changes -- only `sponsor_service.py` and `test_sponsor_blast.py` modified - [x] Tests exist and updated ### PROCESS OBSERVATIONS - **Change failure risk: HIGH.** The `{{business_name}}` substitution bug would cause broken emails to real sponsors. This is a production outreach system with blast capability. The fix is straightforward but must be applied before merge. - **Scope is tight.** Two files changed, clear purpose, tests updated. Good ticket discipline. ### VERDICT: NOT APPROVED One blocker: `{{business_name}}` template placeholders in `DEFAULT_PITCH` will not be substituted by the current `load_email_template()` flow due to dict iteration order. Emails will contain literal `{{business_name}}` text. Fix `get_pitch()` to perform the substitution, update tests to verify, and re-submit.

forgejo_admin added 1 commit 2026-04-06 23:43:06 +00:00

fix: resolve {{business_name}} placeholders in get_pitch() ... de51376b12

QA caught that {{business_name}} in DEFAULT_PITCH would render as literal
text because load_email_template processes business_name before pitch in
the substitution loop. Now get_pitch() resolves the placeholder itself
using sponsor.business_name before returning.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-04-06 23:45:02 +00:00

Author

Owner

Copy link

PR #362 Review

DOMAIN REVIEW

Tech stack: Python / FastAPI / SQLAlchemy (basketball-api).

sponsor_service.py changes:

• CATEGORY_PITCHES dict (9 entries) correctly removed, replaced with single DEFAULT_PITCH string.
• DEFAULT_PITCH content matches Marcus's 2026-04-06 template: opens with nonprofit/EIN status, references national platform + 150K+ monthly Instagram views, includes support categories (tournament fees, gym rentals, gear), sponsorship types (financial, product, food vouchers, community), and return visibility (logo placement, social media, brand exposure, family network). Content is accurate to the issue spec.
• get_pitch() correctly returns custom_pitch if truthy, else DEFAULT_PITCH. The {{business_name}} substitution via .replace() is applied to whichever pitch is selected -- this means custom pitches containing {{business_name}} will also get substitution, which is a reasonable and intentional behavior (documented in the docstring).
• SponsorCategory import retained for use in list_sponsors() and blast_sponsors() signatures -- no dead import.

test_sponsor_blast.py changes:

• CATEGORY_PITCHES import removed, DEFAULT_PITCH not imported (tests verify behavior, not the constant directly). Good practice.
• test_returns_custom_pitch_when_set -- verifies custom_pitch override still works. Passes string without {{business_name}}, gets it back unchanged.
• test_returns_default_pitch_when_no_custom -- verifies universal pitch content ("registered nonprofit organization", "150,000+ views per month"), confirms {{business_name}} is resolved and literal placeholder is absent. Solid.
• test_pitch_ignores_category -- iterates all 9 SponsorCategory values with same business_name, asserts all return identical pitch. Correctly uses unique email per sponsor to avoid DB constraint violations. Replaces the old test_all_nine_categories_have_pitches.
• test_business_name_substituted_in_default -- explicit test for {{business_name}} resolution with "Joe's Pizza". Covers the key substitution behavior.

4 tests covering: custom override, default content, category independence, and placeholder substitution. Coverage is thorough.

BLOCKERS

None.

NITS

1. {{business_name}} in custom_pitch is a silent behavior change. The old get_pitch() returned custom_pitch verbatim. The new one runs .replace("{{business_name}}", ...) on it. If any existing custom_pitch values in the database contain the literal string {{business_name}}, they will now get substituted. This is almost certainly the desired behavior (and the docstring documents it), but there is no test for a custom_pitch that contains {{business_name}}. Consider adding one to lock in this behavior explicitly.

2. Escaped unicode in string literals -- \\u2014 (em dash) and \\u2022 (bullet) are used throughout DEFAULT_PITCH. These work correctly in Python string concatenation, but actual unicode characters would be more readable. Non-blocking style preference.

SOP COMPLIANCE

• Branch named after issue: 361-universal-sponsor-pitch
• PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related
• Related references parent epic (#316) and blast endpoint (#325)
• No secrets committed
• No unrelated changes (2 files, both directly relevant)
• Tests exist and PR body reports 9/9 passed + ruff clean

PROCESS OBSERVATIONS

Clean, focused change. Net -30 lines (76 additions, 106 deletions) -- simplification that removes category complexity in favor of a single proven template. Low change failure risk since the blast endpoint behavior is simplified, not expanded. The _make_sponsor helper accepting business_name and email kwargs implies good test fixture design that predates this PR.

VERDICT: APPROVED

## PR #362 Review ### DOMAIN REVIEW **Tech stack**: Python / FastAPI / SQLAlchemy (basketball-api). **sponsor_service.py changes**: - `CATEGORY_PITCHES` dict (9 entries) correctly removed, replaced with single `DEFAULT_PITCH` string. - `DEFAULT_PITCH` content matches Marcus's 2026-04-06 template: opens with nonprofit/EIN status, references national platform + 150K+ monthly Instagram views, includes support categories (tournament fees, gym rentals, gear), sponsorship types (financial, product, food vouchers, community), and return visibility (logo placement, social media, brand exposure, family network). Content is accurate to the issue spec. - `get_pitch()` correctly returns `custom_pitch` if truthy, else `DEFAULT_PITCH`. The `{{business_name}}` substitution via `.replace()` is applied to whichever pitch is selected -- this means custom pitches containing `{{business_name}}` will also get substitution, which is a reasonable and intentional behavior (documented in the docstring). - `SponsorCategory` import retained for use in `list_sponsors()` and `blast_sponsors()` signatures -- no dead import. **test_sponsor_blast.py changes**: - `CATEGORY_PITCHES` import removed, `DEFAULT_PITCH` not imported (tests verify behavior, not the constant directly). Good practice. - `test_returns_custom_pitch_when_set` -- verifies custom_pitch override still works. Passes string without `{{business_name}}`, gets it back unchanged. - `test_returns_default_pitch_when_no_custom` -- verifies universal pitch content ("registered nonprofit organization", "150,000+ views per month"), confirms `{{business_name}}` is resolved and literal placeholder is absent. Solid. - `test_pitch_ignores_category` -- iterates all 9 `SponsorCategory` values with same `business_name`, asserts all return identical pitch. Correctly uses unique `email` per sponsor to avoid DB constraint violations. Replaces the old `test_all_nine_categories_have_pitches`. - `test_business_name_substituted_in_default` -- explicit test for `{{business_name}}` resolution with "Joe's Pizza". Covers the key substitution behavior. 4 tests covering: custom override, default content, category independence, and placeholder substitution. Coverage is thorough. ### BLOCKERS None. ### NITS 1. **`{{business_name}}` in custom_pitch is a silent behavior change**. The old `get_pitch()` returned `custom_pitch` verbatim. The new one runs `.replace("{{business_name}}", ...)` on it. If any existing `custom_pitch` values in the database contain the literal string `{{business_name}}`, they will now get substituted. This is almost certainly the desired behavior (and the docstring documents it), but there is no test for a custom_pitch that contains `{{business_name}}`. Consider adding one to lock in this behavior explicitly. 2. **Escaped unicode in string literals** -- `\\u2014` (em dash) and `\\u2022` (bullet) are used throughout `DEFAULT_PITCH`. These work correctly in Python string concatenation, but actual unicode characters would be more readable. Non-blocking style preference. ### SOP COMPLIANCE - [x] Branch named after issue: `361-universal-sponsor-pitch` - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related - [x] Related references parent epic (#316) and blast endpoint (#325) - [x] No secrets committed - [x] No unrelated changes (2 files, both directly relevant) - [x] Tests exist and PR body reports 9/9 passed + ruff clean ### PROCESS OBSERVATIONS Clean, focused change. Net -30 lines (76 additions, 106 deletions) -- simplification that removes category complexity in favor of a single proven template. Low change failure risk since the blast endpoint behavior is simplified, not expanded. The `_make_sponsor` helper accepting `business_name` and `email` kwargs implies good test fixture design that predates this PR. ### VERDICT: APPROVED

forgejo_admin merged commit 2f8406a8d5 into main 2026-04-06 23:48:03 +00:00

forgejo_admin deleted branch 361-universal-sponsor-pitch 2026-04-06 23:48:03 +00:00

forgejo_admin referenced this pull request from a commit 2026-04-06 23:48:03 +00:00

fix: universal sponsor pitch template (#361) (#362)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

domain:backend

  

domain:devops

  

domain:frontend

  

status:approved

QA passed, awaiting merge approval

  

status:in-progress

Dev agent is actively working

  

status:needs-fix

QA found issues, back to dev

  

status:qa

PR submitted, awaiting QA review

  

type:bug

Bug fix

  

type:devops

Infrastructure/CI/config work

  

type:feature

New functionality

No labels

domain:backend

domain:devops

domain:frontend

status:approved

status:in-progress

status:needs-fix

status:qa

type:bug

type:devops

type:feature

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo_admin/basketball-api!362

No description provided.