Skip to content

Conversation

@notAreYouScared
Copy link
Member

@notAreYouScared notAreYouScared commented Oct 26, 2025

Closes #1802

Replaces current error log manager with a new one, tested with a 10MB error log file that was 49,000 lines long. Loading time was 0.38 seconds

Actions are View, Download, Upload(to logs.pelican.dev), and delete.
image

Actions are delete, download, upload, back

image

Stacktrace:
image

image image

@notAreYouScared notAreYouScared self-assigned this Oct 26, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 26, 2025

Caution

Review failed

The head commit changed during the review from fda9dcb to fe81e7b.

📝 Walkthrough

Walkthrough

Replaces AchyutN log viewer with Boquizo plugin, adds new Filament admin pages ListLogs and ViewLogs (with log upload/download/delete actions), adds upload-to-external-service workflow, new translations, a Blade list-logs view, CSS template scanning directive, and updates composer dependency and post-install script.

Changes

Cohort / File(s) Summary
Filament Admin Pages
app/Filament/Admin/Pages/ListLogs.php, app/Filament/Admin/Pages/ViewLogs.php
Add new ListLogs page with table config (date and level columns), record actions (ViewLogAction, DownloadAction, inline uploadLogs) and end-of-list DeleteAction. Add getHeaderActions in ViewLogs including Delete, Download, Back, and an uploadLogs action that validates file, reads last ≤1000 lines, POSTs multipart to https://logs.pelican.dev, and shows success/failure notifications and an action to open returned URL. Exceptions produce danger notifications.
Filament Panel Providers
app/Providers/Filament/AdminPanelProvider.php, app/Providers/Filament/AppPanelProvider.php
Replace usage of AchyutN\FilamentLogViewer with Boquizo\FilamentLogViewerPlugin; register ListLogs and ViewLogs with fluent plugin configuration and retain authorization/navigation grouping.
Views
resources/views/filament/components/list-logs.blade.php
New Blade view that wraps the logs table in a Filament page component.
Translations
lang/en/admin/log.php
New English translation file providing keys for log UI, actions, tooltips, upload dialogs, error messages, and labels.
Styling / Tailwind scanning
resources/css/app.css
Append @source directive to include filament-log-viewer package Blade templates for CSS scanning.
Dependencies / Scripts
composer.json
Remove achyutn/filament-log-viewer; add gboquizosanchez/filament-log-viewer ^2.1; add post-install-cmd PHP one-liner to ensure FILAMENT_LOG_VIEWER_DRIVER=raw exists in .env.

Sequence Diagram(s)

sequenceDiagram
  participant Admin as Admin UI (Filament)
  participant View as ViewLogs/ListLogs Page
  participant Storage as Local Storage (storage/logs)
  participant HTTP as logs.pelican.dev (HTTP API)
  participant Browser as User Browser

  Note over Admin,View: User triggers "Upload Logs" action (button/modal)
  Browser->>View: confirm upload (modal)
  View->>Storage: check file exists
  alt file not found
    View-->>Browser: show danger notification "log_not_found"
  else file found
    View->>Storage: read last ≤1000 lines
    View->>HTTP: multipart POST (field c => content, e => 14d)
    alt HTTP failure (non-2xx)
      HTTP-->>View: non-2xx status
      View-->>Browser: show danger notification with status
    else HTTP success (2xx + JSON)
      HTTP-->>View: JSON { url: "<uploaded_url>" }
      View-->>Browser: show persistent success notification with URL and "View Logs" action
      Browser->>Browser: user clicks "View Logs" -> opens URL in new tab
    end
  end
  Note right of View: exceptions caught -> danger notification with message
Loading

Possibly related PRs

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: replacing the current log viewer with a new improved version.
Description check ✅ Passed The description is related to the changeset, providing context about the replacement, performance metrics, supported actions, and UI screenshots.
Linked Issues check ✅ Passed The PR addresses issue #1802 by replacing the log viewer plugin, which resolves the email text readability issue through the new plugin implementation and CSS @source directive.
Out of Scope Changes check ✅ Passed All changes are directly related to replacing the log viewer: plugin swap, page customizations, translations, CSS updates, and dependency changes are all within scope.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gboquizosanchez
Copy link

gboquizosanchez commented Oct 26, 2025

Hi gboquizosanchez here.

I came from here -> gboquizosanchez/filament-log-viewer#36

I have just commented it's necessary in certain plugins to establish a custom theme in Filament v4 because of the new version of the tailwindCSS v4.


The process is simple. Here there is a fantastic guide, but I can summarize it into:

1) In your four panels you have to register a line as ->viteTheme('resources/css/filament/admin/theme.css')
2) In theme.css register those lines:

@import '../../../../vendor/filament/filament/resources/css/theme.css';

@source '../../../../app/Filament/**/*';
@source '../../../../resources/views/filament/**/*';

@source '../../../../vendor/gboquizosanchez/filament-log-viewer/resources/views/**/*.blade.php';

The third first lines come from this step.

3) It won't be necessary to add that css in vite config because you've already mapped it globally.

I'm not sure if in your current https://github.com/pelican-dev/panel/blob/main/resources/css/app.css can be included and will work.


EDIT: I have just tested into your application and you just add into app.css file:

@source '../../vendor/gboquizosanchez/filament-log-viewer/resources/views/**/*.blade.php';

All will work properly:

image

I hope those lines can be helpful. If not, I'll glad to help you'all in the way I can.

Cheers!

@notAreYouScared
Copy link
Member Author

EDIT: I have just tested into your application and you just add into app.css file:

@source '../../vendor/gboquizosanchez/filament-log-viewer/resources/views/**/*.blade.php';

Thanks! i just tested it <3

@rmartinoscar
Copy link
Member

Guess we have to update the workflow matrix :')

Boy132
Boy132 previously requested changes Oct 28, 2025
@notAreYouScared notAreYouScared marked this pull request as ready for review November 7, 2025 22:00
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
composer.json (1)

6-6: CRITICAL: PHP 8.2 constraint still present despite PR title.

The PR title states "Remove php 8.2 support" and comments indicate the new log viewer plugin requires PHP 8.3+, but the constraint still allows ^8.2.

Apply this diff to enforce PHP 8.3 minimum:

-        "php": "^8.2 || ^8.3 || ^8.4",
+        "php": "^8.3 || ^8.4",

Also update the platform constraint:

         "platform": {
-            "php": "8.2"
+            "php": "8.3"
         }
♻️ Duplicate comments (1)
resources/views/filament/components/view-log.blade.php (1)

1-3: Verify if this view is actually referenced.

The ViewLogs class doesn't define a $view property, so it will use the default view from BaseViewLog. This custom view may be unused unless explicitly configured elsewhere.

Run the following script to check if this view is referenced:

#!/bin/bash
# Check if view-log.blade.php is referenced anywhere in the codebase
rg -n "view-log|viewLog" --type=php -C2
🧹 Nitpick comments (4)
composer.json (1)

84-85: Post-install script could be more robust.

The strpos check will match commented lines or partial matches. Consider using a more precise regex pattern to detect the actual key.

-            "php -r \"$env = file_get_contents('.env'); if (strpos($env, 'FILAMENT_LOG_VIEWER_DRIVER=') === false) { file_put_contents('.env', rtrim($env) . PHP_EOL . 'FILAMENT_LOG_VIEWER_DRIVER=raw' . PHP_EOL); }\""
+            "php -r \"$env = file_get_contents('.env'); if (!preg_match('/^\\s*FILAMENT_LOG_VIEWER_DRIVER\\s*=/m', $env)) { file_put_contents('.env', rtrim($env) . PHP_EOL . 'FILAMENT_LOG_VIEWER_DRIVER=raw' . PHP_EOL); }\""
app/Filament/Admin/Pages/ViewLogs.php (1)

23-97: Extract duplicated upload logic to a shared service.

The upload action logic (lines 31-96) is duplicated between ViewLogs and ListLogs. Consider extracting this to a dedicated service class or trait.

Create a new service class:

namespace App\Services;

use Filament\Notifications\Notification;
use Illuminate\Support\Facades\Http;

class LogUploadService
{
    public static function upload(string $logDate): ?string
    {
        $logPath = storage_path('logs/' . $logDate);
        
        if (!file_exists($logPath)) {
            Notification::make()
                ->title(trans('admin/log.actions.log_not_found'))
                ->body(trans('admin/log.actions.log_not_found_description', ['filename' => $logDate]))
                ->danger()
                ->send();
            return null;
        }

        $lines = file($logPath, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
        $totalLines = count($lines);
        $uploadLines = $totalLines <= 1000 ? $lines : array_slice($lines, -1000);
        $content = implode("\n", $uploadLines);

        try {
            $response = Http::asMultipart()->post('https://logs.pelican.dev', [
                ['name' => 'c', 'contents' => $content],
                ['name' => 'e', 'contents' => '14d'],
            ]);

            if ($response->failed()) {
                Notification::make()
                    ->title(trans('admin/log.actions.failed_to_upload'))
                    ->body(trans('admin/log.actions.failed_to_upload_description', ['status' => $response->status()]))
                    ->danger()
                    ->send();
                return null;
            }

            $data = $response->json();
            return $data['url'] ?? null;
        } catch (\Exception $e) {
            Notification::make()
                ->title(trans('admin/log.actions.failed_to_upload'))
                ->body($e->getMessage())
                ->danger()
                ->send();
            return null;
        }
    }
}

Then use it in both classes:

->action(function () {
    $url = LogUploadService::upload($this->record->date);
    
    if ($url) {
        Notification::make()
            ->title(trans('admin/log.actions.log_upload'))
            ->body($url)
            ->success()
            ->actions([
                Action::make('viewLogs')
                    ->label(trans('admin/log.actions.view_logs'))
                    ->url($url)
                    ->openUrlInNewTab(true),
            ])
            ->persistent()
            ->send();
    }
})
app/Filament/Admin/Pages/ListLogs.php (2)

46-52: Extract hardcoded URL to a constant.

The URL https://logs.pelican.dev is duplicated on lines 52 and 71. Consider extracting it to a class constant or configuration value for easier maintenance.

Apply this diff to add a constant at the class level:

 class ListLogs extends BaseListLogs
 {
     protected string $view = 'filament.components.list-logs';
+    private const LOGS_SERVICE_URL = 'https://logs.pelican.dev';

Then update line 52:

-                    ->modalDescription(fn ($record) => trans('admin/log.actions.upload_logs_description', ['file' => $record['date'], 'url' => 'https://logs.pelican.dev']))
+                    ->modalDescription(fn ($record) => trans('admin/log.actions.upload_logs_description', ['file' => $record['date'], 'url' => self::LOGS_SERVICE_URL]))

And line 71:

-                        $hbUrl = 'https://logs.pelican.dev';
+                        $hbUrl = self::LOGS_SERVICE_URL;

80-80: Consider making expiration configurable.

The expiration value '14d' is hardcoded. Depending on your use case, you might want to make this configurable through settings or environment variables.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 49f24e3 and 29e6760.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (9)
  • app/Filament/Admin/Pages/ListLogs.php (1 hunks)
  • app/Filament/Admin/Pages/ViewLogs.php (1 hunks)
  • app/Providers/Filament/AdminPanelProvider.php (2 hunks)
  • app/Providers/Filament/AppPanelProvider.php (2 hunks)
  • composer.json (2 hunks)
  • lang/en/admin/log.php (1 hunks)
  • resources/css/app.css (1 hunks)
  • resources/views/filament/components/list-logs.blade.php (1 hunks)
  • resources/views/filament/components/view-log.blade.php (1 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 (1)
app/Providers/Filament/AdminPanelProvider.php (4)
app/Filament/Admin/Pages/ListLogs.php (1)
  • ListLogs (18-124)
app/Filament/Admin/Pages/ViewLogs.php (1)
  • ViewLogs (14-102)
app/helpers.php (1)
  • user (127-130)
app/Models/User.php (1)
  • can (358-371)
🔇 Additional comments (10)
resources/css/app.css (1)

27-27: LGTM! Resolves the white-on-white text issue.

This @source directive ensures Tailwind v4 scans the new log viewer templates, fixing the unreadable email preview text reported in issue #1802.

resources/views/filament/components/list-logs.blade.php (1)

1-3: LGTM!

Standard Filament page wrapper pattern, correctly referenced by the ListLogs page class.

app/Providers/Filament/AppPanelProvider.php (1)

5-5: LGTM! Clean migration to the new log viewer package.

The namespace and class changes align with the dependency update in composer.json.

Also applies to: 29-29

app/Providers/Filament/AdminPanelProvider.php (1)

5-7: LGTM! Well-structured plugin configuration.

The migration to the new log viewer plugin is clean, with proper wiring of custom page classes and translations.

Also applies to: 40-46

app/Filament/Admin/Pages/ListLogs.php (6)

1-20: LGTM!

The namespace, imports, and class structure are well-organized and appropriate for the functionality being implemented.


22-25: LGTM!

The heading method correctly returns a translated string with appropriate type hints.


27-40: LGTM!

The table configuration and column definitions are appropriate for displaying log file information with multiple severity levels.


42-45: LGTM!

The view and download actions are properly configured with appropriate icons.


53-64: LGTM!

The file existence validation is properly implemented with appropriate user feedback.


120-122: LGTM!

The delete action is properly configured with an appropriate icon.

@notAreYouScared notAreYouScared changed the title Remove php 8.2 support and add new error log manager Replace current panel log viewer with new and improved log viewer Nov 7, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
app/Filament/Admin/Pages/ListLogs.php (1)

100-101: Validate response structure before accessing the URL key.

The code directly accesses $data['url'] without validating the response structure. The json() method can return null, an array, or a stdClass object. Boy132 confirmed in ViewLogs.php that this pattern throws "Cannot use object of type stdClass as array" when the response is a JSON object.

Apply this diff to add validation:

                         $data = $response->json();
+                        
+                        if (!is_array($data) || !isset($data['url'])) {
+                            Notification::make()
+                                ->title(trans('admin/log.actions.failed_to_upload'))
+                                ->body(trans('admin/log.actions.invalid_response'))
+                                ->danger()
+                                ->send();
+
+                            return;
+                        }
+
                         $url = $data['url'];

Ensure the translation key admin/log.actions.invalid_response exists in your language file.

app/Filament/Admin/Pages/ViewLogs.php (1)

72-73: Fix confirmed runtime error: validate response structure before array access.

Boy132 confirmed this code throws "Cannot use object of type stdClass as array" when viewing a log. The json() method returns a stdClass object when the response is a JSON object, but the code attempts array access with $data['url'].

Apply this diff to fix the error:

                         $data = $response->json();
+                        
+                        if (!is_array($data) || !isset($data['url'])) {
+                            Notification::make()
+                                ->title(trans('admin/log.actions.failed_to_upload'))
+                                ->body(trans('admin/log.actions.invalid_response'))
+                                ->danger()
+                                ->send();
+
+                            return;
+                        }
+
                         $url = $data['url'];

Ensure the translation key admin/log.actions.invalid_response exists in your language file.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f77bc8 and 7a1c427.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • app/Filament/Admin/Pages/ListLogs.php (1 hunks)
  • app/Filament/Admin/Pages/ViewLogs.php (1 hunks)
  • lang/en/admin/log.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lang/en/admin/log.php
🧰 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

@notAreYouScared notAreYouScared merged commit 2b5403a into main Nov 9, 2025
25 checks passed
@notAreYouScared notAreYouScared deleted the charles/drop8.2 branch November 9, 2025 00:31
@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTML Email Preview in Log Viewer doesn't show text

6 participants