Open
Conversation
📝 WalkthroughWalkthroughThis change adds a complete template management feature to the application. It introduces a new Vue Router routes configuration with Sequence Diagram(s)sequenceDiagram
actor User
participant Router as Vue Router
participant View as TemplatesView
participant Library as TemplateLibrary
participant Client as Template API Client
participant Backend as Backend API
User->>Router: Navigate to /templates
Router->>View: Render TemplatesView
View->>Library: Mount TemplateLibrary
Library->>Library: Set isLoading = true
Library->>Client: getTemplates(0, 1000)
Client->>Backend: GET /templates
Backend-->>Client: Template list
Client-->>Library: Return templates
Library->>Library: Set templates data
Library->>Library: Set isLoading = false
Library->>View: Render template cards
View-->>User: Display template library UI
sequenceDiagram
actor User
participant Router as Vue Router
participant View as TemplateEditView
participant Client as Template API Client
participant Backend as Backend API
User->>Router: Navigate to /templates/:id/edit
Router->>View: Render TemplateEditView
View->>View: Extract templateId from route
View->>View: Set isLoading = true
View->>Client: getTemplate(templateId)
Client->>Backend: GET /templates/:id
Backend-->>Client: Template data
Client-->>View: Return template
View->>View: Populate form with title/content/text
View->>View: Set isLoading = false
User->>View: Edit form fields & click Save
View->>View: Validate form.title exists
View->>View: Create CreateTemplateRequest
View->>View: Set isSaving = true
View->>Client: updateTemplate(request, templateId)
Client->>Backend: PUT/POST /templates/:id
Backend-->>Client: Success response
Client-->>View: Return result
View->>View: Set saveSuccess = true
View->>View: Set isSaving = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@assets/vue/components/templates/TemplateLibrary.vue`:
- Around line 38-43: The template rendering accesses
templateItem.images[0].mimetype directly which throws when images is an empty
array; update getTemplateImage(templateItem) (and any other places around the
same component block referenced at lines 101-105) to guard against missing/empty
images by first checking templateItem.images && templateItem.images.length > 0
(or using optional chaining) before accessing images[0], and return a safe
fallback (e.g., a placeholder image URL or null) when no preview image exists so
the library render won't break.
- Around line 6-15: The "New Template" button (and the similar inactive buttons
around the component) are interactive but have no click handlers—either wire
them to real handlers or disable them until implemented; to quickly fix, add the
disabled attribute and aria-disabled plus a visual disabled style to the "New
Template" <button> and the other inactive buttons (apply to the buttons at the
other ranges noted) or alternatively bind them to placeholder methods like
createTemplate/copyTemplate/deleteTemplate that emit a TODO event or open a "Not
implemented" toast; update class names to include a disabled state (e.g., remove
hover:bg-ext-wf3 and add opacity-50 cursor-not-allowed) so users get correct
affordance.
In `@assets/vue/views/TemplateEditView.vue`:
- Around line 177-189: The plain-text generator in populateTextFromContent
currently strips tags but leaves HTML entities (e.g., &, ) intact;
after the existing tag/style/script stripping and trimming (in
populateTextFromContent operating on form.value.content and setting
form.value.text) decode HTML entities by using the browser DOM decode approach
(e.g., create a temporary element, set its innerHTML to the intermediate string
and read .textContent) or an equivalent HTML-entity decoder, then assign the
decoded result to form.value.text so entities are converted to their correct
characters.
In `@src/Controller/TemplatesController.php`:
- Around line 15-22: In the index(Request $request) action (the controller
method named index for the #[Route('/', name: 'templates', ...)]), change the
hard-coded page label passed to the template: replace the 'page' => 'Campaigns'
entry with the correct templates page label (e.g. 'Templates' or the appropriate
i18n key) so the /templates route renders the correct SPA title.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e71d3c1e-c314-4329-a42d-3cc1d562866d
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
assets/router/index.jsassets/vue/components/templates/TemplateLibrary.vueassets/vue/views/TemplateEditView.vueassets/vue/views/TemplatesView.vuepackage.jsonsrc/Controller/TemplatesController.php
Comment on lines
+6
to
+15
| <button | ||
| class="px-4 py-2 bg-ext-wf1 hover:bg-ext-wf3 text-white text-sm font-medium rounded-lg flex items-center gap-2 transition-colors" | ||
| type="button" | ||
| > | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"> | ||
| <path d="M5 12h14"></path> | ||
| <path d="M12 5v14"></path> | ||
| </svg> | ||
| New Template | ||
| </button> |
There was a problem hiding this comment.
Wire or disable the inactive template actions.
These buttons are enabled but have no handlers, so clicks silently do nothing. Either implement them now or ship them disabled/hidden until supported.
Proposed temporary disablement
<button
- class="px-4 py-2 bg-ext-wf1 hover:bg-ext-wf3 text-white text-sm font-medium rounded-lg flex items-center gap-2 transition-colors"
+ class="px-4 py-2 bg-ext-wf1 hover:bg-ext-wf3 text-white text-sm font-medium rounded-lg flex items-center gap-2 transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
type="button"
+ disabled
+ title="Template creation is not implemented yet"
> <button
- class="px-3 py-2 border border-slate-200 rounded-lg hover:bg-slate-50 transition-colors"
+ class="px-3 py-2 border border-slate-200 rounded-lg hover:bg-slate-50 transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
type="button"
aria-label="Copy template"
+ disabled
+ title="Copy is not implemented yet"
> <button
- class="px-3 py-2 border border-slate-200 rounded-lg hover:bg-red-50 transition-colors"
+ class="px-3 py-2 border border-slate-200 rounded-lg hover:bg-red-50 transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
type="button"
aria-label="Delete template"
+ disabled
+ title="Delete is not implemented yet"
>Happy to help sketch the create/copy/delete handlers if you want to track that separately.
Also applies to: 67-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@assets/vue/components/templates/TemplateLibrary.vue` around lines 6 - 15, The
"New Template" button (and the similar inactive buttons around the component)
are interactive but have no click handlers—either wire them to real handlers or
disable them until implemented; to quickly fix, add the disabled attribute and
aria-disabled plus a visual disabled style to the "New Template" <button> and
the other inactive buttons (apply to the buttons at the other ranges noted) or
alternatively bind them to placeholder methods like
createTemplate/copyTemplate/deleteTemplate that emit a TODO event or open a "Not
implemented" toast; update class names to include a disabled state (e.g., remove
hover:bg-ext-wf3 and add opacity-50 cursor-not-allowed) so users get correct
affordance.
Comment on lines
+38
to
+43
| <div class="aspect-video bg-gradient-to-br from-slate-100 to-slate-200 flex items-center justify-center text-6xl"> | ||
| <img | ||
| :src="getTemplateImage(templateItem)" | ||
| :alt="`Template #${templateItem.id}`" | ||
| class="w-full h-full object-cover" | ||
| > |
There was a problem hiding this comment.
Guard templates that do not have preview images.
[] is truthy, so images[0].mimetype throws when a template has an empty images array and can break the whole library render.
Proposed fix
<div class="aspect-video bg-gradient-to-br from-slate-100 to-slate-200 flex items-center justify-center text-6xl">
<img
+ v-if="getTemplateImage(templateItem)"
:src="getTemplateImage(templateItem)"
:alt="`Template #${templateItem.id}`"
class="w-full h-full object-cover"
>
+ <span v-else class="text-sm text-slate-500">No preview</span>
</div> const getTemplateImage = (templateItem) => {
- const images = templateItem?.images
- if (!images) return '';
+ const image = Array.isArray(templateItem?.images) ? templateItem.images[0] : null
+ if (!image?.mimetype || !image?.data) return ''
- return `data:${images[0].mimetype};base64,${images[0].data}`;
+ return `data:${image.mimetype};base64,${image.data}`
}Also applies to: 101-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@assets/vue/components/templates/TemplateLibrary.vue` around lines 38 - 43,
The template rendering accesses templateItem.images[0].mimetype directly which
throws when images is an empty array; update getTemplateImage(templateItem) (and
any other places around the same component block referenced at lines 101-105) to
guard against missing/empty images by first checking templateItem.images &&
templateItem.images.length > 0 (or using optional chaining) before accessing
images[0], and return a safe fallback (e.g., a placeholder image URL or null)
when no preview image exists so the library render won't break.
Comment on lines
+177
to
+189
| const populateTextFromContent = () => { | ||
| if (!form.value.content) { | ||
| form.value.text = '' | ||
| return | ||
| } | ||
|
|
||
| form.value.text = form.value.content | ||
| .replace(/<style[\s\S]*?<\/style>/gi, ' ') | ||
| .replace(/<script[\s\S]*?<\/script>/gi, ' ') | ||
| .replace(/<[^>]+>/g, ' ') | ||
| .replace(/\s+/g, ' ') | ||
| .trim() | ||
| } |
There was a problem hiding this comment.
Decode HTML entities when generating the text version.
The current regex removes tags but leaves entities like & and in the plain-text email.
Proposed fix
const populateTextFromContent = () => {
if (!form.value.content) {
form.value.text = ''
return
}
- form.value.text = form.value.content
- .replace(/<style[\s\S]*?<\/style>/gi, ' ')
- .replace(/<script[\s\S]*?<\/script>/gi, ' ')
- .replace(/<[^>]+>/g, ' ')
+ const parser = new DOMParser()
+ const doc = parser.parseFromString(form.value.content, 'text/html')
+ doc.querySelectorAll('script, style').forEach((element) => element.remove())
+
+ form.value.text = (doc.body.textContent || '')
.replace(/\s+/g, ' ')
.trim()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const populateTextFromContent = () => { | |
| if (!form.value.content) { | |
| form.value.text = '' | |
| return | |
| } | |
| form.value.text = form.value.content | |
| .replace(/<style[\s\S]*?<\/style>/gi, ' ') | |
| .replace(/<script[\s\S]*?<\/script>/gi, ' ') | |
| .replace(/<[^>]+>/g, ' ') | |
| .replace(/\s+/g, ' ') | |
| .trim() | |
| } | |
| const populateTextFromContent = () => { | |
| if (!form.value.content) { | |
| form.value.text = '' | |
| return | |
| } | |
| const parser = new DOMParser() | |
| const doc = parser.parseFromString(form.value.content, 'text/html') | |
| doc.querySelectorAll('script, style').forEach((element) => element.remove()) | |
| form.value.text = (doc.body.textContent || '') | |
| .replace(/\s+/g, ' ') | |
| .trim() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@assets/vue/views/TemplateEditView.vue` around lines 177 - 189, The plain-text
generator in populateTextFromContent currently strips tags but leaves HTML
entities (e.g., &, ) intact; after the existing tag/style/script
stripping and trimming (in populateTextFromContent operating on
form.value.content and setting form.value.text) decode HTML entities by using
the browser DOM decode approach (e.g., create a temporary element, set its
innerHTML to the intermediate string and read .textContent) or an equivalent
HTML-entity decoder, then assign the decoded result to form.value.text so
entities are converted to their correct characters.
Comment on lines
+15
to
+22
| #[Route('/', name: 'templates', methods: ['GET'])] | ||
| public function index(Request $request): Response | ||
| { | ||
| return $this->render('@PhpListFrontend/spa.html.twig', [ | ||
| 'page' => 'Campaigns', | ||
| 'api_token' => $request->getSession()->get('auth_token'), | ||
| 'api_base_url' => $this->getParameter('api_base_url'), | ||
| ]); |
There was a problem hiding this comment.
Use the templates page label here.
Line 19 still passes Campaigns, so /templates renders the wrong SPA title.
Proposed fix
- 'page' => 'Campaigns',
+ 'page' => 'Templates',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Controller/TemplatesController.php` around lines 15 - 22, In the
index(Request $request) action (the controller method named index for the
#[Route('/', name: 'templates', ...)]), change the hard-coded page label passed
to the template: replace the 'page' => 'Campaigns' entry with the correct
templates page label (e.g. 'Templates' or the appropriate i18n key) so the
/templates route renders the correct SPA title.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Summary
Provide a general description of the code changes in your pull request …
were there any bugs you had fixed? If so, mention them. If these bugs have open
GitHub issues, be sure to tag them here as well, to keep the conversation
linked together.
Unit test
Are your changes covered with unit tests, and do they not break anything?
You can run the existing unit tests using this command:
Code style
Have you checked that you code is well-documented and follows the PSR-2 coding
style?
You can check for this using this command:
Other Information
If there's anything else that's important and relevant to your pull
request, mention that information here. This could include benchmarks,
or other information.
If you are updating any of the CHANGELOG files or are asked to update the
CHANGELOG files by reviewers, please add the CHANGELOG entry at the top of the file.
Thanks for contributing to phpList!