Story 2455: Library Highlight Carousel on Homepage#2510
Conversation
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review 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 refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR replaces the homepage "Get Started" code card with a new "Library Highlight" carousel showing editor-curated and randomly selected libraries. Changes include a new HomepageSettings field/migration, form/query scoping via a shared FEATURED_LIBRARY_TIERS constant, a carousel builder function, and a CSS-only carousel template with styling. ChangesLibrary Highlight Carousel
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
5c91f7a to
249333e
Compare
8d97155 to
5f915df
Compare
herzog0
left a comment
herzog0
left a comment
There was a problem hiding this comment.
Heya! Pre-approving with some optional feedback :)
| .order_by("?") | ||
| ) | ||
|
|
||
| remaining_slots = limit - len(libraries) |
There was a problem hiding this comment.
limit here won't be enforced if an admin set more than 3 libraries as highlighted, cause remaining_slots will be negative and bypass the filter below. Any issues with that or do you think we should accept as many highlighted libraries as an admin wishes?
There was a problem hiding this comment.
Yes I think we should allow admin to highlight more than the limits as there are no harm in it. Does that work with you @herzog0 ?
jlchilders11
left a comment
jlchilders11
left a comment
There was a problem hiding this comment.
Pre approving, couple of small nits for efficiency but great work overall :)
5f915df to
da79fd3
Compare
cb3e235 to
4a739ca
Compare
![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
🧹 Nitpick comments (1)
ak/homepage.py (1)
187-187: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winN+1 queries on
first_boost_versionin the slide loop.
library.first_boost_versionexecutes 3 queries per library (exists(),order_by().first(), and the.versionFK access withoutselect_related). With 3 libraries on a high-traffic homepage, that's 9 extra queries per page load — andorder_by("?")prevents result caching.Batch-query the first versions in a single pass instead:
⚡ Proposed batch query to eliminate N+1
library_versions = { lv.library_id: lv for lv in LibraryVersion.objects.filter(library__in=libraries, version=latest) } + # Batch-query the first Boost version per library (replaces per-library first_boost_version calls) + first_versions = {} + for lv in ( + LibraryVersion.objects.filter(library__in=libraries) + .select_related("version") + .order_by("library_id", "version__release_date", "version__name") + ): + if lv.library_id not in first_versions: + first_versions[lv.library_id] = lv.version + slides = [] for library in libraries: lv = library_versions.get(library.id) - first_version = library.first_boost_version + first_version = first_versions.get(library.id) slides.append(🤖 Prompt for 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. In `@ak/homepage.py` at line 187, The homepage slide loop is triggering N+1 queries through library.first_boost_version, so replace the per-library lookup with a batched prefetch/query that resolves first boost versions for all libraries at once. Update the logic around the slide loop and the first_boost_version access so it reuses a single query result map instead of calling exists(), order_by().first(), and version FK loading repeatedly for each library.
🤖 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 `@templates/v3/includes/_library_highlight_carousel.html`:
- Around line 17-53: The single-slide case in library_highlight_carousel
currently skips the inline state rules, so the CSS defaults keep the added-in
text and CTA hidden. Update the template logic around the carousel state styles
and radio-driven overrides so that the visible “Added in” span and documentation
CTA are still shown when total == 1. Use the existing carousel_id, total, and
slide loop in _library_highlight_carousel.html to add a dedicated single-slide
override instead of relying only on the :checked rules.
---
Nitpick comments:
In `@ak/homepage.py`:
- Line 187: The homepage slide loop is triggering N+1 queries through
library.first_boost_version, so replace the per-library lookup with a batched
prefetch/query that resolves first boost versions for all libraries at once.
Update the logic around the slide loop and the first_boost_version access so it
reuses a single query result map instead of calling exists(),
order_by().first(), and version FK loading repeatedly for each library.
🪄 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 Plus
Run ID: a240b474-3690-4637-b243-89f9ce5d0169
📒 Files selected for processing (12)
ak/homepage.pyak/views.pycore/migrations/0009_homepagesettings_highlighted_libraries.pycore/models.pycore/views.pylibraries/models.pystatic/css/v3/components.cssstatic/css/v3/library-highlight-carousel.cssstatic/css/v3/v3-homepage.csstemplates/v3/examples/_v3_example_section.htmltemplates/v3/homepage.htmltemplates/v3/includes/_library_highlight_carousel.html
ee51305 to
92173c1
Compare
3 participants
Issue: #2455
Summary & Context
Replaces the static "Get started" code-block card on the V3 homepage with a new "Explore battle tested libraries" carousel card, wired to real library data and editor-curated via a new Wagtail setting.
Changes
Library Highlight Carousel component
_library_highlight_carousel.html— a CSS-only, radio-driven carousel (no JS) mirroring the testimonial card pattern: hidden radios hold state, only the yellow card translates while the meta row (added-in + dots), divider, and CTA stay static and swap per-slide content via inline<style>rules. Prev/next wrap cyclically.library-highlight-carousel.cssand register it incomponents.cssHomepage wiring
build_library_highlight_carousel()inak/homepage.py: shows editor-curatedhighlighted_librariesfirst (random order), then fills remaining slots with random flagship/core libraries from the latest release; returns slide dicts with name, category tags, description, version, and docs URLEditor configuration
HomepageSettings.highlighted_libraries(M2M toLibrary) with a WagtailFieldPanel, scoped — likefeatured_library— to flagship/core libraries in the latest stable release, A→Z0009_homepagesettings_highlighted_librariesShared constant & demo
FEATURED_LIBRARY_TIERS = [Tier.FLAGSHIP, Tier.CORE]inlibraries/models.pyand reuse it acrossak/homepage.pyandcore/models.py(featured-library + highlight choosers)Risks & Considerations
<style>block keyed bycarousel_id; rendering the component twice on one page requires distinctcarousel_idvalues (the homepage and demo already use different ids).Peer-Testing Guidelines
docker compose exec web python manage.py migrate.Screenshots
Self-review Checklist
Frontend
Summary by CodeRabbit
New Features
Bug Fixes
Chores