feat(search-indexer): Nested document resolution #15949
Conversation
WalkthroughThe changes introduce new asynchronous methods to enhance the synchronization processes within the Changes
Suggested labels
Suggested reviewers
Possibly related PRs
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
|
Datadog ReportAll test runs ✅ 12 Total Test Services: 0 Failed, 11 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (5) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Outside diff range and nitpick comments (8)
libs/cms/src/lib/search/importers/link.service.ts (1)
30-30: Add Explicit Type Annotation tochildEntryIdsTo enhance type safety and code readability, consider adding an explicit type annotation to
childEntryIds.For example:
const childEntryIds: string[] = extractChildEntryIds(entry)libs/cms/src/lib/search/importers/vacancy.service.ts (1)
Line range hint
30-45: Ensure Effective Tree-Shaking and BundlingAs this file is part of the
libsdirectory, confirm that the added code and imports promote effective tree-shaking and bundling practices to avoid increasing bundle sizes unnecessarily.libs/cms/src/lib/search/importers/serviceWebPage.service.ts (1)
31-38: Ensure consistent use of optional chainingIn the condition, optional chaining is used to safely access nested properties:
entry.fields?.organization?.fields?.slugHowever, in the object literal, the access omits optional chaining:
key: entry.fields.organization.fields.slug,While the prior check ensures the existence of these properties, using optional chaining consistently enhances code readability and guards against future changes that might introduce null or undefined values.
Consider updating the code as follows:
- key: entry.fields.organization.fields.slug, + key: entry.fields?.organization?.fields?.slug,libs/cms/src/lib/search/importers/teamList.service.ts (1)
24-25: Consider UsingDate.now()for TimestampInstead of
new Date().getTime(), you can useDate.now()to obtain the current timestamp in milliseconds. It makes the code slightly more concise.Apply the following change:
-const dateUpdated = new Date().getTime().toString() +const dateUpdated = Date.now().toString()libs/cms/src/lib/search/importers/event.service.ts (1)
63-71: Consider defining a TypeScript interface for thetagsarray elementsDefining a specific interface for the objects within the
tagsarray can enhance type safety and improve code readability. This aligns with the project's guidelines for TypeScript usage in defining and exporting types.libs/cms/src/lib/search/importers/manual.service.ts (1)
80-88: Consider simplifying the code with the spread operatorTo improve readability and conciseness, you can use the spread operator combined with
mapto add the child entry ID tags.Apply this diff:
const childEntryIds = extractChildEntryIds(entry) -for (const id of childEntryIds) { - tags.push({ - key: id, - type: 'hasChildEntryWithId', - }) -} +tags.push( + ...childEntryIds.map((id) => ({ + key: id, + type: 'hasChildEntryWithId', + })), +)libs/cms/src/lib/search/importers/utils.ts (2)
245-245: Correct typo in commentIn the comment, "The root node is not it's own child," "it's" should be "its" to indicate possession.
Apply this diff to fix the typo:
value !== rootNode.sys.id // The root node is not it's own child + value !== rootNode.sys.id // The root node is not its own child
221-261: Consider adding unit tests forextractChildEntryIdsGiven the recursive nature and complexity of
extractChildEntryIds, adding unit tests would help ensure its correctness and handle edge cases effectively.Do you want me to help generate unit tests for this function or open a GitHub issue to track this task?
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (23)
- libs/cms/src/lib/search/contentful.service.ts (6 hunks)
- libs/cms/src/lib/search/importers/anchorPage.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/article.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/customPage.service.ts (2 hunks)
- libs/cms/src/lib/search/importers/enhancedAsset.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/event.service.ts (2 hunks)
- libs/cms/src/lib/search/importers/frontpage.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/genericListItem.service.ts (2 hunks)
- libs/cms/src/lib/search/importers/groupedMenu.service.ts (2 hunks)
- libs/cms/src/lib/search/importers/lifeEventPage.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/link.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/manual.service.ts (2 hunks)
- libs/cms/src/lib/search/importers/menu.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/news.service.ts (2 hunks)
- libs/cms/src/lib/search/importers/organizationPage.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/organizationSubpage.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/projectPage.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/serviceWebPage.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/subArticle.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/supportQNA.service.ts (2 hunks)
- libs/cms/src/lib/search/importers/teamList.service.ts (3 hunks)
- libs/cms/src/lib/search/importers/utils.ts (1 hunks)
- libs/cms/src/lib/search/importers/vacancy.service.ts (3 hunks)
Additional context used
Path-based instructions (23)
libs/cms/src/lib/search/importers/link.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."
libs/cms/src/lib/search/importers/groupedMenu.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."
libs/cms/src/lib/search/importers/vacancy.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."
libs/cms/src/lib/search/importers/menu.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."
libs/cms/src/lib/search/importers/organizationPage.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."
libs/cms/src/lib/search/importers/projectPage.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."
libs/cms/src/lib/search/importers/serviceWebPage.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."
libs/cms/src/lib/search/importers/enhancedAsset.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."
libs/cms/src/lib/search/importers/lifeEventPage.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."
libs/cms/src/lib/search/importers/anchorPage.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."
libs/cms/src/lib/search/importers/customPage.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."
libs/cms/src/lib/search/importers/teamList.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."
libs/cms/src/lib/search/importers/frontpage.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."
libs/cms/src/lib/search/importers/genericListItem.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."
libs/cms/src/lib/search/importers/event.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."
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."
libs/cms/src/lib/search/importers/news.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."
libs/cms/src/lib/search/importers/manual.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."
libs/cms/src/lib/search/importers/subArticle.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."
libs/cms/src/lib/search/importers/supportQNA.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."
libs/cms/src/lib/search/importers/utils.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."
libs/cms/src/lib/search/importers/article.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."
libs/cms/src/lib/search/contentful.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 not posted (36)
libs/cms/src/lib/search/importers/link.service.ts (2)
8-8: ImportingextractChildEntryIdsUtility FunctionThe import statement correctly adds
extractChildEntryIdsfrom the utils module, enabling the extraction of child entry IDs for tagging purposes.
40-43: Constructing thetagsArray AppropriatelyThe
tagsarray is correctly constructed by mapping overchildEntryIds. This facilitates efficient lookup of documents by their child entries.libs/cms/src/lib/search/importers/groupedMenu.service.ts (2)
9-9: Importing 'extractChildEntryIds' utility function looks good.The import statement correctly includes
extractChildEntryIdsfrom'./utils', which is necessary for extracting child entry IDs.
31-31: Consider handling entries with no child IDs.Ensure that
extractChildEntryIds(entry)handles cases where an entry might not have any child entries. This will prevent potential issues when mapping over an empty array.libs/cms/src/lib/search/importers/vacancy.service.ts (1)
8-8: Confirm Import Path and Export of 'extractChildEntryIds'Ensure that the
'./utils'module correctly exportsextractChildEntryIdsand that the import path is accurate relative tovacancy.service.ts.libs/cms/src/lib/search/importers/menu.service.ts (3)
8-8: ImportingextractChildEntryIdsenhances modularity and reusability.The addition of
extractChildEntryIdsfrom'./utils'promotes code reusability and keeps theMenuSyncServicefocused on its primary responsibility.
29-29: Efficient extraction of child entry IDs for tagging.Using
extractChildEntryIds(entry)effectively retrieves the IDs of child entries, which enhances the tagging mechanism for better tracking and resolution of nested documents.
41-44: Properly mapping child entry IDs into tags array.The spread operator with
childEntryIds.mapcorrectly adds tags for each child ID, associating them with the type'hasChildEntryWithId'. This approach improves the search indexer's ability to handle nested content structures.libs/cms/src/lib/search/importers/organizationPage.service.ts (2)
8-8: Import of 'extractChildEntryIds' is appropriateThe addition of
extractChildEntryIdsimport aligns with the new functionality and is correctly implemented.
32-34: Ensure 'extractChildEntryIds' handles entries without childrenVerify that
extractChildEntryIdsgracefully handles cases where an entry has no child entries, returning an empty array without errors.libs/cms/src/lib/search/importers/projectPage.service.ts (2)
29-36: Good conditional initialization of 'tags'The code correctly initializes the
tagsarray based on the presence ofentry.fields.slug, ensuring that the slug tag is added only when available.
8-8: Verify that 'extractChildEntryIds' is properly exportedEnsure that the
extractChildEntryIdsfunction is correctly exported from theutilsmodule to prevent import errors.Run the following script to verify the export:
libs/cms/src/lib/search/importers/serviceWebPage.service.ts (2)
8-8: Good use of utility function for extracting child entry IDsImporting
extractChildEntryIdsfrom theutilsmodule enhances code reusability and maintainability. This aligns with the project's emphasis on reusability of components and hooks across different NextJS apps.
41-47: Verify that 'extractChildEntryIds' handles entries without childrenEnsure that the
extractChildEntryIds(entry)function correctly handles cases where an entry might not have child entries, returning an empty array when appropriate. This prevents potential runtime errors when iterating overchildEntryIds.You can confirm the function's behavior with the following test:
Verification successful
extractChildEntryIdscorrectly handles entries without childrenThe implementation of
extractChildEntryIdsis robust and properly handles cases where an entry might not have child entries. It initializes an empty arraychildIdsand only adds IDs when valid child entries are found. If no children are present, the function will naturally return an empty array, preventing potential runtime errors when iterating overchildEntryIds.Key points:
- The function uses a recursive approach to explore all nested structures.
- It includes checks for null/undefined values and avoids circular references.
- Child IDs are only added when specific conditions are met (sys.id that's not the root node's ID).
No changes are required as the current implementation is safe and efficient.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that 'extractChildEntryIds' returns an empty array for entries without children. # Test: Search for the implementation of 'extractChildEntryIds' and check its return value. ast-grep --lang typescript --pattern $'function extractChildEntryIds($_) { $$$ }'Length of output: 84
Script:
#!/bin/bash # Search for extractChildEntryIds using ripgrep rg "extractChildEntryIds" -n # Use a more flexible ast-grep pattern to find the function implementation ast-grep --lang typescript --pattern "extractChildEntryIds = $_" # Look for the file where extractChildEntryIds is used rg "extractChildEntryIds\(" -n libs/cms/src/lib/search/importers/serviceWebPage.service.tsLength of output: 5152
Script:
#!/bin/bash # Display the content of the extractChildEntryIds function sed -n '/export const extractChildEntryIds/,/^}/p' libs/cms/src/lib/search/importers/utils.tsLength of output: 189
Script:
#!/bin/bash # Display the content of the extractChildEntryIds function using ripgrep rg -A 30 "export const extractChildEntryIds" libs/cms/src/lib/search/importers/utils.tsLength of output: 911
libs/cms/src/lib/search/importers/enhancedAsset.service.ts (2)
8-8: EnsureextractChildEntryIdsis correctly exported and bundledThe
extractChildEntryIdsfunction is imported from'./utils'. Please verify that this utility function is properly exported in theutilsmodule and that it will be included in the build process for effective tree-shaking and bundling practices.
54-57: Confirm the format of tags for child entriesEnsure that the structure of the tags generated for
childEntryIdsaligns with the expected schema. Specifically, verify that thekeyandtypeproperties are correctly used and that downstream services correctly interpret thehasChildEntryWithIdtype.libs/cms/src/lib/search/importers/lifeEventPage.service.ts (2)
13-13: Importing 'extractChildEntryIds' Utility FunctionThe import statement for
extractChildEntryIdsis correct, promoting code reusability and maintainability.
53-56: Correctly Mapping 'childEntryIds' to 'tags'The mapping of
childEntryIdsto thetagsarray is implemented correctly, enhancing document metadata and facilitating better tracking of parent-child relationships.libs/cms/src/lib/search/importers/anchorPage.service.ts (2)
40-42: Child Entry IDs are correctly extracted and assignedThe extraction of child entry IDs using
extractChildEntryIds(entry)and assigning them tochildEntryIdsenhances the document's metadata for future lookups of child documents.
54-57: Tags field appropriately incorporates child entry IDsMapping the
childEntryIdsto thetagsarray with the specified structure correctly facilitates tracking relationships between entries, aligning with the intended functionality.libs/cms/src/lib/search/importers/customPage.service.ts (2)
8-8: Import statement for 'extractChildEntryIds' is correctThe import of
extractChildEntryIdsfrom'./utils'is appropriate and aligns with module usage practices.
54-62: Verify usage of 'hasChildEntryWithId' tags in the systemEnsure that the
hasChildEntryWithIdtag type is correctly handled in the indexing and search processes to enable nested document resolution as intended.Run the following script to locate where
hasChildEntryWithIdis used in the codebase:Consider reviewing and updating the indexing and search logic to ensure that documents tagged with
hasChildEntryWithIdare effectively associated with their child entries during search operations.libs/cms/src/lib/search/importers/teamList.service.ts (2)
2-3: Appropriate Import StatementsThe necessary modules and types are correctly imported, ensuring proper functionality.
6-7: Proper Use of TypeScript Types and UtilitiesGood use of TypeScript types and utility functions to enhance code clarity and reusability.
libs/cms/src/lib/search/importers/frontpage.service.ts (2)
8-8: ImportingextractChildEntryIdsutility functionThe import statement correctly includes the
extractChildEntryIdsfunction from './utils'.
61-64: Verify thatchildEntryIdsis always an array before mappingTo ensure safe execution of the
mapfunction, confirm thatchildEntryIdsis always an array. If there's a possibility it could beundefinedornull, themapmethod would throw an error.Run the following script to check the return type of
extractChildEntryIds:Verification successful
Verification:
childEntryIdsis consistently an arrayBased on the implementation of
extractChildEntryIdsin utils.ts and its usage across multiple services, we can confirm thatchildEntryIdsis always an array. The function initializes an emptystring[]array and populates it through recursive traversal. While an explicit return type annotation would have been clearer, the consistent usage of.map()on the result across various services strongly indicates that it always returns an array.Given this implementation, the current usage in frontpage.service.ts is safe:
...childEntryIds.map((id) => ({ key: id, type: 'hasChildEntryWithId', })),No additional safeguards are necessary in this case.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if `extractChildEntryIds` always returns an array. # Expected result: Function should always return an array type. # Search for the function definition and inspect its return type. rg --type ts -A 5 -B 2 'function extractChildEntryIds' # Alternatively, if using TypeScript types: rg --type ts 'extractChildEntryIds.*:.*\[\]'Length of output: 103
Script:
#!/bin/bash # Search for childEntryIds in TypeScript files rg --type ts 'childEntryIds' # Look for similar function names related to extracting child entry IDs rg --type ts 'extract.*ChildEntry.*' # Check for imports in the file where childEntryIds is used rg --type ts -A 10 'import.*from' libs/cms/src/lib/search/importers/frontpage.service.tsLength of output: 10253
Script:
#!/bin/bash # Examine the implementation of extractChildEntryIds in utils.ts rg --type ts -A 20 'export const extractChildEntryIds' libs/cms/src/lib/search/importers/utils.ts # Check for any type annotations or safeguards rg --type ts 'extractChildEntryIds.*:.*\[\]' libs/cms/src/lib/search/importers/utils.tsLength of output: 692
libs/cms/src/lib/search/importers/genericListItem.service.ts (1)
9-9: Confirmed: 'extractChildEntryIds' import added correctlyThe import statement now includes
extractChildEntryIds, ensuring the function is available for use.libs/cms/src/lib/search/importers/organizationSubpage.service.ts (3)
12-12: Importing 'extractChildEntryIds' is appropriate.The addition of
extractChildEntryIdsto the imports ensures that the utility function is available for use in the code below.
52-55: Correct extraction of child entry IDs for tagging.The code correctly extracts child entry IDs from the entry using
extractChildEntryIds(entry). This enhances the document's metadata by allowing future lookups to determine parent-child relationships.
75-78: Properly tagging documents with child entry IDs.Including the child entry IDs in the
tagsarray ensures that each document is associated with its children. This is essential for maintaining accurate relationships within the search index.libs/cms/src/lib/search/importers/news.service.ts (1)
13-13: Import statement added appropriately.The import of
extractChildEntryIdsfrom./utilsis correctly added and integrates well with the existing imports.libs/cms/src/lib/search/importers/manual.service.ts (1)
14-14: Import statement approvedThe import of
extractChildEntryIdsfrom './utils' is correct and ensures the necessary functionality is included.libs/cms/src/lib/search/importers/subArticle.service.ts (2)
14-14: Import statement forextractChildEntryIdsis appropriateThe addition of
extractChildEntryIdsto the imports is necessary for its usage below.
90-92: EnsureextractChildEntryIdshandles entries without child entriesVerify that
extractChildEntryIds(entry)correctly handles cases where an entry may not have child entries and returns an empty array in such cases to prevent potential errors during mapping.libs/cms/src/lib/search/importers/supportQNA.service.ts (1)
15-15: LGTM!The import statement for
extractChildEntryIdsis correctly added, enhancing the utility functions available.libs/cms/src/lib/search/importers/article.service.ts (1)
18-18: ImportingextractChildEntryIdsThe
extractChildEntryIdsfunction is imported correctly from'./utils', ensuring that the utility is available for use in the service.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/cms/src/lib/search/contentful.service.ts (7 hunks)
Additional context used
Path-based instructions (1)
libs/cms/src/lib/search/contentful.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."
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- libs/cms/src/lib/search/cmsSync.service.ts (1 hunks)
- libs/cms/src/lib/search/contentful.service.ts (8 hunks)
- libs/content-search-indexer/src/lib/indexing.service.ts (1 hunks)
- libs/content-search-indexer/types/src/index.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/cms/src/lib/search/contentful.service.ts
Additional context used
Path-based instructions (3)
libs/content-search-indexer/types/src/index.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."
libs/content-search-indexer/src/lib/indexing.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."
libs/cms/src/lib/search/cmsSync.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 not posted (1)
libs/content-search-indexer/types/src/index.ts (1)
44-44: Addition of optionalgetNextSyncTokenmethod enhances interfaceThe addition of the optional
getNextSyncTokenmethod to theContentSearchImporterinterface increases flexibility, allowing implementations to provide synchronization tokens as needed. The method is properly typed and optional, ensuring backward compatibility with existing implementations.
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
libs/cms/src/lib/search/contentful.service.ts (1)
Line range hint
278-284: Avoid type casting withaswhen possibleCasting
this.contentfulClient.syncusingasreduces type safety. If possible, define the type explicitly or ensure that the Contentful client's typings are correctly extended to accommodate this usage.Consider defining the method with proper typings to enhance maintainability.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/cms/src/lib/search/contentful.service.ts (8 hunks)
Additional context used
Path-based instructions (1)
libs/cms/src/lib/search/contentful.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 not posted (3)
libs/cms/src/lib/search/contentful.service.ts (3)
169-170: Avoid usinganytype forqueryanderrorparametersAs previously noted, using
anyreduces type safety and can introduce bugs. Consider defining more specific types forqueryanderrorto enhance code maintainability.Also applies to: 188-189
195-199: Prevent potential infinite loop whenchunkSizebecomes zeroThis issue was previously identified and remains unresolved. If
chunkSizebecomes1,Math.floor(1 / 2)results in0, which may cause an infinite loop or invalid requests withlimit: 0. Ensure thatchunkSizedoes not fall below1.
537-538: Clarify the comment to accurately reflect code logicThe comment
// Remove duplicatesmight be misleading. The code filters out IDs already present inindexableEntries, effectively excluding entries that have been processed. Consider updating the comment to// Exclude IDs already in indexableEntriesfor clarity.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
libs/cms/src/lib/search/cmsSync.service.ts (1)
162-180: LGTM!The
getNextSyncTokenfunction correctly handles the synchronization process by:
- Handling the 'fromLast'
syncTypeas a special case.- Fetching all pages until the
nextSyncTokenis obtained.- Updating the
nextPageTokenin each iteration to handle pagination effectively.- Waiting for the final token before returning, ensuring a complete synchronization process.
Consider adding the following enhancements to further improve the function:
- Add error handling to handle potential errors from the Contentful service.
- Add logging statements to track the progress of the synchronization process.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- libs/cms/src/lib/search/cmsSync.service.ts (1 hunks)
- libs/cms/src/lib/search/contentful.service.ts (8 hunks)
- libs/content-search-indexer/src/lib/indexing.service.ts (1 hunks)
- libs/content-search-indexer/types/src/index.ts (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- libs/cms/src/lib/search/contentful.service.ts
- libs/content-search-indexer/src/lib/indexing.service.ts
- libs/content-search-indexer/types/src/index.ts
Additional context used
Path-based instructions (1)
libs/cms/src/lib/search/cmsSync.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."
RunarVestmann
added
the
deprecated:automerge
kodiakhq
bot
deleted the
feature/search-indexer-nested-document-resolution
branch
Nested document resolution
What
Rework how we resolve nested entries to root entries during the cms sync process in Elasticsearch.
Instead of fetching the hierarchy from Contentful we instead store the hierarchy in Elasticsearch via tags.
Each root document is tagged with the id's of its children so we can know what needs to be re-indexed in case of an update to a nested entry
Rework also how we fetch data during initial sync (when all data gets requested from Contentful).
Now we call the sync endpoint to get the sync token and then simply call Contentful ourselves to fetch all indexable content types.
(Previously all data was fetched from the sync api, meaning that we were also getting nested content types which we ignored)
A Quick test locally suggests that the re-sync process is now twice as fast since it now takes about 15 minutes and used to take 30-ish minutes
Why
Checklist:
@coderabbitai: ignore
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
These changes collectively improve the efficiency, clarity, and maintainability of the content management system's synchronization processes.