-
Notifications
You must be signed in to change notification settings - Fork 0
allows editing post pubished dates. Auto updates post slug. #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds scheduled-post support: UI changes for slug and a Published Date/Time field, DTOs accept optional Changes
Sequence Diagram(s)mermaid Browser->>AdminUI: fill title/slug/status/published_at Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 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 |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@resources/views/admin/posts/create.php`:
- Around line 248-255: The current change handler only hides/shows the
published-at UI but does not clear or disable the underlying input, causing
stale published_at to be submitted; update the handler tied to
document.getElementById('status') so that when the status is 'published' or
'scheduled' it sets the published-at input (e.g., the element inside
'published-at-wrapper', such as id 'published-at' or name 'published_at') to
enabled and when hidden it clears its value (set to empty) and sets
disabled=true to prevent submission; also invoke the same logic once on page
load (or on DOMContentLoaded) to sync the initial visibility/state.
In `@resources/views/admin/posts/edit.php`:
- Around line 47-51: Add client-side validation to ensure a published date/time
is provided when the post status is set to "scheduled": hook into the post form
submit handler (element id 'post-form'), read the status field (id 'status') and
the published date input (id 'published_at'), and prevent submit with an inline
error (or alert) if status === 'scheduled' and published_at is empty; also
update the helper text near the published_at input (or the display logic in
'published-at-wrapper') to clearly state that scheduled posts require a future
publish date/time.
🧹 Nitpick comments (2)
src/Cms/Services/Post/Updater.php (1)
59-85: Guardpublished_atupdates by status to avoid drafts carrying a publish timestamp.Right now any non-empty
published_atis applied regardless of status. If a user toggles from scheduled/published back to draft (or an API sendspublished_atwith draft), drafts can still get a publish date. Consider applyingpublished_atonly for published/scheduled statuses (and ignore/clear otherwise). Please mirror the guard insrc/Cms/Services/Post/Creator.phpfor parity.♻️ Suggested adjustment
- // Business rule: set published date - if( $publishedAt && trim( $publishedAt ) !== '' ) - { - // Use provided published date - $post->setPublishedAt( new \DateTimeImmutable( $publishedAt ) ); - } - elseif( $status === ContentStatus::PUBLISHED->value && !$post->getPublishedAt() ) - { - // Auto-set to now for published posts when not provided and not already set - $post->setPublishedAt( new \DateTimeImmutable() ); - } + // Business rule: set published date (only for published/scheduled) + if( in_array( $status, [ ContentStatus::PUBLISHED->value, ContentStatus::SCHEDULED->value ], true ) ) + { + if( $publishedAt && trim( $publishedAt ) !== '' ) + { + // Use provided published date + $post->setPublishedAt( new \DateTimeImmutable( $publishedAt ) ); + } + elseif( $status === ContentStatus::PUBLISHED->value && !$post->getPublishedAt() ) + { + // Auto-set to now for published posts when not provided and not already set + $post->setPublishedAt( new \DateTimeImmutable() ); + } + }resources/views/admin/posts/edit.php (1)
20-21: Slug pattern allows leading/trailing hyphens.The pattern
[a-z0-9-]+permits slugs like--test--, while the auto-generation code strips leading/trailing hyphens. Consider a stricter pattern if you want consistency:^[a-z0-9]+(-[a-z0-9]+)*$Otherwise, the changes look good—removing
requiredto enable auto-generation and the helper text update are clear.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
resources/views/admin/posts/create.phpresources/views/admin/posts/edit.phpsrc/Cms/Dtos/posts/create-post-request.yamlsrc/Cms/Dtos/posts/update-post-request.yamlsrc/Cms/Services/Post/Creator.phpsrc/Cms/Services/Post/Updater.php
🧰 Additional context used
🧬 Code graph analysis (3)
src/Cms/Services/Post/Updater.php (2)
src/Cms/Models/Post.php (2)
setPublishedAt(300-304)getPublishedAt(292-295)src/Cms/Models/Page.php (2)
setPublishedAt(306-310)getPublishedAt(298-301)
src/Cms/Services/Post/Creator.php (2)
src/Cms/Models/Post.php (1)
setPublishedAt(300-304)src/Cms/Models/Page.php (1)
setPublishedAt(306-310)
resources/views/admin/posts/edit.php (3)
src/Cms/Models/Tag.php (1)
getSlug(72-75)src/Cms/Models/Post.php (3)
getSlug(96-99)getStatus(251-254)getPublishedAt(292-295)src/Cms/Models/Category.php (1)
getSlug(73-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Cursor Bugbot
- GitHub Check: build-test (sqlite)
- GitHub Check: build-test (postgres)
- GitHub Check: build-test (mysql)
🔇 Additional comments (8)
src/Cms/Dtos/posts/create-post-request.yaml (1)
38-46: DTO update looks consistent with the update flow.Optional
published_atand thescheduledstatus align cleanly with the update DTO.src/Cms/Services/Post/Creator.php (1)
59-81: LGTM — published_at handling matches the update flow.Explicit dates are respected and the auto-set behavior for published posts is preserved.
resources/views/admin/posts/create.php (3)
19-20: Slug input validation/help text looks good.Clear guidance and a tight pattern for URL-safe slugs.
42-50: Scheduled status option + Published Date/Time field integrate cleanly.The UI now aligns with the new status and publish date workflow.
228-245: Auto-slug generation logic is clear and user-friendly.The auto-generated vs. manual edit tracking is straightforward.
src/Cms/Dtos/posts/update-post-request.yaml (1)
38-46: No action required—scheduled status is fully implemented.All concerns have been verified:
ContentStatusenum includesSCHEDULED, theschedule()method inPublisherenforces that scheduled posts must have a futurepublished_atdate, repository methods support querying scheduled posts, and downstream services properly handle the new status. The implementation is complete and correct.resources/views/admin/posts/edit.php (2)
43-43: LGTM!The new "Scheduled" status option follows the same pattern as existing options and integrates well with the published_at visibility logic.
254-281: LGTM!The JavaScript logic is well-implemented:
- Auto-generation respects existing slugs on page load (since
dataset.autoGeneratedis undefined)- Clearing the slug re-enables auto-generation on subsequent title changes
- The published-at visibility toggle correctly matches the server-side initial display condition
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| throw new \InvalidArgumentException( 'Scheduled posts require a published date' ); | ||
| } | ||
| $post->setPublishedAt( $this->parseDateTime( $publishedAt ) ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scheduled posts accept past dates, bypassing validation
Medium Severity
The new scheduling logic in Creator and Updater validates that scheduled posts have a published date, but unlike Publisher::schedule(), it doesn't validate that the date is in the future. This allows creating or updating scheduled posts with past dates, which is logically inconsistent. A post scheduled for a past date would be immediately eligible for publishing, defeating the purpose of scheduling.
Note
Adds scheduling and improves publishing/slug workflows across UI, DTOs, and services.
resources/views/admin/posts/{create,edit}.php): addscheduledstatus; showpublished_atfield forpublished|scheduled; enforce client-side check that scheduled posts include a date; makeslugoptional with[a-z0-9-]+pattern and auto-generate fromtitlewith manual override.create-post-request.yaml,update-post-request.yaml): addpublished_at(datetime-local pattern) and includescheduledinstatusenum.Post/Creator.php,Post/Updater.php): requirepublished_atforscheduled; parse dates withparseDateTime(throws on invalid); ifpublishedwithout a date, auto-set to now (preserves existing date on update); continue generating slug when absent.published_atbehavior on create/update.Written by Cursor Bugbot for commit 4265a66. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.