-
Notifications
You must be signed in to change notification settings - Fork 61
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(ids): new scope for health directorate #17820
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new file has been added that seeds a scope for the health module. The file exports an Changes
Suggested labels
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
✨ 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
Documentation and Community
|
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 (1)
libs/auth-api-lib/seeders/data/scopes-landlaeknir.ts (1)
22-28
: Consider a more specific displayName for the health scope.While the implementation is correct, the
displayName: 'health'
is quite generic compared to other scopes like 'Vaccinations' and 'organ-donations'. Consider using a more specific name that reflects its purpose (referrals, waiting lists, and prescriptions).- displayName: 'health', + displayName: 'health-services',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/auth-api-lib/seeders/data/scopes-landlaeknir.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`libs/**/*`: "Confirm that the code adheres to the following...
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/auth-api-lib/seeders/data/scopes-landlaeknir.ts
🔇 Additional comments (2)
libs/auth-api-lib/seeders/data/scopes-landlaeknir.ts (2)
1-3
: LGTM! File structure follows TypeScript best practices.The imports and composition setup are well-organized and consistent with the existing pattern.
1-29
: Verify scope naming consistency across the codebase.The new scope follows the established pattern. Let's verify that all Landlæknir scopes follow the same naming convention.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for consistent scope naming patterns # Search for scope definitions rg --type typescript "createScope\({[\s\S]*?name: '@landlaeknir\.is/" -g '!node_modules'Length of output: 236
Let's re-run the verification using a simplified regex pattern. This should help us list all occurrences of scope definitions using the expected naming pattern.
🏁 Script executed:
#!/bin/bash # Description: Simplified search for consistent scope naming in the repository. rg --type typescript "name:\s*'@landlaeknir\.is" -g '!node_modules'Length of output: 107
Let's try a modified command targeting all TypeScript files instead of relying on the "typescript" file type.
🏁 Script executed:
#!/bin/bash # Description: Search for consistent scope naming in the repository for TypeScript files. rg -g '*.ts' "createScope\(\{[^}]*name:\s*'@landlaeknir\.is" -nLength of output: 68
Let's try a more simplified command that directly checks for occurrences of scope names starting with
@landlaeknir.is
in all TypeScript files:
🏁 Script executed:
#!/bin/bash # Description: Search for scope name occurrences in TypeScript files rg -g '*.ts' "name:\\s*'@landlaeknir\\.is" -nLength of output: 442
Scope naming verified across the codebase
The scope names in the file
libs/auth-api-lib/seeders/data/scopes-landlaeknir.ts
are consistent with the established pattern. All scopes correctly use the@landlaeknir.is
prefix and follow the expected naming structure.
- Scopes:
@landlaeknir.is/vaccinations:admin
@landlaeknir.is/vaccinations
@landlaeknir.is/organ-donations
@landlaeknir.is/health
View your CI Pipeline Execution ↗ for commit 9c7ec45.
☁️ Nx Cloud last updated this comment at |
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/auth-api-lib/seeders/data/scope-landlaeknir-health.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`libs/**/*`: "Confirm that the code adheres to the following...
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/auth-api-lib/seeders/data/scope-landlaeknir-health.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: formatting
- GitHub Check: linting-workspace
🔇 Additional comments (1)
libs/auth-api-lib/seeders/data/scope-landlaeknir-health.ts (1)
1-1
: LGTM!The imports are clean and follow the modular pattern using helper functions.
export const up = compose( | ||
createScope({ | ||
name: '@landlaeknir.is/health', | ||
displayName: 'health', | ||
description: | ||
'Veitir aðgang að upplýsingum um tilvísanir, biðlista og lyfjaávísanir', | ||
addToClients: ['@island.is/clients/api'], | ||
}), | ||
) |
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.
🛠️ Refactor suggestion
Add TypeScript types and consider internationalization.
- Consider adding TypeScript interface for the scope configuration to improve type safety and documentation:
+interface ScopeConfig {
+ name: string;
+ displayName: string;
+ description: string;
+ addToClients: string[];
+}
export const up = compose(
- createScope({
+ createScope<ScopeConfig>({
name: '@landlaeknir.is/health',
displayName: 'health',
description:
'Veitir aðgang að upplýsingum um tilvísanir, biðlista og lyfjaávísanir',
addToClients: ['@island.is/clients/api'],
}),
)
- Consider adding an English translation for the description to improve maintainability:
description:
- 'Veitir aðgang að upplýsingum um tilvísanir, biðlista og lyfjaávísanir',
+ 'Veitir aðgang að upplýsingum um tilvísanir, biðlista og lyfjaávísanir' +
+ ' | Provides access to information about referrals, waiting lists and prescriptions',
- Consider extracting the client ID to a constant for better reusability:
+const API_CLIENT_ID = '@island.is/clients/api';
export const up = compose(
createScope({
name: '@landlaeknir.is/health',
displayName: 'health',
description:
'Veitir aðgang að upplýsingum um tilvísanir, biðlista og lyfjaávísanir',
- addToClients: ['@island.is/clients/api'],
+ addToClients: [API_CLIENT_ID],
}),
)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const up = compose( | |
createScope({ | |
name: '@landlaeknir.is/health', | |
displayName: 'health', | |
description: | |
'Veitir aðgang að upplýsingum um tilvísanir, biðlista og lyfjaávísanir', | |
addToClients: ['@island.is/clients/api'], | |
}), | |
) | |
interface ScopeConfig { | |
name: string; | |
displayName: string; | |
description: string; | |
addToClients: string[]; | |
} | |
const API_CLIENT_ID = '@island.is/clients/api'; | |
export const up = compose( | |
createScope<ScopeConfig>({ | |
name: '@landlaeknir.is/health', | |
displayName: 'health', | |
description: | |
'Veitir aðgang að upplýsingum um tilvísanir, biðlista og lyfjaávísanir' + | |
' | Provides access to information about referrals, waiting lists and prescriptions', | |
addToClients: [API_CLIENT_ID], | |
}), | |
) |
Scope - Health directorate
What
Add new scope for Landlæknir Health APIs
Why
✨ new new new ✨
New apis for referrals, waiting lists and prescriptions for "Heilsa" 💊 📝 👨🏼⚕️
Checklist:
Summary by CodeRabbit