rewrite phase 9
This commit is contained in:
+15
-116
@@ -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)
|
||||
| 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 | [ ] | [ ] |
|
||||
@@ -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" });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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" });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
|
||||
@@ -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<string, IcalComponent>;
|
||||
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(),
|
||||
|
||||
@@ -30,7 +30,7 @@ export function calculateCountdown(targetDate: Date): CountdownInfo {
|
||||
};
|
||||
}
|
||||
|
||||
if (diffMin < 60) {
|
||||
if (diffMin <= 60) {
|
||||
return {
|
||||
label: `${diffMin}min`,
|
||||
color: "green",
|
||||
|
||||
Reference in New Issue
Block a user