diff --git a/Gemfile b/Gemfile index 0e11a1fa..614a2e95 100644 --- a/Gemfile +++ b/Gemfile @@ -77,5 +77,4 @@ group :development do gem 'database_consistency', require: false gem 'foreman' gem 'rubocop-rails', require: false - gem 'bullet' end diff --git a/Gemfile.lock b/Gemfile.lock index 7d65bab2..f647ff71 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -163,7 +163,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 @@ -197,7 +197,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) @@ -246,7 +246,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) @@ -256,18 +256,18 @@ GEM net-smtp (0.5.1) net-protocol nio4r (2.7.4) - nokogiri (1.18.8) + nokogiri (1.18.9) mini_portile2 (~> 2.8.2) racc (~> 1.4) - nokogiri (1.18.8-aarch64-linux-gnu) + nokogiri (1.18.9-aarch64-linux-gnu) racc (~> 1.4) - nokogiri (1.18.8-arm-linux-gnu) + nokogiri (1.18.9-arm-linux-gnu) racc (~> 1.4) - nokogiri (1.18.8-arm64-darwin) + nokogiri (1.18.9-arm64-darwin) racc (~> 1.4) - nokogiri (1.18.8-x86_64-darwin) + nokogiri (1.18.9-x86_64-darwin) racc (~> 1.4) - nokogiri (1.18.8-x86_64-linux-gnu) + nokogiri (1.18.9-x86_64-linux-gnu) racc (~> 1.4) oj (3.16.11) bigdecimal (>= 3.0) @@ -345,7 +345,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) @@ -353,7 +353,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) @@ -478,7 +478,7 @@ GEM tailwindcss-ruby (3.4.17-arm64-darwin) tailwindcss-ruby (3.4.17-x86_64-darwin) tailwindcss-ruby (3.4.17-x86_64-linux) - thor (1.3.2) + thor (1.4.0) timeout (0.4.3) turbo-rails (2.0.16) actionpack (>= 7.1.0) @@ -500,7 +500,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 08074462..0300deae 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -5,30 +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 do |stats| - stats.sort_by(&:updated_at).reverse - end.sort.reverse - - # Single aggregated query to replace 3 separate COUNT queries - result = current_user.tracked_points.connection.execute(<<~SQL.squish) - SELECT#{' '} - COUNT(*) as total, - COUNT(reverse_geocoded_at) as geocoded, - COUNT(CASE WHEN geodata = '{}' THEN 1 END) as without_data - FROM points#{' '} - WHERE user_id = #{current_user.id} - SQL - - row = result.first - @points_total = row['total'].to_i - @points_reverse_geocoded = row['geocoded'].to_i - @points_reverse_geocoded_without_data = row['without_data'].to_i - - # Precompute year distance data to avoid N+1 queries in view - @year_distances = {} - @stats.each do |year, _stats| - @year_distances[year] = Stat.year_distance(year, current_user) - end + @stats = build_stats + assign_points_statistics + @year_distances = precompute_year_distances end def show @@ -63,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/queries/stats_query.rb b/app/queries/stats_query.rb new file mode 100644 index 00000000..e3ef3503 --- /dev/null +++ b/app/queries/stats_query.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class StatsQuery + def initialize(user) + @user = user + end + + def points_stats + result = user.tracked_points.connection.execute(<<~SQL.squish) + SELECT#{' '} + COUNT(*) as total, + COUNT(reverse_geocoded_at) as geocoded, + COUNT(CASE WHEN geodata = '{}' THEN 1 END) as without_data + FROM points#{' '} + WHERE user_id = #{user.id} + SQL + + row = result.first + + { + total: row['total'].to_i, + geocoded: row['geocoded'].to_i, + without_data: row['without_data'].to_i + } + end + + private + + attr_reader :user +end diff --git a/config/environments/development.rb b/config/environments/development.rb index 36cb4be1..c940de0e 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -3,15 +3,6 @@ require 'active_support/core_ext/integer/time' Rails.application.configure do - config.after_initialize do - Bullet.enable = true - Bullet.alert = true - Bullet.bullet_logger = true - Bullet.console = true - Bullet.rails_logger = true - Bullet.add_footer = true - end - # Settings specified here will take precedence over those in config/application.rb. # In the development environment your application's code is reloaded any time diff --git a/config/environments/test.rb b/config/environments/test.rb index b4884952..e138d076 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -8,12 +8,6 @@ require 'active_support/core_ext/integer/time' # and recreated between test runs. Don't rely on the data there! Rails.application.configure do - config.after_initialize do - Bullet.enable = true - Bullet.bullet_logger = true - Bullet.raise = true # raise an error if n+1 query occurs - end - # Settings specified here will take precedence over those in config/application.rb. # While tests run files are not watched, reloading is not necessary. 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