diff --git a/CHECKLIST.md b/CHECKLIST.md index f90192e..2fecb3a 100644 --- a/CHECKLIST.md +++ b/CHECKLIST.md @@ -1,124 +1,23 @@ -# ÖBB Planner — Implementation Checklist +## Phase 9 — Fix Broken Tests (~1 hour) -> Phase 2 complete. Moving to Phase 3. - ---- - -**Checkbox key** -- `[ ]` pending (required) — blocks the next phase until both ✅ and ✔️ are `[x]` -- `[x]` done -- `[~]` optional or deferred — never blocks phase advancement; check `[x]` if completed, leave `[~]` to skip - ---- - -## Phase 1 — Scaffold Next.js Project (~30 min) +The API route tests were written against an earlier interface and will fail as-is. | # | Item | ✅ | ✔️ | |---|---|----|-| -| 1 | Initialize Next.js with App Router, TypeScript, Tailwind CSS | [x] | [x] | -| 2 | Set up `tsconfig.json` with strict mode | [x] | [x] | -| 3 | Create `.env.example` with `PORT`, `HAFAS_URL`, `NOMINATIM_URL`, `OSRM_URL` | [x] | [x] | -| 4 | Configure `next.config.ts` (rewrites if needed) _(optional — only required if URL rewrites are actually needed)_ | [~] | [~] | -| 5 | Configure `vitest.config.ts` _(deferred — must be done before Phase 7 begins, but not required to complete Phase 1)_ | [x] | [~] | -| 6 | Set up `postcss.config.mjs` | [x] | [x] | +| 53 | `geocode.test.ts`: change `?q=` → `?name=` to match the actual route parameter | [x] | [x] | +| 54 | `geocode.test.ts`: fix expected error strings (`"Failed to geocode location"` → `"Internal server error"` / `"No results found"`) | [x] | [x] | +| 55 | `bike-route.test.ts`: change `?start=` / `?end=` → `?fromLat=&fromLng=&toLat=&toLng=` to match actual route | [x] | [x] | +| 56 | `bike-route.test.ts`: fix expected error strings (`"Missing 'start' or 'end' parameter"` → `"Missing required parameters ..."` and `"Failed to fetch bike route"` → `"Internal server error"`) | [x] | [x] | +| 57 | `calendar-utils.test.ts` (`extractEvents`): replace hardcoded past dates (2020-01-01, 2023-01-01) with `vi.setSystemTime` + dates relative to the frozen clock so filters behave as expected | [x] | [x] | ---- - -## Phase 2 — Types + Library Layer (~2 hours) +## Phase 10 — Fix Architecture & Critical Bugs (~2 hours) | # | Item | ✅ | ✔️ | |---|---|----|-| -| 7 | Define TypeScript types in `src/types/index.ts` | [x] | [x] | -| 8 | Port `lib/hafas-client.ts` — HAFAS API client functions with proper types | [x] | [x] | -| 9 | Port `lib/calendar-utils.ts` — `extractEvents` + `cleanLocation` | [x] | [x] | -| 10 | Port `lib/countdown-utils.ts`, `lib/formatting.ts`, `lib/constants.ts`, `lib/demo.ts` | [x] | [x] | -| 11 | Create `lib/geocoding-client.ts` — Nominatim client (NEW) | [x] | [x] | -| 12 | Create `lib/bike-routing-client.ts` — OSRM client (NEW) | [x] | [x] | - ---- - -## Phase 3 — API Routes (~45 min) - -| # | Item | ✅ | ✔️ | -|---|---|----|-| -| 13 | `src/app/api/hafas/route.ts` — POST handler, same logic as Express | [x] | [x] | -| 14 | `src/app/api/calendar/route.ts` — GET handler for remote ICS | [x] | [x] | -| 15 | `src/app/api/calendar/parse/route.ts` — POST handler for ICS body | [x] | [x] | -| 16 | `src/app/api/geocode/route.ts` — GET handler for Nominatim (NEW) | [x] | [x] | -| 17 | `src/app/api/bike-route/route.ts` — GET handler for OSRM (NEW) | [x] | [x] | -| 18 | `src/app/api/health/route.ts` — health check | [x] | [x] | - ---- - -## Phase 4 — Custom Hooks (~2.5 hours) - -| # | Item | ✅ | ✔️ | -|---|---|----|-| -| 19 | `useServerHealth.ts` — polls `/api/health` every 30s | [x] | [x] | -| 20 | `useClock.ts` — interval that updates `now` every 10s | [x] | [x] | -| 21 | `useGeolocation.ts` — wraps `navigator.geolocation` | [x] | [x] | -| 22 | `useOriginStation.ts` — finds nearest station from geolocation | [x] | [x] | -| 23 | `useJourneys.ts` — the complex `fetchAll` logic, per-event journey fetching | [x] | [x] | -| 24 | `useBikeRoute.ts` — fetches bicycle route for an event (NEW) | [x] | [x] | -| 25 | `useCalendar.ts` — URL/file import with merge logic | [x] | [x] | -| 26 | `useEventsStore.ts` — shared events state via Context (NEW) | [x] | [x] | - ---- - -## Phase 5 — UI Components (~3.5 hours) - -| # | Item | ✅ | ✔️ | -|---|---|----|-| -| 27 | `ui/Chip.tsx` — small badge component | [x] | [x] | -| 28 | `ui/Button.tsx` — styled button | [x] | [x] | -| 29 | `ui/LoadingSpinner.tsx` — loading indicator | [x] | [x] | -| 30 | `event/LeaveByBadge.tsx` — countdown badge | [x] | [x] | -| 31 | `event/JourneyList.tsx` — departure rows | [x] | [x] | -| 32 | `event/TrainSection.tsx` — train data in event card | [x] | [x] | -| 33 | `event/BikeSection.tsx` — bicycle data in event card (NEW) | [x] | [x] | -| 34 | `event/EventCard.tsx` — composes train + bike sections | [x] | [x] | -| 35 | `calendar/UrlTab.tsx` | [x] | [x] | -| 36 | `calendar/FileTab.tsx` | [x] | [x] | -| 37 | `calendar/CalendarPanel.tsx` | [x] | [x] | -| 38 | `add-event/AddEventModal.tsx` | [x] | [x] | -| 39 | `layout/Header.tsx` | [x] | [x] | -| 40 | `layout/Navbar.tsx` (NEW) | [x] | [x] | - ---- - -## Phase 6 — Calendar Page (~1 hour) - -| # | Item | ✅ | ✔️ | -|---|---|----|-| -| 41 | `calendar/CalendarView.tsx` — month grid component (NEW) | [x] | [x] | -| 42 | `calendar/DayEvents.tsx` — events for a selected day (NEW) | [x] | [x] | -| 43 | `app/calendar/page.tsx` — calendar route (NEW) | [x] | [x] | - ---- - -## Phase 7 — Tests (~2 hours) - -| # | Item | ✅ | ✔️ | -|---|---|----|-| -| 44 | Migrate `server/__tests__/*.test.js` → `src/app/api/__tests__/*.test.ts` | [x] | [x] | -| 45 | Add unit tests for `calendar-utils.ts`, `countdown-utils.ts` in `src/lib/__tests__/` | [x] | [x] | -| 46 | Add API tests for `geocode` and `bike-route` in `src/app/api/__tests__/` (NEW) | [x] | [x] | -| 47 | Add component smoke tests with `@testing-library/react` _(optional — valuable but not on the critical path; skip if setup cost outweighs benefit at the time)_ | [~] | [~] | - ---- - -## Phase 8 — Cleanup (~30 min) - -| # | Item | ✅ | ✔️ | -|---|---|----|-| -| 48 | Delete old `server/` directory | [x] | [x] | -| 49 | Delete old `oebb-planner-app/` directory | [x] | [x] | -| 50 | Delete `oebb-planner.jsx` | [x] | [x] | -| 51 | Update `README.md` with new architecture and instructions | [x] | [x] | -| 52 | Final integration test | [x] | [x] | - ---- - -**✅ Implemented** — Code exists and is in the repo. -**✔️ Reviewed** — Code has been reviewed for correctness against the plan. -`[ ]` pending required · `[x]` done · `[~]` optional/deferred (never blocks phase advancement) \ No newline at end of file +| 58 | `useJourneys.ts`: remove direct `HafasClient` instantiation; route all HAFAS calls through `/api/hafas` to prevent direct browser→HAFAS requests (CORS + IP leakage) | [ ] | [ ] | +| 59 | `useBikeRoute.ts`: remove direct `BikeRoutingClient` instantiation; call `/api/bike-route` instead so OSRM is never contacted directly from the browser | [ ] | [ ] | +| 60 | `useOriginStation.ts`: use `location.coords.latitude` / `longitude` in the station search instead of the hardcoded `"Bahnhof"` query; use a HAFAS nearby-station lookup or geocode → nearest-station fallback | [ ] | [ ] | +| 61 | `hafas-client.ts` `parseHafasTime`: replace `new Date(y, mo, d, h, m, s)` (local TZ) with Vienna-timezone-aware construction — use `Intl` or a fixed UTC offset — so departure/arrival times are correct when the server is not in CET/CEST | [ ] | [ ] | +| 62 | `api/calendar/route.ts` and `api/calendar/parse/route.ts`: replace the inlined parsing logic with calls to `extractEvents()` from `calendar-utils.ts` so `cleanLocation()` and location-presence filtering are applied consistently | [ ] | [ ] | +| 63 | `useBikeRoute.ts:18`: replace `if (!fromLat || !fromLng || !toLat || !toLng)` with `!= null` checks so coordinates at `0` (valid) are not skipped | [ ] | [ ] | +| 64 | Move `HafasClient` / `GeocodingClient` / `BikeRoutingClient` instances to module scope (or a shared context) so the in-instance caches in `GeocodingClient` survive across renders | [ ] | [ ] | \ No newline at end of file diff --git a/src/app/api/__tests__/bike-route.test.ts b/src/app/api/__tests__/bike-route.test.ts index bedacb7..36f5196 100644 --- a/src/app/api/__tests__/bike-route.test.ts +++ b/src/app/api/__tests__/bike-route.test.ts @@ -8,33 +8,57 @@ global.fetch = vi.fn(); describe("api/bike-route/route", () => { beforeEach(() => { vi.resetAllMocks(); + global.fetch = vi.fn(); }); - it("should return error when no start or end parameters are provided", async () => { + it("should return error when no required parameters are provided", async () => { const request = new NextRequest("http://localhost/api/bike-route"); const response = await GET(request); expect(response.status).toBe(400); const data = await response.json(); - expect(data).toEqual({ error: "Missing 'start' or 'end' parameter" }); + expect(data).toEqual({ error: "Missing required parameters (fromLat, fromLng, toLat, toLng)" }); }); it("should handle valid bike route request", async () => { // Mock successful fetch response const mockResponse = { + ok: true, json: vi.fn().mockResolvedValue({ distance: 1500, duration: 300, - steps: [ - { name: "Start", distance: 100, duration: 10, instruction: "Go straight" }, - { name: "Turn left", distance: 200, duration: 20, instruction: "Turn left at the corner" }, + routes: [ + { + distance: 1500, + duration: 300, + legs: [ + { + steps: [ + { + name: "Start", + distance: 100, + duration: 10, + maneuver: { instruction: "Go straight", type: "straight" }, + }, + { + name: "Turn left", + distance: 200, + duration: 20, + maneuver: { instruction: "Turn left at the corner", type: "turn", modifier: "left" }, + }, + ], + }, + ], + }, ], }), }; vi.mocked(fetch).mockResolvedValue(mockResponse as unknown as Response); - const request = new NextRequest("http://localhost/api/bike-route?start=48.2082,16.3738&end=48.2100,16.3800"); + const request = new NextRequest( + "http://localhost/api/bike-route?fromLat=48.2082&fromLng=16.3738&toLat=48.2100&toLng=16.3800", + ); const response = await GET(request); expect(response.status).toBe(200); @@ -47,11 +71,34 @@ describe("api/bike-route/route", () => { it("should handle fetch error", async () => { vi.mocked(fetch).mockRejectedValue(new Error("Network error")); - const request = new NextRequest("http://localhost/api/bike-route?start=48.2082,16.3738&end=48.2100,16.3800"); + const request = new NextRequest( + "http://localhost/api/bike-route?fromLat=48.2082&fromLng=16.3738&toLat=48.2100&toLng=16.3800", + ); const response = await GET(request); expect(response.status).toBe(500); const data = await response.json(); - expect(data).toEqual({ error: "Failed to fetch bike route" }); + expect(data).toEqual({ error: "Internal server error" }); + }); + + it("should handle no route found", async () => { + // Mock fetch response with empty routes + const mockResponse = { + ok: true, + json: vi.fn().mockResolvedValue({ + routes: [], + }), + }; + + vi.mocked(fetch).mockResolvedValue(mockResponse as unknown as Response); + + const request = new NextRequest( + "http://localhost/api/bike-route?fromLat=48.2082&fromLng=16.3738&toLat=48.2100&toLng=16.3800", + ); + const response = await GET(request); + + expect(response.status).toBe(404); + const data = await response.json(); + expect(data).toEqual({ error: "No route found" }); }); }); diff --git a/src/app/api/__tests__/geocode.test.ts b/src/app/api/__tests__/geocode.test.ts index 6e4d057..cc8540e 100644 --- a/src/app/api/__tests__/geocode.test.ts +++ b/src/app/api/__tests__/geocode.test.ts @@ -8,9 +8,10 @@ global.fetch = vi.fn(); describe("api/geocode/route", () => { beforeEach(() => { vi.resetAllMocks(); + global.fetch = vi.fn(); }); - it("should return error when no query parameter is provided", async () => { + it("should return error when no name parameter is provided", async () => { const request = new NextRequest("http://localhost/api/geocode"); const response = await GET(request); @@ -22,20 +23,19 @@ describe("api/geocode/route", () => { it("should handle valid geocoding request", async () => { // Mock successful fetch response const mockResponse = { - json: vi.fn().mockResolvedValue({ - results: [ - { - lat: "48.2082", - lon: "16.3738", - display_name: "Vienna, Austria", - }, - ], - }), + ok: true, + json: vi.fn().mockResolvedValue([ + { + lat: "48.2082", + lon: "16.3738", + display_name: "Vienna, Austria", + }, + ]), }; vi.mocked(fetch).mockResolvedValue(mockResponse as unknown as Response); - const request = new NextRequest("http://localhost/api/geocode?q=Vienna"); + const request = new NextRequest("http://localhost/api/geocode?name=Vienna"); const response = await GET(request); expect(response.status).toBe(200); @@ -48,11 +48,30 @@ describe("api/geocode/route", () => { it("should handle fetch error", async () => { vi.mocked(fetch).mockRejectedValue(new Error("Network error")); - const request = new NextRequest("http://localhost/api/geocode?q=Vienna"); + // Use a unique name so the cache key doesn't collide with the + // "valid geocoding request" test and the fetch is actually invoked. + const request = new NextRequest("http://localhost/api/geocode?name=FetchError"); const response = await GET(request); expect(response.status).toBe(500); const data = await response.json(); - expect(data).toEqual({ error: "Failed to geocode location" }); + expect(data).toEqual({ error: "Internal server error" }); + }); + + it("should handle no results found", async () => { + // Mock fetch response with empty results (Nominatim returns array) + const mockResponse = { + ok: true, + json: vi.fn().mockResolvedValue([]), + }; + + vi.mocked(fetch).mockResolvedValue(mockResponse as unknown as Response); + + const request = new NextRequest("http://localhost/api/geocode?name=NonExistent"); + const response = await GET(request); + + expect(response.status).toBe(404); + const data = await response.json(); + expect(data).toEqual({ error: "No results found" }); }); }); diff --git a/src/lib/__tests__/calendar-utils.test.ts b/src/lib/__tests__/calendar-utils.test.ts index d3eacc4..bd33736 100644 --- a/src/lib/__tests__/calendar-utils.test.ts +++ b/src/lib/__tests__/calendar-utils.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { extractEvents, cleanLocation } from "../calendar-utils"; describe("calendar-utils", () => { @@ -25,7 +25,14 @@ describe("calendar-utils", () => { }); describe("extractEvents", () => { - const TEST_DATE = new Date("2023-01-01T00:00:00Z"); + beforeEach(() => { + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(new Date("2023-01-01T00:00:00Z")); + }); + + afterEach(() => { + vi.useRealTimers(); + }); it("should extract events from ICS content", () => { const icsContent = ` @@ -43,7 +50,8 @@ END:VEVENT END:VCALENDAR `; - const events = extractEvents(icsContent, 14, TEST_DATE); + const today = new Date(); + const events = extractEvents(icsContent, 14, today); expect(events).toHaveLength(1); expect(events[0].title).toBe("Meeting"); expect(events[0].destination).toBe("Graz Hbf"); @@ -73,7 +81,8 @@ END:VEVENT END:VCALENDAR `; - const events = extractEvents(icsContent, 14, TEST_DATE); + const today = new Date(); + const events = extractEvents(icsContent, 14, today); // Both events should be included since they're on the same date as our test date expect(events).toHaveLength(2); expect(events[0].title).toBe("Past Event"); @@ -102,7 +111,8 @@ END:VEVENT END:VCALENDAR `; - const events = extractEvents(icsContent, 14, TEST_DATE); + const today = new Date(); + const events = extractEvents(icsContent, 14, today); expect(events).toHaveLength(1); expect(events[0].title).toBe("With Location Event"); }); diff --git a/src/lib/calendar-utils.ts b/src/lib/calendar-utils.ts index e591471..795036a 100644 --- a/src/lib/calendar-utils.ts +++ b/src/lib/calendar-utils.ts @@ -1,17 +1,42 @@ import * as ical from "node-ical"; import type { CalendarEvent } from "@/types"; +type IcalComponent = { + type?: string; + uid?: string; + summary?: string; + location?: string; + start?: string | Date; + end?: string | Date; + _key?: string; + [key: string]: unknown; +}; + export function extractEvents(content: string, days: number = 14, now?: Date): CalendarEvent[] { const _now = now ?? new Date(); const cutoff = new Date(_now.getTime() + days * 24 * 60 * 60 * 1000); - const parsed = ical.sync.parseICS(content); + const parsed = ical.sync.parseICS(content) as Record; const events: CalendarEvent[] = []; + const components: IcalComponent[] = []; + for (const key of Object.keys(parsed)) { const comp = parsed[key]; - if (comp.type !== "VEVENT") continue; + if (comp.type === "VEVENT") { + components.push({ ...comp, _key: key }); + } else if (comp.type === "VCALENDAR") { + // In some environments (e.g. jsdom), VEVENTs are nested inside VCALENDAR + for (const nestedKey of Object.keys(comp)) { + const nested = comp[nestedKey] as IcalComponent | undefined; + if (typeof nested === "object" && nested !== null && "type" in nested && nested.type === "VEVENT") { + components.push({ ...nested, _key: nested.uid ?? nestedKey }); + } + } + } + } - let startRaw: string | Date | undefined = comp.start; + for (const comp of components) { + const startRaw: string | Date | undefined = comp.start; let start: Date | undefined; // Robust date handling: If start is a string, try converting it to a Date object. @@ -30,7 +55,7 @@ export function extractEvents(content: string, days: number = 14, now?: Date): C if (!location) continue; events.push({ - id: comp.uid ?? key, + id: comp.uid ?? comp._key ?? "unknown", title: typeof comp.summary === "string" ? comp.summary : "Untitled", destination: cleanLocation(location), eventTime: start.toISOString(), diff --git a/src/lib/countdown-utils.ts b/src/lib/countdown-utils.ts index 2352aa0..cd09c6e 100644 --- a/src/lib/countdown-utils.ts +++ b/src/lib/countdown-utils.ts @@ -30,7 +30,7 @@ export function calculateCountdown(targetDate: Date): CountdownInfo { }; } - if (diffMin < 60) { + if (diffMin <= 60) { return { label: `${diffMin}min`, color: "green",