forgejo_admin/westside-emails
1
0
Fork 0
Code Issues 3 Pull requests Projects Releases Packages Wiki Activity Actions

fix: replace hardcoded jersey image URLs with template placeholders #11

Merged
forgejo_admin merged 1 commit from 10-jersey-image-placeholders into main 2026-04-08 21:20:07 +00:00
Conversation 2 Commits 1 Files changed 1 +2 -2
forgejo_admin commented 2026-04-08 21:01:24 +00:00
Owner
Copy link

Summary

Replace the two hardcoded Queens jersey image URLs in jersey-reminder.mjml with Jinja-style template placeholders ({{jersey_image_1}} and {{jersey_image_2}}), allowing callers to supply image URLs at render time.

Changes

  • src/jersey-reminder.mjml: Replaced hardcoded MinIO URLs for Option 1 and Option 2 jersey images with {{jersey_image_1}} and {{jersey_image_2}} respectively

Test Plan

  • Compile MJML: npx mjml src/jersey-reminder.mjml -o dist/jersey-reminder.html
  • Verify the compiled HTML contains {{jersey_image_1}} and {{jersey_image_2}} in the <img> tags
  • Verify no hardcoded minio-api.tail5b443a.ts.net URLs remain in the template

Review Checklist

  • MJML compiles without errors
  • Template placeholders match expected variable names
  • No hardcoded image URLs remain

Related Notes

Related

## Summary Replace the two hardcoded Queens jersey image URLs in `jersey-reminder.mjml` with Jinja-style template placeholders (`{{jersey_image_1}}` and `{{jersey_image_2}}`), allowing callers to supply image URLs at render time. ## Changes - `src/jersey-reminder.mjml`: Replaced hardcoded MinIO URLs for Option 1 and Option 2 jersey images with `{{jersey_image_1}}` and `{{jersey_image_2}}` respectively ## Test Plan - Compile MJML: `npx mjml src/jersey-reminder.mjml -o dist/jersey-reminder.html` - Verify the compiled HTML contains `{{jersey_image_1}}` and `{{jersey_image_2}}` in the `<img>` tags - Verify no hardcoded `minio-api.tail5b443a.ts.net` URLs remain in the template ## Review Checklist - [x] MJML compiles without errors - [x] Template placeholders match expected variable names - [x] No hardcoded image URLs remain ## Related Notes - Part 1 of forgejo_admin/westside-emails#10 ## Related - Closes #10
Part 1 of forgejo_admin/westside-emails#10. Swap the two Queens jersey
image URLs for {{jersey_image_1}} and {{jersey_image_2}} so callers can
supply images at render time.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
forgejo_admin commented 2026-04-08 21:02:24 +00:00
Author
Owner
Copy link

QA Review

Scope: 1 file changed, 2 insertions, 2 deletions.

Diff Review

  • src/jersey-reminder.mjml: Two hardcoded MinIO image URLs replaced with {{jersey_image_1}} and {{jersey_image_2}}. Exact substitution, no surrounding markup changed. Alt text, width, border-radius, and padding all preserved.

Checks

  • MJML compiles without errors
  • Placeholder names are consistent Jinja-style ({{jersey_image_1}}, {{jersey_image_2}})
  • No hardcoded minio-api.tail5b443a.ts.net URLs remain in the template
  • PR body references parent issue (Closes #10)
  • Branch naming follows convention (10-jersey-image-placeholders)
  • Commit message is descriptive and includes co-author

Notes

  • dist/ is gitignored, so compiled output is correctly excluded from the commit. Callers will need to compile after checkout.
  • This is Part 1 of #10 -- Closes #10 in the PR body will auto-close the issue on merge. If Part 2 is still pending, consider changing to Part of #10 to avoid premature closure.

VERDICT: APPROVED

## QA Review **Scope**: 1 file changed, 2 insertions, 2 deletions. ### Diff Review - `src/jersey-reminder.mjml`: Two hardcoded MinIO image URLs replaced with `{{jersey_image_1}}` and `{{jersey_image_2}}`. Exact substitution, no surrounding markup changed. Alt text, width, border-radius, and padding all preserved. ### Checks - [x] MJML compiles without errors - [x] Placeholder names are consistent Jinja-style (`{{jersey_image_1}}`, `{{jersey_image_2}}`) - [x] No hardcoded `minio-api.tail5b443a.ts.net` URLs remain in the template - [x] PR body references parent issue (`Closes #10`) - [x] Branch naming follows convention (`10-jersey-image-placeholders`) - [x] Commit message is descriptive and includes co-author ### Notes - `dist/` is gitignored, so compiled output is correctly excluded from the commit. Callers will need to compile after checkout. - This is Part 1 of #10 -- `Closes #10` in the PR body will auto-close the issue on merge. If Part 2 is still pending, consider changing to `Part of #10` to avoid premature closure. **VERDICT: APPROVED**
forgejo_admin commented 2026-04-08 21:11:23 +00:00
Author
Owner
Copy link

PR #11 Review

DOMAIN REVIEW

Tech stack: MJML email templates (HTML email domain).

The change is surgical: two hardcoded MinIO image URLs replaced with Jinja-style placeholders in src/jersey-reminder.mjml.

  • Line 42: https://minio-api.tail5b443a.ts.net/assets/westside/jerseys/IMG_4164.jpeg replaced with {{jersey_image_1}} -- correct.
  • Line 78: https://minio-api.tail5b443a.ts.net/assets/westside/jerseys/IMG_4165.jpeg replaced with {{jersey_image_2}} -- correct.
  • Surrounding attributes (alt, width, border-radius) are untouched.
  • {{name}} and {{jersey_url}} are outside the changed line ranges and unaffected.
  • No hardcoded MinIO URLs remain in the changed sections.

Observation: The PR body mentions compiled HTML should also be updated, but the diff shows only 1 file changed (src/jersey-reminder.mjml). If the repo convention is to commit compiled HTML to dist/, the compiled output is missing from this PR. If compilation happens downstream (CI or at render time), this is fine. Flagging as a nit since the PR body's own test plan says "Compile MJML" but no compiled artifact is in the diff.

BLOCKERS

None.

  • No new functionality requiring tests (template variable substitution in a static MJML file).
  • No user input validation concerns (placeholders are populated server-side at render time).
  • No secrets or credentials (the removed URLs were public asset paths, not credentials).
  • No auth/security path changes.

NITS

  1. Missing compiled HTML: If dist/jersey-reminder.html is tracked in the repo, it should be recompiled and committed to match the MJML source. The diff only shows the MJML change. If it is not tracked (gitignored), disregard.

SOP COMPLIANCE

  • Branch named after issue (10-jersey-image-placeholders matches issue #10)
  • PR body follows template (Summary, Changes, Test Plan, Related)
  • Related references issue (Closes #10)
  • No secrets committed
  • No unnecessary file changes (1 file, 4 lines, exactly scoped)

PROCESS OBSERVATIONS

Clean, minimal change. Low change failure risk. The placeholder approach correctly decouples image assets from the template, enabling division-aware image injection at render time as described in issue #10.

VERDICT: APPROVED

## PR #11 Review ### DOMAIN REVIEW **Tech stack**: MJML email templates (HTML email domain). The change is surgical: two hardcoded MinIO image URLs replaced with Jinja-style placeholders in `src/jersey-reminder.mjml`. - **Line 42**: `https://minio-api.tail5b443a.ts.net/assets/westside/jerseys/IMG_4164.jpeg` replaced with `{{jersey_image_1}}` -- correct. - **Line 78**: `https://minio-api.tail5b443a.ts.net/assets/westside/jerseys/IMG_4165.jpeg` replaced with `{{jersey_image_2}}` -- correct. - Surrounding attributes (`alt`, `width`, `border-radius`) are untouched. - `{{name}}` and `{{jersey_url}}` are outside the changed line ranges and unaffected. - No hardcoded MinIO URLs remain in the changed sections. **Observation**: The PR body mentions compiled HTML should also be updated, but the diff shows only 1 file changed (`src/jersey-reminder.mjml`). If the repo convention is to commit compiled HTML to `dist/`, the compiled output is missing from this PR. If compilation happens downstream (CI or at render time), this is fine. Flagging as a nit since the PR body's own test plan says "Compile MJML" but no compiled artifact is in the diff. ### BLOCKERS None. - No new functionality requiring tests (template variable substitution in a static MJML file). - No user input validation concerns (placeholders are populated server-side at render time). - No secrets or credentials (the removed URLs were public asset paths, not credentials). - No auth/security path changes. ### NITS 1. **Missing compiled HTML**: If `dist/jersey-reminder.html` is tracked in the repo, it should be recompiled and committed to match the MJML source. The diff only shows the MJML change. If it is not tracked (gitignored), disregard. ### SOP COMPLIANCE - [x] Branch named after issue (`10-jersey-image-placeholders` matches issue #10) - [x] PR body follows template (Summary, Changes, Test Plan, Related) - [x] Related references issue (`Closes #10`) - [x] No secrets committed - [x] No unnecessary file changes (1 file, 4 lines, exactly scoped) ### PROCESS OBSERVATIONS Clean, minimal change. Low change failure risk. The placeholder approach correctly decouples image assets from the template, enabling division-aware image injection at render time as described in issue #10. ### VERDICT: APPROVED
forgejo_admin deleted branch 10-jersey-image-placeholders 2026-04-08 21:20:07 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
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/westside-emails!11
No description provided.