feat(web): Do not index org subpages that are 'standalone' #16717
Conversation
RunarVestmann
added
the
deprecated:automerge
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
libs/cms/src/lib/search/importers/organizationSubpage.service.ts (2)
25-27: Consider extracting the theme constant.The implementation correctly filters out standalone organization pages. However, the 'standalone' string literal could be moved to a constant for better maintainability.
+const STANDALONE_THEME = 'standalone' as const; + @Injectable() export class OrganizationSubpageSyncService implements CmsSyncProvider<IOrganizationSubpage> { processSyncData(entries: processSyncDataInput<IOrganizationSubpage>) { return entries.filter( (entry: Entry<any>): entry is IOrganizationSubpage => entry.sys.contentType.sys.id === 'organizationSubpage' && !!entry.fields.title && !!entry.fields.slug && !!entry.fields.organizationPage?.fields?.slug && - entry.fields.organizationPage.fields.theme !== 'standalone', + entry.fields.organizationPage.fields.theme !== STANDALONE_THEME, ) }
25-27: Consider using a more specific type thanEntry<any>.While the type guard ensures type safety, using
Entry<any>in the filter callback could be more specific to improve type checking during development.processSyncData(entries: processSyncDataInput<IOrganizationSubpage>) { return entries.filter( - (entry: Entry<any>): entry is IOrganizationSubpage => + (entry: Entry<IOrganizationSubpage>): entry is IOrganizationSubpage =>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/cms/src/lib/search/importers/organizationSubpage.service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/cms/src/lib/search/importers/organizationSubpage.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (1)
libs/cms/src/lib/search/importers/organizationSubpage.service.ts (1)
25-27: Verify impact on existing organization pages.
The filtering logic looks correct, but let's verify that:
- Regular (non-standalone) organization pages are still being indexed
- The change doesn't affect existing search functionality for other page types
✅ Verification successful
The filtering logic is safe and correctly implemented
Based on the verification results:
- The default theme is 'default' as seen in
organizationPage.model.ts, confirming that regular organization pages will continue to be indexed - The standalone theme check is only present in the organization subpage importer, not affecting other content types
- The search functionality is properly isolated in the content search service, with the filtering happening at the data source level
The change correctly excludes only standalone organization subpages from the global search while maintaining all other search functionality intact.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other theme values to ensure we're not affecting other cases
rg -l "theme.*=.*['\"](?!standalone)['\"]" --type=typescript
# Look for test files that might need updating
fd "organizationSubpage.*test" --type=typescript
Length of output: 545
Script:
#!/bin/bash
# Search for theme-related code with proper regex
rg -l "theme.*standalone" --type ts
# Look for organization page related tests with proper file type
fd "organizationSubpage" -e ts
# Search for other usages of organizationPage theme
ast-grep --pattern 'fields.theme'
# Look for existing search functionality tests
fd "search" -e ts -e spec -e test
Length of output: 4229
Script:
#!/bin/bash
# Check the default theme implementation in organization models
cat libs/cms/src/lib/models/organizationPage.model.ts
# Look for search indexing tests
rg -A 5 "describe.*search.*index" --type ts
# Check the content search service implementation
cat libs/api/domains/content-search/src/lib/contentSearch.service.ts
Length of output: 8620
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16717 +/- ##
==========================================
- Coverage 36.54% 36.53% -0.01%
==========================================
Files 6890 6890
Lines 143639 143640 +1
Branches 40925 40926 +1
==========================================
- Hits 52486 52485 -1
- Misses 91153 91155 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 9 Passed Test ServicesThis report shows up to 10 services
|
kodiakhq
bot
deleted the
feature/search-dont-index-org-subpages-that-are-standalone
branch
Do not index org subpages that are 'standalone'
What
Checklist:
Summary by CodeRabbit