feat(web): Global search alias for 'Vinnumálastofnun' #17204
Conversation
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 (3)
libs/content-search-toolkit/src/queries/search.ts (3)
38-39: Consider extracting organization aliases to a configuration object.While the implementation is functionally correct, consider refactoring the organization aliases into a maintainable configuration:
+const ORGANIZATION_ALIASES: Record<string, string> = { + 'tr': 'Tryggingastofnun', + 'vmst': 'Vinnumálastofnun', +}; // Handle aliases since the search engine has not been configured to support organization aliases -if (queryString.trim().toLowerCase() === 'tr') { - queryString = 'Tryggingastofnun' -} else if (queryString.trim().toLowerCase() === 'vmst') { - queryString = 'Vinnumálastofnun' +const normalizedQuery = queryString.trim().toLowerCase(); +if (normalizedQuery in ORGANIZATION_ALIASES) { + queryString = ORGANIZATION_ALIASES[normalizedQuery]; }This approach:
- Makes it easier to add new aliases
- Reduces duplicate string normalization
- Improves maintainability
- Provides better type safety with TypeScript
37-39: Consider implementing alias support in the search engine configuration.The comment "since the search engine has not been configured to support organization aliases" suggests this is a workaround. Consider creating a ticket to implement proper alias support in the search engine configuration for better scalability.
This could involve:
- Using Elasticsearch synonyms API
- Implementing a proper mapping configuration
- Moving alias handling to the search engine level
Line range hint
1-1: Consider enhancing type safety for organization names.To improve type safety and maintainability, consider defining an enum or union type for organization names and their aliases.
export enum OrganizationName { Tryggingastofnun = 'Tryggingastofnun', Vinnumalastofnun = 'Vinnumálastofnun', } export enum OrganizationAlias { TR = 'tr', VMST = 'vmst', } const ORGANIZATION_ALIASES: Record<OrganizationAlias, OrganizationName> = { [OrganizationAlias.TR]: OrganizationName.Tryggingastofnun, [OrganizationAlias.VMST]: OrganizationName.Vinnumalastofnun, };This would:
- Provide better TypeScript support
- Make organization names and aliases more maintainable
- Help prevent typos and inconsistencies
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/content-search-toolkit/src/queries/search.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/content-search-toolkit/src/queries/search.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."
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17204 +/- ##
==========================================
- Coverage 35.76% 35.76% -0.01%
==========================================
Files 6932 6932
Lines 147982 147984 +2
Branches 42171 42172 +1
==========================================
Hits 52931 52931
- Misses 95051 95053 +2 Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 98 Total Test Services: 0 Failed, 90 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
… (#17266) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Global search alias for 'Vinnumálastofnun'
What
Similar to what we did for 'TR', now users can search 'VMST' and that'll lead to the same results as typing 'Vinnumálastofnun'
Checklist:
Summary by CodeRabbit