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

[tomerqodo]

User description

Benchmark PR discourse#36023

Type: Clean (correct implementation)

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

• Provide optional_* placeholders to body template i18n calls

• Add comprehensive test coverage for optional placeholders in body templates

• Refactor subject method for improved code clarity and maintainability

---

Diagram Walkthrough

flowchart LR
  A["Email Body Template"] -->|"Missing optional_* placeholders"| B["Translation Missing Error"]
  C["Refactored Code"] -->|"Provides optional_re, optional_pm, optional_cat, optional_tags"| D["Body Renders Correctly"]
  E["Helper Methods"] -->|"format_category, format_tags"| F["DRY Code Reuse"]

Loading

File Walkthrough

	Relevant files
Bug fix	message_builder.rb Add optional placeholders to body template i18n                    lib/email/message_builder.rb Extract format_category and format_tags into dedicated helper methods to eliminate code duplication Refactor subject method to improve readability by extracting has_override variable Add optional placeholder arguments (optional_re, optional_pm, optional_cat, optional_tags) to body template i18n call to prevent translation missing errors Simplify template args escaping logic in body method using inline conditional +44/-49
Tests	message_builder_spec.rb Update body template test expectations                                      spec/lib/email/message_builder_spec.rb Update test expectations to include optional placeholder arguments in body template i18n call Verify that optional_re, optional_pm, optional_cat, and optional_tags are passed with empty string defaults +9/-1      user_notifications_spec.rb Add comprehensive optional placeholders body test                spec/mailers/user_notifications_spec.rb Add new test suite "optional placeholders in email body" to verify optional_* placeholders render correctly in body templates Test that optional_tags, optional_cat, optional_pm, and optional_re placeholders are properly substituted with actual values Verify no "translation missing" errors occur when using optional placeholders in custom body templates 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: No audit logs: The changes add translation and formatting logic without introducing or referencing any logging for critical actions, but this area may not require audit trails. Referred Code def body body = nil if @opts[:template] %i[topic_title inviter_name].each do |key| @template_args[key] = escaped_template_arg(key) if @template_args.key?(key) end augmented_template_args = @template_args.merge( optional_re: "", optional_pm: "", optional_cat: format_category, optional_tags: format_tags, ) body = I18n.t("#{@opts[:template]}.text_body_template", augmented_template_args).dup else body = @opts[:body].dup end 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: Missing fallbacks: The code assumes translation keys exist and does not add explicit error handling or fallbacks if I18n lookups fail, though this may be handled elsewhere by the i18n framework. Referred Code body = I18n.t("#{@opts[:template]}.text_body_template", augmented_template_args).dup else body = @opts[:body].dup end 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
Possible issue	Ensure consistent placeholder value generation Populate optional_re and optional_pm placeholders in the email body using the same conditional logic as the email subject for consistency. lib/email/message_builder.rb [193-199] augmented_template_args = @template_args.merge( - optional_re: "", - optional_pm: "", + 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, ) Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that optional_re and optional_pm are hardcoded to empty strings for the body, which is inconsistent with their conditional logic in the subject, and this could lead to unexpected behavior for custom templates.	Medium
More