Add support for using Open Street Map on the Map and Group pages. #5398

cillian · 2020-05-10T14:29:49Z

What? Why?

This lets people use Open Street map.

The map is displayed using https://leafletjs.com/

The search field for the Open Street Map works differently than searching on Google Maps. It matches producers by their name or address instead of matching place names all over the world because this way easier to implement I found.

To enable Open Street Map go to the Admin -> Configuration -> Content section and click 'Open Street Map Enabled'.

The Open Street Map Provider Name setting can be used to configure different tile providers thanks to the Leaflet-providers extension (https://github.com/leaflet-extras/leaflet-providers)

Some tile providers require an API key, this can provided in JSON format e.g. { apiKey: 123 } in the Open Street Map Provider Options setting.

Each tile provider has their own usage policy so this should be checked before enabling Open Street Map.

What should we test?

Enabling Open Street Map in Admin > Configuration > Content.
The map page and search field.
The group map page and search field.

Note I haven't got automated tests working locally yet but I will try check they are passing later.

Release notes

Adds support for displaying maps usings Open Street Map.

Changelog Category: Added

Discourse thread

https://community.openfoodnetwork.org/t/i-need-help-on-configuring-ofn-without-external-data-collection/1889

Documentation updates

https://github.com/openfoodfoundation/openfoodnetwork/wiki/Set-and-Deploy-on-Heroku (Mention Open Street Map is an option in Post-install setup)
https://github.com/openfoodfoundation/ofn-install/wiki/Administration (Add Open Street Map to External Services section)

Matt-Yorkley · 2020-05-10T15:09:22Z

Thanks so much! Leaflet looks like exactly what we needed. 😄

I think we could probably bring back some of the lost search functionality with a bit of tweaking, possibly via the geocoding gem (which also can be configured to not use google), but it would need some investigating.

We'll probably need to make some configuration changes in the ofn-install repo as well, and make sure we can transition without disruption (I'm not asking you to do any of this stuff, just mentioning it).

Exciting PR! It might be a while before it can be merged though 👍

luisramos0 · 2020-05-10T19:13:37Z

You are making many people happy with this PR @cillian :-)

I think that instead of making this PR ready to replace google maps, I'd just adapt the config in this PR to make OSM the default maps solution in OFN.
If I read the code correctly this leaves google maps as the default. Ideally we would solve the original request and make this limited (for now!) solution be the default. Managers can go to the back office and active google maps if they want (I think initially everyone will keep using google maps because of geocoding).

I'd say we should ship this (default OSM + google maps as the extra solution). From there we can gradually improve this OSM solution and make it replace google maps for all instances.
Thoughts?

I'll do a detailed code review when we agree on what's the general approach to this PR.

Anyway, this PR made my day!

RachL · 2020-05-11T06:53:33Z

(I think initially everyone will keep using google maps because of geocoding).

It is true we will need to add some routes and update some details for some farms. But we already do hit for Google and it is super easy on OSM. For the CSA network in France, the farmer can use this tool : https://nominatim.openstreetmap.org/
when adding his address. If he can't find his address, there is a link to add his address or ask support so that someone adds his address onto OSM. It is working pretty good.
So the big question is how our enterprise address field will play with OSM. That's something we should add in the tests here: add a new enterprise and see if it shows up on the map.

Also I understand that in countries like Australia where OSM is really poor in data, the need for Google maps is still there. I think the most important thing is to keep the choice. Whether or not one should be default and not the other I have no opinion. However if OSM is default, we should make sure that the deployment does not change this for all current instance. Could endup with a lot of problems.

luisramos0 · 2020-05-11T09:45:11Z

Just to be clear, making google maps not be the default is what would close #5078. I think #5078 is important.

cillian · 2020-05-15T09:06:34Z

👍 ..sounds good, I'll leave it with you all for now then. If it needs any changes please let me know.

luisramos0

Actually, I am just approving this PR as is.
If we ship this nothing will change for existing instances and other instances can switch to this solution and try OSM. We can then improve this until it becomes the default.

Matt-Yorkley · 2020-05-23T16:19:14Z

I'm just having a little play with this now. It's working really well. I think I'll try to make some improvements to it at some point, but for now I think I agree we can just merge it and iterate.

We should add a notice in the Wiki though: the current default provider has usage restrictions and should only only be used for a bit of testing. It shouldn't be used long-term or on a larger production server. We should also have a look at the alternate free and paid providers that can be used, and maybe recommend one.

app/views/groups/show.html.haml

Matt-Yorkley · 2020-05-23T16:34:55Z

app/models/preference_sections/map_section.rb

@@ -0,0 +1,15 @@
+module PreferenceSections
+  class MapSection


Note: the preferences are hard to find in the config section. We could move them to a new map settings page.

Matt-Yorkley · 2020-05-23T16:37:47Z

app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee

+          markers.push(marker)
+
+    displaySearchField = () ->
+      new Autocomplete('#open-street-map--search',


Note: we can use a different search box using Geocoding (like the /shops search) and get the old search behavior back. If we do that we can probably ditch the Autocomplete javascript dependency. See geocoding gem.

Matt-Yorkley

Thanks @cillian! ❤️

filipefurtad0 · 2020-05-28T14:00:38Z

Hi @cillian - thank you for this PR!

One issue though: I see codeclimate has several types of issues to resolve - perhaps these should be addressed before testing?

luisramos0 · 2020-05-28T19:30:37Z

app/serializers/api/open_street_map_config_serializer.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+#


this line above here should be an empty line. not an empty comment.

and if you have one more minute you can fix the other warning

by doing:

module Api class OpenStreetMapConfigSerializer < ActiveModel::Serializer ... end end

I think we can ignore the other 2 warnings 👍

Good point @filipefurtad0 👍

cillian · 2020-05-28T19:56:24Z

Hi @filipefurtad0, @luisramos0

No problem, that's done now. Let me know if it needs more changes.

luisramos0 · 2020-05-28T19:58:11Z

Nice, thanks @cillian Ready for testing 👍

The map is displayed using https://leafletjs.com/ To enable Open Street Map go to the Admin -> Configuration -> Content section and click 'Open Street Map Enabled'. The 'Open Street Map Provider Name' setting can be used to configure different tile providers thanks to the Leaflet-providers extension (https://github.com/leaflet-extras/leaflet-providers) Some tile providers require an API key, this can provided in JSON format e.g. '{ apiKey: 123 }' in the 'Open Street Map Provider Options' setting. Each tile provider has their own usage policy so this should be checked before enabling Open Street Map. The search field for the Open Street Map works differently than searching on Google Maps. It matches producers by their name or address because it was easier to implement instead of matching place names all over the world.

Matt-Yorkley · 2020-05-31T20:17:34Z

I've rebased this to resolve some merge conflicts and added a slight adjustment that was needed after the rebase.

filipefurtad0 · 2020-06-04T09:49:13Z

Hi @cillian ,

Thanks for tackling this!

The PR definitively removes dependency from google maps, as shown on the console (second pic, below) but somehow the map is not being displayed.

Before this PR

After this PR, and enabling Open Street Map in the Content section (superadmin):

This might be due to staging issues; maps never really worked well on fr-staging - right @RachL? Perhaps a good idea to test this on a different server.

More on this soon @cillian - thanks again 👍

RachL · 2020-06-04T11:53:12Z

@filipefurtad0 yes gmaps is not working in FR. But I thought it was specific to gmaps. Worth checking on another staging.

filipefurtad0 · 2020-06-04T15:48:23Z

Alright,
Different staging and server, show the expected result:

Before this PR (nothing to show on this map, the staging-fr related issue):

After this PR - in staging-es:

Group maps work as well.
Checked that search field now works for enterprise names - thanks for stressing this @luisramos0 👍

Awesome!
Thank you @cillian 🎉

kirstenalarsen · 2020-06-04T21:01:29Z

Can anyone tell me what would happen to Aus enterprise addresses that aren't on osm if we switched this? Would they just disappear from the map? Would we need to ask all existing enterprises to check and create themselves, do it ourselves, or is there an automated way to do it?

Matt-Yorkley · 2020-06-05T06:30:25Z

@kirstenalarsen We're adding an optional alternative, not really changing anything yet. It needs some more work, but the idea is that instances will be able to use it without any of the problems you've listed.

kirstenalarsen · 2020-06-05T11:00:59Z

thanks @Matt-Yorkley I understand that :) I was just wondering if instances like Aus did want to switch to it, if we did switch to it, what would happen?

kirstenalarsen · 2020-06-05T11:01:42Z

or in saying 'it needs more work' you're imagining that there would be solutions to those problems that would mean that we could switch?

luisramos0 · 2020-06-05T11:25:26Z

you're imagining that there would be solutions to those problems that would mean that we could switch?

yes! this issue will help to understand that Kirsten: #5542
The main difference is the search bar is searching for enterprise names, not locations.
Then, because we dont have any "manual geocoding", I think new enterprises will just not appear on the map. I think existing geocoded enterprises would appear on the map.
Is this correct, Matt?

cillian · 2020-06-05T20:02:07Z

That's correct any enterprise with the latitude and longitude already set on the address record should appear. The search bar doesn't search for locations unfortunately although if you type in an address it will filter enterprises matching that address in the drop down.

Matt-Yorkley · 2020-06-05T20:30:52Z

Actually the geocoding we do when an enterprise is saved is handled by a separate gem, which is not connected to the maps javascript. So new enterprises would still appear on the map. 👍

I've already written up a bit of a roadmap detailing the bits of work would need to be done to solve the outstanding issues.

luisramos0 · 2020-06-05T21:33:44Z

wait, but if geocoding still works if you switch to osm maps, then the only problem with using osm is the search. I wonder if that doesn't make some instances want to switch immediately.

Matt-Yorkley · 2020-06-05T22:06:45Z

It's close, but not close enough...

tschumilas · 2020-06-22T22:29:05Z

So - I activated this on OFN_CAN - and there are a few issues you can see: first - on the main page, the map opens to Japan - is there a way to have it default open to the instance? Second - the map formats like a banner image, versus a large square for SOME but not all of our groups pages. See these links: https://openfoodnetwork.ca/map, See this group map: https://openfoodnetwork.ca/groups/https-www-stcatharines-ca-en-experiencein-stcatharinesfarmersmarket-asp#/map versus this group map: https://openfoodnetwork.ca/groups/guelph-farmers-market#/map

Any idea why the difference in the group maps?

And - now that I see this, I probably want to go back to google maps for OFN-CAN until we fix this more - can I just back out? If I just uncheck open street map in configuration - will it go back? (maybe I shouldn't have experimented )

RachL · 2020-06-23T09:59:11Z

@tschumilas yes please switch back to google maps. There is quite a few work to do before we can use this on live instance: #5542

tschumilas · 2020-06-23T11:00:52Z

OK - will do, sorry I jumped the gun here. I'm guessing I just de-select the open street map and presto - I'll be back.?

sigmundpetersen assigned cillian May 11, 2020

luisramos0 approved these changes May 16, 2020

View reviewed changes

Matt-Yorkley reviewed May 23, 2020

View reviewed changes

app/views/groups/show.html.haml Outdated Show resolved Hide resolved

Matt-Yorkley reviewed May 23, 2020

View reviewed changes

Matt-Yorkley approved these changes May 23, 2020

View reviewed changes

luisramos0 reviewed May 28, 2020

View reviewed changes

cillian force-pushed the open-street-map branch 2 times, most recently from 0779607 to 6c0ec4f Compare May 28, 2020 19:54

cillian and others added 3 commits May 31, 2020 18:42

Fix up Code Climate issues with Open Street Map code.

94b85fb

Adjust shared map partial

8e57f9d

Matt-Yorkley force-pushed the open-street-map branch from 6c0ec4f to 8e57f9d Compare May 31, 2020 20:16

Matt-Yorkley mentioned this pull request Jun 1, 2020

Google-free maps #5542

Open

3 tasks

filipefurtad0 self-assigned this Jun 4, 2020

filipefurtad0 added the pr-staged-fr staging.coopcircuits.fr label Jun 4, 2020

filipefurtad0 removed the pr-staged-fr staging.coopcircuits.fr label Jun 4, 2020

filipefurtad0 added the pr-staged-es label Jun 4, 2020

filipefurtad0 removed the pr-staged-es label Jun 4, 2020

luisramos0 merged commit 3a20de1 into openfoodfoundation:master Jun 4, 2020

cillian deleted the open-street-map branch September 17, 2021 17:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for using Open Street Map on the Map and Group pages. #5398

Add support for using Open Street Map on the Map and Group pages. #5398

cillian commented May 10, 2020

Matt-Yorkley commented May 10, 2020 •

edited

Loading

luisramos0 commented May 10, 2020

RachL commented May 11, 2020

luisramos0 commented May 11, 2020

cillian commented May 15, 2020

luisramos0 left a comment

Matt-Yorkley commented May 23, 2020

Matt-Yorkley May 23, 2020

Matt-Yorkley May 23, 2020

Matt-Yorkley left a comment

filipefurtad0 commented May 28, 2020

luisramos0 May 28, 2020 •

edited

Loading

luisramos0 May 28, 2020

cillian commented May 28, 2020

luisramos0 commented May 28, 2020

Matt-Yorkley commented May 31, 2020

filipefurtad0 commented Jun 4, 2020

RachL commented Jun 4, 2020

filipefurtad0 commented Jun 4, 2020 •

edited

Loading

kirstenalarsen commented Jun 4, 2020

Matt-Yorkley commented Jun 5, 2020 •

edited

Loading

kirstenalarsen commented Jun 5, 2020

kirstenalarsen commented Jun 5, 2020

luisramos0 commented Jun 5, 2020 •

edited

Loading

cillian commented Jun 5, 2020

Matt-Yorkley commented Jun 5, 2020 •

edited

Loading

luisramos0 commented Jun 5, 2020

Matt-Yorkley commented Jun 5, 2020

tschumilas commented Jun 22, 2020

RachL commented Jun 23, 2020 •

edited

Loading

tschumilas commented Jun 23, 2020

Add support for using Open Street Map on the Map and Group pages. #5398

Add support for using Open Street Map on the Map and Group pages. #5398

Conversation

cillian commented May 10, 2020

What? Why?

What should we test?

Release notes

Discourse thread

Documentation updates

Matt-Yorkley commented May 10, 2020 • edited Loading

luisramos0 commented May 10, 2020

RachL commented May 11, 2020

luisramos0 commented May 11, 2020

cillian commented May 15, 2020

luisramos0 left a comment

Choose a reason for hiding this comment

Matt-Yorkley commented May 23, 2020

Matt-Yorkley May 23, 2020

Choose a reason for hiding this comment

Matt-Yorkley May 23, 2020

Choose a reason for hiding this comment

Matt-Yorkley left a comment

Choose a reason for hiding this comment

filipefurtad0 commented May 28, 2020

luisramos0 May 28, 2020 • edited Loading

Choose a reason for hiding this comment

luisramos0 May 28, 2020

Choose a reason for hiding this comment

cillian commented May 28, 2020

luisramos0 commented May 28, 2020

Matt-Yorkley commented May 31, 2020

filipefurtad0 commented Jun 4, 2020

RachL commented Jun 4, 2020

filipefurtad0 commented Jun 4, 2020 • edited Loading

kirstenalarsen commented Jun 4, 2020

Matt-Yorkley commented Jun 5, 2020 • edited Loading

kirstenalarsen commented Jun 5, 2020

kirstenalarsen commented Jun 5, 2020

luisramos0 commented Jun 5, 2020 • edited Loading

cillian commented Jun 5, 2020

Matt-Yorkley commented Jun 5, 2020 • edited Loading

luisramos0 commented Jun 5, 2020

Matt-Yorkley commented Jun 5, 2020

tschumilas commented Jun 22, 2020

RachL commented Jun 23, 2020 • edited Loading

tschumilas commented Jun 23, 2020

Matt-Yorkley commented May 10, 2020 •

edited

Loading

luisramos0 May 28, 2020 •

edited

Loading

filipefurtad0 commented Jun 4, 2020 •

edited

Loading

Matt-Yorkley commented Jun 5, 2020 •

edited

Loading

luisramos0 commented Jun 5, 2020 •

edited

Loading

Matt-Yorkley commented Jun 5, 2020 •

edited

Loading

RachL commented Jun 23, 2020 •

edited

Loading