[Fix] Task not assigned to employee when created by an Employee #8781
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes update the task creation process by enhancing how the current employee is handled. The handler now retrieves the current employee ID along with the user, fetches employee details, and checks whether the employee is already part of the task members. If not, the employee is added to the list. Error handling has been improved to manage retrieval failures. The overall structure of task creation remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant H as TaskCreateHandler
participant E as EmployeeService
C->>H: Send Create Task Request
H->>H: Retrieve current user & employee ID
H->>E: Fetch employee details using employee ID
E-->>H: Return employee details
H->>H: Check if employee exists in members list
alt Employee not in list
H->>H: Add employee to task members
end
H-->>C: Return created task
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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
🧹 Nitpick comments (1)
packages/core/src/lib/tasks/commands/handlers/task-create.handler.ts (1)
63-67: Enhance employee validation and member addition logic.While the implementation fixes the core issue, consider these improvements for better efficiency and robustness:
-const employee = await this._employeeService.findOneByIdString(employeeId); -// Ensure the current employee is added to the members list -if (employee && !members.some((member) => member.id === employeeId)) { - members.push(employee); -} +// Add the current employee to members list if they're not already included +if (employeeId && !members.some((member) => member.id === employeeId)) { + try { + const employee = await this._employeeService.findOneByIdString(employeeId); + if (employee) { + members.push(employee); + } + } catch (error) { + this.logger.warn( + `Failed to add current employee (${employeeId}) to task members:`, + error.message + ); + } +}Changes:
- Check employeeId before database lookup
- Only perform employee lookup if needed
- Add error handling for failed employee lookup
- Improve comment to better describe the business logic
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core/src/lib/tasks/commands/handlers/task-create.handler.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
🔇 Additional comments (1)
packages/core/src/lib/tasks/commands/handlers/task-create.handler.ts (1)
59-62: LGTM! Employee retrieval implementation looks good.The changes correctly retrieve both the current user and employee ID from the request context, which is essential for proper task assignment.
|
View your CI Pipeline Execution ↗ for commit 0574b59.
☁️ Nx Cloud last updated this comment at |
| const user = RequestContext.currentUser(); | ||
| const employeeId = RequestContext.currentEmployeeId(); | ||
|
|
||
| const employee = await this._employeeService.findOneByIdString(employeeId); |
There was a problem hiding this comment.
@samuelmbabhazi What if Super Admin will create task?
Note: It will throw exception if Admin will try to create tasks as If they don't have employee record.
There was a problem hiding this comment.
Now everything is fine,
I just check the existence of employeeId,
if it does not exist we move on to the next instruction, no exception is throw
There was a problem hiding this comment.
@samuelmbabhazi I will review PR soon.
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by CodeRabbit