Cambridge-Community-Kitchen / cck-volunteer-web-app

https://volunteer.cckitchen.uk
3 stars 2 forks source link

Regression: all maps links use short pluscodes #26

Closed faceless-stoat closed 9 months ago

faceless-stoat commented 9 months ago

On today's shift, deliverers are reporting that Google Maps links are leading them to random places (Milton Keynes, France, etc) -- both the new "Google Bike/Driving Route" buttons, and the per-recipient "Google Maps →" buttons.

A workaround that is reported to work is to navigate to Cambridge by hand in Google Maps before tapping any links.

Looking at the demo route, I can see that all Maps links are using the short, relative version of plus codes; like 6574+2F rather than 9F426574+2F. (Thanks to @timsk for the tip-off to look at this)

Looking at recent changes, I think the lint fixes in 5677cacc6f are the likely culprit, at least for the individual Google Maps links. (Disclaimer: I don't speak this language, and don't have the ability to run the code myself.)

Before this change, DeliveriesList.jsx would modify item.plus_code in-place with the Cambridge 9f42 prefix, then copy the result to item.plusCode, and pass the resulting item into (I believe) code in Item.jsx. So both plus_code and plusCode would be fully-qualified versions.

After this change, the itemForRendering that gets passed into Item.jsx has plus_code as the fully-qualified version, and plusCode as the short version.

However, Item.jsx doesn't use the plus_code member at all (and as far as I can tell never has), so we get the (wrong) short code everywhere (in Google URLs, in text, etc).

I think the map-for-whole-route links will have been broken since they were added a bit later, in fcff3cf202, but I haven't looked very closely.

(Unimportant opinion bits: IMO we should convert the short codes into full ones at the earliest opportunity -- IMO the short ones are a menace. And also, get the magic string 9f42, meaning "Cambridge", abstracted out somewhere and explained in comments, in case someone who isn't CCK wants to use this code. And obviously the notion of a thing having two members called plus_code and plusCode needs to be burned with fire.)

timsk commented 9 months ago

Thanks for the good write up, @faceless-stoat. I agree with everything except that short pluscodes do have a use case – we should retain them on the printed route sheets because they're quicker and easier to retype by hand. Since pluscodes are case insensitive by design, it's also good to show them in lower case in this context, for the same reason.

In any case where we're passing pluscodes as parameters though, yeah, we should be fully specifying with the 4-character prefix.

pdl commented 9 months ago

Thanks for looking into this @faceless-stoat, and I can confirm that you're right. The snake/camel thing winds me up as well. I almost refactored that code on the spot and then thought 'no, this is just a lint change' and broke it instead.

faceless-stoat commented 9 months ago

(The reported issue is solved by the pull request, this is just quibbling now and doesn't need to keep the issue open.)

short pluscodes do have a use case – we should retain them on the printed route sheets because they're quicker and easier to retype by hand

I did consider this, but: is Google Maps' UX for manually entered short plus codes any better than it was today for ones in URLs? For the URLs, it apparently silently accepted the ambiguous pluscode and presented a single location as its authoritative meaning, where I'd like it to be popping up some warning like "this location is ambiguous; guessing it's around Poitiers somewhere, since you viewed that recently" to alert the user. If you get the same behaviour from typing in a short pluscode, I maintain it's a menace: it could drop a pin somewhere random, and our hypothetical deliverer standing in the rain entering pluscodes from their paper sheet because the app's fallen over is unlikely to know how to rectify this; understanding what went wrong and how to work around it requires quite some familiarity with the detailed semantics of pluscodes. (I haven't checked if the Welcome Pack covers this, but it's the sort of thing that is easy to not take in.) We could print 6574+2f Cambridge on the printed sheets, I suppose (maybe we do already, I can't check), but it'd be easy to miss that the Cambridge is an essential part of the location.

(OsmAnd's rather rudimentary pluscode support isn't any better; if I happen to have viewed e.g. Scotland most recently, it will take a short code and confidently present a single lat/long in Scotland to tap on without any hint that it might not be the right answer. And entering 6574+2f Cambridge doesn't seem to do the right thing.)