Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 38 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughRefactors module statistics to report active module counts segmented by audience (users vs admins) instead of inactive/total, and updates the KPI language string capitalization for "New this month". Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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. ✨ 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14 +/- ##
============================================
- Coverage 19.23% 18.05% -1.19%
- Complexity 7584 7760 +176
============================================
Files 621 665 +44
Lines 40085 42709 +2624
============================================
Hits 7709 7709
- Misses 32376 35000 +2624 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 the current code and only fix it if needed.
Inline comments:
In `@htdocs/modules/system/themes/modern/modern.php`:
- Around line 247-257: The code computes active_modules_admin by subtracting
active_modules_user from active_modules, which is incorrect because hasmain and
hasadmin are independent; instead build a new CriteriaCompo (like the existing
$criteria) or clone/reset it, add Criteria('isactive', 1) and
Criteria('hasadmin', 1) and call $module_handler->getCount(...) to get the
correct active_modules_admin value, then assign that result to
$tpl->assign('active_modules_admin', ...); ensure you reference the same
module_handler, CriteriaCompo and Criteria symbols when locating the fix.
🪄 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: f8292ff1-be42-4aab-9a0e-bd3ba95815f6
📒 Files selected for processing (3)
htdocs/modules/system/themes/modern/language/english/main.phphtdocs/modules/system/themes/modern/modern.phphtdocs/modules/system/themes/modern/xotpl/xo_dashboard.tpl
There was a problem hiding this comment.
Pull request overview
Updates the Modern admin theme dashboard KPIs to provide a clearer breakdown of active modules by role, along with minor label text consistency improvements.
Changes:
- Restricts dashboard rendering to the admin homepage using
$xoops_page. - Splits “Active Modules” KPI details into user-side vs admin-side counts.
- Updates/enhances English language strings for the dashboard KPI labels.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| htdocs/modules/system/themes/modern/xotpl/xo_dashboard.tpl | Updates dashboard visibility condition and displays split active-module KPI details. |
| htdocs/modules/system/themes/modern/modern.php | Adjusts module statistics assigned to the dashboard template (user/admin split). |
| htdocs/modules/system/themes/modern/language/english/main.php | Updates capitalization and adds new language constants for the split KPI labels. |
| $criteria = new CriteriaCompo(); | ||
| $criteria->add(new Criteria('hasmain', 1)); | ||
| // active modules | ||
| $criteria->add(new Criteria('isactive', 1)); | ||
|
|
||
| $active_modules = $module_handler->getCount($criteria); | ||
|
|
||
| $criteria_all = new Criteria('dirname', '', '!='); | ||
| $total_modules = $module_handler->getCount($criteria_all); | ||
| // active modules for user side | ||
| $criteria->add(new Criteria('hasmain', 1)); | ||
| $active_modules_user = $module_handler->getCount($criteria); | ||
|
|
||
| $tpl->assign('active_modules', $active_modules); | ||
| $tpl->assign('total_modules', $total_modules); | ||
| $tpl->assign('inactive_modules', $total_modules - $active_modules); | ||
| $tpl->assign('active_modules_user', $active_modules_user); | ||
| $tpl->assign('active_modules_admin', $active_modules - $active_modules_user); |
There was a problem hiding this comment.
active_modules_admin is currently derived as active_modules - active_modules_user (i.e., active modules without hasmain). That doesn't actually represent “modules for admins”: modules that have both user side (hasmain=1) and an admin side (hasadmin=1) are excluded from the admin count, and modules with neither hasmain nor hasadmin would be included. Consider either (a) counting admin modules with an explicit hasadmin=1 criteria (separate CriteriaCompo) or (b) renaming the label/variable to reflect that it's counting “admin-only / no-user-side” modules and also filtering hasadmin=1 to avoid counting modules with no admin UI.
|
The counting logic is correct — the subtraction works because every active module has at least The only suggestion is on label clarity. "Active Modules For Admins" reads like "all modules with an admin panel" (which is almost every module), not "modules that only appear in admin" (which is the actual metric). Suggested label update in define('_MODERN_ACTIVE_MODULES_USERS', 'User-Facing Modules');
define('_MODERN_ACTIVE_MODULES_ADMINS', 'Admin-Only Modules');And a short comment in // Modules without a frontend entry (hasmain=0) are admin-only by XOOPS design.
// Every active module has at least hasmain=1 or hasadmin=1, so this subtraction
// equals querying isactive=1 AND hasmain=0 without an extra DB round-trip.
$tpl->assign('active_modules_admin', $active_modules - $active_modules_user);That way the dashboard reads:
|
Add comments to clarify admin-only module logic.
| // Modules without a frontend entry (hasmain=0) are admin-only by XOOPS design. | ||
| // Every active module has at least hasmain=1 or hasadmin=1, so this subtraction | ||
| // equals querying isactive=1 AND hasmain=0 without an extra DB round-trip. | ||
| $tpl->assign('active_modules_admin', $active_modules - $active_modules_user); |
There was a problem hiding this comment.
The KPI label says "Admin-Only Modules", but the value is computed as active_modules - active_modules_user, which is effectively “active modules without hasmain=1” (i.e., not user-facing) and does not check hasadmin=1. To avoid misleading counts, either (a) change the label to reflect “modules without frontend entry”, or (b) compute admin-only as isactive=1 AND hasadmin=1 AND hasmain=0 (using a separate criteria/count).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thank you @ggoffy ! |
Summary by CodeRabbit
New Features
Updates