[CORRUPTED] Synthetic Benchmark PR #36023 - FIX: translation missing when using optional_* placeholder in body

[tomerqodo]

User description

Benchmark PR discourse#36023

Type: Corrupted (contains bugs)

Original PR Title: FIX: translation missing when using optional_* placeholder in body
Original PR Description: All the "optional_*" placeholders are meant to be used in the subject but if you were to use them in the body template you would get a "translation missing" error.

This commit ensures these optional placeholders are also provided when i18ning the body even though they don't make a lot of sense.

Internal ref - t/150916
Original PR URL: discourse#36023

---

PR Type

Bug fix, Enhancement

---

Description

• Fix translation missing errors when using optional_* placeholders in email body templates

• Extract category and tags formatting into reusable helper methods

• Ensure optional placeholders are provided to body template i18n calls

• Add comprehensive test coverage for optional placeholders in body templates

---

Diagram Walkthrough

flowchart LR
  A["Email Body Template"] -->|"uses optional_* placeholders"| B["Message Builder"]
  B -->|"extracts formatting logic"| C["format_category & format_tags methods"]
  C -->|"provides to i18n"| D["Body Template Rendering"]
  D -->|"prevents translation missing"| E["Rendered Email Body"]

Loading

File Walkthrough

	Relevant files
Bug fix, enhancement, refactoring	message_builder.rb Extract formatting helpers and fix optional placeholders in body lib/email/message_builder.rb Extract category and tags formatting into separate format_category and format_tags helper methods Refactor subject method to use extracted helpers and improve code readability Ensure optional_* placeholders are passed to body template i18n calls Simplify template args escaping logic in body method +36/-49
Tests	message_builder_spec.rb Update body template test expectations                                      spec/lib/email/message_builder_spec.rb Update test expectations to include optional_* placeholders in body template i18n calls Verify that optional_re, optional_pm, optional_cat, and optional_tags are merged into template args +9/-1      user_notifications_spec.rb Add tests for optional placeholders in body templates        spec/mailers/user_notifications_spec.rb Add new test suite for optional placeholders in email body templates Verify that optional_tags, optional_cat, optional_pm, and optional_re render correctly in custom body templates Ensure no "translation missing" errors occur when using optional placeholders in body Clean up translation overrides after test execution +46/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing Audit Logs: New paths rendering email subjects and bodies (including translation overrides and placeholder handling) perform critical messaging actions without adding or verifying any audit logging of who triggered rendering and outcomes. Referred Code has_override = TranslationOverride.exists?( locale: I18n.locale, translation_key: "#{@opts[:template]}.subject_template", ) if @opts[:template] && has_override augmented_template_args = @template_args.merge( site_name: @template_args[:email_prefix], optional_re: @opts[:add_re_to_subject] ? I18n.t("subject_re") : "", optional_pm: @opts[:private_reply] ? @template_args[:subject_pm] : "", optional_cat: format_category, optional_tags: format_tags, topic_title: @template_args[:topic_title] ? @template_args[:topic_title] : "", ) subject = I18n.t("#{@opts[:template]}.subject_template", augmented_template_args) elsif @opts[:use_site_subject] subject = String.new(SiteSetting.email_subject) subject.gsub!("%{site_name}", @template_args[:email_prefix]) subject.gsub!("%{optional_re}", @opts[:add_re_to_subject] ? I18n.t("subject_re") : "") ... (clipped 74 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Unchecked I18n Call: Calls to I18n.t for subject and body rendering may fail or return missing translations but added code does not handle errors or provide fallbacks beyond default behavior. Referred Code subject = I18n.t("#{@opts[:template]}.subject_template", augmented_template_args) elsif @opts[:use_site_subject] subject = String.new(SiteSetting.email_subject) subject.gsub!("%{site_name}", @template_args[:email_prefix]) subject.gsub!("%{optional_re}", @opts[:add_re_to_subject] ? I18n.t("subject_re") : "") subject.gsub!("%{optional_pm}", @opts[:private_reply] ? @template_args[:subject_pm] : "") subject.gsub!("%{optional_cat}", format_tags) subject.gsub!("%{optional_tags}", format_category) if @template_args[:topic_title] subject.gsub!("%{topic_title}", @template_args[:topic_title]) end elsif @opts[:use_topic_title_subject] subject = @opts[:add_re_to_subject] ? I18n.t("subject_re") : "" subject = "#{subject}#{@template_args[:topic_title]}" elsif @opts[:template] subject = I18n.t("#{@opts[:template]}.subject_template", @template_args) else subject = @opts[:subject] end DiscoursePluginRegistry.apply_modifier(:message_builder_subject, subject, @opts, @to) ... (clipped 57 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	The PR introduces a significant regression The PR incorrectly swaps the format_tags and format_category methods when replacing placeholders in the email subject for site-wide settings. This bug needs to be fixed to ensure categories and tags are displayed correctly. Examples: lib/email/message_builder.rb [125-126] subject.gsub!("%{optional_cat}", format_tags) subject.gsub!("%{optional_tags}", format_category) Solution Walkthrough: Before: # lib/email/message_builder.rb def subject # ... elsif @opts[:use_site_subject] # ... subject.gsub!("%{optional_cat}", format_tags) subject.gsub!("%{optional_tags}", format_category) # ... end end def format_category # returns formatted category string end def format_tags # returns formatted tags string end After: # lib/email/message_builder.rb def subject # ... elsif @opts[:use_site_subject] # ... subject.gsub!("%{optional_cat}", format_category) subject.gsub!("%{optional_tags}", format_tags) # ... end end def format_category # returns formatted category string end def format_tags # returns formatted tags string end Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant regression in the subject method where placeholders for optional_cat and optional_tags are swapped, breaking email subject formatting.	High
Possible issue	Pass optional placeholders to body Augment the @template_args hash with the optional_* placeholder values before calling I18n.t for the email body to prevent translation errors. lib/email/message_builder.rb [193] -body = I18n.t("#{@opts[:template]}.text_body_template", @template_args).dup +augmented_template_args = + @template_args.merge( + optional_re: @opts[:add_re_to_subject] ? I18n.t("subject_re") : "", + optional_pm: @opts[:private_reply] ? @template_args[:subject_pm] : "", + optional_cat: format_category, + optional_tags: format_tags, + ) +body = I18n.t("#{@opts[:template]}.text_body_template", augmented_template_args).dup Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly points out that the PR's main goal of allowing optional placeholders in the email body is not fully implemented, as the new values are not passed to the body template, which would cause translation errors.	High
	Fix swapped subject placeholder replacements Swap the format_tags and format_category method calls in the gsub! replacements for %{optional_cat} and %{optional_tags} to ensure the correct values are inserted into the email subject. lib/email/message_builder.rb [125-126] -subject.gsub!("%{optional_cat}", format_tags) -subject.gsub!("%{optional_tags}", format_category) +subject.gsub!("%{optional_cat}", format_category) +subject.gsub!("%{optional_tags}", format_tags) Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a copy-paste error where the arguments for replacing category and tag placeholders in the email subject are swapped, which is a clear bug introduced by the PR.	Medium
More