adds dynamic page management to the cms. #21
Conversation
WalkthroughAdds full CMS page functionality: routes, DB migration, Page model, repository + interface, admin and public controllers, create/update/delete services, Editor.js rendering, shortcode/widget system, events, and admin/public views. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Admin as Admin User
participant Ctrl as Admin/Pages Controller
participant Creator as Page Creator
participant Repo as DatabasePageRepository
participant DB as Database
participant Event as Dispatcher
Admin->>Ctrl: POST /admin/pages (form)
Ctrl->>Creator: create(title, content, authorId, status, ...)
Creator->>Creator: generateSlug_if_missing()
Creator->>Repo: create(Page)
Repo->>DB: INSERT pages (...)
DB-->>Repo: inserted id, timestamps
Repo-->>Creator: persisted Page
Creator-->>Ctrl: Page
Ctrl->>Event: dispatch PageCreatedEvent(Page)
Ctrl-->>Admin: redirect (flash success)
sequenceDiagram
autonumber
participant User as Public User
participant Ctrl as Pages Controller
participant Repo as DatabasePageRepository
participant Renderer as EditorJsRenderer
participant Parser as ShortcodeParser
participant Widget as WidgetRenderer
participant View as Template
User->>Ctrl: GET /pages/:slug
Ctrl->>Repo: findBySlug(slug)
Repo-->>Ctrl: Page (published)
Ctrl->>Repo: incrementViewCount(id)
Ctrl->>Renderer: render(page.getContent())
Renderer->>Parser: parse(shortcodes inside HTML)
Parser->>Widget: render(widgetName, attrs)
Widget-->>Parser: widget HTML
Parser-->>Renderer: final HTML
Renderer-->>Ctrl: contentHtml
Ctrl->>View: renderHtml({ContentHtml,...})
View-->>User: HTML response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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: 11
🧹 Nitpick comments (19)
resources/views/admin/pages/create.php (1)
107-155: Consider pinning EditorJS CDN dependencies instead of using@latestAll EditorJS scripts are loaded from jsDelivr with the
@latesttag. That can introduce unexpected breaking changes or behavior shifts when upstream releases new versions.Prefer pinning explicit versions (e.g.
@2.29.1) or hosting the assets yourself so that editor behavior is deterministic across deployments.resources/views/admin/pages/edit.php (1)
126-134: Same CDN versioning concern as create view (@latestEditorJS scripts)As in the create view, loading EditorJS and its tools from jsDelivr with
@latestcan cause unexpected breakage when upstream releases new versions. Consider pinning explicit versions or hosting them locally to keep the admin editor stable.src/Cms/Services/Content/EditorJsRenderer.php (1)
183-186: Remove unused$dataparameter inrenderDelimiter
renderDelimiter()doesn’t use its$dataparameter, which matches the PHPMD warning and slightly clutters the API.If you don’t intend to use the block data for delimiters, simplify the signature:
- private function renderDelimiter( array $data ): string + private function renderDelimiter(): string { return "<hr class='my-4'>\n"; }And adjust the call site:
- 'delimiter' => $this->renderDelimiter( $data ), + 'delimiter' => $this->renderDelimiter(),This keeps the intent clear and eliminates the static analysis warning.
src/Cms/Controllers/Pages.php (1)
33-48: Controller wiring is clear but tightly coupled to concrete implementationsDirectly instantiating
DatabasePageRepository,DatabasePostRepository, andWidgetRendererin the constructor works but makes the controller harder to test and swap out implementations. Long‑term, consider injecting these (or their interfaces) from a composition root/container instead of pulling them fromRegistryinside the controller.src/Cms/Services/Widget/WidgetRegistry.php (1)
15-86: Widget registry integration with shortcodes is straightforward and correctRegister/unregister keep the in‑memory map and the
ShortcodeParserin sync, and closures delegate cleanly toIWidget::render(). If you want to tighten things later, you could (a) document that laterregister()calls override earlier widgets with the same name and/or (b) add a/** @var IWidget[] */docblock on$_widgetsfor IDE/static‑analysis clarity, but functionally this looks good.src/Cms/Services/Page/Updater.php (1)
39-73: Update logic is sound; watch duplicated updatedAt handlingTitle/content/status/template/meta fields and optional slug are applied correctly, and the “set
publishedAtonly when first publishing” rule is implemented as described. Note that you setupdatedAton the entity, butDatabasePageRepository::update()also overwritesupdated_atin the database with its ownDateTimeImmutable, so the stored timestamp may differ slightly from the in‑memory one; either rely on the repository timestamp only or pass the entity’supdatedAtthrough if you want them to stay in lockstep.src/Cms/Controllers/Admin/Pages.php (2)
33-47: Admin controller wiring is fine; unused$requestparams are harmlessThe constructor’s use of
DatabasePageRepositoryplus Creator/Updater/Deleter matches the rest of the codebase and keeps page logic in the services. Inindex()andcreate(),$requestis currently unused; that’s expected given the router’s signature, so you can safely ignore the PHPMD hints or, if they’re noisy, prefix the parameter as/* Request */ $requestor similar to silence “unused parameter” warnings without changing behavior.Also applies to: 55-94, 102-127
183-205: Align unauthorized handling between edit/update and destroyIn
edit()andupdate(), unauthorized access results in a thrownRuntimeException, which will likely bubble up as a 500, while indestroy()it triggers a redirect with a friendly flash message. For a smoother admin UX (and clearer security posture), consider standardizing on one approach—typically a redirect with an “unauthorized” flash or a proper 403 response—in all three places.Also applies to: 243-252, 304-313
src/Cms/Services/Widget/Widget.php (1)
50-61: Avoid clobbering locals when extracting template data inview()
extract( $data );can overwrite local variables in the template scope; elsewhere (e.g.,Sender::template) you already useEXTR_SKIP. For consistency and safety, prefer the same here.Proposed change:
- extract( $data ); + extract( $data, EXTR_SKIP );src/Cms/Services/Widget/WidgetRenderer.php (2)
46-49:categoryattribute in latest-posts widget is documented but unusedThe docblock lists a
categoryfilter, butrenderLatestPosts()currently ignores it and always calls$this->_postRepository->getPublished( $limit );. Either wire this through (ifIPostRepositorysupports it) or update the comment to avoid misleading consumers.
105-138: Make contact-form field IDs unique per widget instanceThe contact form hard-codes
id="name",id="email", andid="message". If multiple contact-form widgets are rendered on the same page, you’ll end up with duplicate IDs and label associations may break.You can derive IDs from
$formIdto keep them unique while keepingnameattributes stable for the backend:- $html .= " <div class='mb-3'>\n"; - $html .= " <label for='name' class='form-label'>Name</label>\n"; - $html .= " <input type='text' id='name' name='name' class='form-control' required>\n"; - $html .= " </div>\n"; + $html .= " <div class='mb-3'>\n"; + $html .= " <label for='{$formId}_name' class='form-label'>Name</label>\n"; + $html .= " <input type='text' id='{$formId}_name' name='name' class='form-control' required>\n"; + $html .= " </div>\n"; @@ - $html .= " <div class='mb-3'>\n"; - $html .= " <label for='email' class='form-label'>Email</label>\n"; - $html .= " <input type='email' id='email' name='email' class='form-control' required>\n"; - $html .= " </div>\n"; + $html .= " <div class='mb-3'>\n"; + $html .= " <label for='{$formId}_email' class='form-label'>Email</label>\n"; + $html .= " <input type='email' id='{$formId}_email' name='email' class='form-control' required>\n"; + $html .= " </div>\n"; @@ - $html .= " <div class='mb-3'>\n"; - $html .= " <label for='message' class='form-label'>Message</label>\n"; - $html .= " <textarea id='message' name='message' class='form-control' rows='5' required></textarea>\n"; - $html .= " </div>\n"; + $html .= " <div class='mb-3'>\n"; + $html .= " <label for='{$formId}_message' class='form-label'>Message</label>\n"; + $html .= " <textarea id='{$formId}_message' name='message' class='form-control' rows='5' required></textarea>\n"; + $html .= " </div>\n";src/Cms/Repositories/DatabasePageRepository.php (3)
76-80: Slug uniqueness should ultimately be enforced at the DB layerBoth
create()andupdate()guard against duplicate slugs viafindBySlug(), but this is subject to race conditions under concurrent writes and doesn’t replace a database-level unique index onpages.slug.Consider:
- Adding a unique index on
slugin the migration/schema.- Catching integrity violations from PDO (e.g. unique constraint errors) and converting them into the same
Slug already existsexception, while keeping the current pre-check as a nicer UX precondition.Also applies to: 121-126
145-159:update()ignoresPage::getUpdatedAt()set by services
Page\Updater::update()already sets$page->setUpdatedAt( new DateTimeImmutable() );, but the repository discards that and always uses a new timestamp forupdated_at. This breaks the layering contract and can lead to subtle timing discrepancies or make testing harder.You can honor the entity’s
updatedAtwhen present, falling back to “now” otherwise:- $page->getViewCount(), - (new DateTimeImmutable())->format( 'Y-m-d H:i:s' ), + $page->getViewCount(), + $page->getUpdatedAt() + ? $page->getUpdatedAt()->format( 'Y-m-d H:i:s' ) + : (new DateTimeImmutable())->format( 'Y-m-d H:i:s' ),
301-305: Avoid unnecessary author lookups when noauthor_idis set
mapRowToPage()always calls$this->loadAuthor( $page->getAuthorId() );, which will run aSELECT(and then returnnull) even whenauthor_idis0/NULL. GivenloadAuthor()is already defensive about missing tables, this is safe but wasteful.A small guard avoids these pointless queries:
- // Load relationships - $page->setAuthor( $this->loadAuthor( $page->getAuthorId() ) ); + // Load relationships + if( $page->getAuthorId() > 0 ) + { + $page->setAuthor( $this->loadAuthor( $page->getAuthorId() ) ); + }Also applies to: 315-335
src/Cms/Models/Page.php (5)
18-31: Consider initializing required scalars or making them nullable
_title,_slug, and_authorIdare non-nullable typed properties but are only set via setters (orfromArray). If any code instantiatesnew Page()and accesses these before initialization, PHP will throw an error at runtime. You may want to either:
- give them safe defaults in the property declarations/constructor, or
- make them nullable and tighten validation at the service/repository layer.
This is especially relevant if the model is ever used outside
Page::fromArray().Also applies to: 51-54
209-245: KeepauthorIdandauthorin sync and verifyUserresolution
setAuthor()helpfully updates_authorIdwhen aUserwith an ID is provided, butsetAuthorId()does not clear or adjust_author. That’s fine as long as consumers treatauthorIdas the source of truth, but it can drift if both setters are used. Consider documenting this invariant or clearing_authorwhensetAuthorId()is called to avoid stale relations.Also, make sure the
Usertype here resolves to the intended class (either by being in the same namespace or via an explicituseimport) so that theBelongsToattribute and type hints align.
247-278: Optional: validate status values against the defined constants
setStatus()accepts any string, whileisPublished()/isDraft()assume values match the defined constants. To prevent invalid statuses from creeping in (e.g., typos), you could validate insetStatus()(or introduce a small guard/enum) and either normalize or reject unknown values.
357-423: Datetime hydration assumes strings orDateTimeImmutableonlyIn
fromArray(), thepublished_at/created_at/updated_atbranches assume each value is either a string or already aDateTimeImmutable. If the repository or callers ever pass aDateTimeor anotherDateTimeInterfaceimplementation, this will result in aTypeErrorat theset*At(DateTimeImmutable $...)calls.If you expect broader inputs, consider:
- widening the setter type to
DateTimeInterface, or- explicitly normalizing non-
DateTimeImmutableinstances toDateTimeImmutablehere.This keeps the hydration path robust to upstream changes.
438-455: Be aware of potential divergence between in-memoryviewCountand DB
toArray()persists_viewCount, whileDatabasePageRepository::incrementViewCount(int $id)(fromsrc/Cms/Repositories/DatabasePageRepository.php) directly updates the DB counter. If you call the repository increment method on aPageinstance and then re-use that same instance, its_viewCountwill be stale unless you refresh it or also callincrementViewCount()on the entity.Not necessarily a bug, but worth keeping in mind or documenting in the repository/service layer to avoid subtle off‑by‑one issues in stats displays. (Based on relevant_code_snippets.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
resources/config/routes.yaml(2 hunks)resources/database/migrate/20250113000000_create_pages_table.php(1 hunks)resources/views/admin/pages/create.php(1 hunks)resources/views/admin/pages/edit.php(1 hunks)resources/views/admin/pages/index.php(1 hunks)resources/views/pages/show.php(1 hunks)src/Cms/Controllers/Admin/Pages.php(1 hunks)src/Cms/Controllers/Pages.php(1 hunks)src/Cms/Events/PageCreatedEvent.php(1 hunks)src/Cms/Events/PageDeletedEvent.php(1 hunks)src/Cms/Events/PagePublishedEvent.php(1 hunks)src/Cms/Events/PageUpdatedEvent.php(1 hunks)src/Cms/Models/Page.php(1 hunks)src/Cms/Repositories/DatabasePageRepository.php(1 hunks)src/Cms/Repositories/IPageRepository.php(1 hunks)src/Cms/Services/Content/EditorJsRenderer.php(1 hunks)src/Cms/Services/Content/ShortcodeParser.php(1 hunks)src/Cms/Services/Page/Creator.php(1 hunks)src/Cms/Services/Page/Deleter.php(1 hunks)src/Cms/Services/Page/Updater.php(1 hunks)src/Cms/Services/Widget/IWidget.php(1 hunks)src/Cms/Services/Widget/Widget.php(1 hunks)src/Cms/Services/Widget/WidgetRegistry.php(1 hunks)src/Cms/Services/Widget/WidgetRenderer.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (23)
src/Cms/Services/Widget/IWidget.php (2)
src/Cms/Services/Widget/Widget.php (3)
Widget(12-74)getDescription(17-20)getAttributes(25-28)src/Cms/Services/Widget/WidgetRenderer.php (1)
render(33-41)
src/Cms/Events/PageCreatedEvent.php (4)
src/Cms/Events/PageDeletedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Events/PagePublishedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Events/PageUpdatedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Models/Page.php (1)
__construct(51-54)
src/Cms/Events/PageUpdatedEvent.php (4)
src/Cms/Events/PageCreatedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Events/PageDeletedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Events/PagePublishedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Models/Page.php (1)
__construct(51-54)
resources/database/migrate/20250113000000_create_pages_table.php (3)
resources/database/migrate/20250116000000_create_posts_table.php (1)
change(13-34)src/Cms/Repositories/DatabasePageRepository.php (1)
create(74-109)src/Cms/Services/Page/Creator.php (1)
create(39-70)
src/Cms/Services/Content/EditorJsRenderer.php (4)
src/Cms/Services/Content/ShortcodeParser.php (3)
ShortcodeParser(15-188)__construct(20-23)parse(64-81)src/Cms/Controllers/Pages.php (1)
__construct(33-48)src/Cms/Services/Widget/WidgetRenderer.php (2)
__construct(21-24)render(33-41)src/Cms/Services/Widget/Widget.php (1)
sanitizeHtml(69-73)
src/Cms/Services/Page/Updater.php (4)
src/Cms/Controllers/Admin/Pages.php (2)
__construct(33-47)update(231-285)src/Cms/Models/Page.php (12)
__construct(51-54)setTitle(84-88)setContent(126-130)setStatus(258-262)setTemplate(152-156)setMetaTitle(169-173)setMetaDescription(186-190)setMetaKeywords(203-207)setSlug(101-105)getPublishedAt(283-286)setPublishedAt(291-295)setUpdatedAt(351-355)src/Cms/Repositories/DatabasePageRepository.php (2)
__construct(30-33)update(114-162)src/Cms/Repositories/IPageRepository.php (1)
update(44-44)
src/Cms/Services/Widget/WidgetRegistry.php (2)
src/Cms/Services/Content/ShortcodeParser.php (4)
ShortcodeParser(15-188)__construct(20-23)register(31-34)unregister(41-44)src/Cms/Services/Widget/IWidget.php (2)
getName(20-20)render(28-28)
resources/views/admin/pages/index.php (4)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Page.php (9)
getTitle(76-79)getSlug(93-96)getStatus(250-253)getAuthor(229-232)getTemplate(144-147)getViewCount(300-303)getUpdatedAt(343-346)getId(59-62)isPublished(267-270)src/Cms/Models/User.php (1)
getUsername(78-81)src/Cms/Auth/helpers.php (1)
csrf_field(122-126)
src/Cms/Services/Widget/Widget.php (3)
src/Cms/Services/Widget/IWidget.php (2)
getDescription(35-35)getAttributes(42-42)src/Cms/Services/Email/Sender.php (1)
template(105-132)src/Cms/Services/Content/EditorJsRenderer.php (1)
sanitizeHtml(231-237)
src/Cms/Services/Page/Creator.php (4)
src/Cms/Controllers/Admin/Pages.php (2)
__construct(33-47)create(102-127)src/Cms/Models/Page.php (12)
__construct(51-54)setTitle(84-88)setSlug(101-105)setContent(126-130)setTemplate(152-156)setMetaTitle(169-173)setMetaDescription(186-190)setMetaKeywords(203-207)setAuthorId(220-224)setStatus(258-262)setCreatedAt(334-338)setPublishedAt(291-295)src/Cms/Repositories/DatabasePageRepository.php (2)
__construct(30-33)create(74-109)src/Cms/Repositories/IPageRepository.php (1)
create(36-36)
resources/views/pages/show.php (2)
src/Cms/Models/Page.php (5)
getTitle(76-79)getPublishedAt(283-286)getUpdatedAt(343-346)getAuthor(229-232)getViewCount(300-303)src/Cms/Models/User.php (1)
getUsername(78-81)
src/Cms/Controllers/Admin/Pages.php (8)
src/Cms/Controllers/Content.php (3)
Content(60-302)getSessionManager(222-230)redirect(240-251)src/Cms/Repositories/DatabasePageRepository.php (8)
DatabasePageRepository(20-336)__construct(30-33)all(178-203)getByAuthor(224-242)create(74-109)findById(38-51)update(114-162)delete(167-173)src/Cms/Services/Page/Creator.php (3)
Creator(16-85)__construct(20-23)create(39-70)src/Cms/Services/Page/Updater.php (3)
Updater(16-74)__construct(20-23)update(39-73)src/Cms/Services/Page/Deleter.php (3)
Deleter(15-39)__construct(19-22)delete(30-38)src/Cms/Services/Auth/CsrfToken.php (1)
CsrfToken(15-71)src/Cms/Models/Page.php (3)
__construct(51-54)getId(59-62)getAuthorId(212-215)src/Cms/Auth/SessionManager.php (2)
set(99-103)getFlash(144-150)
src/Cms/Controllers/Pages.php (7)
src/Cms/Repositories/DatabasePageRepository.php (4)
DatabasePageRepository(20-336)__construct(30-33)findBySlug(56-69)incrementViewCount(268-274)src/Cms/Repositories/DatabasePostRepository.php (1)
DatabasePostRepository(22-534)src/Cms/Controllers/Content.php (1)
Content(60-302)src/Cms/Services/Content/EditorJsRenderer.php (3)
EditorJsRenderer(12-238)__construct(16-19)render(27-37)src/Cms/Services/Content/ShortcodeParser.php (2)
ShortcodeParser(15-188)__construct(20-23)src/Cms/Services/Widget/WidgetRenderer.php (3)
WidgetRenderer(17-150)__construct(21-24)render(33-41)src/Cms/Models/Page.php (9)
__construct(51-54)isPublished(267-270)incrementViewCount(317-321)getId(59-62)getContent(110-113)getMetaTitle(161-164)getTitle(76-79)getMetaDescription(178-181)getMetaKeywords(195-198)
resources/views/admin/pages/edit.php (3)
src/Cms/Models/Page.php (14)
getTitle(76-79)getId(59-62)getSlug(93-96)getStatus(250-253)getTemplate(144-147)getPublishedAt(283-286)getCreatedAt(326-329)getUpdatedAt(343-346)getViewCount(300-303)getMetaTitle(161-164)getMetaDescription(178-181)getMetaKeywords(195-198)isPublished(267-270)getContentRaw(118-121)src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Auth/helpers.php (1)
csrf_field(122-126)
resources/views/admin/pages/create.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Auth/helpers.php (1)
csrf_field(122-126)
src/Cms/Repositories/DatabasePageRepository.php (5)
src/Cms/Database/ConnectionFactory.php (2)
ConnectionFactory(17-88)createFromSettings(26-36)src/Cms/Models/Page.php (7)
__construct(51-54)getPublishedAt(283-286)setId(67-71)getId(59-62)incrementViewCount(317-321)fromArray(364-431)setAuthor(237-245)src/Cms/Services/Page/Creator.php (2)
__construct(20-23)create(39-70)src/Cms/Services/Page/Updater.php (2)
__construct(20-23)update(39-73)src/Cms/Repositories/IPageRepository.php (11)
findById(20-20)findBySlug(28-28)create(36-36)update(44-44)delete(52-52)all(62-62)getPublished(71-71)getDrafts(78-78)getByAuthor(87-87)count(95-95)incrementViewCount(103-103)
src/Cms/Events/PageDeletedEvent.php (3)
src/Cms/Events/PageCreatedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Events/PagePublishedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Events/PageUpdatedEvent.php (2)
__construct(16-19)getPage(21-24)
src/Cms/Events/PagePublishedEvent.php (4)
src/Cms/Events/PageCreatedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Events/PageDeletedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Events/PageUpdatedEvent.php (2)
__construct(16-19)getPage(21-24)src/Cms/Models/Page.php (1)
__construct(51-54)
src/Cms/Services/Content/ShortcodeParser.php (4)
src/Cms/Services/Widget/WidgetRenderer.php (3)
WidgetRenderer(17-150)__construct(21-24)render(33-41)src/Cms/Services/Content/EditorJsRenderer.php (2)
__construct(16-19)render(27-37)src/Cms/Services/Widget/WidgetRegistry.php (3)
__construct(20-23)register(30-40)unregister(47-54)src/Cms/Services/Widget/IWidget.php (1)
render(28-28)
src/Cms/Services/Widget/WidgetRenderer.php (3)
src/Cms/Controllers/Pages.php (1)
__construct(33-48)src/Cms/Services/Content/ShortcodeParser.php (1)
__construct(20-23)src/Cms/Models/Post.php (1)
getExcerpt(124-127)
src/Cms/Repositories/IPageRepository.php (5)
src/Cms/Repositories/DatabasePageRepository.php (11)
findById(38-51)findBySlug(56-69)create(74-109)update(114-162)delete(167-173)all(178-203)getPublished(208-211)getDrafts(216-219)getByAuthor(224-242)count(247-263)incrementViewCount(268-274)src/Cms/Services/Page/Creator.php (1)
create(39-70)src/Cms/Services/Page/Updater.php (1)
update(39-73)src/Cms/Services/Page/Deleter.php (1)
delete(30-38)src/Cms/Models/Page.php (1)
incrementViewCount(317-321)
src/Cms/Services/Page/Deleter.php (4)
src/Cms/Controllers/Admin/Pages.php (1)
__construct(33-47)src/Cms/Models/Page.php (2)
__construct(51-54)getId(59-62)src/Cms/Repositories/DatabasePageRepository.php (2)
__construct(30-33)delete(167-173)src/Cms/Repositories/IPageRepository.php (1)
delete(52-52)
src/Cms/Models/Page.php (6)
src/Cms/Events/PageCreatedEvent.php (1)
__construct(16-19)src/Cms/Events/PageDeletedEvent.php (1)
__construct(16-19)src/Cms/Events/PagePublishedEvent.php (1)
__construct(16-19)src/Cms/Events/PageUpdatedEvent.php (1)
__construct(16-19)src/Cms/Repositories/DatabasePageRepository.php (2)
__construct(30-33)incrementViewCount(268-274)src/Cms/Repositories/IPageRepository.php (1)
incrementViewCount(103-103)
🪛 PHPMD (2.15.0)
src/Cms/Services/Content/EditorJsRenderer.php
183-183: Avoid unused parameters such as '$data'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/Pages.php
55-55: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
102-102: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (15)
src/Cms/Events/PagePublishedEvent.php (1)
12-24: Event value object looks correct and consistentSimple wrapper around
Pagewith constructor + getter mirrors the other page events and is sufficient for dispatching publish events.src/Cms/Events/PageDeletedEvent.php (1)
12-24: Deletion event implementation is straightforward and matches the patternEncapsulating
Pagein a dedicated deleted event keeps the lifecycle API coherent with the other page events.src/Cms/Events/PageCreatedEvent.php (1)
12-24: Created event is consistent with the existing event patternThe event cleanly wraps the
Pageinstance and aligns with the other lifecycle events.resources/views/pages/show.php (1)
27-29: Clarify trust/sanitization guarantees for$ContentHtml
$ContentHtmlis injected directly into the page without escaping. This is necessary for rich content, but it also means any upstream data (EditorJS rendering, shortcodes, widgets) must be fully sanitized and trusted at this point. Please confirm that the renderer(s) applied before this view robustly strip/encode unsafe tags and attributes; otherwise this becomes an XSS sink for any user-controlled content.resources/views/admin/pages/create.php (2)
7-105: Admin create form + EditorJS wiring looks soundForm structure, CSRF usage, slug handling, and EditorJS serialization into
contentall look correct and should round‑trip data reliably.
163-180: Slug auto-generation logic is reasonable and respects manual overridesThe slug builder normalizes to lowercase, strips non
[a-z0-9]chars, collapses them into dashes, trims leading/trailing dashes, and only overwrites the field while it is marked auto-generated. This provides a good default UX without fighting manual edits.src/Cms/Services/Page/Deleter.php (1)
15-38: Deletion service correctly guards against non-persisted pagesChecking
!$page->getId()before delegating toIPageRepository::delete()avoids invalid deletes, and delegating the boolean result preserves repository semantics. Implementation looks good.resources/database/migrate/20250113000000_create_pages_table.php (1)
15-35: Pages table schema and indexes align well with the intended usageColumns and defaults match the repository code (slug uniqueness, status, published_at, view tracking), and the chosen indexes (slug, author_id, status, published_at) will support common lookups efficiently. FK to
users.idwith cascade also makes sense for author lifecycles.resources/views/admin/pages/edit.php (1)
70-81: Metadata panel correctly surfaces publish info and audit fieldsDisplaying published/created/updated timestamps and view count directly from the
Pagemodel gives editors good context when updating content. Conditional rendering of the published alert based ongetPublishedAt()is also appropriate.src/Cms/Services/Widget/IWidget.php (1)
13-42: Widget interface contract looks solidThe interface defines a clear, minimal contract for widgets/shortcodes (name, render, description, attributes) and matches how the rest of the system appears to use widgets. No issues from a design or correctness standpoint.
src/Cms/Events/PageUpdatedEvent.php (1)
12-24: Event shape is consistent with existing page eventsThis follows the same pattern as the other Page*Event classes (private Page property + getPage()) and keeps the event payload simple and typed. Looks good.
src/Cms/Controllers/Pages.php (1)
57-88: Public page show flow looks solid; double‑check view name resolutionSlug lookup, publish check, view counting, and Editor.js + shortcode rendering are all wired correctly and use the
PageAPI as expected. One thing to verify: passing'show'as the view name is consistent with howrenderHtml()resolves templates (other controllers often use names like'pages/index'), so confirm it actually points atresources/views/pages/show.php.resources/config/routes.yaml (1)
217-253: New page routes are consistent with existing routing schemeThe admin
admin_pages*routes mirror the existing posts/categories/tags CRUD patterns (HTTP verbs,/admin/...paths,authfilter), andpage_showat/pages/:slugcleanly exposes public page viewing without conflicting with blog URLs. No functional issues stand out here.Also applies to: 291-296
src/Cms/Repositories/IPageRepository.php (1)
12-103: Repository interface shape aligns well with implementation and servicesThe method set (ID/slug lookups, CRUD, filtered queries, count, view-count increment) matches
DatabasePageRepositoryand the Page services’ usage cleanly. No issues spotted.src/Cms/Models/Page.php (1)
15-50: Solid model/ORM design and clear status/template APIThe overall model structure, attributes, and the explicit status/template constants make the Page entity easy to reason about and use from repositories/services. This is a clean foundation for the rest of the CMS page flow.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
resources/views/admin/pages/edit.php (1)
136-147: Safe initialization ofexistingContentfrom PHP looks goodEncoding
getContentRaw()viajson_encode(... JSON_HEX_*)intoexistingContentJson, thenJSON.parseinside a try/catch with a{ blocks: [] }fallback is robust and addresses script-breaking/XSS issues from raw JSON injection.src/Cms/Services/Content/EditorJsRenderer.php (3)
67-75: Header level coercion/clamping resolves header injection concernsCoercing
$data['level']to an int and clamping to 1–6 before interpolating into<h{$level}>keeps the markup well-formed even if block data is malformed or tampered with.
111-144: Harden image attribute escaping and URL handling to avoid XSS
$urland$captionare escaped withoutENT_QUOTESand are interpolated into single-quoted attributes, so a caption or URL containing a single quote can break out of the attribute and inject extra attributes or event handlers. Also, unvalidated URLs can allowjavascript:or similar schemes.Consider:
- Escaping with
ENT_QUOTESand a fixed encoding.- Optionally validating
$urlto allow onlyhttp/https(and possiblymailto) schemes.For example:
- $url = htmlspecialchars( $data['file']['url'] ?? '' ); - $caption = htmlspecialchars( $data['caption'] ?? '' ); + $url = htmlspecialchars( $data['file']['url'] ?? '', ENT_QUOTES, 'UTF-8' ); + $caption = htmlspecialchars( $data['caption'] ?? '', ENT_QUOTES, 'UTF-8' ); ... - $html .= " <img src='{$url}' class='{$imgClass}' alt='{$caption}'>\n"; + // Optionally, validate $url scheme here before emitting + $html .= " <img src='{$url}' class='{$imgClass}' alt='{$caption}'>\n";
191-200:sanitizeHtml()still allows unsafe<a>attributes; consider tightening
sanitizeHtml()usesstrip_tags()with$allowedTags = '<b><strong><i><em><u><a><code><mark>';which preserves<a>along with all attributes (href,onclick, etc.). This means payloads likehref="javascript:..."oronclick="..."can still come through, especially from raw blocks.If untrusted user input can reach these blocks, you likely want to either:
- Remove
<a>from$allowedTagsfor now, or- Introduce attribute-level sanitization for
<a>(e.g. allow only safehrefschemes and strip allon*attributes), or- Delegate to a dedicated HTML sanitizer that supports attribute whitelisting.
A minimal quick hardening would be:
- $allowedTags = '<b><strong><i><em><u><a><code><mark>'; + $allowedTags = '<b><strong><i><em><u><code><mark>';and then reintroduce
<a>once you have attribute-level filtering in place.Also applies to: 234-240
src/Cms/Controllers/Admin/Pages.php (3)
136-197: Store action now has proper CSRF validation and robust error handling
store()validates the CSRF token, logs failures, and redirects with a generic error, which addresses the earlier CSRF gap. The try/catch around creation logs detailed exceptions while only exposing a generic failure message to admins, which is a good balance. The post-create falsy check on$pageis effectively defensive, givenCreator::create()always returns aPage.
253-329: Update action correctly enforces auth, CSRF, and service result
update()enforces ownership/role checks, validates the CSRF token, and then uses the Updater service. It also checks the boolean result and distinguishes update failures from successes in both logs and flash messaging, which addresses prior concerns about blindly assuming success.
336-393: Destroy action cleanly enforces permissions, CSRF, and delete result
destroy()’s flow—auth check, page existence, ownership/role enforcement, CSRF validation, then delete via Deleter with boolean result checking and logging—is consistent and robust. Redirects with generic error/success messages avoid leaking internal exception details.src/Cms/Models/Page.php (1)
107-150: JSON content handling now correctly guards againstjson_encode()failures
setContentArray()now encodes to JSON, checks forfalse, and throws aJsonExceptionwithjson_last_error_msg()when encoding fails. This avoids silently assigningfalseto the string-typed_contentRawand aligns with the expectations noted in earlier feedback.
🧹 Nitpick comments (5)
resources/views/admin/pages/edit.php (3)
126-134: Consider pinning Editor.js CDN versions instead of@latestUsing
@latestfor Editor.js and its tools can introduce breaking changes into your admin UI without code changes. Consider pinning specific versions (and updating intentionally) to keep the editor behavior stable.
167-173: Use a route/helper for the image upload endpoint for better portabilityHardcoding
'/admin/upload/image'assumes the app is mounted at/. If it’s ever deployed under a subdirectory or behind a reverse proxy with a prefix, uploads could break. Consider routing this throughroute_path()(or equivalent) for consistency with the rest of the app.
189-193: Optional: guardonChangeeditor.save()with error handling
onChangeawaitseditor.save()without a try/catch. A save failure here will result in an unhandled promise rejection in the console, even though the submit handler has its own guarded save. Not critical, but you could wrapeditor.save()in a try/catch and log or ignore failures to avoid noisy rejections.src/Cms/Services/Content/EditorJsRenderer.php (1)
186-189: Minor: drop unused$dataparameter inrenderDelimiter()
renderDelimiter(array $data)ignores$dataentirely. If you don’t expect any configuration for delimiters, you can remove the parameter (and adjust the call site) or rename to$_datato appease static analysis.src/Cms/Controllers/Admin/Pages.php (1)
25-48: Consider depending on the repository interface instead of the concrete classThe controller currently type-hints and constructs
DatabasePageRepositorydirectly. If you expect other repository implementations or want easier testing, you might prefer depending onIPageRepositoryand injecting it (or constructing via a factory) instead of hardwiring the concrete class here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
resources/views/admin/pages/edit.php(1 hunks)src/Cms/Controllers/Admin/Pages.php(1 hunks)src/Cms/Models/Page.php(1 hunks)src/Cms/Services/Content/EditorJsRenderer.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
resources/views/admin/pages/edit.php (3)
src/Cms/Models/Page.php (14)
getTitle(76-79)getId(59-62)getSlug(93-96)getStatus(261-264)getTemplate(155-158)getPublishedAt(294-297)getCreatedAt(337-340)getUpdatedAt(354-357)getViewCount(311-314)getMetaTitle(172-175)getMetaDescription(189-192)getMetaKeywords(206-209)isPublished(278-281)getContentRaw(118-121)src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Auth/helpers.php (1)
csrf_field(122-126)
src/Cms/Models/Page.php (2)
src/Cms/Repositories/DatabasePageRepository.php (1)
incrementViewCount(268-274)src/Cms/Repositories/IPageRepository.php (1)
incrementViewCount(103-103)
src/Cms/Controllers/Admin/Pages.php (8)
src/Cms/Controllers/Content.php (3)
Content(60-302)getSessionManager(222-230)redirect(240-251)src/Cms/Repositories/DatabasePageRepository.php (8)
DatabasePageRepository(20-336)__construct(30-33)all(178-203)getByAuthor(224-242)create(74-109)findById(38-51)update(114-162)delete(167-173)src/Cms/Services/Page/Creator.php (3)
Creator(16-85)__construct(20-23)create(39-70)src/Cms/Services/Page/Updater.php (3)
Updater(16-74)__construct(20-23)update(39-73)src/Cms/Services/Page/Deleter.php (3)
Deleter(15-39)__construct(19-22)delete(30-38)src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)validate(51-62)src/Cms/Models/Page.php (4)
__construct(51-54)getId(59-62)getAuthorId(223-226)getTitle(76-79)src/Cms/Auth/SessionManager.php (2)
set(99-103)getFlash(144-150)
src/Cms/Services/Content/EditorJsRenderer.php (4)
src/Cms/Services/Content/ShortcodeParser.php (3)
ShortcodeParser(15-188)__construct(20-23)parse(64-81)src/Cms/Controllers/Pages.php (1)
__construct(33-48)src/Cms/Services/Widget/WidgetRenderer.php (2)
__construct(21-24)render(33-41)src/Cms/Services/Widget/Widget.php (1)
sanitizeHtml(69-73)
🪛 PHPMD (2.15.0)
src/Cms/Controllers/Admin/Pages.php
56-56: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
103-103: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Services/Content/EditorJsRenderer.php
186-186: Avoid unused parameters such as '$data'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (7)
resources/views/admin/pages/edit.php (2)
1-124: Admin edit form structure and escaping look solidForm wiring, CSRF usage, and escaping via
htmlspecialchars()for all user-controlled fields (title, slug, SEO metadata, timestamps) are consistent and appropriate. Slug pattern enforcement and status/template selects align well with the controller/service expectations.
195-207: Submit handler pattern for serializing Editor.js content is correctIntercepting submit, awaiting
editor.save(), writing to#content-json, then callinge.target.submit()ensures the latest content is persisted even ifonChangehasn’t fired recently. This matches the controller’s expectation of acontentPOST field.src/Cms/Services/Content/EditorJsRenderer.php (1)
21-37: Render pipeline and block dispatch are clean and maintainableThe
render()loop plusrenderBlock()matchover block types is straightforward and makes it easy to extend with new tools later while keeping concerns separated per renderer method.Also applies to: 45-62
src/Cms/Controllers/Admin/Pages.php (2)
56-95: Index flow, CSRF token generation, and role-based page listing look correct
index()correctly enforces an authenticated user, initializes a CSRF token viaCsrfToken, scopes the page list by role (admin/editor vs author), and passes flash messages into the view. This matches the repo and view expectations.
103-128: Create action wiring and CSRF token provisioning are consistent
create()mirrorsindex()’s auth check and CSRF initialization, then renders the create form with appropriate metadata and user context. This should work cleanly with thecsrf_field()helper in the view.src/Cms/Models/Page.php (2)
51-55: Timestamp initialization and accessors are consistentInitializing
_createdAtin the constructor and exposing created/updated timestamps via typed getters/setters withDateTimeImmutablekeeps temporal data handling predictable. Overwrites fromfromArray()handle persisted records cleanly.Also applies to: 334-367
368-467: fromArray/toArray mappings cover core fields and relationships coherently
fromArray()supports both string and array content, maps status/template/meta/author/timestamps, and respects theUserrelationship if provided.toArray()mirrors these fields, including formatting timestamps for persistence. This should integrate smoothly withDatabasePageRepository::mapRowToPage()and any serialization flows.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…nto feature/dynamic-content
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/Cms/Services/Content/ShortcodeParser.php (3)
64-82: Regex patterns now correctly support hyphens in shortcode names.The patterns have been updated to use
[\w-]+(line 68), which correctly matches shortcodes like[latest-posts]. This resolves the previous critical issue where hyphenated shortcodes would not parse.
131-177: Built-in shortcode support correctly limited to implemented widgets.The implementation now accurately reflects available functionality—only
latest-postsis declared inhasBuiltInShortcode()(line 176) and handled in the match expression (line 154). This addresses the previous concern about unsupported shortcodes (calendar,pricing,team,testimonials) by removing them from the built-in list until their implementations are added.Error handling with try-catch blocks and logging is appropriate for production use.
52-56:hasShortcode()returns true for built-ins even when_widgetRendereris null.
hasShortcode('latest-posts')will returntrueviahasBuiltInShortcode()even when_widgetRendererisnull, butrenderShortcode()(line 148) will then return an "Unknown shortcode" comment because it checks for the renderer's presence.Apply this diff to make
hasShortcode()consistent with actual rendering capability:public function hasShortcode( string $shortcode ): bool { return isset( $this->_customHandlers[$shortcode] ) - || $this->hasBuiltInShortcode( $shortcode ); + || ( $this->_widgetRenderer && $this->hasBuiltInShortcode( $shortcode ) ); }src/Cms/Services/Page/Creator.php (1)
72-95: Empty-slug issue from the prior review is now resolved for pages.The updated
generateSlug()(Lines 81-94) trims the normalized slug and falls back to'page-' . uniqid()when nothing usable remains, so titles made only of non-ASCII or stripped characters no longer yield an empty slug, addressing the earlier concern about page slugs breaking routing or constraints.
🧹 Nitpick comments (9)
src/Cms/Controllers/Admin/Tags.php (1)
227-251: Avoid duplicating slug logic and tidygenerateSlug()docblock.The controller’s
generateSlug()(Lines 237-251) correctly matches the non-ASCII-safe slug behavior from the services, but it now duplicates the same regex/fallback logic found elsewhere. Consider centralizing this into a shared helper or delegating to the tag service so future changes only need to be made once, and you may also want to drop the@throws \Exceptionannotation here since this method does not throw.src/Cms/Services/Category/Updater.php (1)
67-90: Non-ASCII category names now get a safe fallback slug.The added trimming and fallback in
generateSlug()(Lines 78-89) ensure auto-generated category slugs are never empty, even when the name contains only non-ASCII or otherwise stripped characters. Note that the caller usesempty($slug)before invoking this helper, so values like'0'would also trigger auto-generation; that’s probably fine but worth being aware of.src/Cms/Services/Post/Creator.php (1)
88-111: Slug fallback for posts is solid; consider a shared slug utility.The new fallback in
generateSlug()(Lines 99-110) correctly handles titles that normalize to an empty string and brings posts in line with tags, categories, and pages. Given this identical slug algorithm now lives in several services/controllers, it would be worth extracting a shared slug-generation helper to keep the regex and fallback behavior DRY.src/Cms/Services/Page/Creator.php (1)
39-70: Page creation flow looks solid; consider events/docs for failures.The
create()method (Lines 39-70) cleanly wires up title, Editor.js content, template, meta fields, author, status, timestamps, and auto-setspublishedAtwhen status isPage::STATUS_PUBLISHED, matching the post/category patterns. For parity withCategory\Creator, you might also emit aPageCreatedEventhere (if pages participate in your event pipeline) and consider documenting thatcreate()can throw whenIPageRepository::create()rejects a duplicate slug.src/Cms/Services/Post/Updater.php (1)
88-107: Slug fallback logic is sound and aligns with creation flowThe additional trim plus the empty-slug fallback using
'post-' . uniqid()correctly handles titles that slugify to an empty string (non‑ASCII, punctuation‑only, etc.) and avoids invalid/empty slugs during updates. This also keeps behavior consistent with the page/post creation services. If you ever want more human‑readable fallbacks for non‑ASCII titles, you could later swap in a transliteration step (e.g., usingintl’s transliterator) before the current regex chain, but the current implementation is perfectly fine as is.src/Cms/Controllers/Admin/Pages.php (4)
34-48: Controller wiring is reasonable; consider future DI for testabilityInstantiating
DatabasePageRepositoryand the page services directly in the constructor is consistent with the existing Registry pattern, and the setup looks correct. Longer‑term, if you move further toward dependency injection, this would be a natural candidate to accept anIPageRepository(or factory) via the baseApplication/container instead of constructing it here, which would simplify testing and swapping implementations.
56-95: Index action logic and CSRF token generation look correctAuth lookup, role‑based page querying (
all()vsgetByAuthor()), CSRF token generation, and passing success/error flashes into the view are all wired cleanly and consistent with other controllers. The$requestparameter is not used here, but it’s likely required by the router signature; if you want to quiet static analysis, you could either prefix it with an underscore ($_request) or add a@noinspection/@phpstan-ignore-next-linestyle hint, but functionally this is fine.
103-128: Create action is straightforward; same note on unused RequestThe create action mirrors the index pattern: user check, CSRF token creation, and simple view rendering, which is appropriate. As with
index(),$requestis currently unused; if your tooling complains, consider the same minor suppression or naming convention tweak, otherwise this is acceptable as‑is.
136-197: Store action covers CSRF, logging, and failures appropriatelyThe store handler validates CSRF before processing, logs failed validations, and redirects with a generic error — good from a security and UX standpoint. The creation flow uses the service correctly, passes
nullwhen slug/meta fields are effectively empty, and checks for a falsy$pagebefore logging and redirecting, with exceptions logged server‑side only. Overall this is solid; any additional validation (e.g., enforcing non‑empty titles or status whitelisting) can be layered in later if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/Cms/Controllers/Admin/Pages.php(1 hunks)src/Cms/Controllers/Admin/Tags.php(2 hunks)src/Cms/Services/Category/Creator.php(2 hunks)src/Cms/Services/Category/Updater.php(2 hunks)src/Cms/Services/Content/ShortcodeParser.php(1 hunks)src/Cms/Services/Page/Creator.php(1 hunks)src/Cms/Services/Post/Creator.php(2 hunks)src/Cms/Services/Post/Updater.php(2 hunks)src/Cms/Services/Tag/Creator.php(2 hunks)src/Cms/Services/Widget/WidgetRenderer.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Cms/Services/Widget/WidgetRenderer.php
🧰 Additional context used
🧬 Code graph analysis (3)
src/Cms/Services/Content/ShortcodeParser.php (5)
src/Cms/Services/Widget/Widget.php (1)
Widget(12-74)src/Cms/Services/Widget/WidgetRenderer.php (3)
WidgetRenderer(17-103)__construct(21-24)render(33-40)src/Cms/Services/Content/EditorJsRenderer.php (2)
__construct(16-19)render(27-37)src/Cms/Services/Widget/WidgetRegistry.php (2)
register(30-40)unregister(47-54)src/Cms/Services/Widget/IWidget.php (1)
render(28-28)
src/Cms/Services/Page/Creator.php (7)
src/Cms/Services/Category/Creator.php (4)
Creator(18-90)__construct(22-25)create(36-64)generateSlug(75-89)src/Cms/Services/Post/Creator.php (4)
Creator(18-112)__construct(24-33)create(49-86)generateSlug(97-111)src/Cms/Services/Tag/Creator.php (4)
Creator(15-64)__construct(19-22)create(31-38)generateSlug(49-63)src/Cms/Controllers/Admin/Pages.php (2)
__construct(34-48)create(103-128)src/Cms/Models/Page.php (12)
__construct(51-54)setTitle(84-88)setSlug(101-105)setContent(126-130)setTemplate(163-167)setMetaTitle(180-184)setMetaDescription(197-201)setMetaKeywords(214-218)setAuthorId(231-235)setStatus(269-273)setCreatedAt(345-349)setPublishedAt(302-306)src/Cms/Repositories/IPageRepository.php (1)
create(36-36)src/Cms/Repositories/DatabasePageRepository.php (1)
create(74-109)
src/Cms/Controllers/Admin/Pages.php (9)
src/Cms/Controllers/Content.php (3)
Content(60-302)getSessionManager(222-230)redirect(240-251)src/Cms/Repositories/DatabasePageRepository.php (7)
DatabasePageRepository(20-336)all(178-203)getByAuthor(224-242)create(74-109)findById(38-51)update(114-162)delete(167-173)src/Cms/Services/Page/Creator.php (3)
Creator(16-96)__construct(20-23)create(39-70)src/Cms/Services/Page/Updater.php (2)
Updater(16-74)update(39-73)src/Cms/Services/Page/Deleter.php (2)
Deleter(15-39)delete(30-38)src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)validate(51-62)src/Cms/Models/Page.php (4)
__construct(51-54)getId(59-62)getAuthorId(223-226)getTitle(76-79)src/Cms/Auth/SessionManager.php (2)
set(99-103)getFlash(144-150)src/Cms/Models/User.php (1)
isEditor(154-157)
🪛 PHPMD (2.15.0)
src/Cms/Controllers/Admin/Pages.php
56-56: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
103-103: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (7)
src/Cms/Services/Content/ShortcodeParser.php (2)
1-23: LGTM! Clean structure with optional dependency injection.The class structure is well-organized with clear separation between custom handlers and built-in widget rendering. Optional
WidgetRendererinjection provides flexibility for different use cases.
90-122: Attribute parsing correctly handles hyphens and type conversion.The attribute regex pattern (line 96) now uses
[\w-]+to support hyphenated attribute names likedata-id. The type conversion logic for booleans and numbers is well-implemented and handles common cases appropriately.src/Cms/Services/Tag/Creator.php (1)
40-63: Slug fallback for tags correctly handles non-ASCII names.The updated
generateSlug()logic (Lines 51-62) now guarantees a non-empty slug even when the tag name normalizes to an empty string, and the docblock clearly describes this behavior. This aligns tag slugs with the patterns used for posts, categories, and pages and avoids empty-slug routing/DB edge cases.src/Cms/Services/Category/Creator.php (1)
66-89: Category slug generation is now consistent across create/update.The updated
generateSlug()(Lines 77-88) mirrors the Updater’s trimming and non-ASCII fallback, so auto-generated category slugs are consistently non-empty in both creation and update flows.src/Cms/Controllers/Admin/Pages.php (3)
205-246: Edit action permission checks and CSRF handling are consistentLookup, 404‑style redirect on missing page, permission checks (admin/editor/owner), logging for unauthorized attempts, and CSRF token generation for the form all look correct and consistent with update/destroy. Passing the
pageobject and user into the view aligns with expected templates.
254-330: Update action correctly enforces auth, CSRF, and service result checksThe update flow validates user, ensures the page exists, enforces permissions, validates CSRF, and then delegates to the updater service. Checking the boolean
$successand redirecting with an error when it’s false avoids false “success” flashes, and exception handling logs detailed context while returning a generic message to the admin. The parameter mapping into_pageUpdater->update()matches the service signature and preserves the “don’t overwrite slug on empty input” behavior via$slug ?: null. This looks good.
337-394: Destroy action cleanly handles permissions, CSRF, and failure modesDelete maintains the same pattern: user and page lookup, permission checks with logging, CSRF validation with warning logs, then calling the deleter service and checking its boolean result before deciding which flash message to show. Storing
$pageTitlebefore deletion for logging is a nice touch, and exception handling mirrors the other actions. No issues spotted here.
Adds the ability to create dynamic pages using a block editor.
Adds the ability to create short codes in order to embed data driven content.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.