Add "Edit Draft" AI rewrite button with dedicated prompt#40
Add "Edit Draft" AI rewrite button with dedicated prompt#40ku9nov wants to merge 2 commits intopresswizards:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a message_edit_prompt field and draft-editing feature: DB migration, GPTSettings update, provider exposes new frontend data, controller endpoint (editDraft) with OpenAI call and HTML stripping, route, frontend JS/button and editor functions, and settings blade textarea. Changes
Sequence DiagramsequenceDiagram
participant User
participant Client as Client (JS)
participant Server as FreeScoutGPTController
participant DB as Database
participant OpenAI as OpenAI API
User->>Client: Click "Edit Draft" button
Client->>Client: getReplyDraftText() (strip HTML)
Client->>Server: POST /freescoutgpt/edit-draft (draft + mailbox_id)
Server->>DB: Load GPTSettings (message_edit_prompt)
DB-->>Server: Return settings
Server->>OpenAI: Chat completion request (system prompt + draft)
OpenAI-->>Server: Edited text
Server-->>Client: Respond with edited text (JSON)
Client->>Client: setReplyDraftText(edited text)
Client->>User: Updated draft visible in editor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@Http/Controllers/FreeScoutGPTController.php`:
- Around line 247-249: The $skipClientData flag is only declared inside the
Responses API branch so the standard chat flow still sends client data; move the
declaration of $skipClientData = $request->boolean('skip_client_data') to the
top of the method (or at least before the Responses API conditional) so it's
available to both flows, then use that variable in the existing checks (e.g.,
the if that currently reads $settings->client_data_enabled && !$skipClientData
and any later logic that reads $skipClientData when building
$customerName/$client data) in FreeScoutGPTController to ensure the privacy flag
is honored in both paths.
In `@Providers/FreeScoutGPTServiceProvider.php`:
- Around line 75-95: The injected prompt strings (e.g., $message_edit_prompt,
$responses_api_prompt, $start_message, $modifyPrompt, etc.) are embedded raw
into the JS object freescoutGPTData and can break the script or allow XSS;
replace the manual string concatenation with safe JSON serialization: build a
PHP array/object containing all keys (copiedToClipboard, updateAvailable,
version, start_message, message_edit_prompt, responses_api_prompt, modifyPrompt,
send, editDraft, nothingToEdit, moduleDisabled) and output it via json_encode so
values are properly escaped for JavaScript and cannot inject backticks, ${}, or
</script>.
In `@Public/js/module.js`:
- Around line 200-207: The setReplyDraftText function is injecting
server-provided text into Summernote as HTML without escaping special
characters, allowing XSS; update setReplyDraftText to HTML-escape &, <, >, " and
' (or use a DOM/text-node-based sanitizer) before converting newlines to <br>
and calling $('#body').summernote('code', ...), or use Summernote's safe text
API if available; target the setReplyDraftText function and the
$('#body').summernote('code', ...) call to ensure the value is escaped/sanitized
prior to insertion.
- Around line 234-237: The success handler currently calls
setReplyDraftText(response.answer) unconditionally; update the success function
that includes setReplyDraftText and $(".gpteditbutton i").removeClass(...) to
first validate that response and response.answer exist and are the expected type
(e.g., typeof response.answer === "string" and response.answer.length > 0)
before calling setReplyDraftText; if validation fails, avoid updating the editor
(or pass a safe default like an empty string) and optionally log or handle the
unexpected response so the UI state change (removing the fa-beat-fade class)
still occurs safely.
🧹 Nitpick comments (2)
Http/Controllers/FreeScoutGPTController.php (1)
16-22: Remove or use the unusedstripHtmlToTexthelper.
Line 16–22 is currently unused (static analysis flagged it). Either delete it or wire it intoeditDraftif HTML stripping is intended.🧹 Remove if not needed
- private function stripHtmlToText($html) - { - if (!is_string($html)) { - return ''; - } - return trim(strip_tags($html)); - }Public/js/module.js (1)
224-254: Consider adding a timeout for the AI request.AI API calls can take a significant amount of time (30+ seconds). Without a timeout, the UI could remain in a loading state indefinitely if the server or AI service becomes unresponsive. Adding a timeout with appropriate user feedback would improve reliability.
⏱️ Suggested timeout configuration
$.ajax({ url: '/freescoutgpt/edit-draft', method: 'POST', dataType: 'json', + timeout: 60000, // 60 second timeout headers: csrfToken ? { 'X-CSRF-TOKEN': csrfToken } : {}, data: { mailbox_id: mailbox_id, text: text, skip_client_data: 1 }, // ... error: function(xhr) { $(".gpteditbutton i").removeClass("fa-beat-fade"); + if (xhr && xhr.statusText === 'timeout') { + showFloatingAlert('error', 'Request timed out. Please try again.'); + return; + } // ... rest of error handling } });
|
@ku9nov There are a few issues with the code that CodeRabbit has identified, are you able to address these? I will review in more detail as well. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Public/js/module.js`:
- Around line 189-199: getReplyDraftText currently converts the editor HTML to
plain text using tmp.textContent which collapses <br> and paragraph boundaries;
update getReplyDraftText to preserve line breaks by first transforming the DOM:
replace/insert newline markers for <br> elements and for block-level elements
(e.g., p, div, li) before reading text, then read the node text (tmp.textContent
or tmp.innerText) and trim; locate the getReplyDraftText function and apply this
DOM-to-text conversion after tmp.innerHTML = html and before returning the
trimmed string so drafts keep their original line breaks.
🧹 Nitpick comments (1)
Http/Controllers/FreeScoutGPTController.php (1)
16-22: UsestripHtmlToTextwhen normalizing the draft input.
The helper is currently unused; applying it ineditDraftboth strips HTML and clears the unused-method warning.♻️ Suggested wiring
- $draftText = (string)$request->get('text', ''); - $draftText = trim($draftText); + $draftText = $this->stripHtmlToText($request->get('text', ''));Also applies to: 472-488
| function getReplyDraftText() { | ||
| if (!$('#body').length || typeof $('#body').summernote !== 'function') { | ||
| return ''; | ||
| } | ||
| const html = $('#body').summernote('code') || ''; | ||
| // Convert HTML to plain text for the editing request. | ||
| const tmp = document.createElement('div'); | ||
| tmp.innerHTML = html; | ||
| return (tmp.textContent || tmp.innerText || '').trim(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Preserve line breaks when extracting draft text.
Using textContent first can collapse <br>/paragraphs, flattening the draft before editing.
📝 Suggested tweak
- return (tmp.textContent || tmp.innerText || '').trim();
+ return (tmp.innerText || tmp.textContent || '').trim();🤖 Prompt for AI Agents
In `@Public/js/module.js` around lines 189 - 199, getReplyDraftText currently
converts the editor HTML to plain text using tmp.textContent which collapses
<br> and paragraph boundaries; update getReplyDraftText to preserve line breaks
by first transforming the DOM: replace/insert newline markers for <br> elements
and for block-level elements (e.g., p, div, li) before reading text, then read
the node text (tmp.textContent or tmp.innerText) and trim; locate the
getReplyDraftText function and apply this DOM-to-text conversion after
tmp.innerHTML = html and before returning the trimmed string so drafts keep
their original line breaks.
There was a problem hiding this comment.
Switching the order to innerText does not reliably preserve line breaks and is browser-dependent.
For deterministic behavior, explicit handling of <br> / block elements would be required.
Keeping the current implementation for now.
There was a problem hiding this comment.
@ku9nov, you're absolutely right. innerText strips <br> tags without replacement, and its behavior varies significantly across browsers, while textContent adheres to the W3C standard and is more performant since it doesn't trigger reflows.
For reliable line break preservation, you'd indeed need to explicitly process <br> and block-level elements in the DOM before extracting text. Keeping the current implementation is a sensible choice for consistency and cross-browser compatibility.
✏️ Learnings added
Learnt from: ku9nov
Repo: presswizards/FreeScoutGPT PR: 40
File: Public/js/module.js:189-199
Timestamp: 2026-01-29T10:29:43.858Z
Learning: In the FreeScoutGPT module file `Public/js/module.js`, the `getReplyDraftText()` function uses `textContent` (not `innerText`) to extract text from the Summernote editor for consistency and cross-browser compatibility, accepting that line breaks may be collapsed. Switching to `innerText` is not preferred due to its browser-dependent behavior.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
Hey @presswizards |
|
@ku9nov Ok we'll dig into it more, do some testing, and ensure it's working well across the our test installations, etc. |
2 participants
This PR adds a new Edit Draft action to the reply editor, allowing agents to send their current reply text to AI for grammar, clarity, and tone improvements without generating a new response from the customer message.
Fixes request behavior:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.