Skip to content

Commit bdb4770

Browse files
authored
Merge pull request #154 from denis1011101/feature/court-street-in-picker
feat: tell namesake courts apart by street in the game form
2 parents cbe7e14 + 86662b8 commit bdb4770

10 files changed

Lines changed: 91 additions & 9 deletions

File tree

app/javascript/controllers/court_picker_controller.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ export default class extends Controller {
8080
this.selectTarget.innerHTML = ""
8181
groups.forEach(([cityName, cityCourts]) => {
8282
const parent = grouped && cityName ? this._appendGroup(cityName) : this.selectTarget
83-
cityCourts.forEach(c => parent.appendChild(this._buildOption(String(c.id), c.name)))
83+
const ambiguous = this._ambiguousNames(cityCourts)
84+
cityCourts.forEach(c => parent.appendChild(this._buildOption(String(c.id), this._optionLabel(c, ambiguous))))
8485
})
8586

8687
// Порядок опций задают группы, поэтому запасной вариант берём из самого списка.
@@ -139,6 +140,20 @@ export default class extends Controller {
139140
.map(([cityName, cityCourts]) => [cityName, cityCourts.sort((a, b) => a.name.localeCompare(b.name))])
140141
}
141142

143+
// Одноимённые площадки города различаем улицей: у остальных скобки — лишний шум.
144+
_ambiguousNames(courts) {
145+
const seen = new Set()
146+
const duplicates = new Set()
147+
courts.forEach(c => (seen.has(c.name) ? duplicates.add(c.name) : seen.add(c.name)))
148+
return duplicates
149+
}
150+
151+
_optionLabel(court, ambiguousNames) {
152+
if (!court.street || !ambiguousNames.has(court.name)) return court.name
153+
154+
return `${court.name} (${court.street})`
155+
}
156+
142157
_appendGroup(label) {
143158
const group = document.createElement("optgroup")
144159
group.label = label

app/jobs/geocoding/fetch_court_address_job.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ def perform(court_id, lat = nil, lng = nil)
1414
if result.is_a?(Hash) && result[:address].present?
1515
Rails.cache.write(cache_key, result[:address], expires_in: 1.day)
1616
court.update_column(:city_name, result[:city_name]) if result[:city_name].present?
17-
Rails.logger.info "[Geocoding] cached address for Court##{court.id} -> #{result[:address]} (city: #{result[:city_name]})"
17+
# Улицу храним в базе: адрес живёт только в кэше, а список кортов должен
18+
# различать одноимённые площадки и без похода в геокодер.
19+
court.update_column(:street, result[:street]) if result[:street].present?
20+
Rails.logger.info "[Geocoding] cached address for Court##{court.id} -> #{result[:address]} (city: #{result[:city_name]}, street: #{result[:street]})"
1821
else
1922
Rails.logger.warn "[Geocoding] no address resolved for Court##{court.id}"
2023
end

app/services/geocoding/address_resolver.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class << self
1313
end
1414
self.nominatim_last_request_at = -Float::INFINITY
1515

16-
# Resolves lat/lng to { address: String, city_name: String | nil } or nil.
16+
# Resolves lat/lng to { address: String, city_name: String | nil, street: String | nil } or nil.
1717
# Tries Google first, falls back to Nominatim.
1818
def resolve(lat, lng)
1919
geocode_google_full(lat, lng) || geocode_nominatim_structured(lat, lng)
@@ -55,7 +55,7 @@ def geocode_google_full(lat, lng)
5555
street_line = [ street, number ].compact.join(" ").presence
5656
address = [ street_line, city, country ].compact.join(", ").presence
5757

58-
{ address: address, city_name: city }
58+
{ address: address, city_name: city, street: street_line }
5959
rescue => e
6060
Rails.logger.warn("Google geocoding error: #{e.message}")
6161
nil
@@ -113,7 +113,8 @@ def geocode_nominatim_structured(lat, lng)
113113

114114
addr = data["address"]
115115
city_name = addr && (addr["city"] || addr["town"] || addr["village"] || addr["municipality"])
116-
{ address: data["display_name"], city_name: city_name }
116+
street = addr && [ addr["road"], addr["house_number"] ].compact.join(" ").presence
117+
{ address: data["display_name"], city_name: city_name, street: street }
117118
rescue => e
118119
Rails.logger.warn("Nominatim error: #{e.message}")
119120
nil

app/views/games/_form.html.erb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,9 @@
211211
data-court-picker-default-lng-value="60.597465"
212212
data-court-picker-courts-value="<%= json_escape(sorted_courts.map { |c|
213213
lat, lng = c.coordinates.to_s.split(',').map(&:to_f)
214-
{ id: c.id, name: c.name, lat: lat, lng: lng, surfaces: c.surfaces, environments: c.environments,
215-
city: c.city_name.to_s, country: locations[:city_country][c.city_name].to_s }
214+
{ id: c.id, name: c.name, street: c.street.to_s, lat: lat, lng: lng, surfaces: c.surfaces,
215+
environments: c.environments, city: c.city_name.to_s,
216+
country: locations[:city_country][c.city_name].to_s }
216217
}.to_json) %>"
217218
data-court-picker-country-cities-value="<%= json_escape(locations[:cities_by_country].to_json) %>"
218219
data-court-picker-city-placeholder-value="<%= t("games.form.court_city_any") %>"
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class AddStreetToCourts < ActiveRecord::Migration[8.1]
2+
def change
3+
add_column :courts, :street, :string
4+
end
5+
end

db/schema.rb

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/tasks/geocoding.rake

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,18 @@ namespace :geocoding do
1818

1919
puts "Done. All jobs enqueued."
2020
end
21+
22+
desc "Backfill street for courts that have coordinates but no street"
23+
task backfill_court_streets: :environment do
24+
courts = Court.where(street: [ nil, "" ]).where.not(coordinates: [ nil, "" ])
25+
total = courts.count
26+
puts "Courts to backfill: #{total}"
27+
28+
courts.find_each.with_index do |court, i|
29+
Geocoding::FetchCourtAddressJob.perform_later(court.id)
30+
puts " [#{i + 1}/#{total}] Enqueued Court##{court.id}"
31+
end
32+
33+
puts "Done. All jobs enqueued."
34+
end
2135
end

test/controllers/games_controller_test.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,24 @@ class GamesControllerTest < ActionDispatch::IntegrationTest
601601
only&.destroy
602602
end
603603

604+
test "game form carries the street of every court so the picker can tell namesakes apart" do
605+
greenwich = Court.create!(name: "Squash Territory", city_name: "Yekaterinburg", street: "8 Marta St 46",
606+
moderation_status: "approved")
607+
elmash = Court.create!(name: "Squash Territory", city_name: "Yekaterinburg", street: "Kosmonavtov Ave 108",
608+
moderation_status: "approved")
609+
post session_url, params: { email: "court-picker-street@example.com" }
610+
611+
get new_game_url
612+
613+
assert_response :success
614+
courts = JSON.parse(css_select("[data-court-picker-courts-value]").first["data-court-picker-courts-value"])
615+
streets = courts.select { |court| court["name"] == "Squash Territory" }.map { |court| court["street"] }
616+
assert_equal [ "8 Marta St 46", "Kosmonavtov Ave 108" ], streets.sort
617+
ensure
618+
greenwich&.destroy
619+
elmash&.destroy
620+
end
621+
604622
test "game form offers the comment field" do
605623
post session_url, params: { email: "comment_form_user@example.com" }
606624

test/jobs/geocoding/fetch_court_address_job_test.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,28 @@ class Geocoding::FetchCourtAddressJobTest < ActiveSupport::TestCase
1919
end
2020
end
2121

22+
test "stores the street so the court list can tell namesakes apart" do
23+
with_memory_cache do
24+
with_stubbed_resolver({ address: "Tverskaya St 1, Moscow, Russia", city_name: "Moscow", street: "Tverskaya St 1" }) do
25+
Geocoding::FetchCourtAddressJob.new.perform(@court.id)
26+
27+
assert_equal "Tverskaya St 1", @court.reload.street
28+
end
29+
end
30+
end
31+
32+
test "keeps the stored street when the resolver has none" do
33+
@court.update_columns(street: "Tverskaya St 1")
34+
35+
with_memory_cache do
36+
with_stubbed_resolver({ address: "Somewhere, Moscow, Russia", city_name: "Moscow", street: nil }) do
37+
Geocoding::FetchCourtAddressJob.new.perform(@court.id)
38+
39+
assert_equal "Tverskaya St 1", @court.reload.street
40+
end
41+
end
42+
end
43+
2244
test "does not write cache when resolver returns nil" do
2345
with_memory_cache do
2446
with_stubbed_resolver(nil) do

test/services/geocoding/address_resolver_test.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class Geocoding::AddressResolverTest < ActiveSupport::TestCase
3838
result = resolver.resolve(55.75, 37.62)
3939
assert_equal "Tverskaya St 1, Moscow, Russia", result[:address]
4040
assert_equal "Moscow", result[:city_name]
41+
assert_equal "Tverskaya St 1", result[:street]
4142
end
4243
end
4344
end
@@ -49,14 +50,15 @@ class Geocoding::AddressResolverTest < ActiveSupport::TestCase
4950
test "resolve falls back to Nominatim when Google key is absent" do
5051
nominatim_payload = {
5152
"display_name" => "Tverskaya St, Moscow, Russia",
52-
"address" => { "city" => "Moscow" }
53+
"address" => { "city" => "Moscow", "road" => "Tverskaya St", "house_number" => "1" }
5354
}
5455

5556
with_env("GOOGLE_GEOCODING_API_KEY" => "") do
5657
with_stubbed_fetch_json(nominatim_payload) do |resolver|
5758
result = resolver.resolve(55.75, 37.62)
5859
assert_equal "Tverskaya St, Moscow, Russia", result[:address]
5960
assert_equal "Moscow", result[:city_name]
61+
assert_equal "Tverskaya St 1", result[:street]
6062
end
6163
end
6264
end

0 commit comments

Comments
 (0)