feat: ✨ add TaskStatusEnum #116
Conversation
WalkthroughThe changes introduce a new enumeration, Changes
Poem
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 (
|
![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/tasks/domain/src/lib/value-objects/task-status.enum.ts (1)
1-7: Consider adding JSDoc documentationAdding JSDoc comments would improve IDE support and make the enum's purpose clearer:
+/** + * Represents the possible states of a task in the system. + * @enum {string} + */ export const TaskStatusEnum = { TODO: 'TODO', IN_PROGRESS: 'IN_PROGRESS', DONE: 'DONE', } as const; +/** Type representing valid task status values */ export type TaskStatus = (typeof TaskStatusEnum)[keyof typeof TaskStatusEnum];libs/tasks/domain/src/lib/value-objects/task-status.enum.spec.ts (2)
16-19: Improve immutability test implementationThe current immutability test could be clearer and avoid the static analysis warning:
it('should be immutable', () => { // @ts-expect-error - Testing immutability - expect(() => TaskStatus.TODO = 'SOMETHING_ELSE').toThrow(); + const attemptMutation = () => { + // @ts-expect-error - Testing immutability + TaskStatus.TODO = 'SOMETHING_ELSE'; + }; + expect(attemptMutation).toThrow(); });🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
3-20: Consider adding type safety testsThe test suite could benefit from additional tests that verify type constraints:
it('should not allow invalid status values', () => { // @ts-expect-error - Testing type safety const invalidStatus: TaskStatus = 'INVALID_STATUS'; // Verify type narrowing works const isValidStatus = (status: string): status is TaskStatus => Object.values(TaskStatusEnum).includes(status as TaskStatus); expect(isValidStatus('INVALID_STATUS')).toBe(false); expect(isValidStatus(TaskStatusEnum.TODO)).toBe(true); });🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
libs/tasks/domain/src/index.ts(1 hunks)libs/tasks/domain/src/lib/value-objects/task-status.enum.spec.ts(1 hunks)libs/tasks/domain/src/lib/value-objects/task-status.enum.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
libs/tasks/domain/src/lib/value-objects/task-status.enum.spec.ts
[error] 18-18: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (2)
libs/tasks/domain/src/index.ts (1)
4-4: LGTM! Export follows consistent pattern
The new export follows the established barrel file pattern and maintains a logical grouping with other domain exports.
libs/tasks/domain/src/lib/value-objects/task-status.enum.ts (1)
1-5: LGTM! Well-implemented immutable enum pattern
The implementation uses TypeScript's const assertion pattern correctly, ensuring type safety and immutability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
libs/tasks/domain/src/lib/value-objects/task-status.enum.spec.ts (2)
4-14: Consider using test.each for more maintainable testsWhile the current tests are correct, they could be more maintainable using Jest's test.each pattern. This would reduce code duplication and make it easier to add new status values in the future.
Here's a suggested refactor:
- it('should have TODO status', () => { - expect(TaskStatusEnum.TODO).toBe('TODO'); - }); - - it('should have IN_PROGRESS status', () => { - expect(TaskStatusEnum.IN_PROGRESS).toBe('IN_PROGRESS'); - }); - - it('should have DONE status', () => { - expect(TaskStatusEnum.DONE).toBe('DONE'); - }); + test.each([ + ['TODO'], + ['IN_PROGRESS'], + ['DONE'], + ])('should have %s status', (status) => { + expect(TaskStatusEnum[status]).toBe(status); + });
3-20: Consider adding additional test cases for better coverageThe current test suite covers the basic functionality, but consider adding these test cases for more comprehensive coverage:
- Type checking to ensure only valid enum values are assignable
- Verification that the enum doesn't contain any unexpected values
Would you like me to provide example implementations for these additional test cases?
🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
libs/tasks/domain/src/lib/value-objects/task-status.enum.spec.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
libs/tasks/domain/src/lib/value-objects/task-status.enum.spec.ts
[error] 18-18: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (3)
libs/tasks/domain/src/lib/value-objects/task-status.enum.spec.ts (3)
1-1: LGTM!
The import statement correctly imports the enum under test.
3-3: LGTM!
The test suite is well-structured with a clear and descriptive name.
16-19:
Fix inconsistent enum naming
There's an inconsistency in the enum naming. The import uses TaskStatusEnum but the test uses TaskStatus.
Apply this fix:
it('should be immutable', () => {
// @ts-expect-error - Testing immutability
- expect(() => TaskStatus.TODO = 'SOMETHING_ELSE').toThrow();
+ expect(() => TaskStatusEnum.TODO = 'SOMETHING_ELSE').toThrow();
});Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
|
🎉 This PR is included in version 1.16.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Related #99
Summary by CodeRabbit
New Features
TODO,IN_PROGRESS, andDONE.task-statusenumeration.Tests
TaskStatusenum.