diff --git a/AGENTS.md b/AGENTS.md index 53c4160..c3e01a7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,7 +5,7 @@ - **Name:** UFOund it - **What it is:** UFOund it, is a webapp that displays UFO sightings. - **Dataset:** [NUFORC UFO Sightings (Kaggle)](https://www.kaggle.com/datasets/NUFORC/ufo-sightings), CSV, approx 80,000 reports, approx 15mb. -- **Current state:** The backend is an Apollo Server with `sightings`, `sighting` and `createSighting`, backed by MySQL on the VM. The search page fetches sightings through Apollo Client, with search, sort and "load more" pagination done in the database. The detail page still looks sightings up in the hand-written mock data in `frontend/src/data/`. A temporary footer shows whether the backend is reachable. +- **Current state:** The backend is an Apollo Server with `sightings`, `sighting` and `createSighting`, backed by MySQL on the VM. The search and detail pages fetch sightings through Apollo Client, with search, sort and "load more" pagination done in the database. A temporary footer shows whether the backend is reachable. - **Course context:** NTNU IT2810 project. Must be reproducible, accessible and sustainable. ## Tech stack @@ -69,7 +69,7 @@ frontend/ hooks/ Custom hooks (debounce, sighting search, single sighting) utils/ Pure helpers (formatting) types/ Shared TypeScript types - data/ Mock sightings, still used by the detail page and as test fixtures + tests/ Vitest tests and the mock sightings they use as fixtures graphql/ Apollo Client setup and pagination cache rules (client.ts), queries.ts and mutations.ts backend/ Node + TypeScript GraphQL server (Apollo Server 5) src/ diff --git a/backend/src/db/sightings.ts b/backend/src/db/sightings.ts index ca259a7..79e71e8 100644 --- a/backend/src/db/sightings.ts +++ b/backend/src/db/sightings.ts @@ -132,6 +132,10 @@ export async function findSightings(args: SightingsArgs) { } export async function findSightingById(id: string) { + // MySQL reads '1abc' as 1 when comparing it with a number column, which + // would show the wrong sighting instead of "not found". + if (!/^\d+$/.test(id)) return null + const [rows] = await pool.query( `${SELECT} ${FROM} WHERE s.UFO_SIGHTING_ID = ?`, [id], diff --git a/frontend/src/data/sighting-store.ts b/frontend/src/data/sighting-store.ts deleted file mode 100644 index 7b8c35b..0000000 --- a/frontend/src/data/sighting-store.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { mockSightings } from './mock-sightings' -import type { UfoSighting } from '../types/ufo-sighting' - -let sightings = [...mockSightings] -const listeners = new Set<() => void>() - -export function getSightings(): UfoSighting[] { - return sightings -} - -export function subscribeToSightings(listener: () => void): () => void { - listeners.add(listener) - return () => listeners.delete(listener) -} - -/** - * Adds a sighting the backend has already stored. The list and detail - * pages still read from this store, so without it a new sighting would - * not show up until those pages fetch from GraphQL. - */ -export function addSighting(sighting: UfoSighting) { - sightings = [sighting, ...sightings] - listeners.forEach((listener) => listener()) -} diff --git a/frontend/src/graphql/queries.ts b/frontend/src/graphql/queries.ts index 74b4121..3d39a4a 100644 --- a/frontend/src/graphql/queries.ts +++ b/frontend/src/graphql/queries.ts @@ -71,3 +71,30 @@ export const SIGHTINGS_QUERY: TypedDocumentNode< } } ` + +interface SightingQueryData { + /** Null if no sighting has the id. */ + sighting: UfoSighting | null +} + +/** A single sighting by id, for the detail page. */ +export const SIGHTING_QUERY: TypedDocumentNode< + SightingQueryData, + { id: string } +> = gql` + query Sighting($id: ID!) { + sighting(id: $id) { + id + datetime + city + state + country + shape + durationSeconds + comments + datePosted + latitude + longitude + } + } +` diff --git a/frontend/src/hooks/use-sighting.ts b/frontend/src/hooks/use-sighting.ts index 16d5fd2..6f83ab8 100644 --- a/frontend/src/hooks/use-sighting.ts +++ b/frontend/src/hooks/use-sighting.ts @@ -1,11 +1,20 @@ -import type { UfoSighting } from '../types/ufo-sighting' -import { useSightings } from './use-sightings' +import { useQuery } from '@apollo/client/react' +import { SIGHTING_QUERY } from '../graphql/queries' /** - * Looks up a single sighting by id. Wrapped in a hook so the detail page - * won't need to change when this becomes a GraphQL query. + * Fetches one sighting. `sighting` is null once the load has finished and + * the backend has no sighting with that id, which is not an `error`. */ -export function useSighting(id: string | undefined): UfoSighting | undefined { - const sightings = useSightings() - return sightings.find((sighting) => sighting.id === id) +export function useSighting(id: string) { + const { data, loading, error, refetch } = useQuery(SIGHTING_QUERY, { + variables: { id }, + }) + + // A failed refetch is shown through `error`, so the rejected promise + // needs no handling of its own. + const retry = () => { + refetch().catch(() => {}) + } + + return { sighting: data?.sighting ?? null, loading, error, retry } } diff --git a/frontend/src/hooks/use-sightings.ts b/frontend/src/hooks/use-sightings.ts deleted file mode 100644 index 96a532c..0000000 --- a/frontend/src/hooks/use-sightings.ts +++ /dev/null @@ -1,6 +0,0 @@ -import { useSyncExternalStore } from 'react' -import { getSightings, subscribeToSightings } from '../data/sighting-store' - -export function useSightings() { - return useSyncExternalStore(subscribeToSightings, getSightings, getSightings) -} diff --git a/frontend/src/pages/SightingDetailPage.tsx b/frontend/src/pages/SightingDetailPage.tsx index bca0d57..93e45c9 100644 --- a/frontend/src/pages/SightingDetailPage.tsx +++ b/frontend/src/pages/SightingDetailPage.tsx @@ -1,3 +1,4 @@ +import { CombinedGraphQLErrors } from '@apollo/client' import { useEffect } from 'react' import { Link, useLocation, useParams } from 'react-router' import SightingMap from '../components/SightingMap' @@ -6,6 +7,7 @@ import { formatDate, formatDuration, formatLocation, + formatShape, } from '../utils/format-sighting' import './SightingDetailPage.css' @@ -27,11 +29,15 @@ function getFromSearch(state: unknown): string { } function SightingDetailPage() { - const { id } = useParams() - const sighting = useSighting(id) + const { id = '' } = useParams() + const { sighting, loading, error, retry } = useSighting(id) const location = useLocation() const backTo = { pathname: '/', search: getFromSearch(location.state) } - const title = sighting ? formatLocation(sighting) : 'Sighting not found' + + let title = 'Sighting not found' + if (sighting) title = formatLocation(sighting) + else if (error) title = 'Could not load sighting' + else if (loading) title = 'Loading sighting…' // Route changes keep the list's scroll position, which would open the // detail page scrolled past its heading. @@ -58,7 +64,22 @@ function SightingDetailPage() {
{backLink}

{title}

-

There is no sighting with id “{id}”.

+ {error ? ( +
+ {CombinedGraphQLErrors.is(error) ? ( +

The server could not load this sighting. Try again later.

+ ) : ( + <> +

Check your connection and try again.

+ + + )} +
+ ) : ( + !loading &&

There is no sighting with id “{id}”.

+ )}
) } @@ -83,7 +104,7 @@ function SightingDetailPage() {
Shape
-
{sighting.shape}
+
{formatShape(sighting.shape)}
Duration
@@ -91,7 +112,9 @@ function SightingDetailPage() {
-

{sighting.comments}

+ {sighting.comments && ( +

{sighting.comments}

+ )} ) } diff --git a/frontend/src/pages/SubmitSightingPage.tsx b/frontend/src/pages/SubmitSightingPage.tsx index 920dfe8..13de282 100644 --- a/frontend/src/pages/SubmitSightingPage.tsx +++ b/frontend/src/pages/SubmitSightingPage.tsx @@ -2,10 +2,8 @@ import { CombinedGraphQLErrors } from '@apollo/client' import { useMutation } from '@apollo/client/react' import { useState, type FormEvent } from 'react' import { Link } from 'react-router' -import { addSighting } from '../data/sighting-store' import { CREATE_SIGHTING_MUTATION, - type CreatedSighting, type SightingFieldError, } from '../graphql/mutations' import LocationPicker from '../components/LocationPicker' @@ -91,24 +89,6 @@ function serverFieldErrors(error: unknown): FormErrors | null { return errors } -// The API allows nulls for imported rows, but a submitted sighting always -// has every field except state, so the fallbacks are only for type safety. -function toUfoSighting(created: CreatedSighting) { - return { - id: created.id, - datetime: created.datetime ?? '', - city: created.city ?? '', - state: created.state ?? '', - country: created.country ?? '', - shape: created.shape ?? '', - durationSeconds: created.durationSeconds ?? 0, - comments: created.comments ?? '', - datePosted: created.datePosted ?? '', - latitude: created.latitude ?? 0, - longitude: created.longitude ?? 0, - } -} - function SubmitSightingPage() { const [values, setValues] = useState(initialValues) const [errors, setErrors] = useState({}) @@ -135,6 +115,9 @@ function SubmitSightingPage() { try { const { data } = await createSighting({ + // The cached lists don't contain the new sighting. Dropping them + // makes the search page fetch fresh results when it is opened. + update: (cache) => cache.evict({ fieldName: 'sightings' }), variables: { input: { // datetime-local gives local time without timezone, which is @@ -152,7 +135,6 @@ function SubmitSightingPage() { }, }) if (!data) throw new Error('createSighting returned no data') - addSighting(toUfoSighting(data.createSighting)) setSubmittedId(data.createSighting.id) } catch (error) { const fieldErrors = serverFieldErrors(error) diff --git a/frontend/src/tests/SearchPage.test.tsx b/frontend/src/tests/SearchPage.test.tsx index 9d32c26..9e4715e 100644 --- a/frontend/src/tests/SearchPage.test.tsx +++ b/frontend/src/tests/SearchPage.test.tsx @@ -4,7 +4,7 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { MemoryRouter } from 'react-router' import { of, throwError } from 'rxjs' import { describe, expect, it, vi } from 'vitest' -import { mockSightings } from '../data/mock-sightings' +import { mockSightings } from './mock-sightings' import { createCache } from '../graphql/client' import type { SightingsQueryVariables } from '../graphql/queries' import SearchPage from '../pages/SearchPage' diff --git a/frontend/src/tests/SightingDetailPage.test.tsx b/frontend/src/tests/SightingDetailPage.test.tsx index 8cffd01..3fcf2a8 100644 --- a/frontend/src/tests/SightingDetailPage.test.tsx +++ b/frontend/src/tests/SightingDetailPage.test.tsx @@ -1,7 +1,14 @@ -import { render, screen } from '@testing-library/react' +import { + MockedProvider, + type MockedProviderProps, +} from '@apollo/client/testing/react' +import { GraphQLError } from 'graphql' +import { fireEvent, render, screen } from '@testing-library/react' import { MemoryRouter, Route, Routes } from 'react-router' import { afterEach, describe, expect, it, vi } from 'vitest' +import { SIGHTING_QUERY } from '../graphql/queries' import SightingDetailPage from '../pages/SightingDetailPage' +import { mockSightings } from './mock-sightings' vi.mock('../components/SightingMap', () => ({ default: ({ @@ -19,13 +26,26 @@ vi.mock('../components/SightingMap', () => ({ ), })) -function renderDetailPage(path: string, state?: unknown) { +const found = { + request: { query: SIGHTING_QUERY, variables: { id: '1' } }, + result: { + data: { sighting: { __typename: 'Sighting', ...mockSightings[0] } }, + }, +} + +function renderDetailPage( + path: string, + mocks: MockedProviderProps['mocks'], + state?: unknown, +) { return render( - - - } /> - - , + + + + } /> + + + , ) } @@ -34,12 +54,15 @@ describe('SightingDetailPage', () => { vi.restoreAllMocks() }) - it('renders sighting details and preserves the originating search link', () => { + it('renders sighting details and preserves the originating search link', async () => { vi.spyOn(window, 'scrollTo').mockImplementation(() => {}) - renderDetailPage('/sightings/1', { fromSearch: '?q=San+Marcos' }) + renderDetailPage('/sightings/1', [found], { fromSearch: '?q=San+Marcos' }) expect( - screen.getByRole('heading', { name: 'San Marcos, TX, US' }), + screen.getByRole('heading', { name: 'Loading sighting…' }), + ).toBeInTheDocument() + expect( + await screen.findByRole('heading', { name: 'San Marcos, TX, US' }), ).toBeInTheDocument() expect(screen.getByText('45 min')).toBeInTheDocument() expect(screen.getByText(/This event took place/)).toBeInTheDocument() @@ -54,12 +77,49 @@ describe('SightingDetailPage', () => { expect(document.title).toBe('San Marcos, TX, US · UFOund it') }) - it('handles missing sightings and links back to the unfiltered list', () => { + it('shows fallbacks for a missing shape and hides missing comments', async () => { vi.spyOn(window, 'scrollTo').mockImplementation(() => {}) - renderDetailPage('/sightings/missing') + const sighting = { + ...found.result.data.sighting, + shape: null, + comments: null, + } + renderDetailPage('/sightings/1', [ + { ...found, result: { data: { sighting } } }, + ]) expect( - screen.getByRole('heading', { name: 'Sighting not found' }), + await screen.findByRole('heading', { name: 'San Marcos, TX, US' }), + ).toBeInTheDocument() + expect(screen.getByText('Unknown')).toBeInTheDocument() + expect(screen.queryByText(/This event took place/)).not.toBeInTheDocument() + }) + + it('does not suggest a retry when the server fails', async () => { + vi.spyOn(window, 'scrollTo').mockImplementation(() => {}) + renderDetailPage('/sightings/1', [ + { ...found, result: { errors: [new GraphQLError('Database down')] } }, + ]) + + expect(await screen.findByRole('alert')).toHaveTextContent( + 'The server could not load this sighting', + ) + expect( + screen.queryByRole('button', { name: 'Try again' }), + ).not.toBeInTheDocument() + }) + + it('handles unknown ids and links back to the unfiltered list', async () => { + vi.spyOn(window, 'scrollTo').mockImplementation(() => {}) + renderDetailPage('/sightings/missing', [ + { + request: { query: SIGHTING_QUERY, variables: { id: 'missing' } }, + result: { data: { sighting: null } }, + }, + ]) + + expect( + await screen.findByRole('heading', { name: 'Sighting not found' }), ).toBeInTheDocument() expect( screen.getByText('There is no sighting with id “missing”.'), @@ -70,4 +130,25 @@ describe('SightingDetailPage', () => { expect(screen.queryByTestId('detail-map')).not.toBeInTheDocument() expect(document.title).toBe('Sighting not found · UFOund it') }) + + it('shows an error and lets the user retry', async () => { + vi.spyOn(window, 'scrollTo').mockImplementation(() => {}) + renderDetailPage('/sightings/1', [ + { ...found, error: new Error('Network down') }, + found, + ]) + + expect(await screen.findByRole('alert')).toHaveTextContent( + 'Check your connection', + ) + expect( + screen.getByRole('heading', { name: 'Could not load sighting' }), + ).toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: 'Try again' })) + + expect( + await screen.findByRole('heading', { name: 'San Marcos, TX, US' }), + ).toBeInTheDocument() + }) }) diff --git a/frontend/src/tests/SightingList.test.tsx b/frontend/src/tests/SightingList.test.tsx index dc64f9d..a72d2fd 100644 --- a/frontend/src/tests/SightingList.test.tsx +++ b/frontend/src/tests/SightingList.test.tsx @@ -3,7 +3,7 @@ import type { ComponentProps } from 'react' import { MemoryRouter } from 'react-router' import { describe, expect, it, vi } from 'vitest' import SightingList from '../components/SightingList' -import { mockSightings } from '../data/mock-sightings' +import { mockSightings } from './mock-sightings' vi.mock('../components/SightingMap', () => ({ default: () =>
, diff --git a/frontend/src/tests/SightingMap.test.tsx b/frontend/src/tests/SightingMap.test.tsx index e502d86..51e2f80 100644 --- a/frontend/src/tests/SightingMap.test.tsx +++ b/frontend/src/tests/SightingMap.test.tsx @@ -2,7 +2,7 @@ import type { ReactNode } from 'react' import { render, screen } from '@testing-library/react' import { describe, expect, it } from 'vitest' import SightingMap from '../components/SightingMap' -import { mockSightings } from '../data/mock-sightings' +import { mockSightings } from './mock-sightings' type MapProps = { children: ReactNode diff --git a/frontend/src/tests/SubmitSightingPage.test.tsx b/frontend/src/tests/SubmitSightingPage.test.tsx index 3c9ba69..4c84245 100644 --- a/frontend/src/tests/SubmitSightingPage.test.tsx +++ b/frontend/src/tests/SubmitSightingPage.test.tsx @@ -2,12 +2,13 @@ import { MockedProvider, type MockedProviderProps, } from '@apollo/client/testing/react' +import { InMemoryCache } from '@apollo/client' import { GraphQLError } from 'graphql' import { fireEvent, render, screen } from '@testing-library/react' import { MemoryRouter } from 'react-router' import { describe, expect, it, vi } from 'vitest' -import { getSightings } from '../data/sighting-store' import { CREATE_SIGHTING_MUTATION } from '../graphql/mutations' +import { SIGHTINGS_QUERY } from '../graphql/queries' import SubmitSightingPage from '../pages/SubmitSightingPage' // Leaflet needs a real browser layout, and the map is not under test here. @@ -42,9 +43,12 @@ const created = { longitude: 10.39, } -function renderPage(mocks: MockedProviderProps['mocks']) { +function renderPage( + mocks: MockedProviderProps['mocks'], + cache?: InMemoryCache, +) { return render( - + @@ -86,7 +90,34 @@ describe('SubmitSightingPage', () => { expect( screen.getByRole('link', { name: 'View your sighting' }), ).toHaveAttribute('href', '/sightings/90001') - expect(getSightings()[0]).toMatchObject({ id: '90001', state: '' }) + }) + + it('drops the cached sighting lists so the new one shows up', async () => { + const cache = new InMemoryCache() + cache.writeQuery({ + query: SIGHTINGS_QUERY, + data: { + sightings: { + edges: [], + pageInfo: { hasNextPage: false, endCursor: null }, + totalCount: 0, + }, + }, + }) + renderPage( + [ + { + request: { query: CREATE_SIGHTING_MUTATION, variables: { input } }, + result: { data: { createSighting: created } }, + }, + ], + cache, + ) + + fillForm() + + await screen.findByRole('heading', { name: 'Sighting submitted' }) + expect(cache.extract().ROOT_QUERY).not.toHaveProperty('sightings') }) it('shows field errors returned by the server next to the field', async () => { diff --git a/frontend/src/tests/format-sighting.test.ts b/frontend/src/tests/format-sighting.test.ts index 10bf2a7..8305525 100644 --- a/frontend/src/tests/format-sighting.test.ts +++ b/frontend/src/tests/format-sighting.test.ts @@ -36,6 +36,7 @@ describe('sighting formatters', () => { it('falls back to readable text when the API returns null', () => { expect(formatDate(null)).toBe('Unknown date') + expect(formatDate('not a date')).toBe('Unknown date') expect(formatDuration(null)).toBe('Unknown') expect(formatShape(null)).toBe('Unknown') expect(formatLocation({ ...sighting, city: null })).toBe( diff --git a/frontend/src/tests/hooks.test.tsx b/frontend/src/tests/hooks.test.tsx index 551b733..5998124 100644 --- a/frontend/src/tests/hooks.test.tsx +++ b/frontend/src/tests/hooks.test.tsx @@ -1,8 +1,6 @@ import { act, renderHook } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' -import { mockSightings } from '../data/mock-sightings' import { useDebouncedValue } from '../hooks/use-debounced-value' -import { useSighting } from '../hooks/use-sighting' describe('sighting hooks', () => { afterEach(() => { @@ -23,17 +21,4 @@ describe('sighting hooks', () => { act(() => vi.advanceTimersByTime(1)) expect(result.current).toBe('second') }) - - it('looks up an existing sighting and returns undefined for an unknown id', () => { - const { result, rerender } = renderHook(({ id }) => useSighting(id), { - initialProps: { id: mockSightings[0].id as string | undefined }, - }) - expect(result.current).toEqual(mockSightings[0]) - - rerender({ id: 'not-a-sighting' }) - expect(result.current).toBeUndefined() - - rerender({ id: undefined }) - expect(result.current).toBeUndefined() - }) }) diff --git a/frontend/src/data/mock-sightings.ts b/frontend/src/tests/mock-sightings.ts similarity index 95% rename from frontend/src/data/mock-sightings.ts rename to frontend/src/tests/mock-sightings.ts index a020365..18200ac 100644 --- a/frontend/src/data/mock-sightings.ts +++ b/frontend/src/tests/mock-sightings.ts @@ -1,10 +1,6 @@ import type { UfoSighting } from '../types/ufo-sighting' -/** - * Hand-written sample sightings. The search page now reads from GraphQL, - * but the detail page still looks sightings up here, and tests use them as - * fixtures. Remove from the app once the detail page uses GraphQL. - */ +/** Hand-written sample sightings, used as fixtures by the tests. */ export const mockSightings: UfoSighting[] = [ { id: '1', diff --git a/frontend/src/utils/format-sighting.ts b/frontend/src/utils/format-sighting.ts index c5a09d9..73fcade 100644 --- a/frontend/src/utils/format-sighting.ts +++ b/frontend/src/utils/format-sighting.ts @@ -3,9 +3,11 @@ import type { UfoSighting } from '../types/ufo-sighting' export function formatDate(date: string | null) { if (!date) return 'Unknown date' - return new Intl.DateTimeFormat('en', { - dateStyle: 'medium', - }).format(new Date(date)) + // Intl throws a RangeError for an invalid date. + const parsed = new Date(date) + if (Number.isNaN(parsed.getTime())) return 'Unknown date' + + return new Intl.DateTimeFormat('en', { dateStyle: 'medium' }).format(parsed) } export function formatDuration(seconds: number | null) {