[CORRUPTED] Synthetic Benchmark PR #36210 - FEATURE: Automatically add 'Add Translation' post menu when content localization enabled

[tomerqodo]

User description

Benchmark PR discourse#36210

Type: Corrupted (contains bugs)

Original PR Title: FEATURE: Automatically add 'Add Translation' post menu when content localization enabled
Original PR Description: Many users have been tripping over not seeing this post menu item, so we'll add it automatically.

Meta: https://meta.discourse.org/t/automatically-add-the-add-translation-post-menu-button-when-content-localization-is-enabled/389557
Original PR URL: discourse#36210

---

PR Type

Enhancement, Tests

---

Description

• Automatically add 'Add Translation' button to post menu when content localization enabled

• Insert button after edit button or at start if edit unavailable

• Handle both visible post_menu and post_menu_hidden_items settings

• Update system tests to use new post action button API instead of direct selectors

---

Diagram Walkthrough

flowchart LR
  A["content_localization_enabled setting changed"] --> B["Check post_menu and post_menu_hidden_items"]
  B --> C["Find edit button position"]
  C --> D["Insert addTranslation after edit or at start"]
  D --> E["Update site settings"]
  E --> F["Post menu updated with translation button"]

Loading

File Walkthrough

	Relevant files
Enhancement	014-track-setting-changes.rb Add translation button auto-insertion on localization enable config/initializers/014-track-setting-changes.rb Add logic to handle content_localization_enabled setting changes Iterate over post_menu and post_menu_hidden_items settings Insert addTranslation button after edit button position or at start if edit doesn't exist Skip insertion if addTranslation already exists in the menu +12/-0
Tests	content_localization_setting_spec.rb Test content localization setting auto-configuration          spec/initializers/content_localization_setting_spec.rb New test file for content localization setting changes Test adding addTranslation after edit button in post_menu Test adding addTranslation as first button when edit doesn't exist Test preventing duplicate addTranslation entries Test same scenarios for post_menu_hidden_items setting Test that setting is not modified when localization is disabled +81/-0    post_translation_spec.rb Refactor tests to use post action button API                          spec/system/post_translation_spec.rb Reorder site setting initialization to enable localization after menu configuration Add post_menu_hidden_items configuration to test setup Replace direct DOM selectors with page object methods click_post_action_button Update all test cases to use show_more and add_translation action buttons instead of direct element selection Fix inconsistent post reference (use post instead of markdown_post in one test) +30/-14

---

[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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Logic bug edge case: The condition checks include?("addTranslation") before inserting, which prevents insertion when absent and allows duplicates when present, and no handling exists for empty strings or nil settings. Referred Code current_items = SiteSetting.get(setting_name).split("|") if current_items.include?("addTranslation") edit_index = current_items.index("edit") insert_position = edit_index ? edit_index + 1 : 0 current_items.insert(insert_position, "addTranslation") SiteSetting.set(setting_name, current_items.join("|")) end 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 logging: The code mutates site settings in response to content_localization_enabled without emitting any audit log or trace of who triggered the change or what was modified. Referred Code if name == :content_localization_enabled %i[post_menu post_menu_hidden_items].each do |setting_name| current_items = SiteSetting.get(setting_name).split("|") if current_items.include?("addTranslation") edit_index = current_items.index("edit") insert_position = edit_index ? edit_index + 1 : 0 current_items.insert(insert_position, "addTranslation") SiteSetting.set(setting_name, current_items.join("|")) end end end 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: Missing validation: The code reads and rewrites post_menu and post_menu_hidden_items without validating/normalizing input (empty values, unexpected separators, or malicious entries) which could corrupt settings. Referred Code %i[post_menu post_menu_hidden_items].each do |setting_name| current_items = SiteSetting.get(setting_name).split("|") if current_items.include?("addTranslation") edit_index = current_items.index("edit") insert_position = edit_index ? edit_index + 1 : 0 current_items.insert(insert_position, "addTranslation") SiteSetting.set(setting_name, current_items.join("|")) end 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	Fix inverted logic for adding button Invert the conditional logic to add addTranslation only when it is not already present, and ensure this only happens when content_localization_enabled is being set to true. config/initializers/014-track-setting-changes.rb [91-101] -if name == :content_localization_enabled +if name == :content_localization_enabled && new_value == true %i[post_menu post_menu_hidden_items].each do |setting_name| current_items = SiteSetting.get(setting_name).split("|") - if current_items.include?("addTranslation") + if !current_items.include?("addTranslation") edit_index = current_items.index("edit") insert_position = edit_index ? edit_index + 1 : 0 current_items.insert(insert_position, "addTranslation") SiteSetting.set(setting_name, current_items.join("|")) end end end Apply / Chat Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical logic bug where the condition is inverted, causing the feature to not work as intended, and also points out the logic should only run when the setting is enabled.	High
	Pass correct object to method Pass the markdown_post object instead of markdown_post.post_number to the click_post_action_button method to match the expected argument type and fix the test. spec/system/post_translation_spec.rb [257-272] it "displays raw markdown when toggle is enabled" do topic_page.visit_topic(topic) - topic_page.click_post_action_button(markdown_post.post_number, :show_more) - topic_page.click_post_action_button(markdown_post.post_number, :add_translation) + topic_page.click_post_action_button(markdown_post, :show_more) + topic_page.click_post_action_button(markdown_post, :add_translation) find(".update-translations-menu__add .post-action-menu__add-translation").click expect(composer).to be_opened translation_preview.raw_toggle.toggle expect(translation_preview).to have_raw_markdown_content raw_content = translation_preview.raw_markdown_content expect(raw_content).to include("# Heading") expect(raw_content).to include("**Bold**") expect(raw_content).to include("[link](https://example.com)") expect(raw_content).to include("*italic*") end Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a bug in a test case where a method is called with an argument of the wrong type (Integer instead of a post object), which would cause the test to fail.	Medium
More