feat: support multiple source entity page sets in locator CTAs#1063
Open
mkouzel-yext wants to merge 3 commits intomainfrom
Open
feat: support multiple source entity page sets in locator CTAs#1063mkouzel-yext wants to merge 3 commits intomainfrom
mkouzel-yext wants to merge 3 commits intomainfrom
Conversation
When resolving the URL for the primary CTA in Locator result cards, the URL template is now read from whichever source entity page set in __.locatorSourcePageSets includes the result entity in the Locator config rather than the __.pathInfo.sourceEntityPageSetTemplate field or the legacy __.entityPageSetUrlTemplates field. The __.locatorSourcePageSets field on the stream document contains the entity type API name, internal saved search ID, and a pathInfo object for each source entity page set of the locator. The pathInfo object contains the necessary data to resolve the CTA url for a search result: URL template, primary locale, and whether to include a locale prefix for the primary locale. Makes use of the existing resolveUrlFromPathInfo to handle actual URL resolution. Also updates Puck options for facet fields to include all options that apply to either entity type. J=WAT-5361 TEST=manual Confirmed that CTA urls used the template of the correct page set and that expected facet options were shown without duplicates.
Contributor
|
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. |
Contributor
WalkthroughThis PR introduces multi-entity type support to the locator component and refactors URL resolution to use source page set metadata. New types are defined to represent locator configuration and source page set information. The Locator component is updated to resolve multiple entity types and aggregate facet options across them. A new Sequence Diagram(s)sequenceDiagram
participant Comp as Locator Component
participant Parse as parseJsonObject
participant Type as Type Resolution
participant Facet as Facet Aggregation
participant UI as UI Rendering
Comp->>Parse: Parse _pageset and locatorSourcePageSets from __
Parse-->>Comp: Parsed JSON objects or defaults
Comp->>Type: getAnyEntityType from env/config/pageset
Type-->>Comp: Resolved single entity type
Comp->>Type: getAllEntityTypeScopes from sources
Type-->>Comp: Array of entity type scopes
Comp->>Facet: Collect facet options per entity type
Facet->>Facet: Deduplicate by value
Facet->>Facet: Sort by label
Facet-->>Comp: Aggregated facet options
Comp->>UI: Render FilterSearch with entity types
UI-->>Comp: User interaction
sequenceDiagram
participant Card as LocatorResultCard
participant Resolve as resolveUrlFromSourcePageSets
participant Parse as parseJsonObject
participant Match as pageSetIncludesEntity
participant PathInfo as resolveUrlFromPathInfo
Card->>Resolve: Call with profile, streamDocument
Resolve->>Parse: Parse locatorSourcePageSets JSON
Parse-->>Resolve: LocatorSourcePageSetInfo array
Resolve->>Match: Find matching page set for entity
Match-->>Resolve: Matching SourcePageSet or undefined
Resolve->>PathInfo: Create doc with pathInfo & resolve URL
PathInfo-->>Resolve: Final URL string
Resolve-->>Card: URL or undefined
Card->>Card: Render CTA if liveVisibility && URL exists
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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
🧹 Nitpick comments (1)
packages/visual-editor/src/utils/types/StreamDocument.ts (1)
42-45: UnifysavedFiltertyping across locator scope models.
EntityTypeScope.savedFilter(Line 44) isstring, whileLocatorSourcePageSetInfo.savedFilter(Line 50) isnumber. This type split forces downstream casts and makes scope matching easier to misuse.♻️ Proposed type alignment
export type EntityTypeScope = { entityType?: string; - savedFilter?: string; + savedFilter?: string | number; };Also applies to: 47-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/utils/types/StreamDocument.ts` around lines 42 - 45, EntityTypeScope.savedFilter is typed as string while LocatorSourcePageSetInfo.savedFilter is a number, causing casts and misuse; make their types identical (choose a single canonical type—e.g., string) by updating the savedFilter property on both EntityTypeScope and LocatorSourcePageSetInfo to that type, then update any callers/assignments that assume the old type (remove casts or convert values) so downstream code consistently treats savedFilter as the unified type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/visual-editor/src/components/Locator.tsx`:
- Around line 121-123: The precedence of entityTypeEnvVar is inverted: change
the logic in the Locator component (where entityDocument, entityTypeEnvVar,
DEFAULT_ENTITY_TYPE, and _pageset are used) to check the env override first—if
entityTypeEnvVar is set and entityDocument._env?.[entityTypeEnvVar] exists
return that; otherwise fall back to entityDocument._pageset (if present) and
finally DEFAULT_ENTITY_TYPE; apply the same env-first ordering to the similar
block covering lines 125-135.
In `@packages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.ts`:
- Around line 34-37: The code uses profile?.savedFilters when building
savedFiltersForEntity but the runtime/profile shape uses savedFilterIds; update
the reference so savedFiltersForEntity = profile?.savedFilterIds ?? [] (i.e.,
read savedFilterIds from the profile object) to ensure scoped page-set matching
works correctly for valid entities and avoid returning undefined.
- Around line 78-80: Replace the falsy check that treats 0 as missing by using a
nullish check: in the predicate that currently uses !pageSetInfo?.savedFilter,
change it to pageSetInfo?.savedFilter == null (or pageSetInfo?.savedFilter ===
null || pageSetInfo?.savedFilter === undefined) so numeric savedFilter values
like 0 are treated as present; keep the rest of the condition with
savedFiltersForEntity.includes(pageSetInfo.savedFilter.toString()) unchanged.
This affects the logic around pageSetInfo.savedFilter in
resolveUrlFromSourcePageSets.ts (references: pageSetInfo, savedFilter,
savedFiltersForEntity).
---
Nitpick comments:
In `@packages/visual-editor/src/utils/types/StreamDocument.ts`:
- Around line 42-45: EntityTypeScope.savedFilter is typed as string while
LocatorSourcePageSetInfo.savedFilter is a number, causing casts and misuse; make
their types identical (choose a single canonical type—e.g., string) by updating
the savedFilter property on both EntityTypeScope and LocatorSourcePageSetInfo to
that type, then update any callers/assignments that assume the old type (remove
casts or convert values) so downstream code consistently treats savedFilter as
the unified type.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/visual-editor/src/components/Locator.tsxpackages/visual-editor/src/components/LocatorResultCard.tsxpackages/visual-editor/src/utils/types/StreamDocument.tspackages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.test.tspackages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.ts
Comment on lines
121
to
123
| if (!entityDocument._pageset && entityTypeEnvVar) { | ||
| return entityDocument._env?.[entityTypeEnvVar] || DEFAULT_ENTITY_TYPE; | ||
| } |
Contributor
There was a problem hiding this comment.
entityTypeEnvVar precedence is currently inverted.
The docblock says env var is priority #1, but Line 121 only consults it when _pageset is missing. If both exist, env-var override is ignored.
🐛 Proposed fix
const getAnyEntityType = (entityTypeEnvVar?: string) => {
const entityDocument: StreamDocument = useDocument();
- if (!entityDocument._pageset && entityTypeEnvVar) {
- return entityDocument._env?.[entityTypeEnvVar] || DEFAULT_ENTITY_TYPE;
+ if (entityTypeEnvVar) {
+ const envEntityType = entityDocument._env?.[entityTypeEnvVar];
+ if (envEntityType) {
+ return envEntityType;
+ }
}
const pageSet = parseJsonObject(entityDocument?._pageset);Also applies to: 125-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/visual-editor/src/components/Locator.tsx` around lines 121 - 123,
The precedence of entityTypeEnvVar is inverted: change the logic in the Locator
component (where entityDocument, entityTypeEnvVar, DEFAULT_ENTITY_TYPE, and
_pageset are used) to check the env override first—if entityTypeEnvVar is set
and entityDocument._env?.[entityTypeEnvVar] exists return that; otherwise fall
back to entityDocument._pageset (if present) and finally DEFAULT_ENTITY_TYPE;
apply the same env-first ordering to the similar block covering lines 125-135.
Comment on lines
+34
to
+37
| // contains the internal saved search IDs that apply to the entity; if no saved search IDs | ||
| // apply, the property is not included in the search response | ||
| const savedFiltersForEntity: string[] = profile?.savedFilters ?? []; | ||
|
|
Contributor
There was a problem hiding this comment.
Use the correct profile field for saved filter IDs.
Line 36 reads profile.savedFilters, but matching data is provided as savedFilterIds in the profile shape used by these tests and runtime payloads. This prevents scoped page-set matching and returns undefined for valid entities.
🐛 Proposed fix
- const savedFiltersForEntity: string[] = profile?.savedFilters ?? [];
+ const savedFiltersForEntity: string[] =
+ profile?.savedFilterIds ?? profile?.savedFilters ?? [];📝 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.
Suggested change
| // contains the internal saved search IDs that apply to the entity; if no saved search IDs | |
| // apply, the property is not included in the search response | |
| const savedFiltersForEntity: string[] = profile?.savedFilters ?? []; | |
| // contains the internal saved search IDs that apply to the entity; if no saved search IDs | |
| // apply, the property is not included in the search response | |
| const savedFiltersForEntity: string[] = | |
| profile?.savedFilterIds ?? profile?.savedFilters ?? []; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.ts` around
lines 34 - 37, The code uses profile?.savedFilters when building
savedFiltersForEntity but the runtime/profile shape uses savedFilterIds; update
the reference so savedFiltersForEntity = profile?.savedFilterIds ?? [] (i.e.,
read savedFilterIds from the profile object) to ensure scoped page-set matching
works correctly for valid entities and avoid returning undefined.
Comment on lines
+78
to
+80
| (!pageSetInfo?.savedFilter || | ||
| savedFiltersForEntity.includes(pageSetInfo.savedFilter.toString())) | ||
| ); |
Contributor
There was a problem hiding this comment.
Avoid falsy checks for savedFilter presence.
Line 78 uses !pageSetInfo?.savedFilter, which treats 0 as “not present”. Use a nullish check instead.
🛡️ Proposed fix
- (!pageSetInfo?.savedFilter ||
- savedFiltersForEntity.includes(pageSetInfo.savedFilter.toString()))
+ (pageSetInfo?.savedFilter == null ||
+ savedFiltersForEntity.includes(String(pageSetInfo.savedFilter)))📝 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.
Suggested change
| (!pageSetInfo?.savedFilter || | |
| savedFiltersForEntity.includes(pageSetInfo.savedFilter.toString())) | |
| ); | |
| (pageSetInfo?.savedFilter == null || | |
| savedFiltersForEntity.includes(String(pageSetInfo.savedFilter))) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.ts` around
lines 78 - 80, Replace the falsy check that treats 0 as missing by using a
nullish check: in the predicate that currently uses !pageSetInfo?.savedFilter,
change it to pageSetInfo?.savedFilter == null (or pageSetInfo?.savedFilter ===
null || pageSetInfo?.savedFilter === undefined) so numeric savedFilter values
like 0 are treated as present; keep the rest of the condition with
savedFiltersForEntity.includes(pageSetInfo.savedFilter.toString()) unchanged.
This affects the logic around pageSetInfo.savedFilter in
resolveUrlFromSourcePageSets.ts (references: pageSetInfo, savedFilter,
savedFiltersForEntity).
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.
When resolving the URL for the primary CTA in Locator result cards, the URL template is now read from whichever source entity page set in
__.locatorSourcePageSetsincludes the result entity in the Locator config rather than the__.pathInfo.sourceEntityPageSetTemplatefield or the legacy__.entityPageSetUrlTemplatesfield. The__.locatorSourcePageSetsfield on the stream document contains the entity type API name, internal saved search ID, and a pathInfo object for each source entity page set of the locator. The pathInfo object contains the necessary data to resolve the CTA url for a search result: URL template, primary locale, and whether to include a locale prefix for the primary locale. Makes use of the existing resolveUrlFromPathInfo to handle actual URL resolution.Also updates Puck options for facet fields to include all options that apply to either entity type.
J=WAT-5361
TEST=manual
Confirmed that CTA urls used the template of the correct page set and that expected facet options were shown without duplicates.
Demo showing that page-set-specific URLs are used (restaurant URLs are prefixed with "restaurant", hospital URLs are prefixed with "hospital"):
Screen.Recording.2026-02-25.at.5.53.43.PM.mov