[CLEAN] Synthetic Benchmark PR #36156 - FEATURE: allow quoting an image from the lightbox

[tomerqodo]

User description

Benchmark PR discourse#36156

Type: Clean (correct implementation)

Original PR Title: FEATURE: allow quoting an image from the lightbox
Original PR Description: Also, fix quoting logic so when a draft is closed it will be reloaded prior to adding quote.

Original PR URL: discourse#36156

---

PR Type

Enhancement

---

Description

• Add quote image button to lightbox for inserting images into composer

• Refactor quoting logic to handle draft reloading before adding quotes

• Extract image markdown building into reusable utility module

• Update lightbox element selection to use page object helpers

• Disable flaky timezone-dependent test in calendar plugin

---

Diagram Walkthrough

flowchart LR
  A["Lightbox UI"] -->|"quote button click"| B["quoteImage function"]
  B -->|"build markdown"| C["buildImageMarkdown"]
  B -->|"check draft"| D["Draft.get"]
  D -->|"draft exists"| E["append to draft reply"]
  D -->|"no draft"| F["open composer with quote"]
  E -->|"update composer"| G["Composer"]
  F -->|"open"| G

Loading

File Walkthrough

	Relevant files
Enhancement	6 files quote-image.js New quote image functionality for lightbox                              +113/-0  markdown-image-builder.js Reusable image markdown builder utility                                    +37/-0    lightbox.js Integrate quote button into lightbox UI                                    +50/-2    post-decorations.js Pass post model to lightbox decorator                                        +2/-2      to-markdown.js Use shared image markdown builder                                                +13/-10  photoswipe.scss Add styling for quote image button                                              +4/-0
Bug fix	1 files topic.js Refactor quoting to reload drafts before adding                    +21/-11
Tests	6 files lightbox_spec.rb Add quote image test and improve selectors                              +30/-7    photoswipe.rb Add quote button accessor to PhotoSwipe component                +13/-0    quote-image-test.js Comprehensive unit tests for quote image feature                  +277/-0  markdown-image-builder-test.js Unit tests for image markdown builder                                        +157/-0  local-dates-quoting-test.js Add draft endpoint mocks to local dates tests                        +9/-0      post_event_spec.rb Disable flaky timezone-dependent test                                        +3/-1
Documentation	1 files client.en.yml Add i18n string for quote image button                                      +1/-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: 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 new image quoting action modifies drafts/composer state but adds no auditing or logging of the action, user, or outcome. Referred Code export default async function quoteImage(slideElement, slideData) { try { const ownerContext = helperContext(); if (!slideElement || !ownerContext) { return false; } const owner = getOwner(ownerContext); if (!owner) { return false; } const markdown = buildImageMarkdown(slideElement, slideData); if (!markdown) { return false; } const composer = owner.lookup("service:composer"); if (!composer) { ... (clipped 50 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: Swallowed errors: The try/catch in quoteImage returns false on any exception without logging, obscuring failure causes and hindering debugging. Referred Code export default async function quoteImage(slideElement, slideData) { try { const ownerContext = helperContext(); if (!slideElement || !ownerContext) { return false; } const owner = getOwner(ownerContext); if (!owner) { return false; } const markdown = buildImageMarkdown(slideElement, slideData); if (!markdown) { return false; } const composer = owner.lookup("service:composer"); if (!composer) { ... (clipped 51 lines) 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: Insufficient sanitization: The image markdown builder only sanitizes alt text and constructs URLs directly from inputs without validating or constraining src values, which may risk malformed or unsafe URLs. Referred Code export function buildImageMarkdown(imageData) { const { src, alt, width, height, title, escapeTablePipe = false, fallbackAlt, } = imageData; if (!src) { return ""; } const altText = sanitizeAlt(alt, { fallback: fallbackAlt }); const pipe = escapeTablePipe ? "\\|" : "|"; const suffix = width && height ? `${pipe}${width}x${height}` : ""; const titleSuffix = title ? ` "${title}"` : ""; return ``; ... (clipped 1 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	Consolidate duplicated draft handling logic The logic for handling existing drafts when adding a quote is duplicated in topic.js and quote-image.js. This should be extracted into a single shared utility or service method to avoid inconsistencies. Examples: frontend/discourse/app/controllers/topic.js [561-571] const draftData = await Draft.get(composerOpts.draftKey); if (draftData.draft) { const data = JSON.parse(draftData.draft); composerOpts.draftSequence = draftData.draft_sequence; composerOpts.reply = data.reply + "\n" + quotedText; } else { composerOpts.quote = quotedText; } ... (clipped 1 lines) frontend/discourse/app/lib/lightbox/quote-image.js [98-108] const draftData = await Draft.get(composerOpts.draftKey); if (draftData.draft) { const data = JSON.parse(draftData.draft); composerOpts.draftSequence = draftData.draft_sequence; composerOpts.reply = data.reply + "\n" + quote; } else { composerOpts.quote = quote; } ... (clipped 1 lines) Solution Walkthrough: Before: // In topic.js#selectText and other methods const composerOpts = { ... }; const quotedText = buildQuote(...); if (composer is closed) { const draftData = await Draft.get(composerOpts.draftKey); if (draftData.draft) { const data = JSON.parse(draftData.draft); composerOpts.reply = data.reply + "\n" + quotedText; } else { composerOpts.quote = quotedText; } composer.open(composerOpts); } After: // In a new shared utility or composer service method async function openComposerWithQuote(composer, composerOpts, quotedText) { const draftData = await Draft.get(composerOpts.draftKey); if (draftData.draft) { const data = JSON.parse(draftData.draft); composerOpts.draftSequence = draftData.draft_sequence; composerOpts.reply = data.reply + "\n" + quotedText; } else { composerOpts.quote = quotedText; } await composer.open(composerOpts); } // In topic.js#selectText and other methods if (composer is closed) { await openComposerWithQuote(composer, composerOpts, quotedText); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies significant code duplication for draft handling logic introduced and repeated across topic.js and the new quote-image.js, and addressing it would improve code quality and maintainability.	Medium
Possible issue	Prevent "undefined" string in composer To prevent "undefined" from being added to the composer, ensure data.reply defaults to an empty string if it is falsy before concatenating it with the quoted text. frontend/discourse/app/controllers/topic.js [564-566] const data = JSON.parse(draftData.draft); composerOpts.draftSequence = draftData.draft_sequence; -composerOpts.reply = data.reply + "\n" + quotedText; +composerOpts.reply = (data.reply || "") + "\n" + quotedText; Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a bug where the string "undefined" could be prepended to the composer text if a draft exists but has no reply content, and provides a correct fix.	Medium
More