Refactor workflow resource handling and introduce new models for bett…#9982
Refactor workflow resource handling and introduce new models for bett…#9982nishagii wants to merge 1 commit intowso2:masterfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 57 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe pull request introduces new workflow-scoped resource type definitions and updates the approval workflows module to use proper TypeScript typing instead of Changes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
6bc990c to
9d6408b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
features/admin.approval-workflows.v1/models/workflow-resources.ts (1)
24-46: Remove redundant JSDoc blocks for self-explanatory model declarations.These comments only restate the symbol names and make the models file noisier.
✂️ Proposed cleanup
-/** - * Workflow-scoped role list response interface. - */ export interface WorkflowRoleListInterface extends RoleListInterface {} -/** - * Workflow-scoped group list response interface. - */ export interface WorkflowGroupListInterface extends GroupListInterface {} -/** - * Workflow-scoped application list response interface. - */ export interface WorkflowApplicationListInterface extends ApplicationListInterface {} -/** - * Workflow-scoped user store list item interface. - */ export interface WorkflowUserStoreListItemInterface extends UserStoreListItem {} -/** - * Union of all possible workflow resource list response types. - */ export type WorkflowResourceListResponse = | WorkflowRoleListInterface | WorkflowGroupListInterface | WorkflowApplicationListInterface | WorkflowUserStoreListItemInterface[];As per coding guidelines, "Interfaces and type definitions should skip JSDoc if the name and properties are self-explanatory" and "Remove comments that restate code, provide verbose step-by-step narration, or state obvious descriptions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/admin.approval-workflows.v1/models/workflow-resources.ts` around lines 24 - 46, Remove the redundant JSDoc comment blocks that simply restate symbol names in this file: delete the comment lines above WorkflowRoleListInterface, WorkflowGroupListInterface, WorkflowApplicationListInterface, WorkflowUserStoreListItemInterface and the final "Union of all possible workflow resource list response types." comment so the file contains only the interface declarations (WorkflowRoleListInterface, WorkflowGroupListInterface, WorkflowApplicationListInterface, WorkflowUserStoreListItemInterface) without self-explanatory JSDoc noise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@features/admin.approval-workflows.v1/models/workflow-resources.ts`:
- Around line 24-46: Remove the redundant JSDoc comment blocks that simply
restate symbol names in this file: delete the comment lines above
WorkflowRoleListInterface, WorkflowGroupListInterface,
WorkflowApplicationListInterface, WorkflowUserStoreListItemInterface and the
final "Union of all possible workflow resource list response types." comment so
the file contains only the interface declarations (WorkflowRoleListInterface,
WorkflowGroupListInterface, WorkflowApplicationListInterface,
WorkflowUserStoreListItemInterface) without self-explanatory JSDoc noise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 21b10b57-c126-4508-9876-9e4db4459ca3
📒 Files selected for processing (6)
features/admin.approval-workflows.v1/components/rules/role-audience-value-selector.tsxfeatures/admin.approval-workflows.v1/components/rules/workflow-resource-autocomplete.tsxfeatures/admin.approval-workflows.v1/components/rules/workflow-resource-list-select.tsxfeatures/admin.approval-workflows.v1/hooks/use-get-workflow-resources.tsfeatures/admin.approval-workflows.v1/models/workflow-resources.tsfeatures/admin.approval-workflows.v1/utils/resource-utils.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9982 +/- ##
=======================================
Coverage 56.01% 56.01%
=======================================
Files 42 42
Lines 1023 1023
Branches 254 231 -23
=======================================
Hits 573 573
Misses 450 450
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
This pull request introduces type safety improvements and better normalization for workflow resource data handling in the approval workflows feature. The main changes include the introduction of new workflow-scoped resource interfaces and types, refactoring of hooks and utility functions to use these types, and removal of unnecessary type assertions and explicit
anyusage.Type system improvements:
WorkflowResourceListResponse) inworkflow-resources.tsto represent the possible resource list responses (roles, groups, applications, user stores) in a type-safe manner.useGetWorkflowResourceshook to useWorkflowResourceListResponseas the default data type, improving type safety for all consumers of the hook. [1] [2]Refactoring and code cleanup:
useGetWorkflowResources(role-audience-value-selector.tsx,workflow-resource-autocomplete.tsx,workflow-resource-list-select.tsx) to remove unnecessaryanytypes and type assertions, and to use the new workflow resource types. [1] [2] [3]normalizeResourceResponseutility to accept the new union type and improved its logic for handling different resource response shapes, with more explicit type narrowing and comments. [1] [2] [3]Dependency updates:
These changes collectively improve type safety, maintainability, and clarity when working with workflow-related resources in the admin approval workflows feature.