Skip to content

Fix Liquid errors in no-ticket reminder notification template#11212

Merged
nbudin merged 6 commits intomainfrom
fix-no-ticket-reminder-liquid-errors
Mar 5, 2026
Merged

Fix Liquid errors in no-ticket reminder notification template#11212
nbudin merged 6 commits intomainfrom
fix-no-ticket-reminder-liquid-errors

Conversation

@nbudin
Copy link
Contributor

@nbudin nbudin commented Mar 5, 2026

Summary

  • Created SignupRankedChoiceDrop so Liquid templates can access target_run on queue items
  • Added to_liquid to SignupRankedChoice (moved before private so it's publicly callable)
  • Added title_suffix to RunDrop's delegate list so templates can use it
  • Fixed .to_a call in notifier so the queue_items collection is Liquid-iterable
  • Fixed the ticket URL in the template — absolute_url expects a relative path, not an absolute one

Test plan

  • All existing RemindQueueWithoutTicketTest tests pass
  • New test asserts rendered email body (text and HTML parts) contains no Liquid errors

🤖 Generated with Claude Code

nbudin and others added 6 commits March 5, 2026 12:33
Three bugs caused Liquid errors when rendering the template:

1. SignupRankedChoice lacked to_liquid, so passing the AR relation to
   Liquid caused "undefined method 'to_liquid'" errors. Added
   SignupRankedChoiceDrop and to_liquid on the model; also convert
   queue_items to an array in the notifier so Liquid can iterate it.

2. RunDrop didn't expose title_suffix, causing the run title suffix
   to be inaccessible in the template.

3. The ticket URL used `convention.url | append: '/ticket/new' |
   absolute_url`, but absolute_url expects a relative path. Changed
   to `'/ticket/new' | absolute_url`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nder

Move to_liquid before private in SignupRankedChoice so it's callable by
the Liquid template engine. Add a test asserting the rendered email body
contains no Liquid errors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Liquid treats empty strings as truthy, so use != blank instead of a
simple presence check to avoid rendering empty parentheses for events
with no title suffix.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers the errors.add branch in ensure_all_fields_point_at_the_same_convention,
bringing signup_ranked_choice.rb to 100% line coverage.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
merge-cobertura keeps each test suite as a separate package, so the
coverage action's per-package comparison flags false regressions when
a file is fully covered in unit tests but not in system tests. The new
script merges all three reports into a single package by summing hits
per line, giving an accurate combined coverage for each file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
Overall Coverage 🟢 56.67% 🟢 100% 🟢 43.33%

Minimum allowed coverage is 0%, this run produced 100%

@nbudin nbudin added bug patch Bumps the patch version number on release labels Mar 5, 2026
@nbudin nbudin marked this pull request as ready for review March 5, 2026 21:42
@nbudin nbudin merged commit a8f3d24 into main Mar 5, 2026
19 checks passed
@nbudin nbudin deleted the fix-no-ticket-reminder-liquid-errors branch March 5, 2026 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug patch Bumps the patch version number on release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant