Repository navigation
Add a required country to every place, a country API filter and a map country filter - #337
Merged
Conversation
Contributor
|
@propcgamer20-png is attempting to deploy a commit to the studentsuite Team on Vercel. A member of the Team first needs to authorize it. |
1 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Fixes #288, steps 2 to 5 (step 1, the optional field in the schema, type and validator, was already on
main).Step 2, backfill (
data/places/*.json). All 2,005 records now carrycountry. I mapped eachlat/lngto a country with a one-off script using the Natural Earth 10m country polygons (point-in-polygon, then nearest border within 0.15 degrees for coastal points); the script is not committed and nothing new is a runtime dependency. 67 distinct codes, the largest being US 538, IN 220, TR 181, AE 122, BR 101, NG 95, CA 94, GB 87. Natural Earth tags TaiwanCN-TW; the field takesTW.Checks on the mapping:
al_ain(AE, plus one SAT centre, Future International School id 52059, whose coordinates are in Oman and which OpenStreetMap also places in Oman),arona(Arona, Italy and Arona, Tenerife, Spain),kingston(Kingston, Ontario and Kingston, Pennsylvania), and the two below. Each record keeps the country of its own coordinates; splitting the slugs is left for a follow-up.detroit-gov_offices-01) in Canada; an OpenStreetMap reverse lookup says US, so that one is set toUSby hand. The other 19 agree with their addresses.tokyohad coordinates in the wrong country, which the backfill exposed.67350International School of the Sacred Heart (Hiroo, Shibuya) pointed at Thalassery, Kerala, India; it is now 35.652712, 139.72131 (OpenStreetMap).67294Global Indian International School Tokyo, Nishi-Kasai pointed at Kuala Lumpur, Malaysia; it is now 35.65675, 139.85882 (the coordinates on its Wikipedia page, which I could not cross-check against OpenStreetMap, so the pin is the campus neighbourhood). Bothgmaps_linkvalues were updated to match.Step 3, required.
countryis inrequiredindata/places.schema.json, sonpm run validatenow fails on a missing country and on"india"(see Proof). It stays optional on the TypeScriptPlacetype because a signed-in visitor's private saved places have none.data/CONTRIBUTING.mdand the three docs-page examples show the field.Step 4, API.
GET /api/places?country=inmatches case-insensitively (src/lib/places-api.ts,src/app/api/places/route.ts). A value that is not two letters, such asIndia, is a 400; so is a well-formed code no place uses (ZZ), via a newunknownCountryError./docs/places-apidescribes the parameter.Step 5, map. A "Filter by country" select (
country-filter.tsx) in the map panel; picking a country narrows the city filter to its cities and clears a selected city that is not in it.countryis a share-link parameter (src/lib/share.ts), so it survives a reload and works for/embedtoo, and it is part of Reset and of the active-filter summary.Proof
"country": "india"on the first recordnpm run validate:placesexits 1 (country must be a two-letter upper-case ISO 3166-1 alpha-2 code); with the field removed it reportsmissing required field "country"; restored,npm run validatepasses. A node one-liner overdata/places/*.jsonreports 2005 records and 0 withoutcountry.GET /api/places?country=in&limit=1returns 200 withcountry: "IN";country=ZZandcountry=Indiareturn 400 (route tests).places-api.test.ts,route.test.ts,share.test.ts,places.test.tsandplaces-map.test.tsx(country filter, URL round trip, shared link, city dropped on country change).npm run typecheck,npm run lint,npm run format:checkclean;npm run buildsucceeds; unit tests all pass.Not verified
/mapin a browser, so the layout of the new select in the sidebar and the mobile sheet is unchecked; behaviour is covered by the component tests.Type of change