feat(calendar): unified calendar view with meetings, milestones, and sprints#164
Conversation
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 8 issue(s) (1 critical) (3 warning).
server/app.ts
Calendar feature is well-structured with good test coverage, but the Python script dependency doesn't exist in the repo, and the fetch hook doesn't handle HTTP error responses properly.
- 🔴 unsafe_assumptions (L675): CALENDAR_SCRIPT references
${BASE_REPO}/command_center/calendar_api.pybut this file does not exist in the repository. The route will always fall into the catch branch, returning an error payload. Either the script should be added to the repo, or this dependency should be documented. - 🟡 unsafe_assumptions (L694):
JSON.parse(stdout)on the Python script output has no validation. Malformed or unexpected output will throw and be caught, but valid JSON with an unexpected shape will be passed straight to the client. Consider validating with a Zod schema (consistent with the project'sapi-schemas.tspattern).[fixable]
frontend/src/hooks/useCalendarData.ts
Calendar feature is well-structured with good test coverage, but the Python script dependency doesn't exist in the repo, and the fetch hook doesn't handle HTTP error responses properly.
- 🟡 bugs (L62): The fetch chain calls
r.json()without checkingr.ok. A 400/401 response will be parsed as JSON and stored in state — the resulting object (e.g.{ error: "..." }) has noeventsorsprintsarrays, so the component would render with undefined data. Should checkr.okand reject on error status codes.[fixable]
frontend/src/pages/__tests__/CalendarView.test.tsx
Calendar feature is well-structured with good test coverage, but the Python script dependency doesn't exist in the repo, and the fetch hook doesn't handle HTTP error responses properly.
- 🔵 missing_tests: No test for the Day/Week view toggle (
viewDaysstate change). The toggle buttons are rendered but never exercised in tests. Also no test for the 'Today' button click (handleToday).[fixable] - 🔵 missing_tests: No test for the expanded state of EventCard — clicking a meeting or milestone card should reveal detail section (
.cal-event-detail). This is a key interaction path.[fixable]
frontend/src/pages/CalendarView.tsx
Calendar feature is well-structured with good test coverage, but the Python script dependency doesn't exist in the repo, and the fetch hook doesn't handle HTTP error responses properly.
- 🟡 bugs (L155):
const today = new Date().toISOString().slice(0, 10)is computed once on render outside of state/effect, buthandleTodaycaptures it as a closure. If the component stays mounted across midnight,todaybecomes stale. Minor in practice, but the same value is also used as initial state forbaseDate, which is fine.[fixable] - 🔵 style: CalendarView is 256 lines with multiple sub-components (EventCard, SprintBar) and utility functions inlined. Consider extracting EventCard and SprintBar into separate component files to match the project convention of short, focused files.
[fixable]
frontend/src/styles/global.css
Calendar feature is well-structured with good test coverage, but the Python script dependency doesn't exist in the repo, and the fetch hook doesn't handle HTTP error responses properly.
- 🔵 style: 308 lines of calendar CSS appended to an already 2700-line global.css. The project has separate style files (e.g. desktop.css). Consider a dedicated
calendar.cssto keep files manageable.[fixable]
| // --- Calendar API --- | ||
|
|
||
| const execFileAsync = promisify(execFile); | ||
| const CALENDAR_SCRIPT = join(BASE_REPO, 'command_center', 'calendar_api.py'); |
There was a problem hiding this comment.
🔴 unsafe_assumptions: CALENDAR_SCRIPT references ${BASE_REPO}/command_center/calendar_api.py but this file does not exist in the repository. The route will always fall into the catch branch, returning an error payload. Either the script should be added to the repo, or this dependency should be documented.
| [CALENDAR_SCRIPT, '--date', dateParam, '--days', String(daysParam)], | ||
| { timeout: CALENDAR_TIMEOUT_MS }, | ||
| ); | ||
| const data = JSON.parse(stdout); |
There was a problem hiding this comment.
🟡 unsafe_assumptions: JSON.parse(stdout) on the Python script output has no validation. Malformed or unexpected output will throw and be caught, but valid JSON with an unexpected shape will be passed straight to the client. Consider validating with a Zod schema (consistent with the project's api-schemas.ts pattern). [fixable]
|
|
||
| fetch(`/api/calendar?date=${date}&days=${days}`) | ||
| .then((r) => r.json()) | ||
| .then((result: CalendarData) => { |
There was a problem hiding this comment.
🟡 bugs: The fetch chain calls r.json() without checking r.ok. A 400/401 response will be parsed as JSON and stored in state — the resulting object (e.g. { error: "..." }) has no events or sprints arrays, so the component would render with undefined data. Should check r.ok and reject on error status codes. [fixable]
| const d = addDays(baseDate, i); | ||
| map.set(d, []); | ||
| } | ||
| for (const evt of events) { |
There was a problem hiding this comment.
🟡 bugs: const today = new Date().toISOString().slice(0, 10) is computed once on render outside of state/effect, but handleToday captures it as a closure. If the component stays mounted across midnight, today becomes stale. Minor in practice, but the same value is also used as initial state for baseDate, which is fine. [fixable]
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 7 issue(s) (1 critical) (3 warning).
frontend/src/styles/desktop.css
Solid feature addition with good test coverage and clean structure; main concern is the desktop grid being hardcoded to 7 columns regardless of view mode, and UTC-based date grouping that will misplace events near midnight.
- 🔴 bugs (L468): Desktop grid is hardcoded to
repeat(7, 1fr)but the view supports day mode (viewDays=1). When viewing 1 day on desktop, a single column will render in a 7-column grid, leaving 6 empty columns. The grid-template-columns should be dynamic or useauto-fill.[fixable]
server/app.ts
Solid feature addition with good test coverage and clean structure; main concern is the desktop grid being hardcoded to 7 columns regardless of view mode, and UTC-based date grouping that will misplace events near midnight.
- 🟡 unsafe_assumptions (L675):
CALENDAR_SCRIPTis resolved once at module load usingBASE_REPO(the repo Mitzo points at). IfREPO_PATHdoesn't containcommand_center/calendar_api.py, the route silently returns empty data. TheexistsSyncguard handles this, but there's no way for the frontend to distinguish 'no events today' from 'calendar script is missing' since the error is in an optional field the frontend never displays.[fixable] - 🟡 unsafe_assumptions (L680):
execFileAsync('python3', [CALENDAR_SCRIPT, ...])assumespython3is onPATH. On some systems (Windows, minimal containers) this may not exist. Consider making the Python path configurable or detecting it at startup.[fixable]
frontend/src/pages/CalendarView.tsx
Solid feature addition with good test coverage and clean structure; main concern is the desktop grid being hardcoded to 7 columns regardless of view mode, and UTC-based date grouping that will misplace events near midnight.
- 🟡 bugs (L7):
toLocalDate()converts ISO datetime strings to dates usingnew Date(isoStr).toISOString().slice(0, 10)which returns UTC date. An event at e.g.2026-04-10T23:00:00-04:00is actually2026-04-11in UTC and would be grouped under April 11 instead of April 10 (the user's local date). Should use local date extraction instead.[fixable]
frontend/src/components/EventCard.tsx
Solid feature addition with good test coverage and clean structure; main concern is the desktop grid being hardcoded to 7 columns regardless of view mode, and UTC-based date grouping that will misplace events near midnight.
- 🔵 missing_tests: No unit tests for EventCard or SprintBar components. The CalendarView integration test covers them indirectly, but the EventCard has non-trivial rendering logic (attendee truncation at 5, time formatting, hangout link rendering) that would benefit from dedicated tests.
- 🔵 style (L4): Local
formatTime()shadows the existingformatTimeexport fromlib/formatTime.ts. While they do different things (clock time vs relative time), the name collision may confuse maintainers. Consider naming thisformatClockTimeto distinguish it.[fixable]
frontend/src/hooks/useCalendarData.ts
Solid feature addition with good test coverage and clean structure; main concern is the desktop grid being hardcoded to 7 columns regardless of view mode, and UTC-based date grouping that will misplace events near midnight.
- 🔵 style (L3):
CalendarEventhastype: 'meeting' | 'milestone' | 'sprint'butsprintis never used — sprints are a separateSprintInfotype in a separate array. The'sprint'variant inCalendarEvent.typeis dead code. Also, the Zod schema inapi-schemas.tsonly allowsz.enum(['meeting', 'milestone']), so asprint-typed event would fail server validation.[fixable]
|
|
||
| .cal-body { | ||
| display: grid; | ||
| grid-template-columns: repeat(7, 1fr); |
There was a problem hiding this comment.
🔴 bugs: Desktop grid is hardcoded to repeat(7, 1fr) but the view supports day mode (viewDays=1). When viewing 1 day on desktop, a single column will render in a 7-column grid, leaving 6 empty columns. The grid-template-columns should be dynamic or use auto-fill. [fixable]
| // --- Calendar API --- | ||
|
|
||
| const execFileAsync = promisify(execFile); | ||
| const CALENDAR_SCRIPT = join(BASE_REPO, 'command_center', 'calendar_api.py'); |
There was a problem hiding this comment.
🟡 unsafe_assumptions: CALENDAR_SCRIPT is resolved once at module load using BASE_REPO (the repo Mitzo points at). If REPO_PATH doesn't contain command_center/calendar_api.py, the route silently returns empty data. The existsSync guard handles this, but there's no way for the frontend to distinguish 'no events today' from 'calendar script is missing' since the error is in an optional field the frontend never displays. [fixable]
|
|
||
| app.get('/api/calendar', async (req, res) => { | ||
| const dateParam = (req.query.date as string) || new Date().toISOString().slice(0, 10); | ||
| const daysParam = Math.max(1, Math.min(31, parseInt(req.query.days as string) || 7)); |
There was a problem hiding this comment.
🟡 unsafe_assumptions: execFileAsync('python3', [CALENDAR_SCRIPT, ...]) assumes python3 is on PATH. On some systems (Windows, minimal containers) this may not exist. Consider making the Python path configurable or detecting it at startup. [fixable]
| import { EventCard } from '../components/EventCard'; | ||
| import { SprintBar } from '../components/SprintBar'; | ||
|
|
||
| function toLocalDate(isoStr: string): string { |
There was a problem hiding this comment.
🟡 bugs: toLocalDate() converts ISO datetime strings to dates using new Date(isoStr).toISOString().slice(0, 10) which returns UTC date. An event at e.g. 2026-04-10T23:00:00-04:00 is actually 2026-04-11 in UTC and would be grouped under April 11 instead of April 10 (the user's local date). Should use local date extraction instead. [fixable]
| import { useState } from 'react'; | ||
| import type { CalendarEvent } from '../hooks/useCalendarData'; | ||
|
|
||
| function formatTime(isoStr: string): string { |
There was a problem hiding this comment.
🔵 style: Local formatTime() shadows the existing formatTime export from lib/formatTime.ts. While they do different things (clock time vs relative time), the name collision may confuse maintainers. Consider naming this formatClockTime to distinguish it. [fixable]
| @@ -0,0 +1,100 @@ | |||
| import { useState, useEffect, useMemo } from 'react'; | |||
|
|
|||
| export interface CalendarEvent { | |||
There was a problem hiding this comment.
🔵 style: CalendarEvent has type: 'meeting' | 'milestone' | 'sprint' but sprint is never used — sprints are a separate SprintInfo type in a separate array. The 'sprint' variant in CalendarEvent.type is dead code. Also, the Zod schema in api-schemas.ts only allows z.enum(['meeting', 'milestone']), so a sprint-typed event would fail server validation. [fixable]
Unified calendar page that merges Google Calendar events, release milestones, and sprint boundaries via the mgmt calendar_api.py backend. Mobile-first day/week view with responsive desktop grid layout. - GET /api/calendar endpoint shells out to calendar_api.py - useCalendarData hook with date navigation - CalendarView page with EventCard expand/collapse - Sprint bars, milestone badges, attendee chips - Day/Week toggle, prev/next/today navigation - 17 tests (5 server + 5 hook + 7 page) - Quick action added to .mitzo.json - Route at /calendar behind ProtectedRoute Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add script existence check before exec (was #1 critical) - Add Zod CalendarResponse schema for stdout validation (#2) - Check r.ok before parsing fetch response in hook (#3) - Compute today fresh in handleToday to avoid stale date (#6) - Extract EventCard + SprintBar to separate component files (#7) - Extract calendar CSS to dedicated calendar.css (#8) - Add tests: Day/Week toggle, Today button, EventCard expand, milestone expand, non-ok HTTP response handling (#4, #5) 832 tests passing (5 new). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused imports (beforeEach, writeFileSync, result) - Fix toLocalDate to use local timezone instead of UTC - Remove dead 'sprint' variant from CalendarEvent.type union Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Also add .claude/worktrees/ to .prettierignore to prevent worktree artifacts from failing format checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c4924e7 to
ef5367f
Compare
Summary
/calendarroute with a unified calendar view merging Google Calendar events, release milestones (from Smartsheet viacalendar_api.py), and sprint boundariesGET /api/calendarendpoint shells out tomgmt/command_center/calendar_api.pywith graceful fallback on failureNew files
server/app.ts(modified)/api/calendarendpoint +execFileasync importfrontend/src/hooks/useCalendarData.tsfrontend/src/pages/CalendarView.tsxfrontend/src/styles/global.css(modified)frontend/src/styles/desktop.css(modified)frontend/src/App.tsx(modified)/calendarroute behind ProtectedRouteTest plan
🤖 Generated with Claude Code