bellingcat / osm-search

A user friendly way to search OpenStreetMap data for features in proximity to each other.
https://osm-search.bellingcat.com/
MIT License
153 stars 6 forks source link

refactor: API #6

Closed zameji closed 3 months ago

zameji commented 4 months ago

This escalated a bit, main changes:

  1. Updated psycopg from 2 to 3
  2. type hints in most places
  3. Pre-commit for consistent formatting
  4. Restructured code to separate concerns
  5. Added input validation with Pydantic, allowing to drop the ALLOWED_ checks
  6. Restructured query, some primitive optimization (still on my end with a sample query a speedup of factor 1.7x)
  7. Prepared server-side pagination (though some API change would be useful to better support it in frontend, but would take this to separate PR implementing just pagination in front- and backend)
zameji commented 4 months ago

This also addresses #3 (via enabled / disabled paging) and implements #4.

zameji commented 3 months ago

Also migrated to Vue3 (as Vue2 is EoL for a while now). Did also major changes to the UI:

Backend changes

There are still some TODOs open:

@loganwilliams Some further UX improvements seem possible to me, but would require knowing more about how it's used - e.g. do people use the small previews more than the main map?

loganwilliams commented 3 months ago

Thanks for this @zameji, really appreciate your work here. It will take me some time to review it, and I might ask you to break the PR into smaller feature specific chunks. I'll be in touch soon.

Best, Logan

zameji commented 3 months ago

Sure, take your time. Breaking it up is going to be somewhat problematic though, esp. in the frontend, since the migration to Vue3 brought with it a lot of necessary changes. In the backend, the individual commits should be smaller and easier to review by itself. If you have any questions/doubts about the design decisions, let me know - I'm open to trying different approaches.

loganwilliams commented 3 months ago

Thanks @zameji, I understand that it might be hard to break up changes. As a first pass, to separate PR concerns, could you make one PR for front-end changes and one PR for back-end changes?

zameji commented 3 months ago

Sure @loganwilliams I'll look into that tonight / tomorrow night. The current frontend can't work with the new backend without a small change, though, because the interface changed. I'll include that change in the backend PR.

zameji commented 3 months ago

Closing this PR as: