Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a template management feature: new Vue Router routes for listing, creating, and editing templates; three frontend components (TemplatesView, TemplateLibrary, TemplateEditView) that load, create, update, and delete templates via the template API client; a Symfony TemplatesController that serves the SPA for the template routes and injects api context; a small package version bump for the REST API client; and several UI adjustments (icons, action button layouts) across components. Sequence Diagram(s)sequenceDiagram
actor User
participant Browser as Browser
participant Router as Vue Router
participant TemplatesView as TemplatesView
participant Library as TemplateLibrary
participant APIClient as Template API Client
participant Backend as Backend API
User->>Browser: Navigate to /templates
Browser->>Router: route /templates
Router->>TemplatesView: render
TemplatesView->>Library: mount
Library->>Library: isLoading = true
Library->>APIClient: getTemplates(0,1000)
APIClient->>Backend: GET /templates
Backend-->>APIClient: template list
APIClient-->>Library: return templates
Library->>Library: sort & set templates
Library->>TemplatesView: render cards
TemplatesView-->>User: display template library
sequenceDiagram
actor User
participant Browser as Browser
participant Router as Vue Router
participant EditView as TemplateEditView
participant APIClient as Template API Client
participant Backend as Backend API
User->>Browser: Navigate to /templates/:id/edit
Browser->>Router: route /templates/:id/edit
Router->>EditView: render
EditView->>EditView: extract templateId, isLoading = true
EditView->>APIClient: getTemplate(templateId)
APIClient->>Backend: GET /templates/:id
Backend-->>APIClient: template data
APIClient-->>EditView: return template
EditView->>EditView: populate form, isLoading = false
User->>EditView: submit form
EditView->>APIClient: updateTemplate(request, templateId)
APIClient->>Backend: PUT /templates/:id
Backend-->>APIClient: success
APIClient-->>EditView: return result
EditView->>User: show save success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 |
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
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
assets/vue/components/campaigns/CampaignDirectory.vue (1)
110-119: Heads-up: "Requeue" button shows for any non-draft, non-active status.Not introduced by this PR (this is styling-only), but while you're in here: the
v-elseon lines 110/250 means campaigns withstatusKey === 'sent'or'unknown'also get a Requeue button. Onsentrows that sits next to "Copy to draft", which looks a bit odd with the new prominent icon+label styling. Might be worth gating it withv-else-if="campaign.statusKey === 'sent'"(or excluding'unknown') depending on intended UX. Feel free to ignore if that's the intended behavior.Also applies to: 250-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/vue/components/campaigns/CampaignDirectory.vue` around lines 110 - 119, The Requeue button is currently shown for any non-draft, non-active status because it uses a broad v-else; update the template so the button is only rendered for the intended statuses by replacing the ambiguous v-else with a conditional check on campaign.statusKey (e.g., use v-else-if="campaign.statusKey === 'sent'" or v-else-if="campaign.statusKey !== 'unknown'" depending on desired UX) wherever the button appears (the blocks that trigger handleRequeue(campaign.id) and :disabled="isActionLoading(campaign.id)"—including the other occurrence around lines 250-259) so only the correct statuses render the Requeue button.assets/vue/views/ListSubscribersView.vue (1)
76-84:start(play-triangle) icon on "Move Selected" feels off-theme.A "play" icon on a button that moves subscribers between lists is a bit of a semantic mismatch — users typically read that glyph as start/run. Something like an arrow-right, swap, or move icon would communicate intent better. Not blocking, just a nitpick for icon vocabulary consistency across the app.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/vue/views/ListSubscribersView.vue` around lines 76 - 84, The BaseIcon used in the Move Selected button currently passes name="start" which shows a play-triangle and is semantically misleading; change the icon to a more appropriate glyph (e.g., "arrow-right", "swap" or "move") by updating the BaseIcon name in the button that calls moveSelectedSubscribers so the visual matches the action and app icon vocabulary. Locate the button component that contains BaseIcon name="start" (the Move Selected button bound to moveSelectedSubscribers and disabled by hasSelection/selectedTargetListId/actionLoading) and replace the name prop with the chosen icon name across the component/template.
🤖 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/base/BaseIcon.vue`:
- Around line 95-97: The new SVG entries for the pause and start icons in
BaseIcon.vue are inconsistent: remove the unsupported rx attribute from the
start icon's <polygon> (or replace the polygon with a <path> if you want rounded
tips) and normalize stroke-width to match the registry (use stroke-width="2" for
both the pause and start entries); optionally add the same xmlns and
class="lucide …" attributes present on other icons for consistency. Ensure you
update the 'start' and 'pause' string values accordingly and keep
stroke-linejoin/stroke-linecap as-is if you rely on rounded joins.
In `@assets/vue/views/TemplateEditView.vue`:
- Line 153: The component currently calls loadTemplate() only inside onMounted,
which leaves data stale when the route is reused; replace that pattern by
importing and using Vue's watch to observe isCreateMode and templateId (or add a
watch([isCreateMode, templateId], () => loadTemplate(), { immediate: true })) so
loadTemplate() runs on initial mount and whenever the route params/mode change;
update the import line to include watch from 'vue' and remove or stop relying
solely on onMounted for this behavior.
---
Nitpick comments:
In `@assets/vue/components/campaigns/CampaignDirectory.vue`:
- Around line 110-119: The Requeue button is currently shown for any non-draft,
non-active status because it uses a broad v-else; update the template so the
button is only rendered for the intended statuses by replacing the ambiguous
v-else with a conditional check on campaign.statusKey (e.g., use
v-else-if="campaign.statusKey === 'sent'" or v-else-if="campaign.statusKey !==
'unknown'" depending on desired UX) wherever the button appears (the blocks that
trigger handleRequeue(campaign.id) and
:disabled="isActionLoading(campaign.id)"—including the other occurrence around
lines 250-259) so only the correct statuses render the Requeue button.
In `@assets/vue/views/ListSubscribersView.vue`:
- Around line 76-84: The BaseIcon used in the Move Selected button currently
passes name="start" which shows a play-triangle and is semantically misleading;
change the icon to a more appropriate glyph (e.g., "arrow-right", "swap" or
"move") by updating the BaseIcon name in the button that calls
moveSelectedSubscribers so the visual matches the action and app icon
vocabulary. Locate the button component that contains BaseIcon name="start" (the
Move Selected button bound to moveSelectedSubscribers and disabled by
hasSelection/selectedTargetListId/actionLoading) and replace the name prop with
the chosen icon name across the component/template.
🪄 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: 69694bc3-9f6c-4dd6-b867-19f4be120c2e
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
assets/router/index.jsassets/vue/components/base/BaseIcon.vueassets/vue/components/campaigns/CampaignDirectory.vueassets/vue/components/dashboard/QuickActionsCard.vueassets/vue/components/lists/ListDirectory.vueassets/vue/components/subscribers/SubscriberDirectory.vueassets/vue/components/subscribers/SubscriberTable.vueassets/vue/components/templates/TemplateLibrary.vueassets/vue/views/ListSubscribersView.vueassets/vue/views/TemplateEditView.vuepackage.jsonsrc/Controller/TemplatesController.phptemplates/auth/login.html.twig
✅ Files skipped from review due to trivial changes (4)
- package.json
- templates/auth/login.html.twig
- assets/vue/components/subscribers/SubscriberDirectory.vue
- assets/vue/components/dashboard/QuickActionsCard.vue
🚧 Files skipped from review as they are similar to previous changes (3)
- assets/router/index.js
- src/Controller/TemplatesController.php
- assets/vue/components/templates/TemplateLibrary.vue
| pause: `<svg width="18" height="18" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="1.8" stroke-linecap="round" stroke-linejoin="round"><rect x="7" y="5" width="3" height="14" rx="1"></rect><rect x="14" y="5" width="3" height="14" rx="1"></rect></svg>`, | ||
|
|
||
| start: `<svg width="18" height="18" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round"><polygon points="6 4 20 12 6 20 6 4" rx="1"></polygon></svg>`, |
There was a problem hiding this comment.
Minor SVG quirks in the new pause/start icons.
A couple of small polish items on the new icons:
- The
startpolygon hasrx="1", but<polygon>doesn't supportrx/ryin SVG — it's silently ignored, so the corners won't actually be rounded. If you want rounded tips, either use a<path>with rounded joins (yourstroke-linejoin="round"already helps) or just drop the attribute. stroke-widthis inconsistent across the two new icons (1.8forpause,2.5forstart), and different from the2used by every other icon in this registry. Visually they'll look a bit heavier/lighter than their siblings when rendered side-by-side (e.g., inCampaignDirectory.vuewhere both appear asw-3.5 h-3.5).
🎨 Suggested tweak
- pause: `<svg width="18" height="18" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="1.8" stroke-linecap="round" stroke-linejoin="round"><rect x="7" y="5" width="3" height="14" rx="1"></rect><rect x="14" y="5" width="3" height="14" rx="1"></rect></svg>`,
+ pause: `<svg xmlns="http://www.w3.org/2000/svg" width="18" height="18" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><rect x="7" y="5" width="3" height="14" rx="1"></rect><rect x="14" y="5" width="3" height="14" rx="1"></rect></svg>`,
- start: `<svg width="18" height="18" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round"><polygon points="6 4 20 12 6 20 6 4" rx="1"></polygon></svg>`,
+ start: `<svg xmlns="http://www.w3.org/2000/svg" width="18" height="18" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><polygon points="6 4 20 12 6 20 6 4"></polygon></svg>`,Also noticed the other icons include xmlns and an inline class="lucide …" — not a bug, but worth mirroring if you care about registry consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@assets/vue/components/base/BaseIcon.vue` around lines 95 - 97, The new SVG
entries for the pause and start icons in BaseIcon.vue are inconsistent: remove
the unsupported rx attribute from the start icon's <polygon> (or replace the
polygon with a <path> if you want rounded tips) and normalize stroke-width to
match the registry (use stroke-width="2" for both the pause and start entries);
optionally add the same xmlns and class="lucide …" attributes present on other
icons for consistency. Ensure you update the 'start' and 'pause' string values
accordingly and keep stroke-linejoin/stroke-linecap as-is if you rely on rounded
joins.
New Features
Templates section: listing, create and edit pages; responsive template library with add-from-default, preview, edit, and delete flows; full template editor with save, create, update, and derive-text utility.
UI Changes
Updated action buttons and icons across dashboard, lists, campaigns, subscribers and subscriber list views; renamed headings and adjusted navigation targets.
Chore
Internal dependency updated.
---------
Co-authored-by: Tatevik <tatevikg1@gmail.com>
Summary by CodeRabbit
New Features
UI Changes
Chore
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!