diff --git a/Gemfile.lock b/Gemfile.lock index e566ce3e..3beeac65 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -160,7 +160,7 @@ GEM dotenv (= 3.1.8) railties (>= 6.1) drb (2.2.3) - erb (5.0.1) + erb (5.0.2) erubi (1.13.1) et-orbi (1.2.11) tzinfo @@ -194,7 +194,7 @@ GEM actionpack (>= 6.0.0) activesupport (>= 6.0.0) railties (>= 6.0.0) - io-console (0.8.0) + io-console (0.8.1) irb (1.15.2) pp (>= 0.6.0) rdoc (>= 4.0.0) @@ -243,7 +243,7 @@ GEM multi_json (1.15.0) multi_xml (0.7.1) bigdecimal (~> 3.1) - net-imap (0.5.8) + net-imap (0.5.9) date net-protocol net-pop (0.1.2) @@ -342,7 +342,7 @@ GEM zeitwerk (~> 2.6) rainbow (3.1.1) rake (13.3.0) - rdoc (6.14.1) + rdoc (6.14.2) erb psych (>= 4.0.0) redis (5.4.0) @@ -350,7 +350,7 @@ GEM redis-client (0.24.0) connection_pool regexp_parser (2.10.0) - reline (0.6.1) + reline (0.6.2) io-console (~> 0.5) request_store (1.7.0) rack (>= 1.4) @@ -496,7 +496,7 @@ GEM hashdiff (>= 0.4.0, < 2.0.0) webrick (1.9.1) websocket (1.2.11) - websocket-driver (0.7.7) + websocket-driver (0.8.0) base64 websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index a456452f..0300deae 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -5,10 +5,9 @@ class StatsController < ApplicationController before_action :authenticate_active_user!, only: %i[update update_all] def index - @stats = current_user.stats.group_by(&:year).transform_values { |stats| stats.sort_by(&:updated_at).reverse }.sort.reverse - @points_total = current_user.tracked_points.count - @points_reverse_geocoded = current_user.total_reverse_geocoded_points - @points_reverse_geocoded_without_data = current_user.total_reverse_geocoded_points_without_data + @stats = build_stats + assign_points_statistics + @year_distances = precompute_year_distances end def show @@ -43,4 +42,30 @@ class StatsController < ApplicationController redirect_to stats_path, notice: 'Stats are being updated', status: :see_other end + + private + + def assign_points_statistics + points_stats = ::StatsQuery.new(current_user).points_stats + + @points_total = points_stats[:total] + @points_reverse_geocoded = points_stats[:geocoded] + @points_reverse_geocoded_without_data = points_stats[:without_data] + end + + def precompute_year_distances + year_distances = {} + + @stats.each do |year, _stats| + year_distances[year] = Stat.year_distance(year, current_user) + end + + year_distances + end + + def build_stats + current_user.stats.group_by(&:year).transform_values do |stats| + stats.sort_by(&:updated_at).reverse + end.sort.reverse + end end diff --git a/app/jobs/tracks/create_job.rb b/app/jobs/tracks/create_job.rb index 919e5f82..a65805c4 100644 --- a/app/jobs/tracks/create_job.rb +++ b/app/jobs/tracks/create_job.rb @@ -27,6 +27,8 @@ class Tracks::CreateJob < ApplicationJob end def create_error_notification(user, error) + return unless DawarichSettings.self_hosted? + Notifications::Create.new( user: user, kind: :error, diff --git a/app/jobs/trips/calculate_all_job.rb b/app/jobs/trips/calculate_all_job.rb index 0500881c..3710df3e 100644 --- a/app/jobs/trips/calculate_all_job.rb +++ b/app/jobs/trips/calculate_all_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Trips::CalculateAllJob < ApplicationJob - queue_as :default + queue_as :trips def perform(trip_id, distance_unit = 'km') Trips::CalculatePathJob.perform_later(trip_id) diff --git a/app/jobs/trips/calculate_countries_job.rb b/app/jobs/trips/calculate_countries_job.rb index e63365d3..ed5ee884 100644 --- a/app/jobs/trips/calculate_countries_job.rb +++ b/app/jobs/trips/calculate_countries_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Trips::CalculateCountriesJob < ApplicationJob - queue_as :default + queue_as :trips def perform(trip_id, distance_unit) trip = Trip.find(trip_id) diff --git a/app/jobs/trips/calculate_distance_job.rb b/app/jobs/trips/calculate_distance_job.rb index 8a28e06f..15ff83c4 100644 --- a/app/jobs/trips/calculate_distance_job.rb +++ b/app/jobs/trips/calculate_distance_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Trips::CalculateDistanceJob < ApplicationJob - queue_as :default + queue_as :trips def perform(trip_id, distance_unit) trip = Trip.find(trip_id) diff --git a/app/jobs/trips/calculate_path_job.rb b/app/jobs/trips/calculate_path_job.rb index 711cfef8..f1323c5f 100644 --- a/app/jobs/trips/calculate_path_job.rb +++ b/app/jobs/trips/calculate_path_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Trips::CalculatePathJob < ApplicationJob - queue_as :default + queue_as :trips def perform(trip_id) trip = Trip.find(trip_id) diff --git a/app/models/country.rb b/app/models/country.rb index 9ef64687..e40a0c6d 100644 --- a/app/models/country.rb +++ b/app/models/country.rb @@ -12,6 +12,8 @@ class Country < ApplicationRecord end def self.names_to_iso_a2 - pluck(:name, :iso_a2).to_h + Rails.cache.fetch('countries_names_to_iso_a2', expires_in: 1.day) do + pluck(:name, :iso_a2).to_h + end end end diff --git a/app/queries/stats_query.rb b/app/queries/stats_query.rb new file mode 100644 index 00000000..0192a8c8 --- /dev/null +++ b/app/queries/stats_query.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class StatsQuery + def initialize(user) + @user = user + end + + def points_stats + sql = ActiveRecord::Base.sanitize_sql_array([ + <<~SQL.squish, + SELECT + COUNT(id) as total, + COUNT(reverse_geocoded_at) as geocoded, + COUNT(CASE WHEN geodata = '{}'::jsonb THEN 1 END) as without_data + FROM points + WHERE user_id = ? + SQL + user.id + ]) + + result = Point.connection.select_one(sql) + + { + total: result['total'].to_i, + geocoded: result['geocoded'].to_i, + without_data: result['without_data'].to_i + } + end + + private + + attr_reader :user +end diff --git a/app/views/stats/_year.html.erb b/app/views/stats/_year.html.erb index 886e2c96..3d8989b8 100644 --- a/app/views/stats/_year.html.erb +++ b/app/views/stats/_year.html.erb @@ -4,7 +4,7 @@
<%= column_chart( - Stat.year_distance(year, current_user), + @year_distances[year], height: '200px', suffix: " #{current_user.safe_settings.distance_unit}", xtitle: 'Days', diff --git a/app/views/stats/index.html.erb b/app/views/stats/index.html.erb index 96050095..2f14fa36 100644 --- a/app/views/stats/index.html.erb +++ b/app/views/stats/index.html.erb @@ -82,7 +82,7 @@
<% end %> <%= column_chart( - Stat.year_distance(year, current_user).map { |month_name, distance_meters| + @year_distances[year].map { |month_name, distance_meters| [month_name, Stat.convert_distance(distance_meters, current_user.safe_settings.distance_unit).round(2)] }, height: '200px', diff --git a/config/environments/production.rb b/config/environments/production.rb index 1e4b392a..53eedb18 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -29,7 +29,7 @@ Rails.application.configure do # config.assets.css_compressor = :sass # Do not fallback to assets pipeline if a precompiled asset is missed. - config.assets.compile = true + config.assets.compile = false config.assets.content_type = { geojson: 'application/geo+json' diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 87109364..c1966a7f 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -6,6 +6,7 @@ - imports - exports - stats + - trips - tracks - reverse_geocoding - visit_suggesting diff --git a/spec/jobs/tracks/create_job_spec.rb b/spec/jobs/tracks/create_job_spec.rb index bc2648d9..bddf430a 100644 --- a/spec/jobs/tracks/create_job_spec.rb +++ b/spec/jobs/tracks/create_job_spec.rb @@ -151,4 +151,50 @@ RSpec.describe Tracks::CreateJob, type: :job do expect(described_class.new.queue_name).to eq('tracks') end end + + context 'when self-hosted' do + let(:generator_instance) { instance_double(Tracks::Generator) } + let(:notification_service) { instance_double(Notifications::Create) } + let(:error_message) { 'Something went wrong' } + + before do + allow(DawarichSettings).to receive(:self_hosted?).and_return(true) + allow(Tracks::Generator).to receive(:new).and_return(generator_instance) + allow(generator_instance).to receive(:call).and_raise(StandardError, error_message) + allow(Notifications::Create).to receive(:new).and_return(notification_service) + allow(notification_service).to receive(:call) + end + + it 'creates a failure notification when self-hosted' do + described_class.new.perform(user.id) + + expect(Notifications::Create).to have_received(:new).with( + user: user, + kind: :error, + title: 'Track Generation Failed', + content: "Failed to generate tracks from your location data: #{error_message}" + ) + expect(notification_service).to have_received(:call) + end + end + + context 'when not self-hosted' do + let(:generator_instance) { instance_double(Tracks::Generator) } + let(:notification_service) { instance_double(Notifications::Create) } + let(:error_message) { 'Something went wrong' } + + before do + allow(DawarichSettings).to receive(:self_hosted?).and_return(false) + allow(Tracks::Generator).to receive(:new).and_return(generator_instance) + allow(generator_instance).to receive(:call).and_raise(StandardError, error_message) + allow(Notifications::Create).to receive(:new).and_return(notification_service) + allow(notification_service).to receive(:call) + end + + it 'does not create a failure notification' do + described_class.new.perform(user.id) + + expect(notification_service).not_to have_received(:call) + end + end end diff --git a/spec/queries/stats_query_spec.rb b/spec/queries/stats_query_spec.rb new file mode 100644 index 00000000..d4d8517f --- /dev/null +++ b/spec/queries/stats_query_spec.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe StatsQuery do + describe '#points_stats' do + subject(:points_stats) { described_class.new(user).points_stats } + + let(:user) { create(:user) } + let!(:import) { create(:import, user: user) } + + context 'when user has no points' do + it 'returns zero counts for all statistics' do + expect(points_stats).to eq({ + total: 0, + geocoded: 0, + without_data: 0 + }) + end + end + + context 'when user has points' do + let!(:geocoded_point_with_data) do + create(:point, + user: user, + import: import, + reverse_geocoded_at: Time.current, + geodata: { 'address' => '123 Main St' }) + end + + let!(:geocoded_point_without_data) do + create(:point, + user: user, + import: import, + reverse_geocoded_at: Time.current, + geodata: {}) + end + + let!(:non_geocoded_point) do + create(:point, + user: user, + import: import, + reverse_geocoded_at: nil, + geodata: { 'some' => 'data' }) + end + + it 'returns correct counts for all statistics' do + expect(points_stats).to eq({ + total: 3, + geocoded: 2, + without_data: 1 + }) + end + + context 'when another user has points' do + let(:other_user) { create(:user) } + let!(:other_import) { create(:import, user: other_user) } + let!(:other_point) do + create(:point, + user: other_user, + import: other_import, + reverse_geocoded_at: Time.current, + geodata: { 'address' => 'Other Address' }) + end + + it 'only counts points for the specified user' do + expect(points_stats).to eq({ + total: 3, + geocoded: 2, + without_data: 1 + }) + end + end + end + + context 'when all points are geocoded with data' do + before do + create_list(:point, 5, + user: user, + import: import, + reverse_geocoded_at: Time.current, + geodata: { 'address' => 'Some Address' }) + end + + it 'returns correct statistics' do + expect(points_stats).to eq({ + total: 5, + geocoded: 5, + without_data: 0 + }) + end + end + + context 'when all points are without geodata' do + before do + create_list(:point, 3, + user: user, + import: import, + reverse_geocoded_at: Time.current, + geodata: {}) + end + + it 'returns correct statistics' do + expect(points_stats).to eq({ + total: 3, + geocoded: 3, + without_data: 3 + }) + end + end + + context 'when all points are not geocoded' do + before do + create_list(:point, 4, + user: user, + import: import, + reverse_geocoded_at: nil, + geodata: { 'some' => 'data' }) + end + + it 'returns correct statistics' do + expect(points_stats).to eq({ + total: 4, + geocoded: 0, + without_data: 0 + }) + end + end + end +end \ No newline at end of file