fix(regulations-admin): Actor to draft #16356
Conversation
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)
🔇 Additional comments (2)
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
|
![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 (1)
apps/services/regulations-admin-backend/src/app/modules/draft_regulation/draft_regulation.service.ts (1)
258-263: Address the TODO Regarding Ministry InformationThere is a comment indicating that if the user is in delegation from a valid ministry, the ministry should be added to the creation data. Please confirm if this needs to be implemented.
Would you like assistance in implementing this requirement or opening a new GitHub issue to track this task?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- apps/services/regulations-admin-backend/src/app/modules/draft_regulation/draft_regulation.service.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/services/regulations-admin-backend/src/app/modules/draft_regulation/draft_regulation.service.ts (2)
Pattern
apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (2)
apps/services/regulations-admin-backend/src/app/modules/draft_regulation/draft_regulation.service.ts (2)
31-31: Import Statement Updated AppropriatelyThe addition of
MinistrySlugto the imports ensures that all necessary types are available.
271-271: Authors Field Now Reflects the Creator's National IDThe
authorsfield is correctly set to include thecreatorNationalId, ensuring that the author information is accurate.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
apps/services/regulations-admin-backend/src/app/modules/draft_regulation/draft_regulation.service.ts (2)
262-262: Address the TODO about adding ministry informationThere's a comment indicating that if the user is in delegation from a valid ministry, the ministry should be added to the creation data. Consider implementing this functionality or removing the comment if it's no longer needed.
Do you want me to help implement this functionality or open a GitHub issue to track this task?
258-271: Add unit tests for the new delegation logic in the 'create' methodThe changes in the
createmethod introduce new logic based on the user's delegation status. Please ensure that this new functionality is covered by unit tests to verify correct behavior under different scenarios (e.g., whenuser.actoris defined or not).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- apps/services/regulations-admin-backend/src/app/modules/draft_regulation/draft_regulation.service.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/services/regulations-admin-backend/src/app/modules/draft_regulation/draft_regulation.service.ts (2)
Pattern
apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
...vices/regulations-admin-backend/src/app/modules/draft_regulation/draft_regulation.service.ts
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16356 +/- ##
=======================================
Coverage 36.73% 36.74%
=======================================
Files 6804 6804
Lines 140861 140766 -95
Branches 40118 40042 -76
=======================================
- Hits 51751 51718 -33
+ Misses 89110 89048 -62
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 12 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
|
What
If user is in delegation, use actor. Else use user.
Why
We want the names attached to drafts. Not the ministry they are representing.
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes