-
Notifications
You must be signed in to change notification settings - Fork 0
Replace native pdo code with new orm component. #16
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
Conversation
WalkthroughAdds Neuron ORM dependency and initializes it; converts core models to ORM models; adds per-user timezone storage and UI to edit it; centralizes date/date-time formatting helpers and updates views to use them; updates install migration and tests to include users.timezone column. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as User (Browser)
participant Ctrl as ProfileController
participant Svc as Updater
participant Repo as DatabaseUserRepository
participant Model as User Model
Browser->>Ctrl: GET /admin/profile
Ctrl->>Ctrl: DateTimeZone::listIdentifiers()
Ctrl-->>Browser: Render profile form with timezone select
Browser->>Ctrl: POST /admin/profile (includes timezone)
Ctrl->>Svc: update(user, ..., timezone)
Svc->>Model: setTimezone(timezone)
Svc->>Repo: update(user)
Repo->>Repo: UPDATE users SET timezone=?
Repo-->>Svc: success
Svc-->>Ctrl: updated user
Ctrl-->>Browser: redirect / success
sequenceDiagram
participant View as Template
participant Helper as format_user_datetime()
participant Registry as Auth/Settings Registry
participant DateLib as DateTime/DateTimeZone
View->>Helper: format_user_datetime(utcDate, fmt)
Helper->>Registry: fetch Auth.User and Settings
alt user timezone exists
Helper->>DateLib: convert UTC -> userTZ
else fallback
Helper->>DateLib: use system timezone
end
Helper-->>View: formatted string or 'N/A'
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/Cms/Models/Post.php (1)
438-499: Honor thestaticreturn contract infromArray.Line 441 declares a
staticreturn but still instantiatesnew self(), soChildPost::fromArray(...)would hand back aPost, breaking the new contract. Please instantiate withnew static()instead.- $post = new self(); + $post = new static();src/Cms/Models/Category.php (1)
143-175: Match instantiation with the newstaticsignature.Line 146 now promises a
staticreturn, yet it still constructs vianew self(). Subclasses callingCategory::fromArray()would receive the base class. Swap tonew static()to keep the method honest.- $category = new self(); + $category = new static();src/Cms/Models/Tag.php (1)
125-156: Return the subclass fromfromArray.Line 128 advertises
staticbut continues to createnew self(), so inheritance callers get the wrong type. Usenew static()so the runtime instance aligns with the declared return.- $tag = new self(); + $tag = new static();src/Cms/Models/User.php (1)
438-489: Respect thestaticreturn infromArray.Line 441 keeps
new self()while the signature now returnsstatic, so any subclass invokingUser::fromArray()would still get aUser. Please construct withnew static().- $user = new self(); + $user = new static();
🧹 Nitpick comments (1)
resources/views/admin/dashboard/index.php (1)
31-33: Remove redundant null check.The conditional check on Line 31 is now redundant since
format_user_datetime()handles null values by returning 'N/A'. You can simplify this code.Apply this diff to simplify the code:
-<?php if($User->getLastLoginAt()): ?> - <p class="mb-0"><strong>Last Login:</strong> <?= format_user_datetime($User->getLastLoginAt(), 'F j, Y g:i A') ?></p> -<?php endif; ?> +<p class="mb-0"><strong>Last Login:</strong> <?= format_user_datetime($User->getLastLoginAt(), 'F j, Y g:i A') ?></p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
composer.json(1 hunks)resources/views/admin/dashboard/index.php(1 hunks)resources/views/admin/posts/index.php(1 hunks)resources/views/admin/profile/edit.php(1 hunks)resources/views/admin/users/index.php(1 hunks)resources/views/blog/category.php(1 hunks)resources/views/blog/index.php(1 hunks)resources/views/blog/show.php(1 hunks)resources/views/blog/tag.php(1 hunks)src/Bootstrap.php(2 hunks)src/Cms/Cli/Commands/Install/InstallCommand.php(1 hunks)src/Cms/Controllers/Admin/ProfileController.php(3 hunks)src/Cms/Models/Category.php(3 hunks)src/Cms/Models/Post.php(3 hunks)src/Cms/Models/Tag.php(2 hunks)src/Cms/Models/User.php(5 hunks)src/Cms/Repositories/DatabaseUserRepository.php(5 hunks)src/Cms/Services/User/Updater.php(3 hunks)src/Cms/View/helpers.php(1 hunks)tests/Cms/Auth/AuthManagerTest.php(1 hunks)tests/Cms/Repositories/DatabaseUserRepositoryTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
resources/views/blog/index.php (2)
src/Cms/View/helpers.php (1)
format_user_date(163-166)src/Cms/Models/Post.php (1)
getCreatedAt(280-283)
resources/views/admin/users/index.php (2)
src/Cms/View/helpers.php (1)
format_user_datetime(121-146)src/Cms/Models/User.php (1)
getCreatedAt(345-348)
src/Cms/Repositories/DatabaseUserRepository.php (1)
src/Cms/Models/User.php (1)
getTimezone(396-399)
resources/views/admin/posts/index.php (2)
src/Cms/View/helpers.php (1)
format_user_datetime(121-146)src/Cms/Models/Post.php (1)
getCreatedAt(280-283)
resources/views/admin/dashboard/index.php (2)
src/Cms/View/helpers.php (1)
format_user_datetime(121-146)src/Cms/Models/User.php (1)
getLastLoginAt(379-382)
src/Cms/View/helpers.php (2)
src/Cms/Auth/SessionManager.php (1)
get(105-109)src/Cms/Models/User.php (2)
getTimezone(396-399)setTimezone(404-408)
resources/views/blog/show.php (1)
src/Cms/View/helpers.php (1)
format_user_date(163-166)
src/Cms/Services/User/Updater.php (1)
src/Cms/Models/User.php (1)
setTimezone(404-408)
src/Cms/Models/Category.php (2)
src/Cms/Models/Post.php (2)
Table(15-524)fromArray(438-500)src/Cms/Models/Tag.php (2)
Table(15-173)fromArray(125-156)
resources/views/blog/category.php (2)
src/Cms/View/helpers.php (1)
format_user_date(163-166)src/Cms/Models/Post.php (1)
getCreatedAt(280-283)
resources/views/admin/profile/edit.php (1)
src/Cms/Models/User.php (1)
getTimezone(396-399)
src/Cms/Models/Post.php (3)
src/Cms/Models/Category.php (2)
Table(15-193)fromArray(143-175)src/Cms/Models/Tag.php (2)
Table(15-173)fromArray(125-156)src/Cms/Models/User.php (2)
Table(14-490)fromArray(438-489)
src/Cms/Models/Tag.php (2)
src/Cms/Models/Category.php (2)
Table(15-193)fromArray(143-175)src/Cms/Models/Post.php (2)
Table(15-524)fromArray(438-500)
src/Cms/Models/User.php (2)
src/Cms/Models/Category.php (2)
Table(15-193)fromArray(143-175)src/Cms/Models/Post.php (2)
Table(15-524)fromArray(438-500)
src/Bootstrap.php (1)
src/Cms/Database/ConnectionFactory.php (2)
ConnectionFactory(17-88)createFromSettings(26-36)
resources/views/blog/tag.php (2)
src/Cms/View/helpers.php (1)
format_user_date(163-166)src/Cms/Models/Post.php (1)
getCreatedAt(280-283)
🪛 PHPMD (2.15.0)
src/Cms/Models/Category.php
27-27: Avoid unused private fields such as '$_posts'. (undefined)
(UnusedPrivateField)
src/Cms/Models/Tag.php
26-26: Avoid unused private fields such as '$_posts'. (undefined)
(UnusedPrivateField)
src/Cms/Models/User.php
36-36: Avoid unused private fields such as '$_posts'. (undefined)
(UnusedPrivateField)
🔇 Additional comments (18)
tests/Cms/Repositories/DatabaseUserRepositoryTest.php (1)
66-66: LGTM!The timezone column addition aligns with the broader timezone support being introduced in this PR. The default value of 'UTC' and VARCHAR(50) sizing are appropriate for storing timezone identifiers.
tests/Cms/Auth/AuthManagerTest.php (1)
80-80: LGTM!The timezone column addition is consistent with the schema changes across the codebase and properly supports the new timezone functionality.
src/Cms/Cli/Commands/Install/InstallCommand.php (1)
869-869: LGTM!The timezone column addition to the migration template is consistent with the schema changes throughout the PR. The limit of 50 characters and default of 'UTC' are appropriate.
resources/views/blog/tag.php (1)
24-24: LGTM!The switch to
format_user_date()centralizes date formatting logic and adds timezone awareness, consistent with the broader refactoring in this PR.resources/views/admin/posts/index.php (1)
30-30: LGTM!The adoption of
format_user_datetime()centralizes date/time formatting and adds timezone support, aligning with the standardization effort across all views in this PR.resources/views/admin/users/index.php (2)
30-30: LGTM!The switch to
format_user_datetime()centralizes date formatting and adds timezone support, consistent with the broader refactoring across views.
33-33: The review comment is incorrect; this is an intentional naming convention, not a bug.The codebase consistently uses
$User(capital U) for the authenticated user and$user(lowercase u) for item users. In the UserController'sindex()method, the viewData explicitly passes'User' => $user(authenticated) and'users' => $users(list), establishing this pattern deliberately. The same convention appears in theedit()method with'User' => $currentUserand'user' => $user.In the view at line 33, the comparison
if( $User->getId() !== $user->getId() )correctly prevents users from deleting themselves by checking if the authenticated user differs from the current loop user. This naming convention is semantic and consistent throughout the codebase.Likely an incorrect or invalid review comment.
composer.json (1)
18-18: I'll search for information about this PHP package to verify its stability and production readiness.Let me search more specifically for the neuron-php/orm package.
The "neuron-php/orm" package at version 0.1. cannot be located in public Composer registries or repositories.*
Despite extensive searching, I found no evidence of this package on Packagist, GitHub, or related package repositories. The Neuron-PHP framework (neuronphp.com) lists multiple published components (core, patterns, routing, validation, jobs, etc.), but an ORM component does not appear to be publicly available.
This could indicate the package is:
- Private or not yet published to Packagist
- A work-in-progress or internal-only package
- Incorrectly specified in the dependency list
Before proceeding with production deployment, verify that this dependency is correctly named and accessible to your deployment environment, and confirm its stability status directly with your team or package maintainer.
resources/views/blog/category.php (1)
27-27: LGTM! Centralized date formatting applied.The change to use
format_user_date()is consistent with the broader refactoring to centralize date/time rendering with timezone support.resources/views/blog/index.php (1)
20-20: LGTM! Consistent with centralized date formatting.Mirrors the refactoring pattern applied across other blog views.
resources/views/blog/show.php (1)
11-11: LGTM! Consistent date formatting.The refactoring to use
format_user_date()is consistent with the broader pattern.src/Cms/Services/User/Updater.php (1)
44-72: LGTM! Timezone parameter properly integrated.The timezone parameter follows the same pattern as the password handling, with appropriate null/empty checks before updating the user model.
src/Cms/Controllers/Admin/ProfileController.php (2)
62-69: LGTM! Timezone data properly prepared for view.The controller correctly retrieves available timezones and injects them into the view data.
98-128: LGTM! Timezone properly handled in update flow.The timezone value is correctly retrieved from POST data and passed to the updater, consistent with the other optional parameters.
src/Cms/Repositories/DatabaseUserRepository.php (3)
103-126: LGTM! Timezone properly persisted on user creation.The timezone column is correctly added to the INSERT statement and bound with the user's timezone value.
157-190: LGTM! Timezone properly persisted on user update.The timezone field is correctly included in the UPDATE statement and bound with the user's timezone value.
232-262: LGTM! Timezone properly mapped with sensible default.The mapping correctly reads the timezone from the database row and provides a sensible fallback to 'UTC' when the column is missing, ensuring backward compatibility.
src/Bootstrap.php (1)
41-56: Verify error handling around repository operations; runtime failures likely if ORM initialization silently fails.The graceful ORM initialization allows the application to start without a database connection, which is intentional. However, the repository pattern abstraction masks a significant risk: if
Model::setPdo()fails and only logs the error, the first database operation will crash without warning.Search results show the application is database-dependent with extensive repository usage (create, update, delete, find operations) throughout controllers and services. These operations are not guarded by exception handling based on the code examined. When the application attempts its first ORM operation, it will fail with an unhandled exception if the PDO connection was never initialized.
Recommendation: Either (1) make ORM initialization a hard requirement that fails startup if database is unavailable, or (2) add runtime guards and graceful error handling around all repository operations to prevent cascading failures when database operations are attempted.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: ljonesfl <1099983+ljonesfl@users.noreply.github.com>
[WIP] Fix variable shadowing in timezone handling logic
This pr replaces all of the native pdo based database code with the new orm component.
Summary by CodeRabbit
New Features
Chores