Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support unwrapped bbox values in changeset history queries #5473

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support unwrapped bbox values in changeset history queries #5473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/assets/javascripts/index/history.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ OSM.History = function (map) {
var data = { list: "1" };

if (window.location.pathname === "/history") {
data.bbox = map.getBounds().wrap().toBBoxString();
data.bbox = map.getBounds().toBBoxString();
var feedLink = $("link[type=\"application/atom+xml\"]"),
feedHref = feedLink.attr("href").split("?")[0];
feedLink.attr("href", feedHref + "?bbox=" + data.bbox);
Expand Down
32 changes: 21 additions & 11 deletions app/controllers/changesets_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ def index
changesets.where("false")
end
elsif @params[:bbox]
changesets = conditions_bbox(changesets, BoundingBox.from_bbox_params(params))
bbox_array = @params[:bbox].split(",").map(&:to_f)
raise OSM::APIBadUserInput, "The parameter bbox must be of the form min_lon,min_lat,max_lon,max_lat" unless bbox_array.count == 4

changesets = conditions_bbox(changesets, *bbox_array)
elsif @params[:friends] && current_user
changesets = changesets.where(:user => current_user.friends.identifiable)
elsif @params[:nearby] && current_user
Expand Down Expand Up @@ -142,21 +145,28 @@ def unsubscribe
#------------------------------------------------------------

##
# if a bounding box was specified do some sanity checks.
# restrict changesets to those enclosed by a bounding box
def conditions_bbox(changesets, bbox)
if bbox
bbox.check_boundaries
bbox = bbox.to_scaled

changesets.where("min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?",
bbox.max_lon.to_i, bbox.min_lon.to_i,
bbox.max_lat.to_i, bbox.min_lat.to_i)
else
def conditions_bbox(changesets, min_lon, min_lat, max_lon, max_lat)
db_min_lat = (min_lat * GeoRecord::SCALE).to_i
db_max_lat = (max_lat * GeoRecord::SCALE).to_i
db_min_lon = (wrap_lon(min_lon) * GeoRecord::SCALE).to_i
db_max_lon = (wrap_lon(max_lon) * GeoRecord::SCALE).to_i

changesets = changesets.where("min_lat < ? and max_lat > ?", db_max_lat, db_min_lat)

if max_lon - min_lon >= 360
changesets
elsif db_min_lon > db_max_lon
changesets.where("min_lon < ? or max_lon > ?", db_max_lon, db_min_lon)
else
changesets.where("min_lon < ? and max_lon > ?", db_max_lon, db_min_lon)
end
end

def wrap_lon(lon)
((lon + 180) % 360) - 180
end

##
# eliminate empty changesets (where the bbox has not been set)
# this should be applied to all changeset list displays
Expand Down
104 changes: 104 additions & 0 deletions test/controllers/changesets_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,110 @@ def test_index_bbox
check_index_result(changesets)
end

##
# Test bbox spanning across the dateline with changesets close to the dateline
def test_index_bbox_dateline
western_changeset = create(:changeset, :num_changes => 1,
:min_lat => 0 * GeoRecord::SCALE, :min_lon => 176 * GeoRecord::SCALE,
:max_lat => 1 * GeoRecord::SCALE, :max_lon => 178 * GeoRecord::SCALE)
eastern_changeset = create(:changeset, :num_changes => 1,
:min_lat => 0 * GeoRecord::SCALE, :min_lon => -178 * GeoRecord::SCALE,
:max_lat => 1 * GeoRecord::SCALE, :max_lon => -176 * GeoRecord::SCALE)

get history_path(:format => "html", :list => "1")
assert_response :success
check_index_result([eastern_changeset, western_changeset])

# negative longitudes
get history_path(:format => "html", :list => "1", :bbox => "-190,-10,-170,10")
assert_response :success
check_index_result([eastern_changeset, western_changeset])

get history_path(:format => "html", :list => "1", :bbox => "-183,-10,-177,10")
assert_response :success
check_index_result([eastern_changeset, western_changeset])

get history_path(:format => "html", :list => "1", :bbox => "-181,-10,-177,10")
assert_response :success
check_index_result([eastern_changeset])

get history_path(:format => "html", :list => "1", :bbox => "-183,-10,-179,10")
assert_response :success
check_index_result([western_changeset])

get history_path(:format => "html", :list => "1", :bbox => "-181,-10,-179,10")
assert_response :success
check_index_result([])

# positive longitudes
get history_path(:format => "html", :list => "1", :bbox => "170,-10,190,10")
assert_response :success
check_index_result([eastern_changeset, western_changeset])

get history_path(:format => "html", :list => "1", :bbox => "177,-10,183,10")
assert_response :success
check_index_result([eastern_changeset, western_changeset])

get history_path(:format => "html", :list => "1", :bbox => "177,-10,181,10")
assert_response :success
check_index_result([western_changeset])

get history_path(:format => "html", :list => "1", :bbox => "179,-10,183,10")
assert_response :success
check_index_result([eastern_changeset])

get history_path(:format => "html", :list => "1", :bbox => "179,-10,181,10")
assert_response :success
check_index_result([])
end

##
# Test bbox spanning across the dateline with changesets around the globe
def test_index_bbox_global
changeset1 = create(:changeset, :num_changes => 1,
:min_lat => 40 * GeoRecord::SCALE, :min_lon => -150 * GeoRecord::SCALE,
:max_lat => 50 * GeoRecord::SCALE, :max_lon => -140 * GeoRecord::SCALE)
changeset2 = create(:changeset, :num_changes => 1,
:min_lat => -30 * GeoRecord::SCALE, :min_lon => -30 * GeoRecord::SCALE,
:max_lat => -20 * GeoRecord::SCALE, :max_lon => -20 * GeoRecord::SCALE)
changeset3 = create(:changeset, :num_changes => 1,
:min_lat => 60 * GeoRecord::SCALE, :min_lon => 10 * GeoRecord::SCALE,
:max_lat => 70 * GeoRecord::SCALE, :max_lon => 20 * GeoRecord::SCALE)
changeset4 = create(:changeset, :num_changes => 1,
:min_lat => -60 * GeoRecord::SCALE, :min_lon => 150 * GeoRecord::SCALE,
:max_lat => -50 * GeoRecord::SCALE, :max_lon => 160 * GeoRecord::SCALE)

# no bbox, get all changesets
get history_path(:format => "html", :list => "1")
assert_response :success
check_index_result([changeset4, changeset3, changeset2, changeset1])

# large enough bbox within normal range
get history_path(:format => "html", :list => "1", :bbox => "-170,-80,170,80")
assert_response :success
check_index_result([changeset4, changeset3, changeset2, changeset1])

# bbox for [1,2] within normal range
get history_path(:format => "html", :list => "1", :bbox => "-160,-80,0,80")
assert_response :success
check_index_result([changeset2, changeset1])

# bbox for [1,4] containing dateline with negative lon
get history_path(:format => "html", :list => "1", :bbox => "-220,-80,-100,80")
assert_response :success
check_index_result([changeset4, changeset1])

# bbox for [1,4] containing dateline with positive lon
get history_path(:format => "html", :list => "1", :bbox => "100,-80,220,80")
assert_response :success
check_index_result([changeset4, changeset1])

# large enough bbox outside normal range
get history_path(:format => "html", :list => "1", :bbox => "-220,-80,220,80")
assert_response :success
check_index_result([changeset4, changeset3, changeset2, changeset1])
end

##
# Checks the display of the user changesets listing
def test_index_user
Expand Down
Loading