search

IsraelHikingMap / Site

Israel Hiking Map has maps, route planning, and travel information for Israel. This repository holds the files needed for running the Israel Hiking Map site and apps.
https://israelhiking.osm.org.il/
Other
77 stars 31 forks source link

Route import is always done in hiking mode #654

Closed zstadler closed 6 years ago

zstadler commented 6 years ago

Feature

What I expect to happen

When importing a route file, the route mode should be set according to the current route mode, unless the route already has route modes for its segments.

What really happened

The import is always done in hiking mode.

What I did that caused the issue - step by step

  1. Select 4WD routing mode
  2. Open the dolev-zanoach.twl 4WD route from jeepolog.com
  3. Delete the route point near "חורבת חנות" image
  4. The resulting route uses "Hiking" mode image

My environment

HarelM commented 6 years ago

Unfortunately, the default is set when loading the route at the server side while the current routing mode is stored only in the client side. I think that moving the routing mode to the server is an overkill for this feature and I tend to think that fixing the response in the client side is very error prone. Since the user is already editing the route, I tend to think that moving the next route-point is a decent workaround...

zstadler commented 6 years ago

The current behavior is unexpected and annoying, if you are not a hiker. On the other hand, passing the client's routing mode to the server, as part of the existing API call, is a local change that does not have negative side-effects.

HarelM commented 6 years ago

I've looked at the code again and it seem I have misled you. When routing type is not defined routing mechanism defaults to hiker (in order to get routing no matter what). When a file that was not generated by IHM is opened the routing is undefined. I think I can fix this to use current routing instead of sending empty routing type.

HarelM commented 6 years ago

I have ran very few tests on this fix, it will require more testing once published.