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

[tomerqodo]

User description

Benchmark PR discourse#36156

Type: Corrupted (contains bugs)

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, Tests

---

Description

• Add quote image functionality to lightbox with new button

• Refactor quoting logic to handle draft reloading properly

• Extract image markdown building into reusable utility module

• Improve test coverage with new quote-image and markdown-builder tests

• Fix flaky test and improve test selectors for reliability

---

Diagram Walkthrough

flowchart LR
  A["Lightbox Component"] -->|"registers quote button"| B["Quote Image Handler"]
  B -->|"builds markdown"| C["Markdown Image Builder"]
  B -->|"handles composer states"| D["Composer Service"]
  D -->|"loads draft if needed"| E["Draft Service"]
  F["Post Decorator"] -->|"passes post model"| A

Loading

File Walkthrough

	Relevant files
Enhancement	6 files lightbox.js Add quote button registration and image data collection    +50/-2    quote-image.js New module for quoting images from lightbox                            +113/-0  markdown-image-builder.js Extract reusable image markdown building utility                  +37/-0    post-decorations.js Pass post model to lightbox initialization                              +2/-2      to-markdown.js Use shared image markdown builder function                              +13/-10  photoswipe.scss Add styling for quote image button icon                                    +4/-0
Bug fix	1 files topic.js Refactor quoting logic to reload closed drafts                      +21/-11
Tests	6 files lightbox_spec.rb Add quote image test and improve selectors                              +30/-7    photoswipe.rb Add quote button accessor methods to PhotoSwipe component +13/-0    quote-image-test.js Comprehensive unit tests for quote image functionality      +277/-0  markdown-image-builder-test.js Unit tests for markdown image builder utility                        +157/-0  post_event_spec.rb Disable flaky timezone-dependent test                                        +3/-1      local-dates-quoting-test.js Add draft endpoint mocks for quoting tests                              +9/-0
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: 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: New critical user action of quoting an image from the lightbox is implemented without any audit/event logging capturing user identity, action, and 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 51 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 catch block returns false without logging or surfacing context, potentially masking failures and making debugging difficult. Referred Code await composer.open(composerOpts); return true; } catch { return false; } } 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 post context exists before quoting Update canQuoteImage to verify slideData.post exists, ensuring the quote button only appears when an image is quotable. frontend/discourse/app/lib/lightbox/quote-image.js [38-40] export function canQuoteImage(slideElement, slideData) { - return buildImageMarkdown(slideElement, slideData) !== null; + return ( + !!slideData?.post && buildImageMarkdown(slideElement, slideData) !== null + ); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that slideData.post is required to quote an image, and canQuoteImage should check for its existence to prevent showing a non-functional quote button and causing a runtime error.	Medium
	Avoid inserting 'undefined' into composer Prevent inserting "undefined" into the composer by ensuring model.get("reply") is treated as an empty string if it's falsy when appending a quote to a draft. frontend/discourse/app/lib/lightbox/quote-image.js [79-84] if (composer.model?.viewDraft) { const model = composer.model; - model.set("reply", model.get("reply") + "\n" + quote); + model.set("reply", (model.get("reply") || "") + "\n" + quote); composer.openIfDraft(); return true; } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential bug where undefined could be prepended to a quote. The fix is simple and prevents incorrect text from being inserted into the composer.	Medium
High-level	Consolidate duplicated draft handling logic The draft handling logic is duplicated in topic.js and quote-image.js. This logic should be extracted into a shared utility or composer method to improve maintainability. Examples: frontend/discourse/app/controllers/topic.js [561-569] 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; } frontend/discourse/app/lib/lightbox/quote-image.js [98-106] 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; } Solution Walkthrough: Before: // In topic.js#selectText 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); // In quote-image.js#quoteImage const draftData = await Draft.get(composerOpts.draftKey); if (draftData.draft) { const data = JSON.parse(draftData.draft); composerOpts.reply = data.reply + "\n" + quote; // ... } else { composerOpts.quote = quote; } await composer.open(composerOpts); After: // In a new shared utility or composer service method async function openComposerWithQuote(composer, opts, quote) { const draftData = await Draft.get(opts.draftKey); if (draftData.draft) { const data = JSON.parse(draftData.draft); opts.draftSequence = draftData.draft_sequence; opts.reply = (data.reply || "") + "\n" + quote; } else { opts.quote = quote; } return composer.open(opts); } // In topic.js#selectText and quote-image.js#quoteImage // ... build composerOpts and quotedText await openComposerWithQuote(composer, composerOpts, quotedText); Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies significant code duplication for draft handling introduced in topic.js and quote-image.js, and abstracting this logic would improve maintainability.	Medium
More