fix(reports): correct date parsing and weekly range calculation #340
Conversation
|
Thank you for opening this PR! Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools. Please take a moment to:
More information on how to conduct a self review: This helps make the review process smoother and gives us a clearer understanding of your thought process. Once you've added your self-review, we'll continue from our side. Thank you! |
…tractor#322) - Add parseTaskwarriorDate utility for Taskwarrior date format (YYYYMMDDTHHMMSSZ) - Fix weekly report to use start of week instead of last 7 days - Use end date for completed tasks instead of modified date - Use entry date as fallback for pending tasks without due date - Add count labels on chart bars for better visibility - Refactor isOverdue to use shared date parsing utility Fixes: CCExtractor#322
ShivaGupta-14
force-pushed
the
fix/322-reports-chart-bug
branch
from
660ff59 to
e0922de
Compare
There was a problem hiding this comment.
centralized Taskwarrior date parsing
There was a problem hiding this comment.
refactored isOverdue to use the shared parseTaskwarriorDate utility
| import { ReportsViewProps } from '../../utils/types'; | ||
| import { getStartOfDay } from '../../utils/utils'; | ||
| import { ReportChart } from './ReportChart'; | ||
| import { parseTaskwarriorDate } from '../Tasks/tasks-utils'; |
There was a problem hiding this comment.
using shared parseTaskwarriorDate utility
| return tasks | ||
| .filter((task) => { | ||
| const taskDateStr = task.modified || task.due; | ||
| const taskDateStr = task.end || task.due || task.entry; |
There was a problem hiding this comment.
completed tasks now use end date for filtering and entry as fallback for pending tasks without a due date
| const sevenDaysAgo = getStartOfDay(new Date()); | ||
| sevenDaysAgo.setDate(sevenDaysAgo.getDate() - 7); | ||
| const weeklyData = [{ name: 'This Week', ...countStatuses(sevenDaysAgo) }]; | ||
| const weeklyData = [{ name: 'This Week', ...countStatuses(startOfWeek) }]; |
There was a problem hiding this comment.
fixed weekly report to use startOfWeek, now this ensures "This Week" actually represents the current calendar week, not just the last 7 days
There was a problem hiding this comment.
show values above bars and removed cursor highlight effect (for better UX)
There was a problem hiding this comment.
added tests for parseTaskwarriorDate function
There was a problem hiding this comment.
updated dates, test names, and fallbacks
There was a problem hiding this comment.
updated mock task data to use valid dates
ShivaGupta-14
left a comment
ShivaGupta-14
left a comment
There was a problem hiding this comment.
self review completed! ready for review
| </span> | ||
| <span> | ||
| ongoing: 1 | ||
| ongoing: 0 |
There was a problem hiding this comment.
these should be reverted, as they are opposing the test rules
| </span> | ||
| <span> | ||
| ongoing: 4 | ||
| ongoing: 0 |
There was a problem hiding this comment.
several should've been > 0 as well, as it was before
|
|
||
| if (!dateString) return null; | ||
|
|
||
| const parsed = dateString.replace( |
There was a problem hiding this comment.
regex is mostly hard to read or maintain, shall switch to normal subString methods in js
2 participants
Description
Add parseTaskwarriorDate utility for Taskwarrior date format (YYYYMMDDTHHMMSSZ)
Fix weekly report to use start of week instead of last 7 days
Use end date for completed tasks instead of modified date
Use entry date as fallback for pending tasks without due date
Add count labels on chart bars for better visibility
Refactor isOverdue to use shared date parsing utility
Fixes: Bug: Reports charts always show 0 and weekly range is incorrect #322
Checklist
npx prettier --write .(for formatting)gofmt -w .(for Go backend)npm test(for JS/TS testing)Additional Notes
Video:
Screen.Recording.2025-12-28.at.11.35.29.AM.mov