Submission: Technical Test for Fullstack Developer#4
Conversation
WalkthroughThe changes in this pull request introduce several new API endpoints and components for user authentication, task management, and dashboard navigation in a Next.js application. Key features include user login and logout functionality, CRUD operations for tasks, and a structured dashboard layout. Additionally, new components for task display, filtering, and creation are implemented, along with middleware for authentication and authorization. The project dependencies are also updated to include libraries for date manipulation and JWT handling. 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: 54
🧹 Outside diff range and nitpick comments (31)
nextjs-task-system/middleware.ts (2)
16-24: Review the role-based access control logicCurrently, the middleware restricts users with the
'adm'role from accessing'/my-tasks', and users with the'user'role from accessing'/dashboard'. If administrators should have access to all routes, consider adjusting the role checks to allow'adm'users to access user-specific routes.Consider modifying the role checks to include administrators:
- if (url.pathname === "/my-tasks" && decodeToken.role !== "user") { + if (url.pathname === "/my-tasks" && !["user", "adm"].includes(decodeToken.role)) {
33-33: Use consistent language in commentsThe comment
'// Rutas protegidas'is in Spanish. For consistency and better readability, consider using English for all comments in the codebase.Apply this diff to update the comment:
- matcher: ["/dashboard/:path*", "/my-tasks"], // Rutas protegidas + matcher: ["/dashboard/:path*", "/my-tasks"], // Protected routesnextjs-task-system/app/ui/tasks/Table.tsx (3)
25-37: Ensure accurate sorting for date fieldsSorting date strings lexicographically may lead to incorrect order if the date formats change. Parsing them into
Dateobjects ensures accurate date comparisons.Update the sorting logic for date fields:
const sorted = [...tasks].sort((a, b) => { + let aValue = a[key]!; + let bValue = b[key]!; + if (key === 'creationDate' || key === 'dueDate') { + aValue = new Date(aValue as string); + bValue = new Date(bValue as string); + } - if (a[key]! < b[key]!) return direction === "asc" ? -1 : 1; - if (a[key]! > b[key]!) return direction === "asc" ? 1 : -1; + if (aValue < bValue) return direction === "asc" ? -1 : 1; + if (aValue > bValue) return direction === "asc" ? 1 : -1; return 0; });
25-37: Define a custom sort order for priority levelsIf 'priority' is a string (e.g., 'High', 'Medium', 'Low'), sorting alphabetically may not reflect the actual priority order.
Map priority levels to numerical values for proper sorting:
+ const priorityOrder = { 'High': 3, 'Medium': 2, 'Low': 1 }; const sorted = [...tasks].sort((a, b) => { + let aValue = a[key]!; + let bValue = b[key]!; + if (key === 'priority') { + aValue = priorityOrder[aValue as string]; + bValue = priorityOrder[bValue as string]; + } // Existing date handling logic // ... - if (a[key]! < b[key]!) return direction === "asc" ? -1 : 1; - if (a[key]! > b[key]!) return direction === "asc" ? 1 : -1; + if (aValue < bValue) return direction === "asc" ? -1 : 1; + if (aValue > bValue) return direction === "asc" ? 1 : -1; return 0; });
102-106: Use a dedicated route for editing tasksNavigating to a creation page for editing tasks can be confusing. Consider using a specific edit route.
Update the edit link:
- href={`/dashboard/tasks/create?id=${task.id}`} + href={`/dashboard/tasks/edit?id=${task.id}`}Ensure that the corresponding route and page are implemented.
nextjs-task-system/app/login/page.tsx (1)
3-9: Consider adding error boundary and loading statesWhile the component is well-structured, consider adding error boundaries and loading states at the page level to handle component-level errors and initial loading gracefully.
+import { ErrorBoundary } from 'react-error-boundary'; +import { Suspense } from 'react'; + export default function LoginPage() { return ( <div className="flex min-h-screen items-center justify-center bg-gray-50 px-4 py-12 sm:px-6 lg:px-8"> - <LoginForm /> + <ErrorBoundary fallback={<div>Something went wrong</div>}> + <Suspense fallback={<div>Loading...</div>}> + <LoginForm /> + </Suspense> + </ErrorBoundary> </div> ); }nextjs-task-system/app/dashboard/tasks/create/page.tsx (1)
14-16: Add TypeScript interface for combinedOptionsThe Form component is receiving untyped props. Consider adding proper TypeScript interfaces.
+interface CombinedOptions { + userOptions: Array<{ value: string; label: string }>; + groupOptions: Array<{ value: string; label: string }>; +} + +interface FormProps { + combinedOptions: CombinedOptions; + onSubmitSuccess: () => void; +}nextjs-task-system/app/dashboard/layout.tsx (1)
3-7: Consider adding resize observer for responsive behaviorThe layout could benefit from dynamic resizing behavior based on viewport changes.
+import { useEffect, useState } from 'react'; + export default function DashboardLayout({ children, }: { children: React.ReactNode; }) { + const [isMobile, setIsMobile] = useState(false); + + useEffect(() => { + const resizeObserver = new ResizeObserver(entries => { + setIsMobile(window.innerWidth < 768); + }); + resizeObserver.observe(document.body); + return () => resizeObserver.disconnect(); + }, []);nextjs-task-system/app/my-tasks/layout.tsx (1)
9-17: Enhance accessibility and semantic structureWhile the layout structure is good, consider these improvements for better accessibility:
- <div className="flex h-screen flex-col"> + <div className="flex h-screen flex-col" role="layout"> <header className="w-full bg-gray-50 shadow-md dark:bg-gray-800"> <NavbarUsers /> </header> - <main className="size-full grow overflow-y-auto bg-white p-7 shadow-sm"> + <main className="size-full grow overflow-y-auto bg-white p-7 shadow-sm" aria-label="Main content"> {children} </main> </div>nextjs-task-system/app/ui/tasks/TaskGroup.tsx (1)
11-22: Optimize performance with memoConsider memoizing the component to prevent unnecessary re-renders when parent components update.
-export function TaskGroup({ title, tasks, onTaskUpdate }: TaskGroupProps) { +export const TaskGroup = React.memo(function TaskGroup( + { title, tasks, onTaskUpdate }: TaskGroupProps +) { return ( <div> <h2 className="mb-4 text-xl font-semibold text-gray-700">{title}</h2> <div className="space-y-4"> {tasks.map((task) => ( <TaskCard key={task.id} task={task} onStatusChange={onTaskUpdate} /> ))} </div> </div> ); -} +});nextjs-task-system/app/api/tasks/users/route.ts (2)
10-17: Optimize task filtering logicThe current implementation could be simplified and made more type-safe:
-const userTasks = tasks.filter((task) => { - const assignedToUser = task.assignedTo.id === userId; - const assignedToGroup = task.assignedTo.id && - groupUsers.some(group => group.id === task.assignedTo.id && group.userIds.includes(userId)) - return assignedToGroup || assignedToUser -}); +const userTasks = tasks.filter((task) => { + if ('userIds' in task.assignedTo) { // TypeScript type guard for UserGroup + return task.assignedTo.userIds.includes(userId); + } + return task.assignedTo.id === userId; // UserInput case +});
1-2: Consider implementing data persistenceThe current implementation uses in-memory data from
@/app/lib/data. For production use, consider implementing proper data persistence.Would you like assistance in implementing a database solution?
nextjs-task-system/app/api/tasks/[taskId]/status/route.ts (1)
22-24: Enhance error handlingThe catch block should log the error and provide more specific error messages when possible.
-} catch (error) { - return NextResponse.json({ error: "Failed to update task" }, { status: 500 }); +} catch (error) { + console.error('Error updating task status:', error); + const message = error instanceof Error ? error.message : "Failed to update task"; + return NextResponse.json({ error: message }, { status: 500 }); }nextjs-task-system/app/lib/definitions.ts (2)
36-41: Add type discrimination for CombinedOptionsImprove type safety by ensuring value type matches the type field.
-export type CombinedOptions = { +export type CombinedOptions = { id: number | string; name: string; - type: 'user' | 'group'; - value: UserInput | UserGroup; -}; +} & ( + | { type: 'user'; value: UserInput } + | { type: 'group'; value: UserGroup } +);
1-41: Add JSDoc documentationConsider adding comprehensive documentation for all types to improve maintainability.
Example:
/** * Represents a user in the system * @property id - Unique identifier for the user * @property name - User's display name * @property email - User's email address * @property password - Hashed password * @property role - User's role (user/admin) */ export type User = { // ... existing properties }nextjs-task-system/package.json (1)
14-19: Lock dependency versions for securityUsing caret versions (
^) can lead to automatic minor version updates that might introduce breaking changes or security vulnerabilities. Consider locking the versions for production deployments.- "date-fns": "^4.1.0", - "jwt-decode": "^4.0.0", - "lucide-react": "^0.468.0", - "next-auth": "^4.24.10", + "date-fns": "4.1.0", + "lucide-react": "0.468.0", + "next-auth": "4.24.10",nextjs-task-system/app/lib/auth.ts (1)
16-23: Improve token verification error handlingThe current implementation silently returns null for any verification error, making it difficult to debug issues or provide appropriate error messages.
export async function verifyToken(token: string): Promise<TokenPayload | null> { try { const { payload } = await jwtVerify(token, JWT_SECRET); return payload as TokenPayload; } catch (error) { console.error('Token verification failed:', error); // Different handling for different error types if (error instanceof jwt.TokenExpiredError) { throw new Error('Token has expired'); } return null; } }nextjs-task-system/app/my-tasks/page.tsx (1)
10-12: Add more descriptive error handlingThe current error message doesn't guide users on how to resolve the authentication issue.
- return <p>Error: No authentication token found.</p>; + return ( + <div className="p-4 text-center"> + <p className="text-red-600 mb-2">Authentication Required</p> + <p>Please log in to view your tasks.</p> + </div> + );nextjs-task-system/app/lib/tasksServices.ts (1)
1-36: Consider implementing request timeout and retry logicNetwork requests could hang or fail temporarily. Adding timeout and retry logic would improve reliability.
Consider using a utility like
axiosor implementing retry logic:import axios from 'axios'; const api = axios.create({ timeout: 5000, // 5 seconds retries: 3, retryDelay: 1000, // 1 second });nextjs-task-system/app/ui/tasks/TaskList.tsx (1)
10-12: Optimize task filtering performanceMultiple array filters could be consolidated into a single pass for better performance with large task lists.
- const pendingTasks = tasks.filter((task) => task.status === "pending"); - const inProgressTasks = tasks.filter((task) => task.status === "in progress"); - const completedTasks = tasks.filter((task) => task.status === "completed"); + const groupedTasks = tasks.reduce((acc, task) => { + acc[task.status].push(task); + return acc; + }, { + pending: [], + 'in progress': [], + completed: [] + });nextjs-task-system/app/dashboard/tasks/search.tsx (1)
21-32: Enhance accessibility and user experienceThe search input could benefit from improved accessibility features and user feedback.
Add ARIA attributes and loading state:
<div className="relative flex flex-1 shrink-0"> <label htmlFor="search" className="sr-only"> Search </label> <input + id="search" + type="search" + role="searchbox" + aria-label={placeholder} onChange={(event) => handleSearch(event.target.value)} className="peer block w-full rounded-md border border-gray-200 py-[9px] pl-10 text-sm outline-2 placeholder:text-gray-500" placeholder={placeholder} defaultValue={searchParams.get("query")?.toString()} + minLength={2} + autoComplete="off" /> + <span className="absolute right-3 top-1/2 -translate-y-1/2"> + {/* Add search icon or loading spinner */} + </span> </div>nextjs-task-system/app/ui/Nav/Navbar.tsx (1)
21-24: Add loading state to logout buttonThe logout button should indicate when the logout process is in progress.
Update the button to show loading state:
<div className="flex md:order-2"> - <Button onClick={handleLogout}>Logout</Button> + <Button + onClick={handleLogout} + disabled={isLoggingOut} + isProcessing={isLoggingOut} + > + {isLoggingOut ? 'Logging out...' : 'Logout'} + </Button> <Navbar.Toggle /> </div>nextjs-task-system/app/dashboard/tasks/page.tsx (2)
32-35: Consider translating or removing non-English commentsThe comment "Búsqueda" should be in English for consistency and maintainability.
- {/* Búsqueda */} + {/* Search */}
18-41: Consider implementing loading states and empty statesThe UI doesn't handle loading or empty states, which could lead to a poor user experience.
Consider adding loading and empty states:
return ( <div className="min-h-screen space-y-6 bg-gray-50 px-4 py-6 dark:bg-gray-900 md:py-8"> {/* ... header ... */} <div className="max-w-md"> <Search placeholder="Search by name or group..." /> </div> <div className="overflow-hidden rounded-lg shadow-lg"> - <TableTask tasks={tasks} /> + {tasks.length === 0 ? ( + <div className="p-4 text-center text-gray-500"> + No tasks found. Create a new task to get started. + </div> + ) : ( + <TableTask tasks={tasks} /> + )} </div> </div> );nextjs-task-system/app/dashboard/Sidenav.tsx (1)
41-46: Add confirmation dialog for logout actionThe logout action happens immediately without confirmation, which could lead to accidental logouts.
Consider adding a confirmation dialog:
- <Sidebar.Item as="div" onClick={handleLogout}> + <Sidebar.Item as="div" onClick={() => { + if (window.confirm('Are you sure you want to logout?')) { + handleLogout(); + } + }}>nextjs-task-system/app/lib/fetchParams.ts (1)
1-74: Consider implementing request timeout and retry logicThe API calls lack timeout handling and retry logic, which could lead to hanging requests or unnecessary failures.
Consider implementing:
- Request timeout using AbortController
- Retry logic for transient failures
- Circuit breaker pattern for repeated failures
Would you like me to provide an example implementation?
nextjs-task-system/app/ui/login/login-form.tsx (1)
49-55: Enhance form accessibilityThe email input field could benefit from additional accessibility attributes and better placeholder text.
<TextInput id="email" type="email" placeholder="name@flowbite.com" name="email" required + aria-describedby="email-hint" + placeholder="Enter your work email" + autoComplete="email" />nextjs-task-system/app/ui/tasks/TaskManager.tsx (2)
38-41: Improve code readability with extracted filter conditionsThe filter conditions are complex and could be more readable if extracted into separate functions.
+ const isTaskAssignedToUser = (task: Task) => + task.assignedTo.id === currentUser; + + const isTaskAssignedToUserGroup = (task: Task) => + userGroups.includes(task.assignedTo.id); + const matchesAssignedTo = - (filters.assignedTo === "mine" && task.assignedTo.id === currentUser) || - (filters.assignedTo === "group" && - userGroups.includes(task.assignedTo.id)) || + (filters.assignedTo === "mine" && isTaskAssignedToUser(task)) || + (filters.assignedTo === "group" && isTaskAssignedToUserGroup(task)) || filters.assignedTo === "";
51-71: Extract filter controls into a separate componentThe filter controls section could be extracted into a separate component to improve maintainability and reusability.
Consider creating a new component
TaskFiltersto handle the filter controls:interface TaskFiltersProps { filters: { priority: string; assignedTo: string }; onFilterChange: (key: string, value: string) => void; } function TaskFilters({ filters, onFilterChange }: TaskFiltersProps) { return ( <div className="mb-6 flex space-x-4"> <select className="rounded border p-2" onChange={(e) => onFilterChange("priority", e.target.value)} value={filters.priority} > <option value="">All Priorities</option> <option value="low">Low</option> <option value="medium">Medium</option> <option value="high">High</option> </select> <select className="rounded border p-2" onChange={(e) => onFilterChange("assignedTo", e.target.value)} value={filters.assignedTo} > <option value="">All Tasks</option> <option value="mine">My Tasks</option> <option value="group">Group Tasks</option> </select> </div> ); }nextjs-task-system/app/ui/tasks/create-form.tsx (1)
91-104: Improve user selection handlingThe current implementation has the following issues:
- Potentially duplicates the assigned user in options
- Lacks organization of users/groups
<Select id="assignedTo" name="assignedTo" defaultValue={taskData?.assignedTo?.id} required > - <option>{taskData?.assignedTo?.name}</option> + <optgroup label="Users"> + {combinedOptions + .filter(option => option.type === 'user') + .map((option) => ( + <option key={option.id} value={option.id}> + {option.name} + </option> + ))} + </optgroup> + <optgroup label="Groups"> + {combinedOptions + .filter(option => option.type === 'group') + .map((option) => ( + <option key={option.id} value={option.id}> + {option.name} + </option> + ))} + </optgroup> </Select>nextjs-task-system/app/ui/tasks/TaskCard.tsx (1)
35-46: Improve comment management featuresThe comment handling implementation needs the following improvements:
- Add comment length validation
- Implement rate limiting
- Add comment editing/deletion capabilities
+const MAX_COMMENT_LENGTH = 500; +const COMMENT_COOLDOWN = 1000; // 1 second +let lastCommentTime = 0; const handleAddComment = async (e: React.FormEvent) => { e.preventDefault(); - if (!comment.trim()) return; + const trimmedComment = comment.trim(); + + if (!trimmedComment) return; + if (trimmedComment.length > MAX_COMMENT_LENGTH) { + console.error(`Comment too long. Maximum length is ${MAX_COMMENT_LENGTH} characters`); + return; + } + + const now = Date.now(); + if (now - lastCommentTime < COMMENT_COOLDOWN) { + console.error('Please wait before adding another comment'); + return; + } + lastCommentTime = now; try { - const { updatedTask } = await updateTaskComments(task.id, comment); + const { updatedTask } = await updateTaskComments(task.id, trimmedComment); onStatusChange(updatedTask); setComment(""); } catch (error) { console.error("Failed to add comment:", error); + // Show user-friendly error message + alert('Failed to add comment. Please try again.'); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
nextjs-task-system/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
nextjs-task-system/app/api/auth/login/route.ts(1 hunks)nextjs-task-system/app/api/auth/logout/route.ts(1 hunks)nextjs-task-system/app/api/tasks/[taskId]/comments/route.ts(1 hunks)nextjs-task-system/app/api/tasks/[taskId]/status/route.ts(1 hunks)nextjs-task-system/app/api/tasks/admin/route.ts(1 hunks)nextjs-task-system/app/api/tasks/users/route.ts(1 hunks)nextjs-task-system/app/dashboard/Sidenav.tsx(1 hunks)nextjs-task-system/app/dashboard/layout.tsx(1 hunks)nextjs-task-system/app/dashboard/page.tsx(1 hunks)nextjs-task-system/app/dashboard/tasks/create/page.tsx(1 hunks)nextjs-task-system/app/dashboard/tasks/page.tsx(1 hunks)nextjs-task-system/app/dashboard/tasks/search.tsx(1 hunks)nextjs-task-system/app/dashboard/users/page.tsx(1 hunks)nextjs-task-system/app/lib/auth.ts(1 hunks)nextjs-task-system/app/lib/data.ts(1 hunks)nextjs-task-system/app/lib/definitions.ts(1 hunks)nextjs-task-system/app/lib/fetchParams.ts(1 hunks)nextjs-task-system/app/lib/fetchUserTasks.ts(1 hunks)nextjs-task-system/app/lib/tasksServices.ts(1 hunks)nextjs-task-system/app/login/page.tsx(1 hunks)nextjs-task-system/app/my-tasks/layout.tsx(1 hunks)nextjs-task-system/app/my-tasks/page.tsx(1 hunks)nextjs-task-system/app/ui/Nav/Navbar.tsx(1 hunks)nextjs-task-system/app/ui/login/login-form.tsx(1 hunks)nextjs-task-system/app/ui/tasks/Table.tsx(1 hunks)nextjs-task-system/app/ui/tasks/TaskCard.tsx(1 hunks)nextjs-task-system/app/ui/tasks/TaskGroup.tsx(1 hunks)nextjs-task-system/app/ui/tasks/TaskList.tsx(1 hunks)nextjs-task-system/app/ui/tasks/TaskManager.tsx(1 hunks)nextjs-task-system/app/ui/tasks/create-form.tsx(1 hunks)nextjs-task-system/middleware.ts(1 hunks)nextjs-task-system/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- nextjs-task-system/app/dashboard/page.tsx
- nextjs-task-system/app/dashboard/users/page.tsx
🔇 Additional comments (2)
nextjs-task-system/app/api/auth/logout/route.ts (1)
1-7: LGTM!
The logout functionality correctly deletes the 'auth-token' cookie and returns a success response.
nextjs-task-system/package.json (1)
16-16: 🛠️ Refactor suggestion
Consolidate authentication dependencies
The project includes both jwt-decode and next-auth. Since next-auth provides comprehensive JWT handling, consider removing jwt-decode to reduce bundle size and potential security vulnerabilities from maintaining multiple auth libraries.
- "jwt-decode": "^4.0.0",Also applies to: 19-19
| } | ||
|
|
||
| try { | ||
| const decodeToken = jwtDecode(token.value); |
There was a problem hiding this comment.
Update the function call to use the correct jwt_decode function
After correcting the import, update the function call to use jwt_decode instead of jwtDecode.
Apply this diff to fix the function call:
- const decodeToken = jwtDecode(token.value);
+ const decodeToken = jwt_decode(token.value);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const decodeToken = jwtDecode(token.value); | |
| const decodeToken = jwt_decode(token.value); |
| @@ -0,0 +1,34 @@ | |||
| import { NextRequest, NextResponse } from "next/server"; | |||
| import { jwtDecode } from "jwt-decode"; | |||
There was a problem hiding this comment.
Correct the import statement for jwtDecode
The jwt-decode library exports a default function. You should import it as import jwt_decode from 'jwt-decode'; instead of destructuring jwtDecode.
Apply this diff to fix the import:
-import { jwtDecode } from "jwt-decode";
+import jwt_decode from "jwt-decode";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { jwtDecode } from "jwt-decode"; | |
| import jwt_decode from "jwt-decode"; |
| const taskId = searchParams.get('id'); | ||
|
|
There was a problem hiding this comment.
Validate taskId before parsing and using
The taskId may be null or invalid. Parsing without validation can lead to unexpected behavior.
Add validation for taskId:
const taskId = searchParams.get('id');
+ if (!taskId || isNaN(parseInt(taskId, 10))) {
+ return NextResponse.json({ message: 'Invalid task ID' }, { status: 400 });
+ }
- const taskIndex = tasks.findIndex(task => task.id === parseInt(taskId));
+ const taskIndex = tasks.findIndex(task => task.id === parseInt(taskId, 10));Also applies to: 57-58
| const taskId = searchParams.get('id'); | ||
|
|
There was a problem hiding this comment.
Ensure taskId is valid before updating a task
As with the DELETE method, validate taskId to prevent errors during task updates.
Add validation for taskId:
const taskId = searchParams.get('id');
+ if (!taskId || isNaN(parseInt(taskId, 10))) {
+ return NextResponse.json({ message: 'Invalid task ID' }, { status: 400 });
+ }
- const taskIndex = tasks.findIndex(task => task.id === parseInt(taskId));
+ const taskIndex = tasks.findIndex(task => task.id === parseInt(taskId, 10));Also applies to: 79-80
| const body = await req.json(); | ||
|
|
||
| let assignedTo = userInputs.find(user => user.id === body.assignedTo); | ||
|
|
||
| if (!assignedTo) { | ||
| assignedTo = groupUsers.find(group => group.id === body.assignedTo); | ||
| } | ||
|
|
||
| const newTask: Task = { | ||
| id: nextId, | ||
| title: body.title, | ||
| description: body.description, | ||
| assignedTo, | ||
| creationDate: new Date().toISOString().split('T')[0], | ||
| dueDate: body.dueDate, | ||
| priority: body.priority, | ||
| status: "pending", | ||
| comments: [], | ||
| }; | ||
| tasks.push(newTask) | ||
| console.log(nextId) | ||
| return NextResponse.json({ message: 'Task created', newTask }, { status: 200 }) |
There was a problem hiding this comment.
Add input validation for task creation
The POST method uses request data without validating it. This can lead to security vulnerabilities or data inconsistencies.
Implement input validation to ensure all required fields are present and valid:
if (!body.title || !body.description || !body.dueDate || !body.priority) {
return NextResponse.json({ message: 'Missing required fields' }, { status: 400 });
}
// Additional validations can be added hereConsider using a validation library like Yup or Joi for robust validation.
| const fetchTask = async () => { | ||
| try { | ||
| const task = tasks.find((task) => task.id === +taskId); | ||
| setTaskData(task); | ||
| } catch (error) { | ||
| setError("Error fetching task"); | ||
| } | ||
| }; | ||
| fetchTask(); | ||
| } |
There was a problem hiding this comment.
Replace client-side task fetching with API call
The current implementation fetches task data from a client-side array, which may not reflect the latest data from the backend. Consider the following improvements:
- Replace with an API call to fetch the latest task data
- Add loading state during initial fetch
- Provide more specific error messages
- Add cleanup for potential race conditions
const fetchTask = async () => {
+ setIsLoading(true);
try {
- const task = tasks.find((task) => task.id === +taskId);
- setTaskData(task);
+ const response = await fetch(`/api/tasks/${taskId}`);
+ if (!response.ok) throw new Error(`HTTP error! status: ${response.status}`);
+ const task = await response.json();
+ setTaskData(task);
} catch (error) {
- setError("Error fetching task");
+ setError(`Failed to fetch task: ${error.message}`);
} finally {
+ setIsLoading(false);
}
};
-fetchTask();
+let mounted = true;
+if (mounted) fetchTask();
+return () => { mounted = false; };Committable suggestion skipped: line range outside the PR's diff.
| async function handleSubmit(event: FormEvent<HTMLFormElement>) { | ||
| event.preventDefault(); | ||
| setIsLoading(true); | ||
| setError(null); | ||
|
|
||
| const formData = new FormData(event.currentTarget); | ||
| const data = Object.fromEntries(formData.entries()); | ||
|
|
||
| try { | ||
| if (taskId) { | ||
| await updateTask(taskId, data); | ||
| } else { | ||
| await createTask(data); | ||
| } | ||
| onSubmitSuccess(); | ||
| } catch (error) { | ||
| setError("Error submitting task"); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance form submission with validation and type safety
The form submission handler could benefit from the following improvements:
- Add input validation before submission
- Implement type safety for form data
- Provide more specific error messages
async function handleSubmit(event: FormEvent<HTMLFormElement>) {
event.preventDefault();
setIsLoading(true);
setError(null);
const formData = new FormData(event.currentTarget);
- const data = Object.fromEntries(formData.entries());
+ const data = {
+ title: formData.get('title') as string,
+ description: formData.get('description') as string,
+ assignedTo: formData.get('assignedTo') as string,
+ dueDate: formData.get('dueDate') as string,
+ priority: formData.get('priority') as string,
+ };
+ // Validate required fields
+ if (!data.title.trim() || !data.assignedTo || !data.dueDate || !data.priority) {
+ setError('Please fill in all required fields');
+ setIsLoading(false);
+ return;
+ }
try {
if (taskId) {
await updateTask(taskId, data);
} else {
await createTask(data);
}
onSubmitSuccess();
} catch (error) {
- setError("Error submitting task");
+ setError(`Failed to ${taskId ? 'update' : 'create'} task: ${error.message}`);
} finally {
setIsLoading(false);
}
}| import React, { useState } from "react"; | ||
| import { Clock, MessageCircle, CheckCircle, Loader } from "lucide-react"; | ||
| import { Task, TaskStatus } from "@/app/lib/definitions"; | ||
| import { updateTaskComments, updateTaskStatus } from "@/app/lib/tasksServices"; | ||
|
|
||
| interface TaskCardProps { | ||
| task: Task; | ||
| onStatusChange: (task: Task) => void; | ||
| } | ||
|
|
||
| const statusIcons = { | ||
| pending: <Clock className="text-yellow-500" />, | ||
| "in progress": <Loader className="text-blue-500" />, | ||
| completed: <CheckCircle className="text-green-500" />, | ||
| }; | ||
|
|
||
| export function TaskCard({ task, onStatusChange }: TaskCardProps) { | ||
| const [comment, setComment] = useState(""); | ||
|
|
||
| const [isCommentFormVisible, setIsCommentFormVisible] = useState(false); | ||
|
|
||
| const handleStatusChange = async () => { | ||
| const statusOrder: TaskStatus[] = ["pending", "in progress", "completed"]; | ||
| const nextStatus = | ||
| statusOrder[(statusOrder.indexOf(task.status) + 1) % statusOrder.length]; | ||
|
|
||
| try { | ||
| const { updatedTask } = await updateTaskStatus(task.id, nextStatus); | ||
| onStatusChange(updatedTask); | ||
| } catch (error) { | ||
| console.error("Failed to update task status:", error); | ||
| } | ||
| }; | ||
|
|
||
| const handleAddComment = async (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| if (!comment.trim()) return; | ||
|
|
||
| try { | ||
| const { updatedTask } = await updateTaskComments(task.id, comment); | ||
| onStatusChange(updatedTask); | ||
| setComment(""); | ||
| } catch (error) { | ||
| console.error("Failed to add comment:", error); | ||
| } | ||
| }; | ||
|
|
||
| const priorityColors = { | ||
| low: "bg-blue-100 text-blue-800", | ||
| medium: "bg-yellow-100 text-yellow-800", | ||
| high: "bg-red-100 text-red-800", | ||
| }; | ||
|
|
||
| return ( | ||
| <div className="relative rounded-lg bg-white p-6 shadow-md transition-shadow hover:shadow-lg"> | ||
| <div className="mb-4 flex items-start justify-between"> | ||
| <h3 className="text-xl font-semibold text-gray-900">{task.title}</h3> | ||
| <button | ||
| onClick={handleStatusChange} | ||
| className="absolute right-4 top-4 flex items-center gap-2 rounded px-3 py-1 text-sm font-medium focus:outline-none focus:ring-2 focus:ring-blue-300" | ||
| disabled={task.status === "completed"} | ||
| > | ||
| {statusIcons[task.status]} | ||
| </button> | ||
| </div> | ||
|
|
||
| <p className="mb-4 text-gray-600">{task.description}</p> | ||
|
|
||
| {/* Fecha límite */} | ||
| <div className="mb-4 flex items-center text-gray-500"> | ||
| <Clock className="mr-2 size-4" /> | ||
| <span>Due: {new Date(task.dueDate).toLocaleDateString()}</span> | ||
| </div> | ||
|
|
||
| {/* Comentarios */} | ||
| <div> | ||
| <div | ||
| className="flex cursor-pointer items-center gap-2 text-gray-500" | ||
| onClick={() => setIsCommentFormVisible(!isCommentFormVisible)} | ||
| > | ||
| <MessageCircle className="size-5" /> | ||
| <span>{task.comments.length} Comments</span> | ||
| </div> | ||
|
|
||
| {/* Formulario condicional */} | ||
| {isCommentFormVisible && ( | ||
| <div className="mt-4"> | ||
| <form onSubmit={handleAddComment} className="flex gap-2"> | ||
| <input | ||
| value={comment} | ||
| onChange={(e) => setComment(e.target.value)} | ||
| placeholder="Add a comment..." | ||
| className="w-full resize-none rounded border border-gray-300 p-1 focus:border-indigo-500 focus:ring focus:ring-indigo-200" | ||
| /> | ||
| </form> | ||
|
|
||
| {/* Lista de comentarios */} | ||
| <ul className="mt-4 space-y-2"> | ||
| {task.comments.map((c, idx) => ( | ||
| <li | ||
| key={idx} | ||
| className="rounded bg-gray-100 p-2 text-gray-800 shadow-sm" | ||
| > | ||
| {c} | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </div> | ||
| )} | ||
| </div> | ||
| <span | ||
| className={`rounded-full px-3 py-1 text-sm font-medium capitalize ${priorityColors[task.priority]}`} | ||
| > | ||
| {task.priority} | ||
| </span> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Implement security measures and proper error handling
The current implementation needs security improvements:
- Sanitize user input
- Implement proper error handling
- Add rate limiting
+import DOMPurify from 'dompurify';
+import { rateLimit } from '@/app/lib/utils';
+const sanitizeInput = (input: string) => DOMPurify.sanitize(input);
+const errorHandler = (error: Error, context: string) => {
+ // Log sanitized error details
+ console.error(`${context}: ${error.message}`);
+ // Return user-friendly message
+ return 'An unexpected error occurred. Please try again.';
+};
export function TaskCard({ task, onStatusChange }: TaskCardProps) {
+ const [error, setError] = useState<string | null>(null);
// ... existing state
const handleStatusChange = async () => {
+ if (!rateLimit('status-change', 1000)) {
+ setError('Please wait before changing status again');
+ return;
+ }
try {
// ... existing code
} catch (error) {
- console.error("Failed to update task status:", error);
+ const message = errorHandler(error, 'Status update failed');
+ setError(message);
}
};
const handleAddComment = async (e: React.FormEvent) => {
e.preventDefault();
- if (!comment.trim()) return;
+ const sanitizedComment = sanitizeInput(comment.trim());
+ if (!sanitizedComment) return;
+
+ if (!rateLimit('add-comment', 1000)) {
+ setError('Please wait before adding another comment');
+ return;
+ }
try {
- const { updatedTask } = await updateTaskComments(task.id, comment);
+ const { updatedTask } = await updateTaskComments(task.id, sanitizedComment);
onStatusChange(updatedTask);
setComment("");
} catch (error) {
- console.error("Failed to add comment:", error);
+ const message = errorHandler(error, 'Comment addition failed');
+ setError(message);
}
};
return (
<div className="relative rounded-lg bg-white p-6 shadow-md">
+ {error && (
+ <div role="alert" className="mb-4 rounded bg-red-100 p-2 text-red-700">
+ {error}
+ </div>
+ )}
// ... existing JSX
</div>
);
}Committable suggestion skipped: line range outside the PR's diff.
| const handleStatusChange = async () => { | ||
| const statusOrder: TaskStatus[] = ["pending", "in progress", "completed"]; | ||
| const nextStatus = | ||
| statusOrder[(statusOrder.indexOf(task.status) + 1) % statusOrder.length]; | ||
|
|
||
| try { | ||
| const { updatedTask } = await updateTaskStatus(task.id, nextStatus); | ||
| onStatusChange(updatedTask); | ||
| } catch (error) { | ||
| console.error("Failed to update task status:", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance task status management
The current status update implementation needs improvements:
- Add status transition validation
- Implement optimistic updates
- Add retry mechanism for failed updates
+const isValidTransition = (current: TaskStatus, next: TaskStatus) => {
+ const transitions = {
+ pending: ['in progress'],
+ 'in progress': ['completed', 'pending'],
+ completed: [],
+ };
+ return transitions[current]?.includes(next) ?? false;
+};
const handleStatusChange = async () => {
const statusOrder: TaskStatus[] = ["pending", "in progress", "completed"];
const nextStatus =
statusOrder[(statusOrder.indexOf(task.status) + 1) % statusOrder.length];
+ if (!isValidTransition(task.status, nextStatus)) {
+ console.error(`Invalid status transition: ${task.status} -> ${nextStatus}`);
+ return;
+ }
+ const originalTask = { ...task };
+ // Optimistic update
+ onStatusChange({ ...task, status: nextStatus });
try {
const { updatedTask } = await updateTaskStatus(task.id, nextStatus);
onStatusChange(updatedTask);
} catch (error) {
console.error("Failed to update task status:", error);
+ // Revert optimistic update
+ onStatusChange(originalTask);
+ // Implement retry mechanism
+ if (error.status !== 400) {
+ setTimeout(() => handleStatusChange(), 5000);
+ }
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleStatusChange = async () => { | |
| const statusOrder: TaskStatus[] = ["pending", "in progress", "completed"]; | |
| const nextStatus = | |
| statusOrder[(statusOrder.indexOf(task.status) + 1) % statusOrder.length]; | |
| try { | |
| const { updatedTask } = await updateTaskStatus(task.id, nextStatus); | |
| onStatusChange(updatedTask); | |
| } catch (error) { | |
| console.error("Failed to update task status:", error); | |
| } | |
| }; | |
| const isValidTransition = (current: TaskStatus, next: TaskStatus) => { | |
| const transitions = { | |
| pending: ['in progress'], | |
| 'in progress': ['completed', 'pending'], | |
| completed: [], | |
| }; | |
| return transitions[current]?.includes(next) ?? false; | |
| }; | |
| const handleStatusChange = async () => { | |
| const statusOrder: TaskStatus[] = ["pending", "in progress", "completed"]; | |
| const nextStatus = | |
| statusOrder[(statusOrder.indexOf(task.status) + 1) % statusOrder.length]; | |
| if (!isValidTransition(task.status, nextStatus)) { | |
| console.error(`Invalid status transition: ${task.status} -> ${nextStatus}`); | |
| return; | |
| } | |
| const originalTask = { ...task }; | |
| // Optimistic update | |
| onStatusChange({ ...task, status: nextStatus }); | |
| try { | |
| const { updatedTask } = await updateTaskStatus(task.id, nextStatus); | |
| onStatusChange(updatedTask); | |
| } catch (error) { | |
| console.error("Failed to update task status:", error); | |
| // Revert optimistic update | |
| onStatusChange(originalTask); | |
| // Implement retry mechanism | |
| if (error.status !== 400) { | |
| setTimeout(() => handleStatusChange(), 5000); | |
| } | |
| } | |
| }; |
| return ( | ||
| <div className="relative rounded-lg bg-white p-6 shadow-md transition-shadow hover:shadow-lg"> | ||
| <div className="mb-4 flex items-start justify-between"> | ||
| <h3 className="text-xl font-semibold text-gray-900">{task.title}</h3> | ||
| <button | ||
| onClick={handleStatusChange} | ||
| className="absolute right-4 top-4 flex items-center gap-2 rounded px-3 py-1 text-sm font-medium focus:outline-none focus:ring-2 focus:ring-blue-300" | ||
| disabled={task.status === "completed"} | ||
| > | ||
| {statusIcons[task.status]} | ||
| </button> | ||
| </div> | ||
|
|
||
| <p className="mb-4 text-gray-600">{task.description}</p> | ||
|
|
||
| {/* Fecha límite */} | ||
| <div className="mb-4 flex items-center text-gray-500"> | ||
| <Clock className="mr-2 size-4" /> | ||
| <span>Due: {new Date(task.dueDate).toLocaleDateString()}</span> | ||
| </div> | ||
|
|
||
| {/* Comentarios */} | ||
| <div> | ||
| <div | ||
| className="flex cursor-pointer items-center gap-2 text-gray-500" | ||
| onClick={() => setIsCommentFormVisible(!isCommentFormVisible)} | ||
| > | ||
| <MessageCircle className="size-5" /> | ||
| <span>{task.comments.length} Comments</span> | ||
| </div> | ||
|
|
||
| {/* Formulario condicional */} | ||
| {isCommentFormVisible && ( | ||
| <div className="mt-4"> | ||
| <form onSubmit={handleAddComment} className="flex gap-2"> | ||
| <input | ||
| value={comment} | ||
| onChange={(e) => setComment(e.target.value)} | ||
| placeholder="Add a comment..." | ||
| className="w-full resize-none rounded border border-gray-300 p-1 focus:border-indigo-500 focus:ring focus:ring-indigo-200" | ||
| /> | ||
| </form> | ||
|
|
||
| {/* Lista de comentarios */} | ||
| <ul className="mt-4 space-y-2"> | ||
| {task.comments.map((c, idx) => ( | ||
| <li | ||
| key={idx} | ||
| className="rounded bg-gray-100 p-2 text-gray-800 shadow-sm" | ||
| > | ||
| {c} | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </div> | ||
| )} | ||
| </div> | ||
| <span | ||
| className={`rounded-full px-3 py-1 text-sm font-medium capitalize ${priorityColors[task.priority]}`} | ||
| > | ||
| {task.priority} | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility and user experience
The UI implementation needs accessibility improvements:
- Add ARIA labels
- Improve keyboard navigation
- Add loading states for async operations
<div className="relative rounded-lg bg-white p-6 shadow-md transition-shadow hover:shadow-lg">
<div className="mb-4 flex items-start justify-between">
- <h3 className="text-xl font-semibold text-gray-900">{task.title}</h3>
+ <h3 className="text-xl font-semibold text-gray-900" id={`task-${task.id}-title`}>
+ {task.title}
+ </h3>
<button
onClick={handleStatusChange}
className="absolute right-4 top-4 flex items-center gap-2 rounded px-3 py-1 text-sm font-medium focus:outline-none focus:ring-2 focus:ring-blue-300"
disabled={task.status === "completed"}
+ aria-label={`Change status from ${task.status}`}
+ aria-describedby={`task-${task.id}-title`}
>
{statusIcons[task.status]}
</button>
</div>
<p className="mb-4 text-gray-600">{task.description}</p>
<div className="mb-4 flex items-center text-gray-500">
<Clock className="mr-2 size-4" />
- <span>Due: {new Date(task.dueDate).toLocaleDateString()}</span>
+ <span role="time" dateTime={task.dueDate}>
+ Due: {new Date(task.dueDate).toLocaleDateString()}
+ </span>
</div>
<div>
<button
- className="flex cursor-pointer items-center gap-2 text-gray-500"
+ className="flex items-center gap-2 text-gray-500"
onClick={() => setIsCommentFormVisible(!isCommentFormVisible)}
+ aria-expanded={isCommentFormVisible}
+ aria-controls={`comments-${task.id}`}
>
<MessageCircle className="size-5" />
<span>{task.comments.length} Comments</span>
</button>
{isCommentFormVisible && (
- <div className="mt-4">
+ <div className="mt-4" id={`comments-${task.id}`}>
<form onSubmit={handleAddComment} className="flex gap-2">
<input
value={comment}
onChange={(e) => setComment(e.target.value)}
placeholder="Add a comment..."
className="w-full resize-none rounded border border-gray-300 p-1 focus:border-indigo-500 focus:ring focus:ring-indigo-200"
+ aria-label="Add a comment"
/>
</form>
- <ul className="mt-4 space-y-2">
+ <ul className="mt-4 space-y-2" aria-label="Comments">
{task.comments.map((c, idx) => (
<li
key={idx}
className="rounded bg-gray-100 p-2 text-gray-800 shadow-sm"
+ tabIndex={0}
>
{c}
</li>
))}
</ul>
</div>
)}
</div>
</div>Committable suggestion skipped: line range outside the PR's diff.
1 participant
Description
This PR implements the required Task Management System for the technical test. The main features developed include:
Task Creation and Assignment:
Admin users can create tasks, assign them to individuals or groups, and define details such as title, description, priority, and due date.
Task Viewing:
Admin users can view all existing tasks, while regular users only have access to tasks assigned to them or their group.
Task Updates:
Users can mark their tasks as completed or in progress, helping track their status effectively.
Responsive User Interface:
Tailwind CSS and Flowbite were used to design a simple, responsive UI optimized for both admin and regular users.
Security:
JWT (JSON Web Tokens) was implemented for user authentication and authorization, ensuring secure access to the system.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores