-
-
Notifications
You must be signed in to change notification settings - Fork 257
fix: use correct log path for upload action #2016
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
fix: use correct log path for upload action #2016
Conversation
Previously, activity log icons only displayed the actor type (user, system, API). This change adds event-specific icons that match the panel's icon conventions used in SubuserPermission and other resources. Changes: - Add EVENT_ICON_MAP constant with 18 event group mappings - Icons include: backup, file, database, schedule, allocation, startup, settings, subuser, console, control, power, sftp, server, auth, user, api-key, reset-password, ssh - Extract getActorIcon() as fallback for unmapped events - Parse event string to determine group (e.g., 'server:backup.create' -> 'backup') This provides visual consistency and better UX when browsing activity logs.
|
I have read the CLA Document and I hereby sign the CLA |
📝 WalkthroughWalkthroughLog file paths in the admin panel are now configurable via prefix and extension settings (defaults: 'laravel-' and '.log'). Additionally, a new icon resolution method is added to the ActivityLog model to map event types to specific icons. Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 0
🧹 Nitpick comments (2)
app/Filament/Admin/Pages/ListLogs.php (1)
59-61: LGTM! Consistent fix applied across both list and view pages.The same configurable path construction is now used in both
ListLogsandViewLogs, ensuring consistent behavior.Optional: Extract duplicated logic to reduce repetition
The 3-line path construction pattern is duplicated between this file and
ViewLogs.php. Consider extracting to a shared trait or helper method:trait BuildsLogPath { protected function buildLogPath(string $date): string { $prefix = config('filament-log-viewer.pattern.prefix', 'laravel-'); $extension = config('filament-log-viewer.pattern.extension', '.log'); return storage_path('logs/' . $prefix . $date . $extension); } }Then use:
$logPath = $this->buildLogPath($record['date']);app/Models/ActivityLog.php (1)
146-216: Consider separating unrelated changes into focused PRs.The icon resolution changes in
ActivityLog.phpappear unrelated to the log path fix described in the PR title and description. While both changes are individually sound, bundling unrelated features in a single PR can make reviews more complex and rollbacks harder.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Filament/Admin/Pages/ListLogs.php(1 hunks)app/Filament/Admin/Pages/ViewLogs.php(1 hunks)app/Models/ActivityLog.php(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-05T22:21:31.863Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1865
File: app/Filament/Admin/Resources/Nodes/Pages/EditNode.php:682-682
Timestamp: 2025-11-05T22:21:31.863Z
Learning: In app/Filament/Admin/Resources/Nodes/Pages/EditNode.php, the diagnostics tab's upload action intentionally does not use the iconButton() modifier, while the pull action does. This UI difference is intentional.
Applied to files:
app/Filament/Admin/Pages/ViewLogs.php
🧬 Code graph analysis (2)
app/Filament/Admin/Pages/ViewLogs.php (1)
app/Traits/ResolvesRecordDate.php (1)
resolveRecordDate(12-44)
app/Models/ActivityLog.php (2)
app/Models/File.php (1)
getIcon(103-118)app/Services/Activity/ActivityLogService.php (1)
event(48-53)
🔇 Additional comments (5)
app/Models/ActivityLog.php (4)
146-169: Icon mapping looks comprehensive and consistent.The EVENT_ICON_MAP provides appropriate icon associations for activity event groups, using tabler icons consistent with the rest of the codebase.
171-195: Event parsing logic is sound for standard event formats.The method correctly extracts event groups from formats like
server:backup.create→backupandbackup.create→backup, then falls back to actor-based icons when no mapping exists.
197-216: Clean refactoring of actor-based icon logic.The previous
getIcon()implementation is now appropriately moved to a privategetActorIcon()helper, maintaining the same fallback behavior while allowing the publicgetIcon()to prioritize event-based icon selection.
222-222: Minor formatting improvements for readability.Added spacing around concatenation operators in
trans_choicecalls, improving code consistency.Also applies to: 311-311
app/Filament/Admin/Pages/ViewLogs.php (1)
36-38: Fix correctly addresses log path construction issue.The configurable prefix and extension ensure the constructed path matches actual log filenames. While the config keys
filament-log-viewer.pattern.prefixandfilament-log-viewer.pattern.extensionare not formally documented in the package documentation, the defaults ('laravel-' and '.log') align with Laravel's standard log naming convention and provide sensible fallback values for proper functionality.
Boy132
left a 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.
Seems like some changes from your other PR snuck in.
notAreYouScared
left a 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.
LGTM
The upload logs action was building the wrong file path, causing 'log not found' errors.
The code used just the date (e.g. 2025-01-15) but the actual files have prefix and extension (e.g. laravel-2025-01-15.log).
Now uses config('filament-log-viewer.pattern.prefix') and config('filament-log-viewer.pattern.extension') to match the package behavior.