feat: add breadcrumbs and reviews schema #843
Conversation
|
Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information. |
commit: |
asanehisa
left a comment
asanehisa
left a comment
There was a problem hiding this comment.
lgtm assuming testing in platform works out!
This reverts commit 66b33ec.
benlife5
requested review from
asanehisa,
briantstephan,
colton-demetriou and
mkilpatrick
mkilpatrick
left a comment
mkilpatrick
left a comment
There was a problem hiding this comment.
What happens with schema editing? Do the reviews or breadcrumbs show up or does this get added post saving and only when applied to the page?
WalkthroughCentralizes directory-parent extraction by adding and exporting getDirectoryParents(streamDocument) in utils, removes duplicate local helpers from Breadcrumbs which now imports the shared helper, and extends getSchema to optionally emit a composite JSON-LD Sequence Diagram(s)sequenceDiagram
participant Breadcrumbs
participant StreamDocument
participant getDirectoryParents
participant getSchema
participant getBreadcrumbsSchema
participant getAggregateRatingSchemaBlock
participant Output as @graph
Breadcrumbs->>StreamDocument: receive document
Breadcrumbs->>getDirectoryParents: getDirectoryParents(streamDocument)
getDirectoryParents-->>Breadcrumbs: [{slug,name}, ...]
StreamDocument->>getSchema: getSchema(streamDocument)
rect rgb(240,248,255)
Note over getSchema: Resolve main entity schema
getSchema->>getSchema: resolveMainSchema()
end
rect rgb(240,245,240)
alt has directory parents
getSchema->>getDirectoryParents: extract directory parents
getDirectoryParents-->>getSchema: [{slug,name}, ...]
getSchema->>getBreadcrumbsSchema: build BreadcrumbList
getBreadcrumbsSchema-->>getSchema: BreadcrumbList schema
end
end
rect rgb(245,240,240)
alt has FIRSTPARTY reviews
getSchema->>getAggregateRatingSchemaBlock: build rating block
getAggregateRatingSchemaBlock-->>getSchema: AggregateRating schema
end
end
rect rgb(250,250,240)
Note over getSchema: Combine resolved pieces into @graph (filtering missing parts)
getSchema-->>Output: { "@graph": [ main, breadcrumbs?, aggregateRating? ] }
end
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/visual-editor/src/utils/getSchema.test.ts (2)
⏰ 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). (4)
🔇 Additional comments (4)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/visual-editor/src/components/testing/screenshots/InsightSection/[desktop] version 7 props with entity values.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (4)
packages/visual-editor/src/components/pageSections/Breadcrumbs.tsx(1 hunks)packages/visual-editor/src/utils/getSchema.test.ts(4 hunks)packages/visual-editor/src/utils/getSchema.ts(2 hunks)packages/visual-editor/src/utils/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/visual-editor/src/utils/getSchema.test.ts (1)
packages/visual-editor/src/utils/getSchema.ts (1)
getSchema(14-60)
packages/visual-editor/src/utils/getSchema.ts (2)
packages/visual-editor/src/utils/resolveYextEntityField.ts (1)
resolveSchemaJson(230-240)packages/visual-editor/src/utils/applyTheme.ts (1)
StreamDocument(19-37)
⏰ 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). (3)
- GitHub Check: call_unit_test / unit_tests (18.x)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: semgrep/ci
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/visual-editor/src/utils/getSchema.ts (2)
46-52: Consider removing unnecessary spread operators.The spread operators on lines 49-50 are redundant since
breadcrumbsSchemaandaggregateRatingSchemaBlockare already objects. You can simplify this to:return { "@graph": [ resolvedSchemaMarkup, - breadcrumbsSchema && { ...breadcrumbsSchema }, - aggregateRatingSchemaBlock && { ...aggregateRatingSchemaBlock }, + breadcrumbsSchema, + aggregateRatingSchemaBlock, ].filter(Boolean), };
91-107: Consider enhancing the TSDoc to clarify behavior.The function correctly extracts directory parents, but the documentation could clarify that it returns the first matching
dm_directoryParents_*key found. While this is likely the expected behavior (typically only one such key should exist), making it explicit would improve clarity:/** * getDirectoryParents returns an array of objects. If no dm_directoryParents or children of - * the directory parent are not the expected objects, returns an empty array. + * the directory parent are not the expected objects, returns an empty array. + * If multiple dm_directoryParents_* keys exist, returns the first valid match. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/visual-editor/src/utils/getSchema.test.ts(4 hunks)packages/visual-editor/src/utils/getSchema.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/visual-editor/src/utils/getSchema.test.ts (2)
packages/visual-editor/src/utils/getSchema.ts (1)
getSchema(14-60)packages/visual-editor/src/utils/index.ts (1)
getSchema(12-12)
packages/visual-editor/src/utils/getSchema.ts (2)
packages/visual-editor/src/utils/resolveYextEntityField.ts (1)
resolveSchemaJson(230-240)packages/visual-editor/src/utils/applyTheme.ts (1)
StreamDocument(19-37)
🔇 Additional comments (9)
packages/visual-editor/src/utils/getSchema.test.ts (5)
5-38: LGTM! Test correctly validates schema without breadcrumbs or reviews.The test properly verifies that a location without directory parents or reviews produces a simple @graph with just the resolved schema markup.
40-89: LGTM! Comprehensive test of default schema generation.This test properly validates the fallback behavior when no custom schemaMarkup is provided, ensuring the default LocalBusiness schema is generated with field placeholders.
91-265: LGTM! Comprehensive test validates full @graph composition.This test properly verifies:
- Main schema resolution
- BreadcrumbList with all directory parents plus the current page (positions 1-5)
- AggregateRating block from FIRSTPARTY reviews
The test addresses previous concerns about including the current page in breadcrumbs (line 247) and correctly expects string values for ratings (lines 257-258).
267-360: LGTM! Test correctly validates directory page schema generation.This test properly verifies that directory entities generate:
- CollectionPage schema with ItemList mainEntity
- BreadcrumbList including the current directory page
- No AggregateRating block (as expected for directory pages)
362-399: LGTM! Test correctly validates root directory edge case.This test properly verifies that the directory root generates only the main CollectionPage schema without breadcrumbs, which is correct since root pages have no directory parents.
packages/visual-editor/src/utils/getSchema.ts (4)
75-89: LGTM! Validation logic is sound.The helper correctly validates that directory parents have the expected structure with
nameandslugstring properties.
109-145: LGTM! Breadcrumb schema generation is correct.This function properly:
- Extracts directory parents using the shared helper
- Maps parents to ListItems with correct position numbering
- Includes the current page as the final breadcrumb item (lines 128-138), addressing the previous review concern
- Uses appropriate
@idvalues (relativePrefixToRoot for parents, pageId for current page)
147-187: LGTM! Aggregate rating schema generation is correct.This function properly:
- Validates that
ref_reviewsAggexists and is an array- Searches for FIRSTPARTY publisher reviews
- Checks that both
averageRatingandreviewCountare defined before use- Converts values to strings (lines 177-178), which is acceptable per Schema.org specification and was confirmed in previous review discussions
- Returns early when FIRSTPARTY is found (efficient, as only one should exist)
14-60: LGTM! Schema generation orchestration is well-structured.The main function properly:
- Resolves the document path for schema resolution
- Conditionally builds a composite
@graphfor non-locator entities with main schema, breadcrumbs, and ratings- Filters out undefined blocks with
filter(Boolean)- Falls back to a single-item
@graphfor locator pages or when conditions aren't met- Handles errors gracefully by returning a valid
@graphwith the default schemaThe implementation correctly addresses all previous review concerns.
Automatically add breadcrumbs and reviews schema to entity pages, if the data is present on the document. Breadcrumbs and reviews get added as separate blocks. Wraps the whole schema in `@graph`. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
3 participants
Automatically add breadcrumbs and reviews schema to entity pages, if the data is present on the document.
Breadcrumbs and reviews get added as separate blocks. Wraps the whole schema in
@graph.