feat: support multiple source entity page sets in locator CTAs#1063
Open
mkouzel-yext wants to merge 8 commits intomainfrom
Open
feat: support multiple source entity page sets in locator CTAs#1063mkouzel-yext wants to merge 8 commits intomainfrom
mkouzel-yext wants to merge 8 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
WalkthroughAdds multi-entity-type locator support and new URL resolution from source page sets. Introduces helpers: getAnyEntityType, getAllEntityTypeScopes, parseJsonObject, getLocatorSourcePageSets, getFacetFieldOptions, and getFacetFieldOptionsForEntityType. _pageset and .locatorSourcePageSets are parsed safely; facet options are aggregated, deduplicated, and sorted across entity types. Adds StreamDocument..locatorSourcePageSets plus exported types LocatorConfig, EntityTypeScope, and LocatorSourcePageSetInfo. Adds resolveUrlFromSourcePageSets(profile, streamDocument, relativePrefixToRoot) and updates LocatorResultCard to use it with a liveVisibility + URL guard. Sequence Diagram(s)sequenceDiagram
participant Locator as Locator Component
participant Parse as parseJsonObject
participant TypeRes as getAnyEntityType / getAllEntityTypeScopes
participant FacetAgg as Facet Aggregation
participant UI as FilterSearch / UI Rendering
Locator->>Parse: Parse _pageset and __.locatorSourcePageSets
Parse-->>Locator: Parsed objects or defaults
Locator->>TypeRes: getAnyEntityType()
TypeRes-->>Locator: single entityType or undefined
Locator->>TypeRes: getAllEntityTypeScopes()
TypeRes-->>Locator: list of entityType scopes
Locator->>FacetAgg: getFacetFieldOptions(entityTypes)
FacetAgg->>FacetAgg: Aggregate, dedupe, sort options
FacetAgg-->>Locator: Aggregated facet options
Locator->>UI: Render FilterSearch with scopes and options
sequenceDiagram
participant Card as LocatorResultCard
participant Resolver as resolveUrlFromSourcePageSets
participant Parse as parseJsonObject
participant Match as pageSetIncludesEntity
participant PathInfo as resolveUrlFromPathInfo
Card->>Resolver: resolveUrlFromSourcePageSets(profile, streamDocument, prefix)
Resolver->>Parse: Parse __.locatorSourcePageSets
Parse-->>Resolver: Array of LocatorSourcePageSetInfo or undefined
Resolver->>Match: Find first matching pageSet for profile.type / savedFilters
Match-->>Resolver: Matching pageSet with pathInfo or none
Resolver->>PathInfo: mergeMeta + resolveUrlFromPathInfo(docWithPathInfo, prefix)
PathInfo-->>Resolver: Resolved URL or undefined
Resolver-->>Card: URL or undefined
Card->>Card: Render CTA only 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)
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
packages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/visual-editor/src/utils/types/StreamDocument.ts (1)
33-45: Consider using JSDoc@deprecatedannotations.The inline comments
// deprecatedare helpful, but using JSDoc@deprecatedtags would provide better IDE support (strikethrough, warnings on usage).📝 Suggested improvement
export type LocatorConfig = { source?: string; experienceKey?: string; - entityType?: string; // deprecated - savedFilter?: string; // deprecated + /** `@deprecated` Use entityTypeScopes instead */ + entityType?: string; + /** `@deprecated` Use entityTypeScopes instead */ + savedFilter?: string; entityTypeScopes?: EntityTypeScope[]; [key: string]: any; // allow any other fields };🤖 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 33 - 45, Replace the inline "// deprecated" comments with JSDoc `@deprecated` annotations on the specific properties so IDEs show warnings/strikethroughs: add /** `@deprecated` */ above the entityType and savedFilter properties in the LocatorConfig type and likewise in the EntityTypeScope type; keep the optional types and any explanatory text in the JSDoc if desired so callers see the deprecation reason and alternative usage.
🤖 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/utils/types/StreamDocument.ts`:
- Around line 47-53: The savedFilter type is inconsistent: EntityTypeScope
defines savedFilter as string while LocatorSourcePageSetInfo uses number; update
LocatorSourcePageSetInfo.savedFilter to string to match EntityTypeScope and the
actual data (savedFiltersForEntity is string[]), and ensure any comparisons with
internalSavedFilterId still convert the number to string (internalSavedFilterId
-> String(internalSavedFilterId)) where needed; alternatively, if the numeric
intent is deliberate, add a clear comment on
LocatorSourcePageSetInfo.savedFilter explaining the semantic difference and
where conversion happens.
---
Nitpick comments:
In `@packages/visual-editor/src/utils/types/StreamDocument.ts`:
- Around line 33-45: Replace the inline "// deprecated" comments with JSDoc
`@deprecated` annotations on the specific properties so IDEs show
warnings/strikethroughs: add /** `@deprecated` */ above the entityType and
savedFilter properties in the LocatorConfig type and likewise in the
EntityTypeScope type; keep the optional types and any explanatory text in the
JSDoc if desired so callers see the deprecation reason and alternative usage.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/visual-editor/src/utils/types/StreamDocument.tspackages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.test.tspackages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.ts
- packages/visual-editor/src/utils/urls/resolveUrlFromSourcePageSets.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/visual-editor/src/components/Locator.tsx (1)
119-123:⚠️ Potential issue | 🟠 MajorHonor env override precedence in
getAnyEntityType.Line 121 still gates env-var lookup behind
!entityDocument._pageset, so the env override is ignored when both sources exist.🐛 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 119 - 123, getAnyEntityType currently prevents reading the env override when entityDocument._pageset is present; change the lookup order to honor the env var precedence: if entityTypeEnvVar is provided, return entityDocument._env?.[entityTypeEnvVar] when defined, otherwise fall back to entityDocument._pageset-derived type (or DEFAULT_ENTITY_TYPE). Update the getAnyEntityType function (and the similar logic in the other block around the same area) to first check entityTypeEnvVar and its value on useDocument() before using _pageset, and ensure DEFAULT_ENTITY_TYPE is used as the final fallback.
🤖 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 1101-1104: The effect that performs the initial search (the
useEffect/block that builds the filter search request using entityType) omits
entityType from its dependency array, causing stale results; update that
effect's dependency list to include entityType so the effect reruns when
entityType changes, and apply the same change to the other similar effect around
line 1210; reference the useEffect/initial-search effect and the entityType
variable when making the dependency update.
---
Duplicate comments:
In `@packages/visual-editor/src/components/Locator.tsx`:
- Around line 119-123: getAnyEntityType currently prevents reading the env
override when entityDocument._pageset is present; change the lookup order to
honor the env var precedence: if entityTypeEnvVar is provided, return
entityDocument._env?.[entityTypeEnvVar] when defined, otherwise fall back to
entityDocument._pageset-derived type (or DEFAULT_ENTITY_TYPE). Update the
getAnyEntityType function (and the similar logic in the other block around the
same area) to first check entityTypeEnvVar and its value on useDocument() before
using _pageset, and ensure DEFAULT_ENTITY_TYPE is used as the final fallback.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (24)
packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] latest version default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] latest version non-default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 24 with filters.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 60 custom heading with site color 2.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 60 custom heading.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 60 custom title and result cards with site color 3.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 64 static headings.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 64 static image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] latest version default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] latest version non-default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 24 with filters.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 60 custom heading with site color 2.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 60 custom heading.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 60 custom title and result cards with site color 3.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 64 static headings.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 64 static image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version non-default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 24 with filters.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 60 custom heading with site color 2.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 60 custom heading.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 60 custom title and result cards with site color 3.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 64 static headings.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 64 static image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (1)
packages/visual-editor/src/components/Locator.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/visual-editor/src/utils/types/StreamDocument.ts (1)
47-53: Document the non-zero contract forinternalSavedFilterId.Line 51 allows any
number, but0is an invalid state in this domain. Please add an inline contract note here (and keep runtime validation where parsed) to avoid accidental acceptance.Suggested minimal diff
export type LocatorSourcePageSetInfo = { pathInfo?: PathInfoShape; entityType?: string; savedFilter?: string; - internalSavedFilterId?: number; + /** Internal saved search ID; must be > 0 (0 is invalid). */ + internalSavedFilterId?: number; [key: string]: any; };Based on learnings: internal saved search IDs should never be 0; 0 indicates an invalid state for fields like
savedFilterandinternalSavedFilterId.🤖 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 47 - 53, Add an inline contract comment to LocatorSourcePageSetInfo documenting that internalSavedFilterId must be a non-zero number (0 is invalid) and indicate the same for savedFilter where applicable; keep existing runtime validation elsewhere when parsing/consuming this type but make the non-zero requirement explicit in the type declaration comment next to the internalSavedFilterId property so future readers and editors won't accept 0 by accident.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/visual-editor/src/utils/types/StreamDocument.ts`:
- Around line 47-53: Add an inline contract comment to LocatorSourcePageSetInfo
documenting that internalSavedFilterId must be a non-zero number (0 is invalid)
and indicate the same for savedFilter where applicable; keep existing runtime
validation elsewhere when parsing/consuming this type but make the non-zero
requirement explicit in the type declaration comment next to the
internalSavedFilterId property so future readers and editors won't accept 0 by
accident.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 24 with filters (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 64 static headings (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 24 with filters (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 64 static headings (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 24 with filters (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 64 static headings (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (1)
packages/visual-editor/src/utils/types/StreamDocument.ts
commit: |
mkilpatrick
reviewed
Feb 26, 2026
Collaborator
mkilpatrick
left a comment
mkilpatrick
left a comment
There was a problem hiding this comment.
Let's sync offline. Just want to understand how we're determining which pageset's url template to use. I think I understand based on the code but want to verify.
| // apply, the property is not included in the search response | ||
| const savedFiltersForEntity: string[] = profile?.savedFilters ?? []; | ||
|
|
||
| console.log( |
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
2 participants
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