Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an admin "slideshow preview" flow, extends the Slideshow model with an Changes
Sequence DiagramsequenceDiagram
actor Admin
participant Controller as admin/category.php
participant Handler as SlideshowHandler
participant Model as Slideshow
participant DB as Database
participant Theme as xoTheme
participant View as Template Engine
Admin->>Controller: Request preview (op=preview&id=CAT_ID)
Controller->>Handler: getSlideshowElements(CAT_ID, display, assets=true, preview=true)
Handler->>DB: query images & slideshow rows (preview may bypass filters)
DB-->>Handler: slideshow row(s) + images
Handler->>Model: decode params/assets, build block['images'], block['params'], block['assets']
Handler-->>Controller: return ['slsTpl'=>..., 'block'=>...]
alt BT3 or BT5 slideshow
Controller->>Theme: add CSS assets (from block.assets)
Controller->>Theme: add JS assets (from block.assets)
end
Controller->>Controller: gen preview identifier (md5(uniqid(catId,...)))
Controller->>View: assign vars (block, preview=true, identifier, urls, slsTpl)
View->>View: render wgslider_admin_category.tpl (includes db:slideshow tpl)
View-->>Admin: rendered preview page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
class/SlideshowHandler.php (1)
167-233:⚠️ Potential issue | 🟡 MinorPotential undefined variable when no images exist.
When
$imageCountis 0, the$blockvariable is never initialized before being accessed at lines 230-231. This could cause an undefined variable warning or error.🐛 Proposed fix to initialize $block
public function getSlideshowElements(int $catId, int $display, bool $assets = false, bool $preview = false): array { $helper = \XoopsModules\Wgslider\Helper::getInstance(); $imageHandler = $helper->getHandler('Image'); $categoryHandler = $helper->getHandler('Category'); + $block = []; $categoryObj = $categoryHandler->get($catId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@class/SlideshowHandler.php` around lines 167 - 233, The method getSlideshowElements can return with $block uninitialized when no images exist; initialize $block early (e.g., set $block = ['images' => []]; and ensure $block['params'] and $block['assets'] are set even if $imageCount is 0) before the image query so later code and the return ['slsTpl' => $slsTpl, 'block' => $block] always reference a defined array; update getSlideshowElements to create $block (and ensure $slsParams/$slsAssets are defined) immediately after building $slsParams/$slsAssets so all return paths are safe.
🧹 Nitpick comments (1)
xoops_version.php (1)
33-33: Version bump looks good, but consider updatingrelease_date.The version is correctly bumped to
1.0.1. Note thatrelease_dateon line 45 andreleaseon line 65 still reference the old date (2026/02/28). Consider updating these to2026/04/10to match the changelog entry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xoops_version.php` at line 33, Update the package metadata to match the new version by changing the 'release_date' and 'release' fields to the new date; specifically locate the 'release_date' key and the 'release' key in xoops_version.php (they currently have '2026/02/28') and set both to '2026/04/10' so the metadata matches the 1.0.1 changelog entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@templates/wgslider_slideshow_bt3.tpl`:
- Around line 14-20: The carousel indicators loop is iterating over $block
(which contains params, assets, images) instead of the image list, producing
extra indicators; change the Smarty foreach in the indicators block to iterate
over $block.images (the same collection used by the slides loop) and keep the
loop name (e.g., name="loop") so that smarty.foreach.loop.index and the
data-target "#myCarousel{$wgslider_identifier}" remain valid; ensure the
condition still checks $block.params.show_indicator before rendering.
In `@templates/wgslider_slideshow_bt5.tpl`:
- Around line 4-8: The carousel indicator loop is iterating over the whole
$block wrapper instead of the slide list, causing one indicator per top-level
key; change the foreach to iterate over the slide array (e.g., foreach
$block.images as $slider_image or foreach $block['images'] as $slider_image
depending on Smarty usage) and keep the existing references to
$smarty.foreach.loop.index and the active check (== 0) so indicators match
slides correctly.
- Around line 45-49: The pause property in the bootstrap.Carousel initialization
is incorrectly wrapped in additional quotes; remove the extra literal quotes
around the pause value so it uses the `@json_encode` output directly (consistent
with wrap and keyboard). Update the pause line in the new bootstrap.Carousel(el,
{ ... }) block to stop surrounding
<{$block.params.pause|default:'hover'|@json_encode}> with quotes so the filter
emits a valid JS literal instead of a quoted string.
---
Outside diff comments:
In `@class/SlideshowHandler.php`:
- Around line 167-233: The method getSlideshowElements can return with $block
uninitialized when no images exist; initialize $block early (e.g., set $block =
['images' => []]; and ensure $block['params'] and $block['assets'] are set even
if $imageCount is 0) before the image query so later code and the return
['slsTpl' => $slsTpl, 'block' => $block] always reference a defined array;
update getSlideshowElements to create $block (and ensure $slsParams/$slsAssets
are defined) immediately after building $slsParams/$slsAssets so all return
paths are safe.
---
Nitpick comments:
In `@xoops_version.php`:
- Line 33: Update the package metadata to match the new version by changing the
'release_date' and 'release' fields to the new date; specifically locate the
'release_date' key and the 'release' key in xoops_version.php (they currently
have '2026/02/28') and set both to '2026/04/10' so the metadata matches the
1.0.1 changelog entry.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ebcbf62-a4eb-4a55-a5a6-3ba8bbb7b7e7
📒 Files selected for processing (18)
admin/category.phpassets/css/admin/style.cssclass/Slideshow.phpclass/SlideshowHandler.phpdocs/changelog.txtlanguage/english/admin.phpsql/mysql.sqlsql/wgslider_1.0.0_migrate.ymlsql/wgslider_1_0_0_migrate.ymltemplates/admin/wgslider_admin_category.tpltemplates/admin/wgslider_admin_image.tpltemplates/admin/wgslider_admin_slideshow.tpltemplates/wgslider_slideshow_bt3.tpltemplates/wgslider_slideshow_bt5.tpltemplates/wgslider_slideshow_default.tpltemplates/wgslider_slideshow_splide.tpltemplates/wgslider_slideshow_swiper.tplxoops_version.php
💤 Files with no reviewable changes (2)
- sql/wgslider_1.0.0_migrate.yml
- sql/wgslider_1_0_0_migrate.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
class/SlideshowHandler.php (1)
167-175: PHPStan flags potential method calls onboolreturn.The static analysis tool flags that
$helper->getHandler('Image')and$helper->getHandler('Category')can returnbool|XoopsObjectHandler|XoopsPersistableObjectHandler, and calling->get(),->getCount(),->getAll()on aboolwould fail. While in practice these handlers are always valid in this module's context, consider adding type assertions or@varannotations to suppress the warnings cleanly.💡 Optional: Add type annotations
public function getSlideshowElements(int $catId, int $display, bool $assets = false, bool $preview = false): array { $helper = \XoopsModules\Wgslider\Helper::getInstance(); + /** `@var` \XoopsModules\Wgslider\ImageHandler $imageHandler */ $imageHandler = $helper->getHandler('Image'); + /** `@var` \XoopsModules\Wgslider\CategoryHandler $categoryHandler */ $categoryHandler = $helper->getHandler('Category');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@class/SlideshowHandler.php` around lines 167 - 175, The helper->getHandler calls in getSlideshowElements can return bool per PHPStan; add explicit type assertions or `@var` annotations to silence warnings and guard usage: after $helper = \XoopsModules\Wgslider\Helper::getInstance(); assert or annotate that $imageHandler is the Image handler and $categoryHandler is the Category handler (e.g. `@var` \XoopsModules\Wgslider\ImageHandler $imageHandler and `@var` \XoopsModules\Wgslider\CategoryHandler $categoryHandler) or perform runtime checks (if ($imageHandler === false) throw/new error) before calling ->get(), ->getCount(), ->getAll() so PHPStan no longer thinks methods might be invoked on bool.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin/category.php`:
- Around line 78-96: The preview case calls
$slideshowHandler->getSlideshowElements but never instantiates
$slideshowHandler; instantiate it the same way as in the list case by calling
$helper->getHandler('Slideshow') (or reuse an existing slideshow handler
variable) before calling getSlideshowElements in the preview branch so
$slideshowHandler is defined when getSlideshowElements($catId, 0, true, true) is
invoked.
---
Nitpick comments:
In `@class/SlideshowHandler.php`:
- Around line 167-175: The helper->getHandler calls in getSlideshowElements can
return bool per PHPStan; add explicit type assertions or `@var` annotations to
silence warnings and guard usage: after $helper =
\XoopsModules\Wgslider\Helper::getInstance(); assert or annotate that
$imageHandler is the Image handler and $categoryHandler is the Category handler
(e.g. `@var` \XoopsModules\Wgslider\ImageHandler $imageHandler and `@var`
\XoopsModules\Wgslider\CategoryHandler $categoryHandler) or perform runtime
checks (if ($imageHandler === false) throw/new error) before calling ->get(),
->getCount(), ->getAll() so PHPStan no longer thinks methods might be invoked on
bool.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d005dd71-7ac8-4953-923e-d35077c7fc67
📒 Files selected for processing (5)
admin/category.phpclass/SlideshowHandler.phptemplates/wgslider_slideshow_bt3.tpltemplates/wgslider_slideshow_bt5.tplxoops_version.php
✅ Files skipped from review due to trivial changes (1)
- xoops_version.php
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
New Features
Style
Refactor
Documentation
Chores