[Improvement] Create task and assign another team member #3353
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes encompass modifications to several files, primarily focusing on enhancing task management functionalities. Key updates include the introduction of a Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 (
|
CREDO23
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (8)
apps/web/app/hooks/features/useTaskInput.ts (1)
155-167: Consider adding assignee validationWhile the members array construction is logically sound, consider adding validation for:
- Duplicate member IDs in the array
- Maximum number of assignees allowed
Example implementation:
members: [ ...(autoAssignTaskAuth && user?.employee.id ? [{id: user?.employee.id}] : []), + // Filter out duplicates and limit to MAX_ASSIGNEES + ...Array.from(new Set([...taskAssignees.current.map(a => a.id)])) + .slice(0, MAX_ASSIGNEES) + .map(id => ({id})) ]apps/web/app/services/client/api/tasks.ts (2)
Line range hint
110-139: Add input validation and error handling.Several improvements could enhance the robustness of this function:
- Input validation for required fields
- Error handling for API calls
- Cookie validation before use
- Input sanitization for user-provided content
Consider applying these improvements:
export async function createTeamTaskAPI(body: Partial<ICreateTask> & { title: string }) { if (GAUZY_API_BASE_SERVER_URL.value) { const organizationId = getOrganizationIdCookie(); const teamId = getActiveTeamIdCookie(); const tenantId = getTenantIdCookie(); const projectId = getActiveProjectIdCookie(); + // Validate required cookies + if (!organizationId || !teamId || !tenantId || !projectId) { + throw new Error('Missing required organization context'); + } + const title = body.title.trim() || ''; + // Validate title after trim + if (!title) { + throw new Error('Task title is required'); + } const datas: ICreateTask = { description: '', teams: [ { id: teamId } ], tags: [], organizationId, tenantId, projectId, estimate: 0, ...body, title // this must be called after ...body }; - await post('/tasks', datas, { tenantId }); - return getTeamTasksAPI(organizationId, tenantId, projectId, teamId); + try { + await post('/tasks', datas, { tenantId }); + return getTeamTasksAPI(organizationId, tenantId, projectId, teamId); + } catch (error) { + // Log error for monitoring + console.error('Failed to create task:', error); + throw error; + } } return api.post<PaginationResponse<ITeamTask>>('/tasks/team', body); }
Line range hint
110-139: Consider standardizing the return type.The function returns different response types based on
GAUZY_API_BASE_SERVER_URL. This could lead to inconsistent handling in the consuming code.Consider standardizing the return type to
Promise<PaginationResponse<ITeamTask>>for both code paths:if (GAUZY_API_BASE_SERVER_URL.value) { // ... existing code ... await post('/tasks', datas, { tenantId }); - return getTeamTasksAPI(organizationId, tenantId, projectId, teamId); + const response = await getTeamTasksAPI(organizationId, tenantId, projectId, teamId); + return response; } -return api.post<PaginationResponse<ITeamTask>>('/tasks/team', body); +const response = await api.post<PaginationResponse<ITeamTask>>('/tasks/team', body); +return response;apps/web/components/pages/kanban/menu-kanban-card.tsx (2)
204-204: LGTM! Well-structured component export.The export of
TeamMembersSelectcomponent is a good architectural decision that promotes reusability. The implementation aligns well with the PR objective of improving task assignment functionality.Consider creating a dedicated components directory for shared team-related components if more components become reusable in the future.
Line range hint
62-63: Track TODOs in issue tracker.Several menu items have TODO comments with unimplemented functionality:
- Estimate task
- Change parent
- Change relations
- Set as next
- Move to
Would you like me to create GitHub issues to track these TODOs? This will help ensure these features are properly implemented in future iterations.
Also applies to: 108-110, 115-117, 122-124, 129-131
apps/web/lib/features/daily-plan/add-task-estimation-hours-modal.tsx (1)
594-594: Implement issue type selection to replace hardcoded 'Bug' valueThe current implementation hardcodes all new tasks as 'Bug' type, which limits task categorization. This should be replaced with a proper issue type selection mechanism.
Would you like me to help implement the issue type selection feature? I can:
- Add an issue type dropdown component
- Integrate it with the task creation form
- Update the createTask payload to use the selected issue type
apps/web/lib/features/task/task-input.tsx (2)
829-830: Use strict equality operator '===' instead of '=='.For type safety and to avoid unexpected type coercion, it's recommended to use '===' when comparing values in JavaScript/TypeScript.
Apply this diff:
- const authMember = useMemo(() => teamMembers.find(member => member.employee.user?.id == user?.id), [teamMembers, user?.id]) + const authMember = useMemo(() => teamMembers.find(member => member.employee.user?.id === user?.id), [teamMembers, user?.id])
838-838: Update label to 'ASSIGNEES' for clarity in multi-select component.Since users can select multiple assignees, updating the label to 'ASSIGNEES' will provide better context.
Apply this diff:
- <span>{t('common.ASSIGNEE')}</span> + <span>{t('common.ASSIGNEES')}</span>Ensure that the corresponding translation key exists in your localization files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
.vscode/settings.json(1 hunks)apps/web/app/hooks/features/useTaskInput.ts(3 hunks)apps/web/app/hooks/features/useTeamTasks.ts(3 hunks)apps/web/app/services/client/api/tasks.ts(1 hunks)apps/web/components/pages/kanban/menu-kanban-card.tsx(2 hunks)apps/web/lib/features/daily-plan/add-task-estimation-hours-modal.tsx(1 hunks)apps/web/lib/features/task/task-input.tsx(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- .vscode/settings.json
🔇 Additional comments (8)
apps/web/app/hooks/features/useTaskInput.ts (3)
67-67: LGTM: Well-typed state for task assignees
The taskAssignees ref is appropriately typed and initialized for storing assignee IDs.
241-241: LGTM: Consistent exposure of task assignees state
The taskAssignees ref is properly exposed in the hook's return value, maintaining consistency with other task-related refs and enabling the new assignment functionality.
Line range hint 67-241: Verify integration with AssigneesSelect component
The hook changes look good and support the new task assignment functionality. Let's verify the integration with the UI components.
✅ Verification successful
Integration between useTaskInput hook and AssigneesSelect component is properly implemented
The verification confirms that the integration is correctly implemented:
- The
taskAssigneesref is properly defined inuseTaskInputhook - The
AssigneesSelectcomponent is correctly implemented intask-input.tsxand receives the required props - The integration flow works as expected:
- Hook maintains assignees state via
taskAssigneesref - Component receives and updates the assignees through the ref
- Task creation includes assignees in the payload
- Hook maintains assignees state via
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration between useTaskInput hook and AssigneesSelect component
# Check for AssigneesSelect component usage
echo "Checking for AssigneesSelect component usage..."
rg -l "AssigneesSelect.*taskAssignees"
# Check for taskAssignees usage in components
echo "Checking for taskAssignees usage in components..."
rg "useTaskInput.*taskAssignees"
Length of output: 331
Script:
#!/bin/bash
# Let's examine the task-input.tsx file to verify the integration
echo "Checking task-input.tsx content..."
cat "apps/web/lib/features/task/task-input.tsx"
# Let's also check for any AssigneesSelect component definition
echo -e "\nChecking for AssigneesSelect component definition..."
rg -l "AssigneesSelect"
# Let's look for any other files that might be using taskAssignees
echo -e "\nChecking for other taskAssignees usages..."
rg "taskAssignees" -A 3 -B 3
Length of output: 31189
apps/web/app/services/client/api/tasks.ts (2)
110-110: LGTM! Function signature simplified.
The removal of the user parameter aligns with the PR objective of allowing task assignment to other team members during creation.
Line range hint 110-139: Verify the impact of these changes on task assignment.
The removal of the user parameter suggests that task assignment is now handled differently. Let's verify this change doesn't break existing functionality.
✅ Verification successful
Task assignment is handled through team association, no issues found
Based on the code analysis:
- Tasks are now created with team association (
teams: [{ id: teamId }]) rather than individual user assignment - Task members are managed through the team membership as shown in the components (
task?.members) - The UI components properly handle task assignees display and member management
- The removal of the
userparameter is consistent with this team-based approach
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the task creation and assignment flow
# Check for any references to the old createTeamTaskAPI signature
echo "Checking for old function signature usage..."
rg -l "createTeamTaskAPI.*user.*\)"
# Check for task assignment handling in related components
echo "Checking task assignment handling..."
rg -l "taskAssignees|members.*task"
# Check for any potential breaking changes in test files
echo "Checking test files..."
fd -e test.ts -e spec.ts | xargs rg "createTeamTaskAPI"
Length of output: 1425
Script:
#!/bin/bash
# Let's analyze the task creation and assignment implementation
# Check the implementation details of task creation
echo "Checking task creation implementation..."
ast-grep --pattern 'export async function createTeamTaskAPI($$$)'
# Check for task assignees handling in the tasks API
echo "Checking task assignees handling..."
rg -A 5 "taskAssignees|assignedTo" apps/web/app/services/client/api/tasks.ts
# Check for task member assignment in related components
echo "Checking task member assignment in components..."
rg -A 5 "members.*task|assignees.*task" apps/web/lib/features/task/
Length of output: 2953
Script:
#!/bin/bash
# Let's check how tasks are updated and how members are assigned
# Check for task update endpoints and member assignment
echo "Checking task update implementations..."
rg -A 10 "updateTask|assignTask|addMember.*task" apps/web/app/services/client/api/tasks.ts
# Check the task interface to understand member structure
echo "Checking task interface definition..."
ast-grep --pattern 'interface ITask {
$$$
}'
# Look for task member assignment mutations
echo "Checking task member assignment mutations..."
rg -A 5 "mutation.*task.*member|mutation.*assign.*task" apps/web/app/services/
Length of output: 1178
apps/web/app/hooks/features/useTeamTasks.ts (2)
Line range hint 292-312: LGTM! Function signature changes align with requirements.
The changes to the createTask function signature effectively support task assignment to other team members while improving naming consistency:
- Renamed
taskNametotitlefor better consistency - Added
membersparameter to support team member assignment - Made
projectIdoptional for flexibility
Line range hint 315-329: Verify API endpoint handling of members parameter.
The implementation correctly includes the members array in the API call. However, let's verify that the API endpoint properly handles the new parameter.
✅ Verification successful
Let me gather more information about the API endpoint and type definitions.
Based on the gathered information, I can now generate the final response:
API endpoint is properly configured to handle members parameter
The implementation is correct as:
- The
ICreateTaskinterface inapps/web/app/interfaces/ITask.tsproperly defines themembersfield asmembers?: { id: string;[x: string]: any }[] - The
createTeamTaskAPIfunction accepts aPartial<ICreateTask>parameter, which includes the members field - The type definition matches the usage in the code under review
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the API endpoint is prepared to handle the members parameter
# Search for API endpoint definition
rg -A 5 "createTeamTaskAPI|POST.*tasks" --type ts
# Search for related type definitions and interfaces
ast-grep --pattern 'interface $interface {
$$$
members?: { id: string }[];
$$$
}'
Length of output: 1688
Script:
#!/bin/bash
# Search for ICreateTask interface definition and related types
ast-grep --pattern 'interface ICreateTask {
$$$
}'
# Search for API implementation details
rg -A 10 "createTeamTaskAPI" --type ts --no-filename
Length of output: 5018
apps/web/lib/features/daily-plan/add-task-estimation-hours-modal.tsx (1)
594-594: LGTM: Property rename aligns with API contract
The change from taskName to title in the task creation payload aligns with the API contract changes mentioned in the summary.
Let's verify the consistency of this property name change across the codebase:
✅ Verification successful
Property rename is consistent with API contract
The verification confirms that the change from taskName to title is correct. The createTask implementation in useTeamTasks.ts shows that the task creation payload uses title as the property name, and there are no remaining instances of taskName being used in task creation contexts. The change aligns with the existing API contract.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of taskName to ensure complete migration
# Expect: No instances of taskName in task creation/update operations
# Search for taskName in task-related operations
rg "taskName.*task.*create" -A 5 -B 5
# Search for title in task creation to confirm new usage
rg "title.*task.*create" -A 5 -B 5
Length of output: 1769
Script:
#!/bin/bash
# Let's search for taskName and title usage in task-related files more broadly
# First, find task-related files
fd -e ts -e tsx task
# Then search for taskName property in these files
rg "taskName.*:" --type ts --type tsx -A 2 -B 2
# Also search for title property in task contexts
rg "title.*:" --type ts --type tsx -A 2 -B 2
# Search for task creation/update mutations or API calls
ast-grep --pattern 'const createTask = $$$'
ast-grep --pattern 'mutation CreateTask($$$) { $$ }'
Length of output: 16386
evereq
deleted the
3130-improvement-create-task--assign-another-team-member-on-create-new-task
branch
Description
Fix #3130
Type of Change
Checklist
Current screenshots
Loom
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes