-
-
Notifications
You must be signed in to change notification settings - Fork 257
Add basic api for plugins #2146
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
📝 WalkthroughWalkthroughAdds a new PluginController with endpoints for listing, viewing, importing (file/URL), installing, updating, uninstalling, enabling, and disabling plugins; introduces request classes for read/write/uninstall/import, a PluginTransformer, Plugin::RESOURCE_NAME, ApiKey exposure, and route registrations under /api/application/plugins. (41 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as PluginController
participant Service as PluginService
participant Storage as Storage/Filesystem
participant DB as Database
participant Transformer as PluginTransformer
Client->>Controller: POST /api/application/plugins/import/file (multipart file)
Controller->>Controller: validate request (WritePluginRequest)
Controller->>Service: importFromFile(file)
Service->>Storage: store/process uploaded file
Service->>DB: create/update Plugin record
Service-->>Controller: Plugin model
Controller->>Transformer: transform(Plugin)
Transformer-->>Controller: JSON resource
Controller-->>Client: 201 Created + transformed Plugin
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
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: 3
🤖 Fix all issues with AI agents
In `@app/Http/Controllers/Api/Application/Plugins/PluginController.php`:
- Around line 87-92: The call in importUrl passes a Stringable from
ImportFilePluginRequest->string('url') into downloadPluginFromUrl which requires
a native string; fix by explicitly casting the request value to string (e.g.
(string) $request->string('url')) before calling downloadPluginFromUrl in the
importUrl method so downloadPluginFromUrl receives a real string.
- Around line 109-114: After calling
$this->pluginService->installPlugin($plugin) the $plugin model isn't reloaded so
the transformer returns stale status/meta; update installPlugin usage to refresh
the model before transforming (e.g. call $plugin->refresh() or re-query via
fresh()) and apply the same pattern in update(), uninstall(), enable(), and
disable() so the response uses the updated Plugin model when passing to
$this->fractal->item($plugin)->transformWith($this->getTransformer(PluginTransformer::class)).
In `@app/Transformers/Api/Application/PluginTransformer.php`:
- Line 35: In PluginTransformer (file: PluginTransformer.php) the
'composer_packages' decoding uses json_decode with JSON_THROW_ON_ERROR which
will throw a JsonException when $model->composer_packages is null or empty;
mirror the null/empty check used for 'panels' (line 33) and only call
json_decode when $model->composer_packages is a non-empty string, otherwise
return an empty array (or suitable default) for the 'composer_packages' field so
decoding never receives null/empty input.
🧹 Nitpick comments (3)
app/Http/Requests/Api/Application/Plugins/ImportFilePluginRequest.php (1)
5-12: Class name does not match its purpose.This request class validates a
urlfield but is namedImportFilePluginRequest. Based on the controller usage,importUrl()uses this class whileimportFile()usesWritePluginRequest. Consider renaming toImportUrlPluginRequestfor clarity.app/Http/Controllers/Api/Application/Plugins/PluginController.php (2)
41-41: Consider boundingper_pageto prevent excessive queries.The
per_pageparameter is taken directly from user input without an upper limit. A malicious or careless client could request an excessively large page size. Consider usingmin()to cap it.♻️ Proposed fix
- ->paginate($request->query('per_page') ?? 10); + ->paginate(min($request->query('per_page') ?? 10, 100));
69-78: Consider validating file presence in request class.The manual file check could be moved to a dedicated
ImportFilePluginRequest(with the current one renamed toImportUrlPluginRequest) for consistency with other endpoints that use request validation. This would centralize validation logic.
No description provided.