-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
move the user model to config #16
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces a configuration-driven approach to model references in the Filament Notes application. A new Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (5)
config/filament-notes.php (1)
4-6
: Add documentation for configuration options.While the configuration structure is clean, it would be helpful to add documentation comments explaining:
- The purpose of the
models
configuration- Available options and their expected values
- Example of how to override with a custom user model
Add documentation comments above the configuration:
<?php +/** + * Filament Notes Configuration + * + * models.user: The user model class used for note ownership and relationships. + * To use a custom user model, override this in your config: + * 'models' => [ + * 'user' => \App\Models\CustomUser::class, + * ] + */ return [ 'models' => [ 'user' => App\Models\User::class, ] ];resources/views/widget/notes-widget.blade.php (1)
9-10
: Improve query readability by extracting conditions.The nested query conditions are complex and could benefit from extraction into a dedicated scope or method.
Consider moving the query logic to the Note model:
$notes = \TomatoPHP\FilamentNotes\Models\Note::query() - ->where('is_public', 0) - ->where('is_pined', 1) - ->where('user_id', auth()->user()->id) - ->orWhere('is_public', 0) - ->where('is_pined', 1) - ->whereHas('noteMetas', function ($q){ - $q->where('key', config('filament-notes.models.user')) - ->where('value',(string)auth()->user()->id); - }) - ->orWhere('is_public', 1) - ->where('is_pined', 1) + ->pinnedNotesForUser(auth()->user()) ->orderBy('created_at', 'desc') ->limit(filament('filament-notes')->widgetLimit) ->get();Add this scope to the Note model:
public function scopePinnedNotesForUser($query, $user) { return $query->where(function($q) use ($user) { $q->where('is_public', 0) ->where('is_pined', 1) ->where(function($sq) use ($user) { $sq->where('user_id', $user->id) ->orWhereHas('noteMetas', function ($mq) use ($user) { $mq->where('key', config('filament-notes.models.user')) ->where('value', (string)$user->id); }); }) ->orWhere(function($sq) { $sq->where('is_public', 1) ->where('is_pined', 1); }); }); }src/Livewire/NoteAction.php (3)
203-203
: Consider making model type options more extensible.While the current implementation works, consider making the model type options configurable as well. This would allow for easier addition of other model types in the future without code changes.
Example approach:
- config('filament-notes.models.user') => 'Users', + ...config('filament-notes.model_types', [ + config('filament-notes.models.user') => 'Users', + ]),
216-216
: Add defensive check for config value.The code assumes the config value exists. Consider adding a fallback or validation to handle cases where the config might be missing.
Example approach:
- $this->note->noteMetas()->where('key', config('filament-notes.models.user'))->delete(); + $userModel = config('filament-notes.models.user') ?? \App\Models\User::class; + $this->note->noteMetas()->where('key', $userModel)->delete();
Line range hint
5-5
: Remove unused User model import.Since the User model is now referenced through configuration, this import is no longer used and can be removed.
-use App\Models\User;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
config/filament-notes.php
(1 hunks)resources/views/widget/notes-widget.blade.php
(1 hunks)src/Filament/Resources/NoteResource.php
(3 hunks)src/Filament/Resources/NoteResource/Pages/ManageNotes.php
(1 hunks)src/Livewire/NoteAction.php
(2 hunks)
🔇 Additional comments (5)
src/Filament/Resources/NoteResource/Pages/ManageNotes.php (1)
31-31
: LGTM! Clean implementation of configurable user model.The change correctly uses the configuration value for the user type.
src/Filament/Resources/NoteResource.php (3)
69-77
: Improve query readability by extracting conditions.Similar to the widget template, the query conditions are complex and could benefit from extraction into a dedicated scope or method.
Consider moving the query logic to the Note model and reusing the same scope suggested for the widget template.
->modifyQueryUsing(function ($query) { - $query - ->where('is_public', true) - ->orWhere('user_id', auth()->id()) - ->orWhere('is_public', false) - ->whereHas('noteMetas', function ($q) { - $q->where('key', config('filament-notes.models.user')) - ->where('value', (string)auth()->user()->id); - }); + $query->visibleToUser(auth()->user()); })Add this scope to the Note model:
public function scopeVisibleToUser($query, $user) { return $query->where(function($q) use ($user) { $q->where('is_public', true) ->orWhere('user_id', $user->id) ->orWhere(function($sq) use ($user) { $sq->where('is_public', false) ->whereHas('noteMetas', function ($mq) use ($user) { $mq->where('key', config('filament-notes.models.user')) ->where('value', (string)$user->id); }); }); }); }
61-65
: LGTM! Clean formatting of conditional groups.The spacing and structure of the group conditions is clean and readable.
112-112
: LGTM! Clean formatting of table configuration.The spacing in the content callback and pagination options is consistent.
Also applies to: 140-140
src/Livewire/NoteAction.php (1)
195-196
: LGTM! Verify config existence.The change correctly moves the user model reference to configuration.
Let's verify the config file exists with the correct structure:
✅ Verification successful
Config verified and correctly implemented ✓
The user model configuration is properly defined in
config/filament-notes.php
and correctly referenced in the code.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the config file exists and contains the user model configuration rg -U "models.*user.*" config/Length of output: 32
Script:
#!/bin/bash # Find all config files and files containing filament-notes configuration echo "=== Looking for config files ===" fd "config" --type f echo -e "\n=== Searching for filament-notes configuration ===" rg "filament-notes" -l echo -e "\n=== Searching for model configurations ===" rg "models\s*=>" -lLength of output: 976
Script:
#!/bin/bash # Check ServiceProvider and Install command for config setup echo "=== Checking ServiceProvider for config registration ===" cat src/FilamentNotesServiceProvider.php echo -e "\n=== Checking Install command for config setup ===" cat src/Console/FilamentNotesInstall.phpLength of output: 3038
Script:
#!/bin/bash # Check the actual config file content echo "=== Checking config file content ===" cat config/filament-notes.php || cat src/config/filament-notes.php || cat ../config/filament-notes.phpLength of output: 199
Summary by CodeRabbit
Release Notes
New Features
Improvements
Refactor