-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add function to get relevant app titles to display as input field #14521
Conversation
📝 WalkthroughWalkthroughA new utility function Changes
✨ Finishing Touches
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
CodeRabbit Configuration File (
|
be25f52
to
1a939f5
Compare
1a939f5
to
2d9b159
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/types/ServiceNames.ts (1)
1-3
: LGTM! Consider adding JSDoc comments.The type definition is well-structured and correctly allows for undefined values. Consider adding JSDoc comments to document the purpose and structure of this type.
+/** + * Represents a mapping of language codes to their corresponding service names. + * Keys are language codes (e.g., 'en', 'nb') and values are either the translated service name or undefined if no translation exists. + */ export type ServiceNames = { [key: string]: string | undefined; };frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/utils/getAppTitlesToDisplay.ts (2)
4-4
: Consider moving recommendedLanguages to a configuration file.The hardcoded array of recommended languages would be better maintained in a dedicated configuration file, making it easier to update and reuse across the application.
6-16
: LGTM! Consider adding JSDoc documentation.The implementation is clean and efficient. Consider adding JSDoc comments to document the function's purpose, parameters, and return value.
+/** + * Generates a mapping of language codes to their corresponding application titles. + * Combines recommended languages, metadata titles, and provided language codes, + * ensuring unique entries and mapping them to their respective titles. + * + * @param appMetadataTitles - Object containing existing language-title mappings + * @param appLangCodesData - Array of additional language codes to include + * @returns Object mapping all unique language codes to their titles (undefined if no title exists) + */ export const getAppTitlesToDisplay = ( appMetadataTitles: ServiceNames, appLangCodesData: string[], ): ServiceNames => {frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/utils/getAppTitlesToDisplay.test.ts (2)
5-10
: Consider moving language code constants to a shared location.These language code constants might be useful in other files. Consider moving them to a shared constants file.
11-76
: Add test cases for edge cases and improve test structure.The test coverage is good but could be enhanced:
Add test cases for:
- Invalid inputs (null/undefined)
- Empty string language codes
- Case sensitivity handling
Consider using beforeEach to set up common test data.
Example structure:
describe('getAppTitlesToDisplay', () => { const appNameNb = 'appNameNb'; let appMetadataTitles: KeyValuePairs<string>; let appLangCodesData: string[]; beforeEach(() => { appMetadataTitles = {}; appLangCodesData = []; }); // ... existing tests ... it('handles null/undefined inputs gracefully', () => { expect(() => getAppTitlesToDisplay(null as any, [])).toThrow(); expect(() => getAppTitlesToDisplay({}, null as any)).toThrow(); }); it('handles empty string language codes', () => { appMetadataTitles = { '': 'empty' }; appLangCodesData = ['']; const result = getAppTitlesToDisplay(appMetadataTitles, appLangCodesData); expect(result).toEqual({ nb: undefined, nn: undefined, en: undefined, '': 'empty' }); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/types/ServiceNames.ts
(1 hunks)frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/utils/getAppTitlesToDisplay.test.ts
(1 hunks)frontend/app-development/layout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/components/Tabs/AboutTab/utils/getAppTitlesToDisplay.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14521 +/- ##
=======================================
Coverage 95.70% 95.70%
=======================================
Files 1902 1903 +1
Lines 24771 24777 +6
Branches 2839 2839
=======================================
+ Hits 23707 23713 +6
Misses 803 803
Partials 261 261 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Description
This PR creates a function
getAppTitlesToDisplay
in a utils folder in the About tab in SettingsModal. The function takes as parameters:applicationmetadata
that has languages as keys and the actual title as the value, andThe function uses these parameters to create a result object of the type
ServiceNames
with keys that equal the union of all languages in both appCodeLanguages and applicationmetadata titles. The values are fetched only from the applicationmetadata titles. If a language does not have an app name available in applicationmetadata titles, the value is undefined.Adding the
skip-manual-testing
label since the function is not used in this PR and there are tests covering the functionality of it.Related Issue(s)
appName
doesn't updatetitle
inapplicationMetaData
#12917Verification
Summary by CodeRabbit
New Features
Tests
getAppTitlesToDisplay
function, covering various input scenarios and edge cases