Feat: Fetch sighting details from the backend #62
Conversation
The detail page now queries the sighting by id instead of reading the mock data. Part of #35
Nothing reads from the store now that the detail page uses GraphQL, so SubmitSightingPage no longer adds sightings to it. The mock sightings move to tests/ because the tests still use them as fixtures.
There was a problem hiding this comment.
Good job getting the detail page over to GraphQL, and nice that the mock store is finally gone. I read through the code and found some things. The first four I think we should fix before merging, the rest can wait.
Should fix
1. New sightings don't show up in the list anymore
SubmitSightingPage.tsx:97
Since addSighting() is removed, nothing updates the list after you submit. The list is read from the Apollo cache, so when you go back to the results the new sighting isn't there until you reload the page. It worked before this PR. I think we need refetchQueries or a cache update on the mutation. The test that checked this was also deleted, so we should add a new one.
2. Empty shape and comment
SightingDetailPage.tsx:99 and :107
shape and comments can be null now that the data comes from the database. Then the page shows "Shape" with nothing next to it, and an empty paragraph for the comment. We already have formatShape() which returns "Unknown", so we can just use that. The comment can be hidden when it's null.
3. formatDate() can crash the page
format-sighting.ts:6-8
If the date string isn't a valid date, Intl.DateTimeFormat.format() throws and the whole detail page goes blank. I don't know if we actually have rows like that in the database, but a check that returns "Unknown date" is cheap to add.
4. Weird ids show the wrong sighting
use-sighting.ts:9-11, backend/src/db/sightings.ts:134-139
The id from the URL is sent straight to the SQL query. MySQL turns '1abc' into 1, so /#/sightings/1abc should end up showing sighting 1 instead of "not found" (I haven't tried it, just read the code). We should check that the id is only digits before we query.
Smaller things
5. We fetch the sighting again even though we have it
client.ts:17
The list already loads all the fields, so when you click a sighting it's in the cache. Right now we still send a new request and show "Loading sighting…". Apollo has a way to read it from the cache with a read function on Query.sighting. Could be its own issue.
6. Screen readers don't get told when loading is done
SightingDetailPage.tsx:60-76
Only the error has role="alert". The list uses aria-live and aria-busy, we could do the same here.
7. "Try again" button has no styling
SightingDetailPage.tsx:68
The one in the list uses .sighting-results__button and has a focus outline. This one is just a plain button, so it looks different.
8. Same fields written three times
queries.ts:85-99
SIGHTING_QUERY, SIGHTINGS_QUERY and the create mutation all list the same eleven fields. A shared fragment would mean we only have to change one place when we add a field.
9. retry is copied
use-sighting.ts:13-17
Same code as in use-sighting-search.ts:76-80. Not important, but could be one helper.
Clear the cached sighting lists after a submit so the new sighting shows up in the search results. Show "Unknown" for a missing shape and hide empty comments. Return "Unknown date" for invalid dates instead of throwing. Return not found for ids that are not plain numbers, since MySQL reads '1abc' as 1. Part of #35
I fixed the first four: Null shape and comments: the page now shows "Unknown" for a missing shape and hides the comment when it's null. Added a test. formatDate(): returns "Unknown date" for invalid dates instead of crashing. Added a test. Weird ids: the backend now returns not found unless the id is only digits. No test for this one, since we have no database tests, but I checked it by hand. I'm leaving 5-9 for follow-up issues |
The detail page said "Check your connection" for every error. If the backend answers with a GraphQL error, such as a database failure, it now says the server could not load the sighting and hides the retry button, since retrying won't help. Network failures keep the old message and the button. Part of #35
Fixed :) |
|
@thortve Could you take another look at this? |
Description
Closes #35
What
The detail page now fetches a sighting by id through Apollo Client (SIGHTING_QUERY and useSighting) instead of reading the mock data.
It shows a loading state, an error state with a "Try again" button, and a clear "no sighting with id …" message for unknown ids.
Removed the mock sighting store (sighting-store.ts, use-sightings.ts), and SubmitSightingPage no longer adds to it.
The mock sightings moved to tests/ because the tests still use them as fixtures.
Updated AGENTS.md to match.
Testing
npm run lint and npm run test pass.
Detail page tests cover success, unknown id and error with retry.
Manual check in the browser at desktop and mobile widths