Admin Tasks Frontend #63
Conversation
e1011
force-pushed
the
admin-tasks-frontend
branch
from
02d8e9f to
69baeea
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
| const parseDateString = (dateStr: string): Date => { | ||
| const [day, month, year] = dateStr.split('/').map(Number); | ||
| // Convert 2-digit year to 4-digit (25 -> 2025) | ||
| const fullYear = year < 100 ? 2000 + year : year; | ||
| return new Date(fullYear, month - 1, day); | ||
| }; | ||
|
|
||
| const formatDateString = (date: Date): string => { | ||
| const month = String(date.getMonth() + 1).padStart(2, '0'); | ||
| const day = String(date.getDate()).padStart(2, '0'); | ||
| const year = date.getFullYear(); | ||
| return `${month}/${day}/${year}`; | ||
| }; | ||
|
|
||
| const handleStartDateChange = (date: Date | null) => { | ||
| if (date) { | ||
| onUpdateField(task.id, 'startDate', formatDateString(date)); | ||
| setIsStartDatePickerOpen(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleEndDateChange = (date: Date | null) => { | ||
| if (date) { | ||
| onUpdateField(task.id, 'endDate', formatDateString(date)); | ||
| setIsEndDatePickerOpen(false); |
There was a problem hiding this comment.
The modal formats user selections as MM/DD/YYYY (formatDateString) but the rest of the flow expects the DD/MM/YY strings produced by the backend mapper (formatDateToISO and the list’s parseDateString both split on day/month/year and add 2000 to the year). After a user picks a new date the stored value becomes 03/15/2025; the ISO converter then treats 03 as the day and adds 2000 to 2025, sending year 4025 with month 15 to /tasks/{id}, and the sorting logic similarly mis-orders the entry. The modal should emit the same format as the list or the converters need to accept the new format before updating the task.
Useful? React with 👍 / 👎.
| const updateTaskField = async ( | ||
| taskId: string, | ||
| field: string | number | symbol, | ||
| value: string | boolean, | ||
| ) => { | ||
| const taskToUpdate = tasks.find((t) => t.id === taskId); | ||
| if (!taskToUpdate) return; | ||
|
|
||
| try { | ||
| const updates: Record<string, string | null> = {}; | ||
|
|
||
| if (field === 'priority') { | ||
| updates.priority = mapPriorityToBackend(value as string); | ||
| } else if (field === 'assignee') { | ||
| const admin = admins.find((a) => a.name === value); | ||
| updates.assigneeId = admin?.id || null; | ||
| } else if (field === 'startDate') { | ||
| updates.startDate = formatDateToISO(value as string); | ||
| } else if (field === 'endDate') { | ||
| updates.endDate = formatDateToISO(value as string); | ||
| } |
There was a problem hiding this comment.
When the user toggles the completion checkbox in the edit modal, updateTaskField receives field === 'completed' but the handler only builds payloads for priority, assignee, startDate and endDate. The resulting updates object is empty, so the PUT /tasks/{id} call is a no-op and the task isn’t marked completed on the server; refreshing the page reverts the change even though the UI toggled locally. The update handler should send a status change (e.g. status: 'completed' / 'pending') or call the existing completeTask endpoint when field === 'completed'.
Useful? React with 👍 / 👎.
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
| // Fetch admins and tasks on mount | ||
| useEffect(() => { | ||
| const fetchData = async () => { | ||
| try { | ||
| setLoading(true); | ||
|
|
||
| // Fetch admins | ||
| const adminsResponse = await getAdmins(); | ||
| const mappedAdmins = adminsResponse.users.map((user, index) => mapUserToAdmin(user, index)); | ||
| setAdmins(mappedAdmins); | ||
|
|
||
| // Get the currently logged-in user | ||
| const authenticatedUser = getCurrentUser(); | ||
| if (authenticatedUser) { | ||
| // Find the current user in the admins list by ID | ||
| const loggedInAdmin = mappedAdmins.find((admin) => admin.id === authenticatedUser.id); | ||
| if (loggedInAdmin) { | ||
| setCurrentUser(loggedInAdmin); | ||
| } else { | ||
| // Fallback: If logged-in user not found in admins list (shouldn't happen), use first admin | ||
| console.warn('Logged-in user not found in admins list, using first admin as fallback'); | ||
| if (mappedAdmins.length > 0) { | ||
| setCurrentUser(mappedAdmins[0]); | ||
| } | ||
| } | ||
| } else { | ||
| // No authenticated user found (shouldn't happen with auth guard), use first admin as fallback | ||
| console.warn('No authenticated user found, using first admin as fallback'); | ||
| if (mappedAdmins.length > 0) { | ||
| setCurrentUser(mappedAdmins[0]); | ||
| } | ||
| } | ||
|
|
||
| // Fetch tasks | ||
| const tasksResponse = await taskAPIClient.getTasks(); | ||
|
|
||
| // Fetch participants for tasks | ||
| const participantIds = [ | ||
| ...new Set( | ||
| tasksResponse.tasks | ||
| .filter((t) => t.participantId) | ||
| .map((t) => t.participantId as string), | ||
| ), | ||
| ]; | ||
|
|
||
| const participantMap = new Map<string, UserResponse>(); | ||
| await Promise.all( | ||
| participantIds.map(async (id) => { | ||
| try { | ||
| const participant = await getUserById(id); | ||
| participantMap.set(id, participant); | ||
| } catch (error) { | ||
| console.error(`Error fetching participant ${id}:`, error); | ||
| } | ||
| }), | ||
| ); |
There was a problem hiding this comment.
The initial load fetches the task list and then issues one getUserById request per unique participantId. With even a moderate task volume this becomes an N+1 pattern, multiplying page load latency and server traffic proportionally to the number of tasks. Consider extending the /tasks response to include participant names or adding a bulk lookup so the UI can resolve participants in a single round trip.
Useful? React with 👍 / 👎.
| const handleTaskCheck = async (taskId: string) => { | ||
| try { | ||
| const task = tasks.find((t) => t.id === taskId); | ||
| if (!task) return; | ||
|
|
||
| if (!task.completed) { | ||
| // Mark as completed | ||
| await taskAPIClient.completeTask(taskId); | ||
| } else { | ||
| // Update to pending | ||
| await taskAPIClient.updateTask(taskId, { status: 'pending' }); | ||
| } | ||
|
|
||
| // Update local state | ||
| setTasks(tasks.map((t) => (t.id === taskId ? { ...t, completed: !t.completed } : t))); | ||
| } catch (error) { | ||
| console.error('Error updating task:', error); | ||
| } | ||
| }; | ||
|
|
||
| const openTaskPopup = (task: Task) => { | ||
| setSelectedTask(task); | ||
| setIsPopupOpen(true); | ||
| }; | ||
|
|
||
| const closeTaskPopup = () => { | ||
| setIsPopupOpen(false); | ||
| setSelectedTask(null); | ||
| }; | ||
|
|
||
| const updateTaskField = async ( | ||
| taskId: string, | ||
| field: string | number | symbol, | ||
| value: string | boolean, | ||
| ) => { | ||
| const taskToUpdate = tasks.find((t) => t.id === taskId); | ||
| if (!taskToUpdate) return; | ||
|
|
||
| try { | ||
| // Handle completed field separately using the dedicated endpoints | ||
| if (field === 'completed') { | ||
| if (value === true) { | ||
| // Mark as completed | ||
| await taskAPIClient.completeTask(taskId); | ||
| } else { | ||
| // Update to pending | ||
| await taskAPIClient.updateTask(taskId, { status: 'pending' }); | ||
| } | ||
|
|
||
| // Update local state | ||
| const updatedTask = { ...taskToUpdate, completed: value as boolean }; | ||
| if (selectedTask?.id === taskId) { | ||
| setSelectedTask(updatedTask); | ||
| } | ||
| setTasks(tasks.map((task) => (task.id === taskId ? updatedTask : task))); | ||
| return; | ||
| } | ||
|
|
||
| const updates: Record<string, string | null> = {}; | ||
|
|
||
| if (field === 'priority') { | ||
| updates.priority = mapPriorityToBackend(value as string); | ||
| } else if (field === 'assignee') { | ||
| const admin = admins.find((a) => a.name === value); | ||
| updates.assigneeId = admin?.id || null; | ||
| } else if (field === 'startDate') { | ||
| updates.startDate = formatDateToISO(value as string); | ||
| } else if (field === 'endDate') { | ||
| updates.endDate = formatDateToISO(value as string); | ||
| } | ||
|
|
||
| // Call API | ||
| await taskAPIClient.updateTask(taskId, updates); | ||
|
|
||
| // Update local state | ||
| const updatedTask = { ...taskToUpdate, [field]: value }; | ||
| if (selectedTask?.id === taskId) { | ||
| setSelectedTask(updatedTask); | ||
| } | ||
| setTasks(tasks.map((task) => (task.id === taskId ? updatedTask : task))); |
There was a problem hiding this comment.
Both handleTaskCheck and updateTaskField call setTasks(tasks.map(...)) after awaiting network calls. Because tasks is captured from the render that started the request, any concurrent change (e.g. another row updated or drag‑and‑drop reassignment) will be discarded when these setters run, causing the later mutation to revert earlier changes. Using the functional form setTasks(prev => prev.map(...)) (as the drag handlers already do) keeps updates based on the latest state and prevents lost edits.
Useful? React with 👍 / 👎.
3 participants
Notion ticket link
Admin task page
Implementation description
This implements the complete frontend for the admin task page, which is connected to the backend:
Steps to test
Here's a video:
2025-10-04.02-47-10.mp4
Testing steps:
docker-compose up db. Run the frontend and backend locallyChecklist