From bc36882e73e1c0936b418591b179058acc225923 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sat, 12 Jul 2025 11:21:38 +0200 Subject: [PATCH] Add name fetcher for places and visits --- .gitignore | 7 + app/jobs/places/bulk_name_fetching_job.rb | 11 + app/jobs/places/name_fetching_job.rb | 11 + app/services/places/name_fetcher.rb | 33 +++ config/schedule.yml | 5 + config/sidekiq.yml | 1 + package-lock.json | 131 +++++++++++ package.json | 7 +- .../places/bulk_name_fetching_job_spec.rb | 26 +++ spec/jobs/places/name_fetching_job_spec.rb | 29 +++ spec/services/places/name_fetcher_spec.rb | 220 ++++++++++++++++++ 11 files changed, 480 insertions(+), 1 deletion(-) create mode 100644 app/jobs/places/bulk_name_fetching_job.rb create mode 100644 app/jobs/places/name_fetching_job.rb create mode 100644 app/services/places/name_fetcher.rb create mode 100644 spec/jobs/places/bulk_name_fetching_job_spec.rb create mode 100644 spec/jobs/places/name_fetching_job_spec.rb create mode 100644 spec/services/places/name_fetcher_spec.rb diff --git a/.gitignore b/.gitignore index 4fe8d20f..1510b45b 100644 --- a/.gitignore +++ b/.gitignore @@ -76,3 +76,10 @@ Makefile /db/*.sqlite3 /db/*.sqlite3-shm /db/*.sqlite3-wal + +# Playwright +node_modules/ +/test-results/ +/playwright-report/ +/blob-report/ +/playwright/.cache/ diff --git a/app/jobs/places/bulk_name_fetching_job.rb b/app/jobs/places/bulk_name_fetching_job.rb new file mode 100644 index 00000000..9c8eeea4 --- /dev/null +++ b/app/jobs/places/bulk_name_fetching_job.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class Places::BulkNameFetchingJob < ApplicationJob + queue_as :default + + def perform + Place.where(name: Place::DEFAULT_NAME).find_each do |place| + Places::NameFetchingJob.perform_later(place.id) + end + end +end diff --git a/app/jobs/places/name_fetching_job.rb b/app/jobs/places/name_fetching_job.rb new file mode 100644 index 00000000..e40391f0 --- /dev/null +++ b/app/jobs/places/name_fetching_job.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class Places::NameFetchingJob < ApplicationJob + queue_as :places + + def perform(place_id) + place = Place.find(place_id) + + Places::NameFetcher.new(place).call + end +end diff --git a/app/services/places/name_fetcher.rb b/app/services/places/name_fetcher.rb new file mode 100644 index 00000000..1bdf5821 --- /dev/null +++ b/app/services/places/name_fetcher.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Places + class NameFetcher + def initialize(place) + @place = place + end + + def call + geodata = Geocoder.search([@place.lat, @place.lon], units: :km, limit: 1, distance_sort: true).first + + return if geodata.blank? + + properties = geodata.data&.dig('properties') + return if properties.blank? + + ActiveRecord::Base.transaction do + @place.name = properties['name'] + @place.city = properties['city'] + @place.country = properties['country'] + @place.geodata = geodata.data if DawarichSettings.store_geodata? + @place.save! + + @place + .visits + .where(name: Place::DEFAULT_NAME) + .update_all(name: properties['name']) + + @place + end + end + end +end diff --git a/config/schedule.yml b/config/schedule.yml index a184df13..863296df 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -34,3 +34,8 @@ tracks_bulk_creating_job: cron: "10 0 * * *" # every day at 00:10 class: "Tracks::BulkCreatingJob" queue: tracks + +place_name_fetching_job: + cron: "30 0 * * *" # every day at 00:30 + class: "Places::NameFetchingJob" + queue: places diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 9ef06b6f..87109364 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -9,3 +9,4 @@ - tracks - reverse_geocoding - visit_suggesting + - places diff --git a/package-lock.json b/package-lock.json index 16af91c8..2ead76e3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,10 @@ "postcss": "^8.4.49", "trix": "^2.1.15" }, + "devDependencies": { + "@playwright/test": "^1.54.1", + "@types/node": "^24.0.13" + }, "engines": { "node": "18.17.1", "npm": "9.6.7" @@ -34,6 +38,22 @@ "@rails/actioncable": "^7.0" } }, + "node_modules/@playwright/test": { + "version": "1.54.1", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.54.1.tgz", + "integrity": "sha512-FS8hQ12acieG2dYSksmLOF7BNxnVf2afRJdCuM1eMSxj6QTSE6G4InGF7oApGgDb65MX7AwMVlIkpru0yZA4Xw==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "playwright": "1.54.1" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=18" + } + }, "node_modules/@rails/actioncable": { "version": "7.1.3", "resolved": "https://registry.npmjs.org/@rails/actioncable/-/actioncable-7.1.3.tgz", @@ -58,6 +78,16 @@ "spark-md5": "^3.0.1" } }, + "node_modules/@types/node": { + "version": "24.0.13", + "resolved": "https://registry.npmjs.org/@types/node/-/node-24.0.13.tgz", + "integrity": "sha512-Qm9OYVOFHFYg3wJoTSrz80hoec5Lia/dPp84do3X7dZvLikQvM1YpmvTBEdIr/e+U8HTkFjLHLnl78K/qjf+jQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "undici-types": "~7.8.0" + } + }, "node_modules/@types/trusted-types": { "version": "2.0.7", "resolved": "https://registry.npmjs.org/@types/trusted-types/-/trusted-types-2.0.7.tgz", @@ -133,6 +163,21 @@ "resolved": "https://registry.npmjs.org/fastparse/-/fastparse-1.1.2.tgz", "integrity": "sha512-483XLLxTVIwWK3QTrMGRqUfUpoOs/0hbQrl2oz4J0pAcm3A3bu84wxTFqGqkJzewCLdME38xJLJAxBABfQT8sQ==" }, + "node_modules/fsevents": { + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz", + "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": "^8.16.0 || ^10.6.0 || >=11.0.0" + } + }, "node_modules/leaflet": { "version": "1.9.4", "resolved": "https://registry.npmjs.org/leaflet/-/leaflet-1.9.4.tgz", @@ -160,6 +205,38 @@ "resolved": "https://registry.npmjs.org/picocolors/-/picocolors-1.1.1.tgz", "integrity": "sha512-xceH2snhtb5M9liqDsmEw56le376mTZkEX/jEb/RxNFyegNul7eNslCXP9FDj/Lcu0X8KEyMceP2ntpaHrDEVA==" }, + "node_modules/playwright": { + "version": "1.54.1", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.54.1.tgz", + "integrity": "sha512-peWpSwIBmSLi6aW2auvrUtf2DqY16YYcCMO8rTVx486jKmDTJg7UAhyrraP98GB8BoPURZP8+nxO7TSd4cPr5g==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "playwright-core": "1.54.1" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=18" + }, + "optionalDependencies": { + "fsevents": "2.3.2" + } + }, + "node_modules/playwright-core": { + "version": "1.54.1", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.54.1.tgz", + "integrity": "sha512-Nbjs2zjj0htNhzgiy5wu+3w09YetDx5pkrpI/kZotDlDUaYk0HVA5xrBVPdow4SAUIlhgKcJeJg4GRKW6xHusA==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "playwright-core": "cli.js" + }, + "engines": { + "node": ">=18" + } + }, "node_modules/postcss": { "version": "8.5.3", "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.3.tgz", @@ -226,6 +303,13 @@ "dependencies": { "dompurify": "^3.2.5" } + }, + "node_modules/undici-types": { + "version": "7.8.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.8.0.tgz", + "integrity": "sha512-9UJ2xGDvQ43tYyVMpuHlsgApydB8ZKfVYTsLDhXkFL/6gfkp+U8xTGdh8pMJv1SpZna0zxG1DwsKZsreLbXBxw==", + "dev": true, + "license": "MIT" } }, "dependencies": { @@ -243,6 +327,15 @@ "@rails/actioncable": "^7.0" } }, + "@playwright/test": { + "version": "1.54.1", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.54.1.tgz", + "integrity": "sha512-FS8hQ12acieG2dYSksmLOF7BNxnVf2afRJdCuM1eMSxj6QTSE6G4InGF7oApGgDb65MX7AwMVlIkpru0yZA4Xw==", + "dev": true, + "requires": { + "playwright": "1.54.1" + } + }, "@rails/actioncable": { "version": "7.1.3", "resolved": "https://registry.npmjs.org/@rails/actioncable/-/actioncable-7.1.3.tgz", @@ -264,6 +357,15 @@ "spark-md5": "^3.0.1" } }, + "@types/node": { + "version": "24.0.13", + "resolved": "https://registry.npmjs.org/@types/node/-/node-24.0.13.tgz", + "integrity": "sha512-Qm9OYVOFHFYg3wJoTSrz80hoec5Lia/dPp84do3X7dZvLikQvM1YpmvTBEdIr/e+U8HTkFjLHLnl78K/qjf+jQ==", + "dev": true, + "requires": { + "undici-types": "~7.8.0" + } + }, "@types/trusted-types": { "version": "2.0.7", "resolved": "https://registry.npmjs.org/@types/trusted-types/-/trusted-types-2.0.7.tgz", @@ -318,6 +420,13 @@ "resolved": "https://registry.npmjs.org/fastparse/-/fastparse-1.1.2.tgz", "integrity": "sha512-483XLLxTVIwWK3QTrMGRqUfUpoOs/0hbQrl2oz4J0pAcm3A3bu84wxTFqGqkJzewCLdME38xJLJAxBABfQT8sQ==" }, + "fsevents": { + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz", + "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==", + "dev": true, + "optional": true + }, "leaflet": { "version": "1.9.4", "resolved": "https://registry.npmjs.org/leaflet/-/leaflet-1.9.4.tgz", @@ -333,6 +442,22 @@ "resolved": "https://registry.npmjs.org/picocolors/-/picocolors-1.1.1.tgz", "integrity": "sha512-xceH2snhtb5M9liqDsmEw56le376mTZkEX/jEb/RxNFyegNul7eNslCXP9FDj/Lcu0X8KEyMceP2ntpaHrDEVA==" }, + "playwright": { + "version": "1.54.1", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.54.1.tgz", + "integrity": "sha512-peWpSwIBmSLi6aW2auvrUtf2DqY16YYcCMO8rTVx486jKmDTJg7UAhyrraP98GB8BoPURZP8+nxO7TSd4cPr5g==", + "dev": true, + "requires": { + "fsevents": "2.3.2", + "playwright-core": "1.54.1" + } + }, + "playwright-core": { + "version": "1.54.1", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.54.1.tgz", + "integrity": "sha512-Nbjs2zjj0htNhzgiy5wu+3w09YetDx5pkrpI/kZotDlDUaYk0HVA5xrBVPdow4SAUIlhgKcJeJg4GRKW6xHusA==", + "dev": true + }, "postcss": { "version": "8.5.3", "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.3.tgz", @@ -368,6 +493,12 @@ "requires": { "dompurify": "^3.2.5" } + }, + "undici-types": { + "version": "7.8.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.8.0.tgz", + "integrity": "sha512-9UJ2xGDvQ43tYyVMpuHlsgApydB8ZKfVYTsLDhXkFL/6gfkp+U8xTGdh8pMJv1SpZna0zxG1DwsKZsreLbXBxw==", + "dev": true } } } diff --git a/package.json b/package.json index 41a83df8..927d52fb 100644 --- a/package.json +++ b/package.json @@ -10,5 +10,10 @@ "engines": { "node": "18.17.1", "npm": "9.6.7" - } + }, + "devDependencies": { + "@playwright/test": "^1.54.1", + "@types/node": "^24.0.13" + }, + "scripts": {} } diff --git a/spec/jobs/places/bulk_name_fetching_job_spec.rb b/spec/jobs/places/bulk_name_fetching_job_spec.rb new file mode 100644 index 00000000..49e72616 --- /dev/null +++ b/spec/jobs/places/bulk_name_fetching_job_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Places::BulkNameFetchingJob, type: :job do + describe '#perform' do + let!(:place1) { create(:place, name: Place::DEFAULT_NAME) } + let!(:place2) { create(:place, name: Place::DEFAULT_NAME) } + let!(:place3) { create(:place, name: 'Other place') } + + it 'enqueues name fetching job for each place with default name' do + expect { described_class.perform_now }.to \ + have_enqueued_job(Places::NameFetchingJob).exactly(2).times + end + + it 'does not process places with custom names' do + expect { described_class.perform_now }.not_to \ + have_enqueued_job(Places::NameFetchingJob).with(place3.id) + end + + it 'can be enqueued' do + expect { described_class.perform_later }.to have_enqueued_job(described_class) + .on_queue('default') + end + end +end diff --git a/spec/jobs/places/name_fetching_job_spec.rb b/spec/jobs/places/name_fetching_job_spec.rb new file mode 100644 index 00000000..d868f845 --- /dev/null +++ b/spec/jobs/places/name_fetching_job_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Places::NameFetchingJob, type: :job do + describe '#perform' do + let(:place) { create(:place, name: Place::DEFAULT_NAME) } + let(:name_fetcher) { instance_double(Places::NameFetcher) } + + before do + allow(Places::NameFetcher).to receive(:new).with(place).and_return(name_fetcher) + allow(name_fetcher).to receive(:call) + end + + it 'finds the place and calls NameFetcher' do + expect(Place).to receive(:find).with(place.id).and_return(place) + expect(Places::NameFetcher).to receive(:new).with(place) + expect(name_fetcher).to receive(:call) + + described_class.perform_now(place.id) + end + + it 'can be enqueued' do + expect { described_class.perform_later(place.id) }.to have_enqueued_job(described_class) + .with(place.id) + .on_queue('places') + end + end +end diff --git a/spec/services/places/name_fetcher_spec.rb b/spec/services/places/name_fetcher_spec.rb new file mode 100644 index 00000000..a2e72b76 --- /dev/null +++ b/spec/services/places/name_fetcher_spec.rb @@ -0,0 +1,220 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Places::NameFetcher do + describe '#call' do + subject(:service) { described_class.new(place) } + + let(:place) do + create( + :place, + name: Place::DEFAULT_NAME, + city: nil, + country: nil, + geodata: {}, + lonlat: 'POINT(10.0 10.0)' + ) + end + + let(:geocoder_result) do + double( + 'geocoder_result', + data: { + 'properties' => { + 'name' => 'Central Park', + 'city' => 'New York', + 'country' => 'United States' + } + } + ) + end + + before do + allow(Geocoder).to receive(:search).and_return([geocoder_result]) + end + + context 'when geocoding is successful' do + it 'calls Geocoder with correct parameters' do + expect(Geocoder).to receive(:search) + .with([place.lat, place.lon], units: :km, limit: 1, distance_sort: true) + .and_return([geocoder_result]) + + service.call + end + + it 'updates place name from geocoder data' do + expect { service.call }.to change(place, :name) + .from(Place::DEFAULT_NAME) + .to('Central Park') + end + + it 'updates place city from geocoder data' do + expect { service.call }.to change(place, :city) + .from(nil) + .to('New York') + end + + it 'updates place country from geocoder data' do + expect { service.call }.to change(place, :country) + .from(nil) + .to('United States') + end + + it 'saves the place' do + expect(place).to receive(:save!) + + service.call + end + + context 'when DawarichSettings.store_geodata? is enabled' do + before do + allow(DawarichSettings).to receive(:store_geodata?).and_return(true) + end + + it 'stores geodata in the place' do + expect { service.call }.to change(place, :geodata) + .from({}) + .to(geocoder_result.data) + end + end + + context 'when DawarichSettings.store_geodata? is disabled' do + before do + allow(DawarichSettings).to receive(:store_geodata?).and_return(false) + end + + it 'does not store geodata in the place' do + expect { service.call }.not_to change(place, :geodata) + end + end + + context 'when place has visits with default name' do + let!(:visit_with_default_name) do + create(:visit, name: Place::DEFAULT_NAME) + end + let!(:visit_with_custom_name) do + create(:visit, name: 'Custom Visit Name') + end + + before do + place.visits << visit_with_default_name + place.visits << visit_with_custom_name + end + + it 'updates visits with default name to the new place name' do + expect { service.call }.to \ + change { visit_with_default_name.reload.name } + .from(Place::DEFAULT_NAME) + .to('Central Park') + end + + it 'does not update visits with custom names' do + expect { service.call }.not_to \ + change { visit_with_custom_name.reload.name } + end + end + + context 'when using transactions' do + it 'wraps updates in a transaction' do + expect(ActiveRecord::Base).to \ + receive(:transaction).and_call_original + + service.call + end + + it 'rolls back changes if save fails' do + allow(place).to receive(:save!).and_raise(ActiveRecord::RecordInvalid) + + expect { service.call }.to raise_error(ActiveRecord::RecordInvalid) + expect(place.reload.name).to eq(Place::DEFAULT_NAME) + end + end + + it 'returns the updated place' do + result = service.call + expect(result).to eq(place) + expect(result.name).to eq('Central Park') + end + end + + context 'when geocoding returns no results' do + before do + allow(Geocoder).to receive(:search).and_return([]) + end + + it 'returns nil' do + expect(service.call).to be_nil + end + + it 'does not update the place' do + expect { service.call }.not_to change(place, :name) + end + + it 'does not call save on the place' do + expect(place).not_to receive(:save!) + + service.call + end + end + + context 'when geocoding returns nil result' do + before do + allow(Geocoder).to receive(:search).and_return([nil]) + end + + it 'returns nil' do + expect(service.call).to be_nil + end + + it 'does not update the place' do + expect { service.call }.not_to change(place, :name) + end + end + + context 'when geocoder result has missing properties' do + let(:incomplete_geocoder_result) do + double( + 'geocoder_result', + data: { + 'properties' => { + 'name' => 'Partial Place', + 'city' => nil, + 'country' => 'United States' + } + } + ) + end + + before do + allow(Geocoder).to receive(:search).and_return([incomplete_geocoder_result]) + end + + it 'updates place with available data' do + service.call + + expect(place.name).to eq('Partial Place') + expect(place.city).to be_nil + expect(place.country).to eq('United States') + end + end + + context 'when geocoder result has no properties' do + let(:no_properties_result) do + double('geocoder_result', data: {}) + end + + before do + allow(Geocoder).to receive(:search).and_return([no_properties_result]) + end + + it 'handles missing properties gracefully' do + expect { service.call }.not_to raise_error + + expect(place.name).to eq(Place::DEFAULT_NAME) + expect(place.city).to be_nil + expect(place.country).to be_nil + end + end + end +end