fix(plugins): settings button for any plugin with a settings page (Mobile API gate unreachable)#191
fix(plugins): settings button for any plugin with a settings page (Mobile API gate unreachable)#191fabiodalez-dev wants to merge 4 commits into
Conversation
The admin Plugins list only rendered a settings button for a few hardcoded
plugins (open-library, api-book-scraper, goodlib, discogs). Any other plugin
with a working settings page — notably Mobile API, whose 'Abilita accesso app
mobile' gate (mobile_api.enabled) lives there — was unreachable from the UI, so
the gate could never be turned on and the app got 'mobile access disabled'.
PluginController::index now flags which active plugins expose a settings page
(hasSettingsPage + getSettingsViewPath), and the list shows a generic
'Impostazioni' link to /admin/plugins/{id}/settings for them. The custom modals
of open-library / api-book-scraper / goodlib are preserved; discogs now flows
through the generic path (same link it already used). No new i18n key.
Verified live: the Mobile API card now shows 'Impostazioni' linking to its
settings page with the app-access toggle; existing plugins unaffected.
PHPStan L5 clean.
|
Warning Review limit reached
More reviews will be available in 15 minutes and 2 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIl controller calcola per ogni plugin attivo se esiste una pagina impostazioni, verificando istanza e metodi disponibili. La vista admin usa questa mappa per mostrare il pulsante Impostazioni solo quando disponibile e non per i plugin con pulsanti custom. ChangesImpostazioni plugin
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Controllers/PluginController.php`:
- Around line 56-62: The settings-page detection in
PluginController::hasSettingsPage is too permissive and does not match the guard
used by settingsPage(). Update the check so it not only verifies
hasSettingsPage() and getSettingsViewPath() are callable, but also confirms the
returned path is a string and points to an existing file, mirroring the logic in
settingsPage() to avoid showing an “Impostazioni” button for invalid views.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bfd71d1b-04d7-423a-9936-3a289eb0c489
📒 Files selected for processing (2)
app/Controllers/PluginController.phpapp/Views/admin/plugins.php
…e API toggle)
The POST /admin/plugins/{id}/settings route always called updateSettings(),
whose hardcoded if/elseif chain bailed with 'plugin non supporta impostazioni'
for mobile-api — so the 'Abilita accesso app mobile' toggle (and push config)
could never be saved from the UI, and the app kept seeing app_access_enabled=false.
The Mobile API settings page is a normal (non-AJAX) form that handles its own
POST inside the view (CSRF + plugin->saveSettings() + success message). The
legacy AJAX handlers require a nested 'settings' payload; when it's absent the
request is such a self-rendering form, so delegate to settingsPage() and let the
view's own POST logic run. Generic — fixes mobile-api and any future
self-rendering settings page; AJAX plugins (which always send 'settings') are
unaffected.
Verified end-to-end: toggling 'Abilita accesso app mobile' now flips
system_settings mobile_api.enabled 0->1. PHPStan L5 clean.
…xists Align the settings-page detection with settingsPage()'s guard (is_string + is_file on getSettingsViewPath), so a plugin declaring a settings page with a missing view file doesn't surface an 'Impostazioni' button that 404s on click. Addresses CodeRabbit review on #191.
|
Superseded by #194 (consolidated mobile-API contract coherence). Changes included there. |
1 participant
Problem
After activating the Mobile API plugin, the app still gets "l'accesso da app mobile è disattivato su questa biblioteca". The cause: the gate that enables app access (
mobile_api.enabled, the "Abilita accesso app mobile" toggle) lives on the plugin's settings page — but the Admin → Plugins list only renders a settings button for a few hardcoded plugins (open-library, api-book-scraper, goodlib, discogs). Mobile API (and any other plugin with a settings page) had no button, so the page/admin/plugins/{id}/settingswas unreachable and the gate could never be turned on.Confirmed live with Playwright: the Mobile API card showed only Disattiva, no settings entry, despite
hasSettingsPage()=trueand a fully working settings page.Fix
PluginController::indexflags which active plugins expose a settings page (hasSettingsPage()+getSettingsViewPath())./admin/plugins/{id}/settingsfor any such plugin not already handled by a custom modal.Impostazioniexists).Verified
Summary by CodeRabbit
New Features
Bug Fixes