Add jobs for plugin install, update and uninstall#2267
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughPlugin install, update, and uninstall actions were moved from synchronous PluginService calls to queued background jobs. Three new unique queued jobs (InstallPlugin, UpdatePlugin, UninstallPlugin) perform the operations and send success/failure notifications; resource actions now dispatch these jobs and use notification keys for background-started states. Changes
Sequence DiagramsequenceDiagram
participant User as User/UI
participant Action as PluginResource Action
participant Queue as Job Queue
participant Job as Plugin Job
participant Service as PluginService
participant Notif as Notification System
User->>Action: Trigger install/update/uninstall
Action->>Queue: Dispatch Job(User, Plugin)
Action-->>User: Return (no immediate redirect)
activate Queue
Queue->>Job: Process Job
Job->>Service: Execute operation (install/update/uninstall)
alt Success
Service-->>Job: Operation complete
Job->>Notif: Send success notification (goto_plugins link)
Notif-->>User: Display success & action link
else Failure
Service-->>Job: Exception
Job->>Job: Report exception
Job->>Notif: Send danger notification with error message
Notif-->>User: Display error
end
deactivate Queue
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (2)
app/Filament/Admin/Resources/Plugins/PluginResource.php (1)
125-141:⚠️ Potential issue | 🟡 Minor
user()is nullable —TypeErrorfrom a null dispatch would be uncaught.
user()returns?User. If it returnsnull, PHP throws aTypeError(which extends\Error, not\Exception) when constructing the job. Thecatch (Exception $exception)block will not intercept it, resulting in an unhandled server error surfacing to the user.While the
->authorize()guard makes this scenario unlikely in practice, the type is still unsafe. The same pattern applies to theexclude_update(Line 148) andexclude_uninstall(Line 224) actions.🛡️ Proposed fix
- ->action(function (Plugin $plugin) { + ->action(function (Plugin $plugin) { + $user = user(); + if ($user === null) { + return; + } try { - InstallPlugin::dispatch(user(), $plugin); + InstallPlugin::dispatch($user, $plugin);Apply the same guard in the
exclude_updateandexclude_uninstallaction closures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Filament/Admin/Resources/Plugins/PluginResource.php` around lines 125 - 141, The action closures call InstallPlugin::dispatch(user(), $plugin) while user() is nullable, so add an explicit guard at the start of each closure (the install action and the closures for exclude_update and exclude_uninstall) that checks if user() is null and, if so, sends a Notification::make()->danger() with an appropriate title/body and returns early; otherwise proceed to dispatch. Alternatively, you can replace catch (Exception $exception) with catch (\Throwable $t) to also intercept TypeError, but the primary fix is to verify user() !== null before calling InstallPlugin::dispatch (and the corresponding dispatch calls in the exclude_update and exclude_uninstall closures).lang/en/admin/plugin.php (1)
47-70:⚠️ Potential issue | 🔴 CriticalMissing
*_failedtranslation keys referenced by the new job classes.
InstallPlugin.php,UpdatePlugin.php, andUninstallPlugin.phpeach referenceinstall_failed,update_failed, anduninstall_failedrespectively in their catch blocks, but these keys do not exist in this file. The existinginstall_error,update_error, anduninstall_errorkeys are already present and used correctly byPluginResource.php. The fix belongs in the job files (changing_failed→_error), but this file is the source of truth for what is valid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lang/en/admin/plugin.php` around lines 47 - 70, The catch blocks in the job classes InstallPlugin.php, UpdatePlugin.php, and UninstallPlugin.php reference non-existent translation keys install_failed, update_failed, and uninstall_failed; update those references to use the existing keys install_error, update_error, and uninstall_error respectively so the jobs log/notify using the translation keys defined in the notifications array (replace any occurrences of 'install_failed' → 'install_error', 'update_failed' → 'update_error', and 'uninstall_failed' → 'uninstall_error' inside the job catch blocks).
♻️ Duplicate comments (4)
app/Jobs/Plugin/UninstallPlugin.php (2)
45-45:⚠️ Potential issue | 🔴 CriticalWrong translation key —
uninstall_faileddoes not exist; the correct key isuninstall_error.Same issue as in
UpdatePlugin.php. The lang file definesuninstall_error, notuninstall_failed.🐛 Proposed fix
- ->title(trans('admin/plugin.notifications.uninstall_failed')) + ->title(trans('admin/plugin.notifications.uninstall_error'))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Jobs/Plugin/UninstallPlugin.php` at line 45, The notification title in UninstallPlugin.php uses the wrong translation key; locate the chain that calls ->title(trans('admin/plugin.notifications.uninstall_failed')) inside the UninstallPlugin job and change the key to trans('admin/plugin.notifications.uninstall_error') (mirror the same fix applied in UpdatePlugin.php) so it matches the lang file; ensure any other occurrences in this class use the correct 'uninstall_error' key.
10-10: SameFilament\Actions\Actionimport issue asUpdatePlugin.php— useFilament\Notifications\Actions\Actioninstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Jobs/Plugin/UninstallPlugin.php` at line 10, The file imports the wrong Action class; replace the import "use Filament\Actions\Action" with "use Filament\Notifications\Actions\Action" in the UninstallPlugin job so it uses the notifications Action type; locate the top-of-file import and any type hints or usages of Action inside the UninstallPlugin class (e.g., in methods that build or return actions) to ensure they reference the notifications Action and adjust any docblocks or use-sites if necessary.app/Jobs/Plugin/InstallPlugin.php (2)
45-45:⚠️ Potential issue | 🔴 CriticalWrong translation key —
install_faileddoes not exist; the correct key isinstall_error.Same issue as in
UpdatePlugin.phpandUninstallPlugin.php.🐛 Proposed fix
- ->title(trans('admin/plugin.notifications.install_failed')) + ->title(trans('admin/plugin.notifications.install_error'))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Jobs/Plugin/InstallPlugin.php` at line 45, The notification title in InstallPlugin (where ->title(trans('admin/plugin.notifications.install_failed')) is used) references a non-existent translation key; update the call to use the correct key trans('admin/plugin.notifications.install_error') and make the same replacement in the analogous usages inside UpdatePlugin and UninstallPlugin so all notification titles use 'install_error' (or the appropriate 'update_error'/'uninstall_error' equivalents if those files expect different names).
10-10: SameFilament\Actions\Actionimport issue — useFilament\Notifications\Actions\Actioninstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Jobs/Plugin/InstallPlugin.php` at line 10, The imported symbol is wrong: replace the incorrect import "Filament\Actions\Action" with the correct "Filament\Notifications\Actions\Action" in InstallPlugin.php; locate the use statement at the top of the file (referencing Action) and update it so the class InstallPlugin and any references to Action resolve to Filament\Notifications\Actions\Action, then run a quick static check or IDE resolution to ensure no other references need updating.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Jobs/Plugin/UpdatePlugin.php`:
- Line 45: Replace the incorrect translation key
"admin/plugin.notifications.update_failed" with
"admin/plugin.notifications.update_error" in the danger notification builder
inside UpdatePlugin.php (the code that calls ->title(...)); do the same pattern
fix in InstallPlugin.php (install_failed → install_error) and
UninstallPlugin.php (uninstall_failed → uninstall_error) so the notifications
use the existing keys defined in lang/en/admin/plugin.php.
In `@lang/en/admin/plugin.php`:
- Line 49: Update the string value for the 'background_info' array entry to
correct the grammar from "its finished" to "it's finished" by locating the
'background_info' => 'This process can take a few seconds. You will be notified
once its finished.' entry in plugin.php and replacing the contraction
accordingly so the message reads "This process can take a few seconds. You will
be notified once it's finished."
---
Outside diff comments:
In `@app/Filament/Admin/Resources/Plugins/PluginResource.php`:
- Around line 125-141: The action closures call InstallPlugin::dispatch(user(),
$plugin) while user() is nullable, so add an explicit guard at the start of each
closure (the install action and the closures for exclude_update and
exclude_uninstall) that checks if user() is null and, if so, sends a
Notification::make()->danger() with an appropriate title/body and returns early;
otherwise proceed to dispatch. Alternatively, you can replace catch (Exception
$exception) with catch (\Throwable $t) to also intercept TypeError, but the
primary fix is to verify user() !== null before calling InstallPlugin::dispatch
(and the corresponding dispatch calls in the exclude_update and
exclude_uninstall closures).
In `@lang/en/admin/plugin.php`:
- Around line 47-70: The catch blocks in the job classes InstallPlugin.php,
UpdatePlugin.php, and UninstallPlugin.php reference non-existent translation
keys install_failed, update_failed, and uninstall_failed; update those
references to use the existing keys install_error, update_error, and
uninstall_error respectively so the jobs log/notify using the translation keys
defined in the notifications array (replace any occurrences of 'install_failed'
→ 'install_error', 'update_failed' → 'update_error', and 'uninstall_failed' →
'uninstall_error' inside the job catch blocks).
---
Duplicate comments:
In `@app/Jobs/Plugin/InstallPlugin.php`:
- Line 45: The notification title in InstallPlugin (where
->title(trans('admin/plugin.notifications.install_failed')) is used) references
a non-existent translation key; update the call to use the correct key
trans('admin/plugin.notifications.install_error') and make the same replacement
in the analogous usages inside UpdatePlugin and UninstallPlugin so all
notification titles use 'install_error' (or the appropriate
'update_error'/'uninstall_error' equivalents if those files expect different
names).
- Line 10: The imported symbol is wrong: replace the incorrect import
"Filament\Actions\Action" with the correct
"Filament\Notifications\Actions\Action" in InstallPlugin.php; locate the use
statement at the top of the file (referencing Action) and update it so the class
InstallPlugin and any references to Action resolve to
Filament\Notifications\Actions\Action, then run a quick static check or IDE
resolution to ensure no other references need updating.
In `@app/Jobs/Plugin/UninstallPlugin.php`:
- Line 45: The notification title in UninstallPlugin.php uses the wrong
translation key; locate the chain that calls
->title(trans('admin/plugin.notifications.uninstall_failed')) inside the
UninstallPlugin job and change the key to
trans('admin/plugin.notifications.uninstall_error') (mirror the same fix applied
in UpdatePlugin.php) so it matches the lang file; ensure any other occurrences
in this class use the correct 'uninstall_error' key.
- Line 10: The file imports the wrong Action class; replace the import "use
Filament\Actions\Action" with "use Filament\Notifications\Actions\Action" in the
UninstallPlugin job so it uses the notifications Action type; locate the
top-of-file import and any type hints or usages of Action inside the
UninstallPlugin class (e.g., in methods that build or return actions) to ensure
they reference the notifications Action and adjust any docblocks or use-sites if
necessary.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/Filament/Admin/Resources/Plugins/PluginResource.phpapp/Jobs/Plugin/InstallPlugin.phpapp/Jobs/Plugin/UninstallPlugin.phpapp/Jobs/Plugin/UpdatePlugin.phplang/en/admin/plugin.php
notAreYouScared
approved these changes
Mar 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2183