NSWSESMembers / Lighthouse

Beacon Tools and UI Improvements
Other
15 stars 15 forks source link

Feature - Add what3words address onto Job Register Page #148

Closed andrewturner closed 2 years ago

andrewturner commented 2 years ago

Add a super basic implementation of geocoding addresses to what3words locations. Located under lat/long field within job details.

Only appears on geocoded addresses (that have a lat/long) and parses lat/long to w3w API (https://developer.what3words.com/public-api/docs)

Non-Geocoded Address: image

Geocoded Address image

We have used this for some jobs where addresses were geocoded, but long to spell out via radio.

lucanos commented 2 years ago

An interesting contribution, but is it really adding a feature people need?

I understand the intent of W3W - to use words rather than a set of digits to represent a lat/long set, but would we really use it?

Especially considering that the problem it is trying to fix (digits being misheard) is simply replaced by a worse problem (words being misheard).

///wafers.reason.bluebirds is about 1,000km from ///wafers.reason.bluebird, for example. The only difference being a single letter between a singular and a plural.

And then you have homophones - words which sound the same, but are spelled differently. "There" and "their", for example.

Open to other opinions, but respectfully, I don't think W3W is a direction SES should go in, or Lighthouse.

OSPFNeighbour commented 2 years ago

Hey Andrew, not ignoring this just in the middle of a massive chunk of code. Will have a poke ASAP.

OSPFNeighbour commented 2 years ago

Thoughts:

You have hard coded an API key into this, thats not ideal (Anyone on the chrome store would be able to pull that key out)

As this code publishes beacon data (GPS positions of jobs) to an external party it will need to be discussed with the beacon team as a courtesy. Im in the process of having chats for some other map work so will let you know how that pans out.

vincentkoc commented 2 years ago

Wouldn't it be better to have W3W for job creation? And Likewise as @OSPFNeighbour mentioned it should not have the API key in the code, and the Beacon team should get a heads up as this is pushing potentially sensitive data to an external party.

W3W also gives out free access to emergency organisations like SES where needed, but i suspect this key would need to be stored server side.

OSPFNeighbour commented 2 years ago

I can help with the storage of API keys sever side, just let me finish some other map work with legal about sending data offsite before we jump down this rabbit hole.

vincentkoc commented 2 years ago

@OSPFNeighbour FYI NSW SES needs to request a key from W3W if they have not already, the is a special one for EM organisations. I have the contact somewhere and can email directly if you want (once your ready to go down the rabbit hole) and its cleared to proceed.

OSPFNeighbour commented 2 years ago

Honestly if SES hold the API key then SES can take responsibility for coding it into beacon. This feature sits right in that grey area of lighthouse functionality

On Thu, 27 Jan 2022, 4:05 pm Vincent Koc, @.***> wrote:

@OSPFNeighbour https://github.com/OSPFNeighbour FYI NSW SES needs to request a key from W3W if they have not already, the is a special one for EM organisations. I have the contact somewhere and can email directly if you want (once your ready to go down the rabbit hole) and its cleared to proceed.

— Reply to this email directly, view it on GitHub https://github.com/NSWSESMembers/Lighthouse/pull/148#issuecomment-1022851885, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMH2NWKXHLUWP54MGGITRTUYDHATANCNFSM5K5GZSZA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

andrewturner commented 2 years ago

Thanks for the review and feedback. Totally agree there around API Key storage, not ideal and good pickup.

p.s. as for the job creation side, I have done a little work also around scripting up some fields on the job creation screen to take W3W locations and returning a lat/long/address, but it's far too complex to realistically drop into a browser extension and something that should be done within the application if needed.

lucanos commented 2 years ago

With the utmost respect for Andrew for suggesting this feature, I think that the subsequent discussion has landed on us, as a group, deciding that if W3W is to be supported by Beacon it should be built into Beacon itself, rather than through Lighthouse.

As the last comment on this discussion was three months ago, I am going to close this Pull Request.

If someone thinks that this is still an active point of discussion, they are welcome to reopen this PR, or open a new one.