EC-1209 - Fix og_image variant crash when product has no images - #26
lee-farrell-lu wants to merge 2 commits into
Conversation
When a product has no images, page_image falls back to current_store.social_image. Unlike Spree::Asset attachments, social_image has no named variants registered, causing Rails to raise 'Cannot find variant :og_image for Spree::Store#social_image'. Only use variant: :og_image when the image is a product asset, not when it falls back to the store's social_image.
|
Warning Review limit reached
More reviews will be available in 19 minutes and 13 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe OG image meta tag rendering logic in the meta tags template is refined with a narrowed condition that prevents rendering when the ChangesOG Image Meta Tag Condition
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 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.
🧹 Nitpick comments (1)
storefront/app/views/themes/default/spree/shared/_meta_tags.html.erb (1)
25-29: 💤 Low valueLGTM!
The condition correctly prevents the crash by excluding
Spree::Storerecords from using the:og_imagevariant. Whenpage_imagefalls back tocurrent_store.social_image(which lacks the:og_imagevariant registration), the explicit dimensions are used instead.The logic flow is sound:
- Product pages with product images → use
:og_imagevariant- Product pages without images (store fallback) → use explicit dimensions
- Non-product pages → use explicit dimensions
One optional improvement would be adding an inline comment explaining why the Store exclusion exists:
<%# Product assets have :og_image variant; store social_image does not %> <% if `@product` && !page_image.record.is_a?(Spree::Store) %>This would help future maintainers understand the variant availability constraint, but it's not critical given the PR description documents the issue well.
🤖 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 `@storefront/app/views/themes/default/spree/shared/_meta_tags.html.erb` around lines 25 - 29, Add a short inline comment above the conditional that checks `@product` and page_image.record.is_a?(Spree::Store) to explain why Spree::Store is excluded (product assets register the :og_image variant but the store's social_image does not), so future maintainers understand why the code uses the :og_image variant for product images and explicit width/height for store/non-product images; reference the conditional and the spree_image_url calls that use variant: :og_image vs width/height to locate where to place the comment.
🤖 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.
Nitpick comments:
In `@storefront/app/views/themes/default/spree/shared/_meta_tags.html.erb`:
- Around line 25-29: Add a short inline comment above the conditional that
checks `@product` and page_image.record.is_a?(Spree::Store) to explain why
Spree::Store is excluded (product assets register the :og_image variant but the
store's social_image does not), so future maintainers understand why the code
uses the :og_image variant for product images and explicit width/height for
store/non-product images; reference the conditional and the spree_image_url
calls that use variant: :og_image vs width/height to locate where to place the
comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b2dce52d-e963-4a58-b482-0b8fd1c83089
📒 Files selected for processing (1)
storefront/app/views/themes/default/spree/shared/_meta_tags.html.erb
2 participants
When a product has no images, page_image falls back to current_store.social_image. Unlike
Spree::Assetattachments, social_image has no named variants registered, causing Rails to raise 'Cannot find variant:og_imagefor Spree::Store#social_image'.Only use variant: :og_image when the image is a product asset, not when it falls back to the store's social_image.
From our Sentry logging:
Error: [Sentry] - ActionView::Template::Error: Cannot find variant :og_image for Spree::Store#social_image (ActionView::Template::Error)Summary by CodeRabbit