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.

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

Add profile location #5302

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
102 changes: 93 additions & 9 deletions app/assets/javascripts/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ $(document).ready(function () {
var defaultHomeZoom = 12;
var map, marker, deleted_lat, deleted_lon;

const locationInput = {
dirty: true,
request: null,
countryName: "",
deletedText: null
};

if ($("#map").length) {
map = L.map("map", {
attributionControl: false,
Expand Down Expand Up @@ -73,6 +80,7 @@ $(document).ready(function () {

deleted_lat = null;
deleted_lon = null;
locationInput.deletedText = null;
respondToHomeUpdate();
}).on("moveend", function () {
var lat = $("#home_lat").val().trim(),
Expand All @@ -93,9 +101,26 @@ $(document).ready(function () {
$("#home_lat, #home_lon").on("input", function () {
deleted_lat = null;
deleted_lon = null;
locationInput.deletedText = null;
respondToHomeUpdate();
});

$("#location_name").on("input", function () {
locationInput.dirty = true;
deleted_lat = null;
deleted_lon = null;
locationInput.deletedText = null;

updateLocationWarning();
respondToHomeUpdate(false);
});

$("#location_default_name").on("click", function () {
$("#location_name").val(locationInput.countryName);
$("#location_name_warning").addClass("d-none");
locationInput.dirty = false;
});

$("#home_show").click(function () {
var lat = $("#home_lat").val(),
lon = $("#home_lon").val();
Expand All @@ -104,24 +129,34 @@ $(document).ready(function () {
});

$("#home_delete").click(function () {
var lat = $("#home_lat").val(),
lon = $("#home_lon").val();
let lat = $("#home_lat").val(),
lon = $("#home_lon").val(),
locationName = $("#location_name").val();

$("#home_lat, #home_lon").val("");
$("#location_name_warning").addClass("d-none");
$("#home_lat, #home_lon, #location_name").val("");
deleted_lat = lat;
deleted_lon = lon;
respondToHomeUpdate();
locationInput.deletedText = locationName;

respondToHomeUpdate(false);
$("#home_undelete").trigger("focus");
});

$("#home_undelete").click(function () {
$("#home_lat").val(deleted_lat);
$("#home_lon").val(deleted_lon);
$("#location_name").val(locationInput.deletedText);
deleted_lat = null;
deleted_lon = null;
respondToHomeUpdate();
locationInput.deletedText = null;

updateLocationWarning();
respondToHomeUpdate(false);
$("#home_delete").trigger("focus");
});

updateHomeLocation();
} else {
$("[data-user]").each(function () {
var user = $(this).data("user");
Expand All @@ -133,14 +168,18 @@ $(document).ready(function () {
}
}

function respondToHomeUpdate() {
var lat = $("#home_lat").val().trim(),
function respondToHomeUpdate(updateLocationName = true) {
let lat = $("#home_lat").val().trim(),
lon = $("#home_lon").val().trim(),
locationName = $("#location_name").val().trim(),
location;

try {
if (lat && lon) {
location = L.latLng(lat, lon);
if (updateLocationName) {
updateHomeLocation();
}
}
$("#home_lat, #home_lon").removeClass("is-invalid");
} catch (error) {
Expand All @@ -150,8 +189,11 @@ $(document).ready(function () {

$("#home_message").toggleClass("invisible", Boolean(location));
$("#home_show").prop("hidden", !location);
$("#home_delete").prop("hidden", !location);
$("#home_undelete").prop("hidden", !(!location && deleted_lat && deleted_lon));
$("#home_delete").prop("hidden", !location && !locationName);
$("#home_undelete").prop("hidden", !(
(!location || !locationName) &&
((deleted_lat && deleted_lon) || locationInput.deletedText)
));
if (location) {
marker.setLatLng([lat, lon]);
marker.addTo(map);
Expand All @@ -178,6 +220,48 @@ $(document).ready(function () {
}
}

function updateHomeLocation() {
const lat = $("#home_lat").val().trim();
const lon = $("#home_lon").val().trim();
if (!lat || !lon) {
return;
}

const geocodeUrl = "/geocoder/search_osm_nominatim_reverse";
const params = {
format: "json",
lat,
lon,
zoom: 3
};

if (locationInput.request) locationInput.request.abort();
locationInput.request = $.ajax({
url: geocodeUrl,
method: "POST",
data: params,
success: function (country) {
locationInput.request = null;
locationInput.countryName = country;
$("#location_default_name").text(I18n.t("javascripts.profiles.edit.location_autofill", { country }));
if (locationInput.dirty) {
updateLocationWarning();
} else {
$("#location_name").val(locationInput.countryName);
}
}
});
}

function updateLocationWarning() {
const locationName = $("#location_name").val();
if (!locationInput.dirty || locationName.includes(locationInput.countryName)) {
$("#location_name_warning").addClass("d-none");
} else {
$("#location_name_warning").removeClass("d-none");
}
}

updateAuthUID();

$("select#user_auth_provider").on("change", updateAuthUID);
Expand Down
6 changes: 5 additions & 1 deletion app/controllers/geocoder_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,11 @@ def search_osm_nominatim_reverse
:type => object_type, :id => object_id)
end

render :action => "results"
if params[:format] == "json"
render :html => @results.pluck(:name).join(", ")
else
render :action => "results"
end
rescue StandardError => e
host = URI(Settings.nominatim_url).host
@error = "Error contacting #{host}: #{e}"
Expand Down
1 change: 1 addition & 0 deletions app/controllers/profiles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def update

current_user.home_lat = params[:user][:home_lat]
current_user.home_lon = params[:user][:home_lon]
current_user.location_name = params[:user][:location_name]

if current_user.save
flash[:notice] = t ".success"
Expand Down
1 change: 1 addition & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# home_lat :float
# home_lon :float
# home_zoom :integer default(3)
# location_name :string
# pass_salt :string
# email_valid :boolean default(FALSE), not null
# new_email :string
Expand Down
6 changes: 6 additions & 0 deletions app/views/profiles/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@
<button type="button" id="home_undelete" class="btn btn-outline-primary" hidden><%= t ".undelete" %></button>
</div>
</div>
<div class="row my-2">
<%= f.text_field :location_name, :wrapper_class => "col-sm-4 d-flex flex-column pe-3", :class => "mt-auto", :id => "location_name" %>
<div id="location_name_warning" class="col-sm-4 pt-2 align-self-end d-none">
<button type="button" id="location_default_name" class="btn btn-outline-primary"></button>
</div>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" name="updatehome" value="1" <% unless current_user.home_location? %> checked <% end %> id="updatehome" />
<label class="form-check-label" for="updatehome"><%= t ".update home location on click" %></label>
Expand Down
8 changes: 8 additions & 0 deletions app/views/users/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@
<div class='text-body-secondary'>
<small>
<dl class="list-inline">
<% if @user.location_name && @user.location_name.strip.present? %>
<dt class="list-inline-item m-0">
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-geo-alt-fill" viewBox="0 0 16 16">
<path d="M8 16s6-5.686 6-10A6 6 0 0 0 2 6c0 4.314 6 10 6 10m0-7a3 3 0 1 1 0-6 3 3 0 0 1 0 6" />
</svg>
</dt>
<dd class="list-inline-item"><%= @user.location_name %></dd>
<% end %>
<dt class="list-inline-item m-0"><%= t ".mapper since" %></dt>
<dd class="list-inline-item"><%= l @user.created_at.to_date, :format => :long %></dd>
<dt class="list-inline-item m-0"><%= t ".last map edit" %></dt>
Expand Down
3 changes: 3 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3176,6 +3176,9 @@ en:
ninth: "9th"
tenth: "10th"
time: "Time"
profiles:
edit:
location_autofill: "Autofill %{country}"
query:
node: Node
way: Way
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20241030090336_add_user_location_name.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddUserLocationName < ActiveRecord::Migration[7.2]
def change
add_column :users, :location_name, :string
end
end
4 changes: 3 additions & 1 deletion db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1509,7 +1509,8 @@ CREATE TABLE public.users (
tou_agreed timestamp without time zone,
diary_comments_count integer DEFAULT 0,
note_comments_count integer DEFAULT 0,
creation_address inet
creation_address inet,
location_name character varying
);


Expand Down Expand Up @@ -3397,6 +3398,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('23'),
('22'),
('21'),
('20241030090336'),
('20241023004427'),
('20241022141247'),
('20240913171951'),
Expand Down
18 changes: 18 additions & 0 deletions test/controllers/geocoder_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,24 @@ def test_search_osm_nominatim_reverse
end
end

##
# Test the nominatim reverse JSON search
def test_search_osm_nominatim_reverse_json
with_http_stubs "nominatim" do
post geocoder_search_osm_nominatim_reverse_path(:lat => 51.7632, :lon => -0.0076, :zoom => 15, :format => "json"), :xhr => true
assert_response :success
assert_select "body", :text => "Broxbourne, Hertfordshire, East of England, England, United Kingdom"

post geocoder_search_osm_nominatim_reverse_path(:lat => 51.7632, :lon => -0.0076, :zoom => 17, :format => "json"), :xhr => true
assert_response :success
assert_select "body", :text => "Dinant Link Road, Broxbourne, Hertfordshire, East of England, England, EN11 8HX, United Kingdom"

post geocoder_search_osm_nominatim_reverse_path(:lat => 13.7709, :lon => 100.50507, :zoom => 19, :format => "json"), :xhr => true
assert_response :success
assert_select "body", :text => "MM Steak&Grill, ถนนศรีอยุธยา, บางขุนพรหม, กรุงเทพมหานคร, เขตดุสิต, กรุงเทพมหานคร, 10300, ประเทศไทย"
end
end

private

def latlon_check(query, lat, lon)
Expand Down
22 changes: 22 additions & 0 deletions test/system/user_location_change_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
require "application_system_test_case"

class UserLocationChangeTest < ApplicationSystemTestCase
def setup
stub_request(:get, /.*gravatar.com.*d=404/).to_return(:status => 404)
end

test "User can change their location" do
user = create(:user)
sign_in_as(user)

visit user_path(user)
assert_no_selector ".bi.bi-geo-alt-fill"

visit edit_profile_path
fill_in "Location name", :with => "Test Location"
click_on "Update Profile"

visit user_path(user)
assert_text "Test Location"
end
end