Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactors breadcrumbs in three page components to derive items from a computed array and emit a schema.org BreadcrumbList; makes ProfileCard Person schema include explicit Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/web-app/app/pages/[pageSlug]/index.vue (1)
37-47: Schema.org requires absolute URLs for breadcrumb items.Same issue as in
reviews.vue: theitemproperty should contain absolute URLs rather than relative paths for proper structured data validation.Apply the same fix as suggested in the
reviews.vuefile to construct absolute URLs usingapp.url.apps/web-app/app/pages/[pageSlug]/points.vue (1)
48-58: Schema.org requires absolute URLs for breadcrumb items.Same issue as in
reviews.vueandindex.vue: theitemproperty should contain absolute URLs rather than relative paths for proper structured data validation.Apply the same fix as suggested in the
reviews.vuefile to construct absolute URLs usingapp.url.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web-app/app/components/user/ProfileCard.vue(1 hunks)apps/web-app/app/pages/[pageSlug]/index.vue(2 hunks)apps/web-app/app/pages/[pageSlug]/points.vue(2 hunks)apps/web-app/app/pages/[pageSlug]/reviews.vue(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: build
🔇 Additional comments (4)
apps/web-app/app/pages/[pageSlug]/reviews.vue (1)
56-72: LGTM! Breadcrumb items are well-structured.The computed breadcrumb items array is cleanly organized with proper labels, icons, and routing.
apps/web-app/app/components/user/ProfileCard.vue (1)
38-42: LGTM! Schema.org Person definition is correctly structured.The explicit
@typeand quoted keys align with schema.org conventions, and theurlproperty correctly uses an absolute URL.apps/web-app/app/pages/[pageSlug]/index.vue (1)
22-33: LGTM! Breadcrumb items are well-structured.The computed breadcrumb items array is cleanly organized for the main page view.
apps/web-app/app/pages/[pageSlug]/points.vue (1)
28-44: LGTM! Breadcrumb items are well-structured.The computed breadcrumb items array is cleanly organized for the points page view.
| useSchemaOrg([ | ||
| defineBreadcrumb({ | ||
| '@type': 'BreadcrumbList', | ||
| 'itemListElement': breadcrumbItems.value.map((item, index) => ({ | ||
| '@type': 'ListItem', | ||
| 'position': index + 1, | ||
| 'name': item.label, | ||
| 'item': item.to ?? undefined, | ||
| })), | ||
| }), | ||
| ]) |
There was a problem hiding this comment.
Schema.org requires absolute URLs for breadcrumb items.
The item property in BreadcrumbList should contain absolute URLs (e.g., https://example.com/page) rather than relative paths. This impacts SEO and structured data validation by search engines.
Consider constructing absolute URLs using the app configuration:
+const { app } = useAppConfig()
+
useSchemaOrg([
defineBreadcrumb({
'@type': 'BreadcrumbList',
'itemListElement': breadcrumbItems.value.map((item, index) => ({
'@type': 'ListItem',
'position': index + 1,
'name': item.label,
- 'item': item.to ?? undefined,
+ 'item': item.to ? `${app.url}${item.to}` : undefined,
})),
}),
])Alternatively, omit the item property entirely when to is undefined:
useSchemaOrg([
defineBreadcrumb({
'@type': 'BreadcrumbList',
'itemListElement': breadcrumbItems.value.map((item, index) => ({
'@type': 'ListItem',
'position': index + 1,
'name': item.label,
- 'item': item.to ?? undefined,
+ ...(item.to && { 'item': `${app.url}${item.to}` }),
})),
}),
])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useSchemaOrg([ | |
| defineBreadcrumb({ | |
| '@type': 'BreadcrumbList', | |
| 'itemListElement': breadcrumbItems.value.map((item, index) => ({ | |
| '@type': 'ListItem', | |
| 'position': index + 1, | |
| 'name': item.label, | |
| 'item': item.to ?? undefined, | |
| })), | |
| }), | |
| ]) | |
| const { app } = useAppConfig() | |
| useSchemaOrg([ | |
| defineBreadcrumb({ | |
| '@type': 'BreadcrumbList', | |
| 'itemListElement': breadcrumbItems.value.map((item, index) => ({ | |
| '@type': 'ListItem', | |
| 'position': index + 1, | |
| 'name': item.label, | |
| 'item': item.to ? `${app.url}${item.to}` : undefined, | |
| })), | |
| }), | |
| ]) |
🤖 Prompt for AI Agents
In apps/web-app/app/pages/[pageSlug]/reviews.vue around lines 76 to 86, the
BreadcrumbList currently sets each item's "item" to the raw `to` (which may be a
relative path); change this to produce absolute URLs or omit the property when
`to` is undefined: obtain the site base URL from runtime config (e.g.
useRuntimeConfig().public.siteUrl or equivalent), then map breadcrumbItems so
that for entries with a `to` value you build an absolute URL (e.g. new
URL(item.to, base).toString()) and assign that to `item`, and for entries
without `to` either set `item` to undefined or omit the `item` key entirely so
only absolute URLs appear in the Schema.org BreadcrumbList.
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web-app/app/pages/[pageSlug]/index.vue">
<violation number="1" location="apps/web-app/app/pages/[pageSlug]/index.vue:35">
P2: The `computed()` property's reactivity is not being used. `breadcrumbItems.value` is read once during setup, so changes to `page` or `mainCategory` won't update the breadcrumbs. Consider using `watch` or `watchEffect` if reactivity is needed, or remove `computed()` if it's not.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| }, | ||
| ]) | ||
|
|
||
| useBreadcrumb().setItems(breadcrumbItems.value) |
There was a problem hiding this comment.
P2: The computed() property's reactivity is not being used. breadcrumbItems.value is read once during setup, so changes to page or mainCategory won't update the breadcrumbs. Consider using watch or watchEffect if reactivity is needed, or remove computed() if it's not.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web-app/app/pages/[pageSlug]/index.vue, line 35:
<comment>The `computed()` property's reactivity is not being used. `breadcrumbItems.value` is read once during setup, so changes to `page` or `mainCategory` won't update the breadcrumbs. Consider using `watch` or `watchEffect` if reactivity is needed, or remove `computed()` if it's not.</comment>
<file context>
@@ -32,6 +32,20 @@ useBreadcrumb().setItems([
},
])
+useBreadcrumb().setItems(breadcrumbItems.value)
+
+useSchemaOrg([
</file context>
| useBreadcrumb().setItems(breadcrumbItems.value) | |
| watch(breadcrumbItems, (items) => useBreadcrumb().setItems(items), { immediate: true }) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web-app/app/pages/[pageSlug].vue (2)
88-95: Consider handling undefined rating data and clarifying ratingCount vs reviewCount.Two observations:
ratingCountandreviewCountare both assignedpage.value?.reviewsCount. Per Schema.org, these have different semantics—ratingCountis total ratings,reviewCountis total reviews. If users can rate without writing a review, these values may differ. If your data model doesn't distinguish them, this is fine.If
page.value?.ratingorpage.value?.reviewsCountisundefined, the schema will emit undefined values, which may produce invalid structured data. Consider conditionally includingaggregateRatingonly when the data exists:useSchemaOrg([ defineLocalBusiness({ name: page.value?.title, url: `${app.url}/${params.pageSlug}`, priceRange: '₽', - aggregateRating: { + aggregateRating: page.value?.rating ? { '@type': 'AggregateRating', 'ratingValue': page.value?.rating, 'ratingCount': page.value?.reviewsCount, 'reviewCount': page.value?.reviewsCount, 'bestRating': 5, 'worstRating': 1, - }, + } : undefined, }), ])
87-87: Verify priceRange value aligns with Schema.org recommendations.The
priceRangevalue'₽'is the currency symbol but doesn't indicate relative price level. Schema.org typically expects indicators like'$','$$','$$$'to convey cheap/moderate/expensive. Consider using'₽₽'or similar if you want to convey a price tier, or confirm this is the intended representation for your use case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web-app/app/pages/[pageSlug].vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
Summary by cubic
Added Schema.org BreadcrumbList to page breadcrumbs to improve SEO and enable rich snippets. Also made the Person schema explicit in ProfileCard and updated LocalBusiness schema with priceRange and embedded aggregate rating.
New Features
Refactors
Written for commit dd64b43. Summary will update automatically on new commits.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.