Added DISCORD_INVITE_LINK #459

Merged

johnsturgeon merged 3 commits from move-discord-invite-url-to-config-454 into main

2026-09-03 13:28:49 +02:00

johnsturgeon commented

2026-09-03 13:23:55 +02:00

Owner

Closes #454

johnsturgeon added 1 commit

2026-09-03 13:23:56 +02:00

Added DISCORD_INVITE_LINK

Claude Code Review / claude-review (pull_request) Successful in 1m5s

Details

Python FastAPI Jinja Linting / build (pull_request) Successful in 27s

Details

Tests / pytest (pull_request) Successful in 13s

Details

e288f30f03

Closes #454

forgejo-actions commented

2026-09-03 13:25:00 +02:00

This change is clean and well-scoped. Here's my review:

Review: PR #459 — Added DISCORD_INVITE_LINK

This replaces three hardcoded https://discord.gg/f25zmnF links with a config.DISCORD_INVITE_LINK setting sourced from 1Password, following the established config pattern. The wiring is correct: both home routes already pass config into the template context (app/main.py:164, app/main.py:184), the mailer injects the value into the email body dict (app/mailer.py:54, app/mailer.py:68), and no stale discord.gg references remain. The change looks good.

A few small observations, none blocking:

Nits

app/config/config.py:89 — .rstrip("/") is defensive normalization copied from SITE_BASE_URL (config.py:106), but unlike SITE_BASE_URL the invite link is only ever used bare — never as a base for path concatenation like f"{url}/picks". It's harmless, so keep it if you prefer consistency, but it isn't buying anything here.
app/templates/home.j2:22 — the file still ends without a trailing newline (\ No newline at end of file). Untouched by this PR, so purely optional to fix while you're in here.
The unrelated  →  swap on app/templates/home.j2:5 is a sensible modernization ( is a deprecated element) and preserves the class/style, so behavior is unchanged. Just flagging that it's bundled in with the invite-link change.

Notes (no action needed)

Test suite is unaffected: tests/conftest.py:126 seeds env vars from config/op.env, which this PR updates, so DISCORD_INVITE_LINK gets an auto-generated test-DISCORD_INVITE_LINK placeholder and nothing KeyErrors.
There's no test asserting the invite link renders from config in the home page or emails, but that gap predates this PR and matches the existing coverage level — not worth adding on its own.

Approving as-is.

This change is clean and well-scoped. Here's my review: --- ## Review: PR #459 — Added DISCORD_INVITE_LINK This replaces three hardcoded `https://discord.gg/f25zmnF` links with a `config.DISCORD_INVITE_LINK` setting sourced from 1Password, following the established config pattern. The wiring is correct: both `home` routes already pass `config` into the template context (`app/main.py:164`, `app/main.py:184`), the mailer injects the value into the email body dict (`app/mailer.py:54`, `app/mailer.py:68`), and no stale `discord.gg` references remain. The change looks good. A few small observations, none blocking: ### Nits - `app/config/config.py:89` — `.rstrip("/")` is defensive normalization copied from `SITE_BASE_URL` (`config.py:106`), but unlike `SITE_BASE_URL` the invite link is only ever used bare — never as a base for path concatenation like `f"{url}/picks"`. It's harmless, so keep it if you prefer consistency, but it isn't buying anything here. - `app/templates/home.j2:22` — the file still ends without a trailing newline (`\ No newline at end of file`). Untouched by this PR, so purely optional to fix while you're in here. - The unrelated `` → `` swap on `app/templates/home.j2:5` is a sensible modernization (`` is a deprecated element) and preserves the `class`/`style`, so behavior is unchanged. Just flagging that it's bundled in with the invite-link change. ### Notes (no action needed) - Test suite is unaffected: `tests/conftest.py:126` seeds env vars from `config/op.env`, which this PR updates, so `DISCORD_INVITE_LINK` gets an auto-generated `test-DISCORD_INVITE_LINK` placeholder and nothing KeyErrors. - There's no test asserting the invite link renders from config in the home page or emails, but that gap predates this PR and matches the existing coverage level — not worth adding on its own. Approving as-is.

johnsturgeon added 1 commit

2026-09-03 13:27:01 +02:00

Missed one

Claude Code Review / claude-review (pull_request) Successful in 49s

Details

Python FastAPI Jinja Linting / build (pull_request) Has been cancelled

Details

Tests / pytest (pull_request) Has been cancelled

Details

0e4e29115d

johnsturgeon added 1 commit

2026-09-03 13:28:04 +02:00