-
Notifications
You must be signed in to change notification settings - Fork 37
Update generationtTemplateConfigs.ts #1711
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughUpdated generationTemplateConfigs to set lockWholeSheet to false for the HCM_ADMIN_CONSOLE_BOUNDARY_DATA sheet in both the user and facility modules. No other configurations were modified. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, 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: 1
🔭 Outside diff range comments (2)
health-services/project-factory/src/server/config/generationtTemplateConfigs.ts (2)
15-17: Make locking intent consistent or explicit across similar sheetsIn user.module: HCM_ADMIN_CONSOLE_USER_LIST has no lockWholeSheet (implicitly unlocked?). In userCredential.module: the same sheet is explicitly locked (lockWholeSheet: true). Consider either:
- making lockWholeSheet explicit for all sheets, or
- documenting the default (e.g., default = unlocked) and only setting true when needed.
This reduces ambiguity for future maintainers.
Also applies to: 28-31
3-3: Strengthen typing and consider correcting the identifier typo
- Replace any with a typed config to catch schema/key drift at compile time.
- Optional: Rename generationtTemplateConfigs → generationTemplateConfigs for clarity. If renaming now is risky, add an alias export to ease future migration.
Here’s a lightweight typing approach and an alias export (non-breaking):
type SheetConfig = { sheetName: string; schemaName: string; lockWholeSheet?: boolean; // default assumed false }; type ModuleConfig = { sheets: SheetConfig[] }; export type GenerationTemplateConfigs = { user: ModuleConfig; userCredential: ModuleConfig; facility: ModuleConfig; boundary: ModuleConfig; }; // Keep existing export to avoid breaking imports export const generationtTemplateConfigs: GenerationTemplateConfigs = { // ...existing object... }; // Non-breaking alias for clearer naming going forward export { generationtTemplateConfigs as generationTemplateConfigs };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
health-services/project-factory/src/server/config/generationtTemplateConfigs.ts(2 hunks)
🔇 Additional comments (2)
health-services/project-factory/src/server/config/generationtTemplateConfigs.ts (2)
19-22: Change aligns with PR objective — unlocking boundary-data sheets looks correctBoth occurrences of HCM_ADMIN_CONSOLE_BOUNDARY_DATA now have lockWholeSheet: false. This matches the PR intent and keeps the change surface minimal.
Also applies to: 46-49
19-22: Boundary-data sheet unlocking has no downstream impactAfter reviewing the codebase:
- The only consumer of
lockWholeSheetislockSheetAccordingToConfiginsheetManageUtils.ts, which solely applies Excel protection—no branching or business logic checks this flag.- All schema- and validation-based checks for boundaries (e.g., in
createAndSearch.ts) operate on column values, not on sheet protection.- The process-stage configs (
processTemplateConfigs) lock boundary sheets independently and are unrelated to the generation configs you’ve updated.- No column-level protection or validation rules reference or depend on
lockWholeSheet.Unlocking
"HCM_ADMIN_CONSOLE_BOUNDARY_DATA"in the generation templates is safe and requires no further changes.
| sheetName: "HCM_ADMIN_CONSOLE_BOUNDARY_DATA", | ||
| schemaName: "boundary-data", | ||
| lockWholeSheet: true | ||
| lockWholeSheet: false | ||
| } |
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.
🧹 Nitpick (assertive)
Add a brief comment explaining why these sheets are unlocked
A short rationale (e.g., “allow admins to edit boundary data inline”) helps avoid future toggles back to true during refactors.
Apply this minimal inline comment near each change:
{
sheetName: "HCM_ADMIN_CONSOLE_BOUNDARY_DATA",
schemaName: "boundary-data",
- lockWholeSheet: false
+ lockWholeSheet: false // Intentionally unlocked to allow inline boundary data edits
}Also applies to: 46-49
🤖 Prompt for AI Agents
In
health-services/project-factory/src/server/config/generationtTemplateConfigs.ts
around lines 19-22 (and similarly for lines 46-49), the objects set
lockWholeSheet: false lack an explanatory comment; add a concise inline comment
next to each lockWholeSheet: false (e.g., “// allow admins to edit boundary data
inline” or appropriate domain-specific reason) so future readers understand why
the sheet is intentionally unlocked.
Summary by CodeRabbit