Conversation
Feat/attendence fix
|
@Methexx is attempting to deploy a commit to the dizzpy's projects team on Vercel, but is not a member of this team. To resolve this issue, you can:
To read more about collaboration on Vercel, click here. |
There was a problem hiding this comment.
Pull request overview
This pull request implements attendance management features and fixes timezone-related bugs in the attendance system. The changes include removing QR code functionality, implementing a new members management page with check-in/check-out capabilities, updating the attendance monitoring dashboard to use real API data, and standardizing date handling to use UTC across backend and frontend.
Changes:
- Fixed timezone mismatch bugs by standardizing all date calculations to use UTC on both backend and frontend
- Removed QR code-based attendance functionality in favor of manual admin check-in/check-out
- Added new members management page with user listing, search, pagination, and attendance status tracking
- Updated attendance monitoring dashboard to fetch and display real-time data from the backend
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/lib/api/users.ts | New API module for fetching user data with pagination and search |
| frontend/lib/api/index.ts | Added exports for users and attendance API modules |
| frontend/lib/api/attendance.ts | New comprehensive attendance API with data transformation and stats calculation |
| frontend/app/admin/members/page.tsx | Complete members management page with check-in/check-out functionality |
| frontend/app/admin/attendance/page.tsx | Updated to use real API data instead of mock data with auto-refresh |
| backend/src/routes/attendanceRoutes.js | Removed QR code routes that are no longer needed |
| backend/src/models/Attendance.js | Updated getTodayAttendance to use UTC date ranges consistently |
| backend/src/controllers/attendanceController.js | Added getTodayUTC helper and updated all date calculations to use UTC |
| TIMEZONE_FIX_SUMMARY.md | Documentation explaining the timezone bug and fix |
| ATTENDANCE_CODE_ANALYSIS.md | Detailed code walkthrough of the bug chain |
| ATTENDANCE_BUG_ANALYSIS.md | Root cause analysis of the attendance status bug |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| console.log('✅ [Attendance API] Mapped records:', mappedData); | ||
|
|
||
| return { | ||
| ...response, | ||
| data: mappedData, | ||
| success: true, | ||
| }; | ||
| } | ||
|
|
||
| console.log('❌ [Attendance API] No data returned:', response); |
There was a problem hiding this comment.
Production code should not include console.log statements. These debug logs on lines 62, 73, 82, 118, and 127 should be removed before merging to production, or replaced with a proper logging framework that can be configured per environment.
| console.log('✅ [Attendance API] Mapped records:', mappedData); | |
| return { | |
| ...response, | |
| data: mappedData, | |
| success: true, | |
| }; | |
| } | |
| console.log('❌ [Attendance API] No data returned:', response); | |
| return { | |
| ...response, | |
| data: mappedData, | |
| success: true, | |
| }; | |
| } |
| const fetchAttendanceStatus = async (membersList: User[]) => { | ||
| const statusMap: Record<string, 'checked-in' | 'checked-out'> = {}; | ||
| try { | ||
| const { getAllAttendance } = await import('@/lib/api/attendance'); | ||
| const attendanceRes = await getAllAttendance(); | ||
|
|
||
| if (attendanceRes.success && attendanceRes.data) { | ||
| // Get today's date - use UTC to match backend format | ||
| const today = new Date(); | ||
| const todayKey = today.getUTCFullYear() + '-' + | ||
| String(today.getUTCMonth() + 1).padStart(2, '0') + '-' + | ||
| String(today.getUTCDate()).padStart(2, '0'); | ||
|
|
||
| console.log('🔍 [Members] Today key (UTC):', todayKey); | ||
| console.log('🔍 [Members] Total attendance records:', attendanceRes.data.length); | ||
|
|
||
| // Create a map of user statuses from today's attendance | ||
| attendanceRes.data.forEach((record) => { | ||
| console.log('📝 [Members] Record - ID:', record.memberId, 'Date:', record.date, 'Status:', record.status); | ||
| // Exact date match only for today's records | ||
| if (record.date === todayKey) { | ||
| statusMap[record.memberId] = record.status === 'inside' ? 'checked-in' : 'checked-out'; | ||
| } | ||
| }); | ||
| console.log('✅ [Members] Attendance status map from today:', statusMap); | ||
| } | ||
| } catch (err) { | ||
| console.error('❌ [Members] Failed to fetch attendance:', err); | ||
| } | ||
|
|
||
| // Set default status for users not in attendance records | ||
| membersList.forEach((user) => { | ||
| if (!statusMap[user.id]) { | ||
| statusMap[user.id] = 'checked-out'; | ||
| } | ||
| }); | ||
|
|
||
| setAttendanceStatus(statusMap); | ||
| }; |
There was a problem hiding this comment.
The fetchAttendanceStatus function is referenced in the useCallback dependency array on line 55, but it's defined after the useEffect that calls fetchUsers. This creates a situation where fetchAttendanceStatus is not included in the dependencies of fetchUsers, which could lead to stale closures. Consider either moving fetchAttendanceStatus before fetchUsers and including it in the dependencies, or extracting the attendance status logic into a separate hook.
| {openMenuId === user.id && ( | ||
| <div className="absolute right-0 top-full mt-1 bg-[var(--color-background)] border border-[var(--color-outline)] rounded-lg shadow-lg z-10"> | ||
| <button | ||
| onClick={() => handleCheckIn(user.id)} | ||
| disabled={actionLoading === user.id} | ||
| className="w-full px-4 py-2 text-left text-sm hover:bg-[var(--color-surface)] transition-colors disabled:opacity-60 text-green-600 hover:text-green-700 flex items-center gap-2 first:rounded-t-lg" | ||
| > | ||
| {actionLoading === user.id ? ( | ||
| <Loader2 className="w-4 h-4 animate-spin" /> | ||
| ) : ( | ||
| <span>✓</span> | ||
| )} | ||
| Check In | ||
| </button> | ||
| <button | ||
| onClick={() => handleCheckOut(user.id)} | ||
| disabled={actionLoading === user.id} | ||
| className="w-full px-4 py-2 text-left text-sm hover:bg-[var(--color-surface)] transition-colors disabled:opacity-60 text-red-600 hover:text-red-700 flex items-center gap-2 last:rounded-b-lg" | ||
| > | ||
| {actionLoading === user.id ? ( | ||
| <Loader2 className="w-4 h-4 animate-spin" /> | ||
| ) : ( | ||
| <span>✕</span> | ||
| )} | ||
| Check Out | ||
| </button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
The dropdown menu is not dismissed when clicking outside of it. Consider adding an onClick handler to close the menu when clicking outside, or using a proper dropdown component that handles this automatically. This affects user experience as the menu remains open indefinitely.
| // Helper function to get today's date at midnight UTC | ||
| function getTodayUTC() { | ||
| const today = new Date(); | ||
| return new Date(Date.UTC( | ||
| today.getUTCFullYear(), | ||
| today.getUTCMonth(), | ||
| today.getUTCDate(), | ||
| 0, 0, 0, 0 | ||
| )); | ||
| } |
There was a problem hiding this comment.
The getTodayUTC helper function is defined but consider whether this should be a shared utility function rather than defined inline in the controller. If other parts of the application need UTC date handling, extracting this to a utils module would improve code reusability and maintainability.
|
|
||
| records.forEach((record) => { | ||
| const isToday = record.date === todayKey; | ||
| if (record.status === 'inside') { |
There was a problem hiding this comment.
The computeStatsFromRecords function counts all records with status 'inside' for membersInside, regardless of date. This means membersInside could include users from previous days who never checked out. The logic should filter by today's date first before counting members with 'inside' status to accurately reflect who is currently inside today.
| if (record.status === 'inside') { | |
| if (isToday && record.status === 'inside') { |
| console.log('📡 [Attendance] Fetching records...'); | ||
| const recordsRes = await getAllAttendance(); | ||
| console.log('📋 [Attendance] Records response:', recordsRes); | ||
|
|
||
| // Sample data - replace with actual API call | ||
| const stats = { | ||
| membersInside: 34, | ||
| totalCheckIns: 23, | ||
| totalCheckOuts: 12, | ||
| if (recordsRes.success && recordsRes.data) { | ||
| console.log('✅ [Attendance] Setting records:', recordsRes.data); | ||
| setAttendanceRecords(recordsRes.data); | ||
| const computed = computeStatsFromRecords(recordsRes.data); | ||
| console.log('✅ [Attendance] Computed stats:', computed); | ||
| setStats(computed); | ||
| } | ||
| } catch (err) { | ||
| console.error('❌ [Attendance] Error:', err); |
There was a problem hiding this comment.
Production code should not include console.log statements. Debug logs on lines 58, 60, 63, 66, and 70 should be removed or replaced with a proper logging framework.
| useEffect(() => { | ||
| fetchData(); | ||
| // Auto-refresh every 30 seconds | ||
| const interval = setInterval(fetchData, 30000); | ||
| return () => clearInterval(interval); | ||
| }, []); |
There was a problem hiding this comment.
The auto-refresh interval (line 80) is not cleaned up when the component unmounts if fetchData is still in progress. This could lead to state updates on unmounted components. Consider adding an AbortController or a flag to prevent state updates after unmount.
| if (params.page) query.set('page', String(params.page)); | ||
| if (params.limit) query.set('limit', String(params.limit)); | ||
| if (params.q) query.set('q', params.q); | ||
| if (params.status) query.set('status', params.status); |
There was a problem hiding this comment.
The query parameters include falsy values (0 for page, empty strings) which should be handled. Line 34 checks 'if (params.page)' which will exclude page 0 if pagination is 0-indexed, though typically pagination is 1-indexed. However, if params.limit is 0, it would be excluded from the query which might not be the intended behavior. Consider using explicit checks like 'params.page != null' or 'params.page !== undefined' for more precise handling.
| if (params.page) query.set('page', String(params.page)); | |
| if (params.limit) query.set('limit', String(params.limit)); | |
| if (params.q) query.set('q', params.q); | |
| if (params.status) query.set('status', params.status); | |
| if (params.page != null) query.set('page', String(params.page)); | |
| if (params.limit != null) query.set('limit', String(params.limit)); | |
| if (params.q != null) query.set('q', params.q); | |
| if (params.status != null) query.set('status', params.status); |
| export async function checkInMember(memberId: string): Promise<ApiResponse<AttendanceRecord>> { | ||
| return apiClient<AttendanceRecord>('/attendance/admin/checkin', { | ||
| method: 'POST', | ||
| body: JSON.stringify({ userId: memberId }), | ||
| }); | ||
| } | ||
|
|
||
| // Check out a member (admin) | ||
| export async function checkOutMember(memberId: string): Promise<ApiResponse<AttendanceRecord>> { | ||
| return apiClient<AttendanceRecord>('/attendance/admin/checkout', { | ||
| method: 'POST', | ||
| body: JSON.stringify({ userId: memberId }), | ||
| }); | ||
| } |
There was a problem hiding this comment.
The checkInMember and checkOutMember functions return ApiResponse<AttendanceRecord>, but the backend likely returns the raw backend format (not the transformed frontend AttendanceRecord format). This means the response won't match the expected type structure. The functions should either transform the response data like getAllAttendance does, or the return type should reflect the actual backend response structure.
| attendanceSchema.statics.getTodayAttendance = async function (userId) { | ||
| const startOfDay = new Date(); | ||
| startOfDay.setHours(0, 0, 0, 0); | ||
| const endOfDay = new Date(); | ||
| endOfDay.setHours(23, 59, 59, 999); | ||
| // Get today's date in UTC (midnight) | ||
| const today = new Date(); | ||
| const startOfDay = new Date(Date.UTC( | ||
| today.getUTCFullYear(), | ||
| today.getUTCMonth(), | ||
| today.getUTCDate(), | ||
| 0, 0, 0, 0 | ||
| )); | ||
|
|
||
| const endOfDay = new Date(Date.UTC( | ||
| today.getUTCFullYear(), | ||
| today.getUTCMonth(), | ||
| today.getUTCDate(), | ||
| 23, 59, 59, 999 | ||
| )); |
There was a problem hiding this comment.
The getTodayAttendance method duplicates the UTC date calculation logic found in the controller's getTodayUTC helper. Consider creating a shared utility function for UTC date handling to avoid code duplication and ensure consistency across the codebase.
No description provided.