From 57ecda2b1b5aab3acffdf0949a27b4d0ca24f3ab Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Fri, 12 Sep 2025 21:08:45 +0200 Subject: [PATCH] Extract stats sharing logic to its own controller --- app/controllers/api/v1/areas_controller.rb | 4 +- app/controllers/api/v1/settings_controller.rb | 2 +- .../api/v1/subscriptions_controller.rb | 2 +- app/controllers/api/v1/visits_controller.rb | 10 +- app/controllers/exports_controller.rb | 2 +- app/controllers/imports_controller.rb | 16 +- app/controllers/settings/users_controller.rb | 13 +- app/controllers/shared/stats_controller.rb | 54 ++++++ app/controllers/stats_controller.rb | 80 +-------- app/controllers/trips_controller.rb | 10 +- app/controllers/visits_controller.rb | 2 +- app/models/stat.rb | 28 +++ app/views/stats/public_month.html.erb | 8 +- config/initializers/devise.rb | 2 +- config/routes.rb | 11 +- spec/models/stat_spec.rb | 160 ++++++++++++++++++ spec/requests/api/v1/areas_spec.rb | 4 +- spec/requests/api/v1/settings_spec.rb | 2 +- spec/requests/api/v1/subscriptions_spec.rb | 2 +- spec/requests/api/v1/visits_spec.rb | 34 ++-- spec/requests/exports_spec.rb | 2 +- spec/requests/settings/users_spec.rb | 2 +- spec/requests/shared/stats_spec.rb | 157 +++++++++++++++++ spec/requests/stats_spec.rb | 146 ---------------- spec/requests/trips_spec.rb | 4 +- .../users/export_import_integration_spec.rb | 84 +++++---- 26 files changed, 508 insertions(+), 333 deletions(-) create mode 100644 app/controllers/shared/stats_controller.rb create mode 100644 spec/requests/shared/stats_spec.rb diff --git a/app/controllers/api/v1/areas_controller.rb b/app/controllers/api/v1/areas_controller.rb index 4ccebd7c..81e20d17 100644 --- a/app/controllers/api/v1/areas_controller.rb +++ b/app/controllers/api/v1/areas_controller.rb @@ -15,7 +15,7 @@ class Api::V1::AreasController < ApiController if @area.save render json: @area, status: :created else - render json: { errors: @area.errors.full_messages }, status: :unprocessable_entity + render json: { errors: @area.errors.full_messages }, status: :unprocessable_content end end @@ -23,7 +23,7 @@ class Api::V1::AreasController < ApiController if @area.update(area_params) render json: @area, status: :ok else - render json: { errors: @area.errors.full_messages }, status: :unprocessable_entity + render json: { errors: @area.errors.full_messages }, status: :unprocessable_content end end diff --git a/app/controllers/api/v1/settings_controller.rb b/app/controllers/api/v1/settings_controller.rb index 10620730..7404ec01 100644 --- a/app/controllers/api/v1/settings_controller.rb +++ b/app/controllers/api/v1/settings_controller.rb @@ -18,7 +18,7 @@ class Api::V1::SettingsController < ApiController status: :ok else render json: { message: 'Something went wrong', errors: current_api_user.errors.full_messages }, - status: :unprocessable_entity + status: :unprocessable_content end end diff --git a/app/controllers/api/v1/subscriptions_controller.rb b/app/controllers/api/v1/subscriptions_controller.rb index 2da2e97d..cc510d67 100644 --- a/app/controllers/api/v1/subscriptions_controller.rb +++ b/app/controllers/api/v1/subscriptions_controller.rb @@ -15,6 +15,6 @@ class Api::V1::SubscriptionsController < ApiController render json: { message: 'Failed to verify subscription update.' }, status: :unauthorized rescue ArgumentError => e ExceptionReporter.call(e) - render json: { message: 'Invalid subscription data received.' }, status: :unprocessable_entity + render json: { message: 'Invalid subscription data received.' }, status: :unprocessable_content end end diff --git a/app/controllers/api/v1/visits_controller.rb b/app/controllers/api/v1/visits_controller.rb index 248e5ea7..4ec4173b 100644 --- a/app/controllers/api/v1/visits_controller.rb +++ b/app/controllers/api/v1/visits_controller.rb @@ -19,7 +19,7 @@ class Api::V1::VisitsController < ApiController render json: Api::VisitSerializer.new(service.visit).call else error_message = service.errors || 'Failed to create visit' - render json: { error: error_message }, status: :unprocessable_entity + render json: { error: error_message }, status: :unprocessable_content end end @@ -34,7 +34,7 @@ class Api::V1::VisitsController < ApiController # Validate that we have at least 2 visit IDs visit_ids = params[:visit_ids] if visit_ids.blank? || visit_ids.length < 2 - return render json: { error: 'At least 2 visits must be selected for merging' }, status: :unprocessable_entity + return render json: { error: 'At least 2 visits must be selected for merging' }, status: :unprocessable_content end # Find all visits that belong to the current user @@ -52,7 +52,7 @@ class Api::V1::VisitsController < ApiController if merged_visit&.persisted? render json: Api::VisitSerializer.new(merged_visit).call, status: :ok else - render json: { error: service.errors.join(', ') }, status: :unprocessable_entity + render json: { error: service.errors.join(', ') }, status: :unprocessable_content end end @@ -71,7 +71,7 @@ class Api::V1::VisitsController < ApiController updated_count: result[:count] }, status: :ok else - render json: { error: service.errors.join(', ') }, status: :unprocessable_entity + render json: { error: service.errors.join(', ') }, status: :unprocessable_content end end @@ -84,7 +84,7 @@ class Api::V1::VisitsController < ApiController render json: { error: 'Failed to delete visit', errors: visit.errors.full_messages - }, status: :unprocessable_entity + }, status: :unprocessable_content end rescue ActiveRecord::RecordNotFound render json: { error: 'Visit not found' }, status: :not_found diff --git a/app/controllers/exports_controller.rb b/app/controllers/exports_controller.rb index efd2d502..0c59e1bf 100644 --- a/app/controllers/exports_controller.rb +++ b/app/controllers/exports_controller.rb @@ -27,7 +27,7 @@ class ExportsController < ApplicationController ExceptionReporter.call(e) - redirect_to exports_url, alert: "Export failed to initiate: #{e.message}", status: :unprocessable_entity + redirect_to exports_url, alert: "Export failed to initiate: #{e.message}", status: :unprocessable_content end def destroy diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index 3ee75a95..96049978 100644 --- a/app/controllers/imports_controller.rb +++ b/app/controllers/imports_controller.rb @@ -13,9 +13,9 @@ class ImportsController < ApplicationController def index @imports = policy_scope(Import) - .select(:id, :name, :source, :created_at, :processed, :status) - .order(created_at: :desc) - .page(params[:page]) + .select(:id, :name, :source, :created_at, :processed, :status) + .order(created_at: :desc) + .page(params[:page]) end def show; end @@ -43,7 +43,7 @@ class ImportsController < ApplicationController raw_files = Array(files_params).reject(&:blank?) if raw_files.empty? - redirect_to new_import_path, alert: 'No files were selected for upload', status: :unprocessable_entity and return + redirect_to new_import_path, alert: 'No files were selected for upload', status: :unprocessable_content and return end created_imports = [] @@ -62,7 +62,7 @@ class ImportsController < ApplicationController else redirect_to new_import_path, alert: 'No valid file references were found. Please upload files using the file selector.', - status: :unprocessable_entity and return + status: :unprocessable_content and return end rescue StandardError => e if created_imports.present? @@ -74,7 +74,7 @@ class ImportsController < ApplicationController Rails.logger.error e.backtrace.join("\n") ExceptionReporter.call(e) - redirect_to new_import_path, alert: e.message, status: :unprocessable_entity + redirect_to new_import_path, alert: e.message, status: :unprocessable_content end def destroy @@ -117,7 +117,7 @@ class ImportsController < ApplicationController # Extract filename and extension basename = File.basename(original_name, File.extname(original_name)) extension = File.extname(original_name) - + # Add current datetime timestamp = Time.current.strftime('%Y%m%d_%H%M%S') "#{basename}_#{timestamp}#{extension}" @@ -126,6 +126,6 @@ class ImportsController < ApplicationController def validate_points_limit limit_exceeded = PointsLimitExceeded.new(current_user).call - redirect_to imports_path, alert: 'Points limit exceeded', status: :unprocessable_entity if limit_exceeded + redirect_to imports_path, alert: 'Points limit exceeded', status: :unprocessable_content if limit_exceeded end end diff --git a/app/controllers/settings/users_controller.rb b/app/controllers/settings/users_controller.rb index f00a28ce..6545f387 100644 --- a/app/controllers/settings/users_controller.rb +++ b/app/controllers/settings/users_controller.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true class Settings::UsersController < ApplicationController - before_action :authenticate_self_hosted!, except: [:export, :import] - before_action :authenticate_admin!, except: [:export, :import] + before_action :authenticate_self_hosted!, except: %i[export import] + before_action :authenticate_admin!, except: %i[export import] before_action :authenticate_user! def index @@ -19,7 +19,7 @@ class Settings::UsersController < ApplicationController if @user.update(user_params) redirect_to settings_users_url, notice: 'User was successfully updated.' else - redirect_to settings_users_url, notice: 'User could not be updated.', status: :unprocessable_entity + redirect_to settings_users_url, notice: 'User could not be updated.', status: :unprocessable_content end end @@ -33,7 +33,7 @@ class Settings::UsersController < ApplicationController if @user.save redirect_to settings_users_url, notice: 'User was successfully created' else - redirect_to settings_users_url, notice: 'User could not be created.', status: :unprocessable_entity + redirect_to settings_users_url, notice: 'User could not be created.', status: :unprocessable_content end end @@ -43,7 +43,7 @@ class Settings::UsersController < ApplicationController if @user.destroy redirect_to settings_url, notice: 'User was successfully deleted.' else - redirect_to settings_url, notice: 'User could not be deleted.', status: :unprocessable_entity + redirect_to settings_url, notice: 'User could not be deleted.', status: :unprocessable_content end end @@ -90,8 +90,7 @@ class Settings::UsersController < ApplicationController end def validate_archive_file(archive_file) - unless archive_file.content_type == 'application/zip' || - archive_file.content_type == 'application/x-zip-compressed' || + unless ['application/zip', 'application/x-zip-compressed'].include?(archive_file.content_type) || File.extname(archive_file.original_filename).downcase == '.zip' redirect_to edit_user_registration_path, alert: 'Please upload a valid ZIP file.' and return diff --git a/app/controllers/shared/stats_controller.rb b/app/controllers/shared/stats_controller.rb new file mode 100644 index 00000000..e660dbcf --- /dev/null +++ b/app/controllers/shared/stats_controller.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +class Shared::StatsController < ApplicationController + before_action :authenticate_user!, except: [:show] + before_action :authenticate_active_user!, only: [:update] + + def show + @stat = Stat.find_by(sharing_uuid: params[:uuid]) + + unless @stat&.public_accessible? + return redirect_to root_path, + alert: 'Shared stats not found or no longer available' + end + + @year = @stat.year + @month = @stat.month + @user = @stat.user + @is_public_view = true + @data_bounds = @stat.calculate_data_bounds + + render 'stats/public_month' + end + + def update + @year = params[:year].to_i + @month = params[:month].to_i + @stat = current_user.stats.find_by(year: @year, month: @month) + + return head :not_found unless @stat + + if params[:enabled] == '1' + @stat.enable_sharing!(expiration: params[:expiration] || 'permanent') + sharing_url = shared_stat_url(@stat.sharing_uuid) + + render json: { + success: true, + sharing_url: sharing_url, + message: 'Sharing enabled successfully' + } + else + @stat.disable_sharing! + + render json: { + success: true, + message: 'Sharing disabled successfully' + } + end + rescue StandardError + render json: { + success: false, + message: 'Failed to update sharing settings' + }, status: :unprocessable_content + end +end diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 522cd6f1..f72a6bed 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true class StatsController < ApplicationController - before_action :authenticate_user!, except: [:public_show] - before_action :authenticate_active_user!, only: %i[update update_all update_sharing] + before_action :authenticate_user! + before_action :authenticate_active_user!, only: %i[update update_all] def index @stats = build_stats @@ -52,54 +52,6 @@ class StatsController < ApplicationController redirect_to stats_path, notice: 'Stats are being updated', status: :see_other end - def update_sharing - @year = params[:year].to_i - @month = params[:month].to_i - @stat = current_user.stats.find_by(year: @year, month: @month) - - return head :not_found unless @stat - - if params[:enabled] == '1' - @stat.enable_sharing!(expiration: params[:expiration] || 'permanent') - sharing_url = public_stat_url(@stat.sharing_uuid) - - render json: { - success: true, - sharing_url: sharing_url, - message: 'Sharing enabled successfully' - } - else - @stat.disable_sharing! - - render json: { - success: true, - message: 'Sharing disabled successfully' - } - end - rescue StandardError - render json: { - success: false, - message: 'Failed to update sharing settings' - }, status: :unprocessable_entity - end - - def public_show - @stat = Stat.find_by(sharing_uuid: params[:uuid]) - - unless @stat&.public_accessible? - return redirect_to root_path, - alert: 'Shared stats not found or no longer available' - end - - @year = @stat.year - @month = @stat.month - @user = @stat.user - @is_public_view = true - @data_bounds = calculate_data_bounds(@stat) - - render 'public_month' - end - private def assign_points_statistics @@ -132,32 +84,4 @@ class StatsController < ApplicationController stats.sort_by(&:updated_at).reverse end.sort.reverse end - - def calculate_data_bounds(stat) - start_date = Date.new(stat.year, stat.month, 1).beginning_of_day - end_date = start_date.end_of_month.end_of_day - - points_relation = stat.user.points.where(timestamp: start_date.to_i..end_date.to_i) - point_count = points_relation.count - - return nil if point_count.zero? - - bounds_result = ActiveRecord::Base.connection.exec_query( - "SELECT MIN(latitude) as min_lat, MAX(latitude) as max_lat, - MIN(longitude) as min_lng, MAX(longitude) as max_lng - FROM points - WHERE user_id = $1 - AND timestamp BETWEEN $2 AND $3", - 'data_bounds_query', - [stat.user.id, start_date.to_i, end_date.to_i] - ).first - - { - min_lat: bounds_result['min_lat'].to_f, - max_lat: bounds_result['max_lat'].to_f, - min_lng: bounds_result['min_lng'].to_f, - max_lng: bounds_result['max_lng'].to_f, - point_count: point_count - } - end end diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index 1880002b..00764a96 100644 --- a/app/controllers/trips_controller.rb +++ b/app/controllers/trips_controller.rb @@ -16,9 +16,9 @@ class TripsController < ApplicationController end @photo_sources = @trip.photo_sources - if @trip.path.blank? || @trip.distance.blank? || @trip.visited_countries.blank? - Trips::CalculateAllJob.perform_later(@trip.id, current_user.safe_settings.distance_unit) - end + return unless @trip.path.blank? || @trip.distance.blank? || @trip.visited_countries.blank? + + Trips::CalculateAllJob.perform_later(@trip.id, current_user.safe_settings.distance_unit) end def new @@ -34,7 +34,7 @@ class TripsController < ApplicationController if @trip.save redirect_to @trip, notice: 'Trip was successfully created. Data is being calculated in the background.' else - render :new, status: :unprocessable_entity + render :new, status: :unprocessable_content end end @@ -42,7 +42,7 @@ class TripsController < ApplicationController if @trip.update(trip_params) redirect_to @trip, notice: 'Trip was successfully updated.', status: :see_other else - render :edit, status: :unprocessable_entity + render :edit, status: :unprocessable_content end end diff --git a/app/controllers/visits_controller.rb b/app/controllers/visits_controller.rb index a22e60e5..bc8c1d8c 100644 --- a/app/controllers/visits_controller.rb +++ b/app/controllers/visits_controller.rb @@ -22,7 +22,7 @@ class VisitsController < ApplicationController if @visit.update(visit_params) redirect_back(fallback_location: visits_path(status: :suggested)) else - render :edit, status: :unprocessable_entity + render :edit, status: :unprocessable_content end end diff --git a/app/models/stat.rb b/app/models/stat.rb index a8fc1e0a..58ad79f5 100644 --- a/app/models/stat.rb +++ b/app/models/stat.rb @@ -81,6 +81,34 @@ class Stat < ApplicationRecord ) end + def calculate_data_bounds + start_date = Date.new(year, month, 1).beginning_of_day + end_date = start_date.end_of_month.end_of_day + + points_relation = user.points.where(timestamp: start_date.to_i..end_date.to_i) + point_count = points_relation.count + + return nil if point_count.zero? + + bounds_result = ActiveRecord::Base.connection.exec_query( + "SELECT MIN(latitude) as min_lat, MAX(latitude) as max_lat, + MIN(longitude) as min_lng, MAX(longitude) as max_lng + FROM points + WHERE user_id = $1 + AND timestamp BETWEEN $2 AND $3", + 'data_bounds_query', + [user.id, start_date.to_i, end_date.to_i] + ).first + + { + min_lat: bounds_result['min_lat'].to_f, + max_lat: bounds_result['max_lat'].to_f, + min_lng: bounds_result['min_lng'].to_f, + max_lng: bounds_result['max_lng'].to_f, + point_count: point_count + } + end + private def generate_sharing_uuid diff --git a/app/views/stats/public_month.html.erb b/app/views/stats/public_month.html.erb index 15b15532..a9506424 100644 --- a/app/views/stats/public_month.html.erb +++ b/app/views/stats/public_month.html.erb @@ -90,7 +90,9 @@
-

📈 Daily Activity

+

+ <%= icon 'trending-up' %> Daily Activity +

<%= column_chart( @stat.daily_distance.map { |day, distance_meters| @@ -132,7 +134,9 @@
-

🌍 Countries & Cities

+

+ <%= icon 'earth' %> Countries & Cities +

<% @stat.toponyms.each_with_index do |country, index| %>
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index e93579b9..7b207ed3 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -302,7 +302,7 @@ Devise.setup do |config| # When set to false, does not sign a user in automatically after their password is # changed. Defaults to true, so a user is signed in automatically after changing a password. # config.sign_in_after_change_password = true - config.responder.error_status = :unprocessable_entity + config.responder.error_status = :unprocessable_content config.responder.redirect_status = :see_other if Rails.env.production? && !DawarichSettings.self_hosted? diff --git a/config/routes.rb b/config/routes.rb index 96848f71..7e653ec4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -75,14 +75,17 @@ Rails.application.routes.draw do to: 'stats#update', as: :update_year_month_stats, constraints: { year: /\d{4}/, month: /\d{1,2}|all/ } + # Public shared stats routes + get 'shared/stats/:uuid', to: 'shared/stats#show', as: :shared_stat + # Backward compatibility + get 'shared/stats/:uuid', to: 'shared/stats#show', as: :public_stat + + # Sharing management endpoint (requires auth) patch 'stats/:year/:month/sharing', - to: 'stats#update_sharing', + to: 'shared/stats#update', as: :sharing_stats, constraints: { year: /\d{4}/, month: /\d{1,2}/ } - # Public shared stats page - get 'shared/stats/:uuid', to: 'stats#public_show', as: :public_stat - root to: 'home#index' if SELF_HOSTED diff --git a/spec/models/stat_spec.rb b/spec/models/stat_spec.rb index ee4c477f..5bda16d5 100644 --- a/spec/models/stat_spec.rb +++ b/spec/models/stat_spec.rb @@ -102,5 +102,165 @@ RSpec.describe Stat, type: :model do expect(subject).to eq(points) end end + + describe '#calculate_data_bounds' do + let(:stat) { create(:stat, year: 2024, month: 6, user:) } + let(:user) { create(:user) } + + context 'when stat has points' do + before do + # Create test points within the month (June 2024) + create(:point, + user:, + latitude: 40.6, + longitude: -74.1, + timestamp: Time.new(2024, 6, 1, 12, 0).to_i) + create(:point, + user:, + latitude: 40.8, + longitude: -73.9, + timestamp: Time.new(2024, 6, 15, 15, 0).to_i) + create(:point, + user:, + latitude: 40.7, + longitude: -74.0, + timestamp: Time.new(2024, 6, 30, 18, 0).to_i) + + # Points outside the month (should be ignored) + create(:point, + user:, + latitude: 41.0, + longitude: -75.0, + timestamp: Time.new(2024, 5, 31, 23, 59).to_i) # May + create(:point, + user:, + latitude: 39.0, + longitude: -72.0, + timestamp: Time.new(2024, 7, 1, 0, 1).to_i) # July + end + + it 'returns correct bounding box for points within the month' do + result = stat.calculate_data_bounds + + expect(result).to be_a(Hash) + expect(result[:min_lat]).to eq(40.6) + expect(result[:max_lat]).to eq(40.8) + expect(result[:min_lng]).to eq(-74.1) + expect(result[:max_lng]).to eq(-73.9) + expect(result[:point_count]).to eq(3) + end + + context 'with points from different users' do + let(:other_user) { create(:user) } + + before do + # Add points from a different user (should be ignored) + create(:point, + user: other_user, + latitude: 50.0, + longitude: -80.0, + timestamp: Time.new(2024, 6, 15, 12, 0).to_i) + end + + it 'only includes points from the stat user' do + result = stat.calculate_data_bounds + + expect(result[:min_lat]).to eq(40.6) + expect(result[:max_lat]).to eq(40.8) + expect(result[:min_lng]).to eq(-74.1) + expect(result[:max_lng]).to eq(-73.9) + expect(result[:point_count]).to eq(3) # Still only 3 points from the stat user + end + end + + context 'with single point' do + let(:single_point_user) { create(:user) } + let(:single_point_stat) { create(:stat, year: 2024, month: 7, user: single_point_user) } + + before do + create(:point, + user: single_point_user, + latitude: 45.5, + longitude: -122.65, + timestamp: Time.new(2024, 7, 15, 14, 30).to_i) + end + + it 'returns bounds with same min and max values' do + result = single_point_stat.calculate_data_bounds + + expect(result[:min_lat]).to eq(45.5) + expect(result[:max_lat]).to eq(45.5) + expect(result[:min_lng]).to eq(-122.65) + expect(result[:max_lng]).to eq(-122.65) + expect(result[:point_count]).to eq(1) + end + end + + context 'with edge case coordinates' do + let(:edge_user) { create(:user) } + let(:edge_stat) { create(:stat, year: 2024, month: 8, user: edge_user) } + + before do + # Test with extreme coordinate values + create(:point, + user: edge_user, + latitude: -90.0, # South Pole + longitude: -180.0, # Date Line West + timestamp: Time.new(2024, 8, 1, 0, 0).to_i) + create(:point, + user: edge_user, + latitude: 90.0, # North Pole + longitude: 180.0, # Date Line East + timestamp: Time.new(2024, 8, 31, 23, 59).to_i) + end + + it 'handles extreme coordinate values correctly' do + result = edge_stat.calculate_data_bounds + + expect(result[:min_lat]).to eq(-90.0) + expect(result[:max_lat]).to eq(90.0) + expect(result[:min_lng]).to eq(-180.0) + expect(result[:max_lng]).to eq(180.0) + expect(result[:point_count]).to eq(2) + end + end + end + + context 'when stat has no points' do + let(:empty_user) { create(:user) } + let(:empty_stat) { create(:stat, year: 2024, month: 10, user: empty_user) } + + it 'returns nil' do + result = empty_stat.calculate_data_bounds + + expect(result).to be_nil + end + end + + context 'when stat has points but none within the month timeframe' do + let(:empty_month_user) { create(:user) } + let(:empty_month_stat) { create(:stat, year: 2024, month: 9, user: empty_month_user) } + + before do + # Create points outside the target month + create(:point, + user: empty_month_user, + latitude: 40.7, + longitude: -74.0, + timestamp: Time.new(2024, 8, 31, 23, 59).to_i) # August + create(:point, + user: empty_month_user, + latitude: 40.8, + longitude: -73.9, + timestamp: Time.new(2024, 10, 1, 0, 1).to_i) # October + end + + it 'returns nil when no points exist in the month' do + result = empty_month_stat.calculate_data_bounds + + expect(result).to be_nil + end + end + end end end diff --git a/spec/requests/api/v1/areas_spec.rb b/spec/requests/api/v1/areas_spec.rb index 7be57513..c5f15948 100644 --- a/spec/requests/api/v1/areas_spec.rb +++ b/spec/requests/api/v1/areas_spec.rb @@ -49,7 +49,7 @@ RSpec.describe '/api/v1/areas', type: :request do post api_v1_areas_url, headers: { 'Authorization' => "Bearer #{user.api_key}" }, params: { area: invalid_attributes } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end @@ -85,7 +85,7 @@ RSpec.describe '/api/v1/areas', type: :request do patch api_v1_area_url(area), headers: { 'Authorization' => "Bearer #{user.api_key}" }, params: { area: invalid_attributes } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end diff --git a/spec/requests/api/v1/settings_spec.rb b/spec/requests/api/v1/settings_spec.rb index 3f6673e5..470563b7 100644 --- a/spec/requests/api/v1/settings_spec.rb +++ b/spec/requests/api/v1/settings_spec.rb @@ -47,7 +47,7 @@ RSpec.describe 'Api::V1::Settings', type: :request do it 'returns http unprocessable entity' do patch "/api/v1/settings?api_key=#{api_key}", params: { settings: { route_opacity: 'invalid' } } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end it 'returns an error message' do diff --git a/spec/requests/api/v1/subscriptions_spec.rb b/spec/requests/api/v1/subscriptions_spec.rb index 85e657e4..19fea01c 100644 --- a/spec/requests/api/v1/subscriptions_spec.rb +++ b/spec/requests/api/v1/subscriptions_spec.rb @@ -102,7 +102,7 @@ RSpec.describe 'Api::V1::Subscriptions', type: :request do post '/api/v1/subscriptions/callback', params: { token: token } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) expect(JSON.parse(response.body)['message']).to eq('Invalid subscription data received.') end end diff --git a/spec/requests/api/v1/visits_spec.rb b/spec/requests/api/v1/visits_spec.rb index a250635c..d5e9317a 100644 --- a/spec/requests/api/v1/visits_spec.rb +++ b/spec/requests/api/v1/visits_spec.rb @@ -81,9 +81,9 @@ RSpec.describe 'Api::V1::Visits', type: :request do let(:existing_place) { create(:place, latitude: 52.52, longitude: 13.405) } it 'creates a new visit' do - expect { + expect do post '/api/v1/visits', params: valid_create_params, headers: auth_headers - }.to change { user.visits.count }.by(1) + end.to change { user.visits.count }.by(1) expect(response).to have_http_status(:ok) end @@ -100,9 +100,9 @@ RSpec.describe 'Api::V1::Visits', type: :request do end it 'creates a place for the visit' do - expect { + expect do post '/api/v1/visits', params: valid_create_params, headers: auth_headers - }.to change { Place.count }.by(1) + end.to change { Place.count }.by(1) created_place = Place.last expect(created_place.name).to eq('Test Visit') @@ -114,9 +114,9 @@ RSpec.describe 'Api::V1::Visits', type: :request do it 'reuses existing place when coordinates are exactly the same' do create(:visit, user: user, place: existing_place) - expect { + expect do post '/api/v1/visits', params: valid_create_params, headers: auth_headers - }.not_to change { Place.count } + end.not_to(change { Place.count }) json_response = JSON.parse(response.body) expect(json_response['place']['id']).to eq(existing_place.id) @@ -132,7 +132,7 @@ RSpec.describe 'Api::V1::Visits', type: :request do it 'returns unprocessable entity status' do post '/api/v1/visits', params: missing_name_params, headers: auth_headers - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end it 'returns error message' do @@ -144,9 +144,9 @@ RSpec.describe 'Api::V1::Visits', type: :request do end it 'does not create a visit' do - expect { + expect do post '/api/v1/visits', params: missing_name_params, headers: auth_headers - }.not_to change { Visit.count } + end.not_to(change { Visit.count }) end end end @@ -199,7 +199,7 @@ RSpec.describe 'Api::V1::Visits', type: :request do it 'renders a JSON response with errors for the visit' do put "/api/v1/visits/#{visit.id}", params: invalid_attributes, headers: auth_headers - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end @@ -234,7 +234,7 @@ RSpec.describe 'Api::V1::Visits', type: :request do it 'returns an error when fewer than 2 visits are specified' do post '/api/v1/visits/merge', params: { visit_ids: [visit1.id] }, headers: auth_headers - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) json_response = JSON.parse(response.body) expect(json_response['error']).to include('At least 2 visits must be selected') end @@ -264,7 +264,7 @@ RSpec.describe 'Api::V1::Visits', type: :request do post '/api/v1/visits/merge', params: { visit_ids: [visit1.id, visit2.id] }, headers: auth_headers - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) json_response = JSON.parse(response.body) expect(json_response['error']).to include('Failed to merge visits') end @@ -316,7 +316,7 @@ RSpec.describe 'Api::V1::Visits', type: :request do post '/api/v1/visits/bulk_update', params: invalid_update_params, headers: auth_headers - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) json_response = JSON.parse(response.body) expect(json_response['error']).to include('Invalid status') end @@ -329,9 +329,9 @@ RSpec.describe 'Api::V1::Visits', type: :request do context 'when visit exists and belongs to current user' do it 'deletes the visit' do - expect { + expect do delete "/api/v1/visits/#{visit.id}", headers: auth_headers - }.to change { user.visits.count }.by(-1) + end.to change { user.visits.count }.by(-1) expect(response).to have_http_status(:no_content) end @@ -363,9 +363,9 @@ RSpec.describe 'Api::V1::Visits', type: :request do end it 'does not delete the visit' do - expect { + expect do delete "/api/v1/visits/#{other_user_visit.id}", headers: auth_headers - }.not_to change { Visit.count } + end.not_to(change { Visit.count }) end end diff --git a/spec/requests/exports_spec.rb b/spec/requests/exports_spec.rb index 89658348..8fd9f43c 100644 --- a/spec/requests/exports_spec.rb +++ b/spec/requests/exports_spec.rb @@ -70,7 +70,7 @@ RSpec.describe '/exports', type: :request do it 'renders a response with 422 status (i.e. to display the "new" template)' do post(exports_url, params:) - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end diff --git a/spec/requests/settings/users_spec.rb b/spec/requests/settings/users_spec.rb index 51079587..a80694ae 100644 --- a/spec/requests/settings/users_spec.rb +++ b/spec/requests/settings/users_spec.rb @@ -64,7 +64,7 @@ RSpec.describe '/settings/users', type: :request do it 'renders a response with 422 status (i.e. to display the "new" template)' do post settings_users_url, params: { user: invalid_attributes } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end diff --git a/spec/requests/shared/stats_spec.rb b/spec/requests/shared/stats_spec.rb new file mode 100644 index 00000000..23d5a131 --- /dev/null +++ b/spec/requests/shared/stats_spec.rb @@ -0,0 +1,157 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Shared::Stats', type: :request do + before do + stub_request(:any, 'https://api.github.com/repos/Freika/dawarich/tags') + .to_return(status: 200, body: '[{"name": "1.0.0"}]', headers: {}) + end + + context 'public sharing' do + let(:user) { create(:user) } + let(:stat) { create(:stat, :with_sharing_enabled, user:, year: 2024, month: 6) } + + describe 'GET /shared/stats/:uuid' do + context 'with valid sharing UUID' do + before do + # Create some test points for data bounds calculation + create_list(:point, 5, user:, timestamp: Time.new(2024, 6, 15).to_i) + end + + it 'renders the public month view' do + get shared_stat_url(stat.sharing_uuid) + + expect(response).to have_http_status(:success) + expect(response.body).to include('Monthly Digest') + expect(response.body).to include('June 2024') + end + + it 'includes required content in response' do + get shared_stat_url(stat.sharing_uuid) + + expect(response.body).to include('June 2024') + expect(response.body).to include('Monthly Digest') + expect(response.body).to include('data-public-stat-map-uuid-value') + expect(response.body).to include(stat.sharing_uuid) + end + end + + context 'with invalid sharing UUID' do + it 'redirects to root with alert' do + get shared_stat_url('invalid-uuid') + + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Shared stats not found or no longer available') + end + end + + context 'with expired sharing' do + let(:stat) { create(:stat, :with_sharing_expired, user:, year: 2024, month: 6) } + + it 'redirects to root with alert' do + get shared_stat_url(stat.sharing_uuid) + + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Shared stats not found or no longer available') + end + end + + context 'with disabled sharing' do + let(:stat) { create(:stat, :with_sharing_disabled, user:, year: 2024, month: 6) } + + it 'redirects to root with alert' do + get shared_stat_url(stat.sharing_uuid) + + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq('Shared stats not found or no longer available') + end + end + + context 'when stat has no points' do + it 'renders successfully' do + get shared_stat_url(stat.sharing_uuid) + + expect(response).to have_http_status(:success) + expect(response.body).to include('Monthly Digest') + end + end + end + + describe 'PATCH /stats/:year/:month/sharing' do + context 'when user is signed in' do + let!(:stat_to_share) { create(:stat, user:, year: 2024, month: 6) } + + before { sign_in user } + + context 'enabling sharing' do + it 'enables sharing and returns success' do + patch sharing_stats_path(year: 2024, month: 6), + params: { enabled: '1' }, + as: :json + + expect(response).to have_http_status(:success) + + json_response = JSON.parse(response.body) + expect(json_response['success']).to be(true) + expect(json_response['sharing_url']).to be_present + expect(json_response['message']).to eq('Sharing enabled successfully') + + stat_to_share.reload + expect(stat_to_share.sharing_enabled?).to be(true) + expect(stat_to_share.sharing_uuid).to be_present + end + + it 'sets custom expiration when provided' do + patch sharing_stats_path(year: 2024, month: 6), + params: { enabled: '1', expiration: '1_week' }, + as: :json + + expect(response).to have_http_status(:success) + stat_to_share.reload + expect(stat_to_share.sharing_enabled?).to be(true) + end + end + + context 'disabling sharing' do + let!(:enabled_stat) { create(:stat, :with_sharing_enabled, user:, year: 2024, month: 7) } + + it 'disables sharing and returns success' do + patch sharing_stats_path(year: 2024, month: 7), + params: { enabled: '0' }, + as: :json + + expect(response).to have_http_status(:success) + + json_response = JSON.parse(response.body) + expect(json_response['success']).to be(true) + expect(json_response['message']).to eq('Sharing disabled successfully') + + enabled_stat.reload + expect(enabled_stat.sharing_enabled?).to be(false) + end + end + + context 'when stat does not exist' do + it 'returns not found' do + patch sharing_stats_path(year: 2024, month: 12), + params: { enabled: '1' }, + as: :json + + expect(response).to have_http_status(:not_found) + end + end + end + + context 'when user is not signed in' do + it 'returns unauthorized' do + patch sharing_stats_path(year: 2024, month: 6), + params: { enabled: '1' }, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + end + end + end +end \ No newline at end of file diff --git a/spec/requests/stats_spec.rb b/spec/requests/stats_spec.rb index 27d85789..8c008e85 100644 --- a/spec/requests/stats_spec.rb +++ b/spec/requests/stats_spec.rb @@ -112,150 +112,4 @@ RSpec.describe '/stats', type: :request do end end - context 'public sharing' do - let(:user) { create(:user) } - let(:stat) { create(:stat, :with_sharing_enabled, user:, year: 2024, month: 6) } - - describe 'GET /public/:uuid' do - context 'with valid sharing UUID' do - before do - # Create some test points for data bounds calculation - create_list(:point, 5, user:, timestamp: Time.new(2024, 6, 15).to_i) - end - - it 'renders the public month view' do - get public_stat_url(stat.sharing_uuid) - - expect(response).to have_http_status(:success) - expect(response.body).to include('Monthly Digest') - expect(response.body).to include('June 2024') - end - - it 'includes required content in response' do - get public_stat_url(stat.sharing_uuid) - - expect(response.body).to include('June 2024') - expect(response.body).to include('Monthly Digest') - expect(response.body).to include('data-public-stat-map-uuid-value') - expect(response.body).to include(stat.sharing_uuid) - end - end - - context 'with invalid sharing UUID' do - it 'redirects to root with alert' do - get public_stat_url('invalid-uuid') - - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to eq('Shared stats not found or no longer available') - end - end - - context 'with expired sharing' do - let(:stat) { create(:stat, :with_sharing_expired, user:, year: 2024, month: 6) } - - it 'redirects to root with alert' do - get public_stat_url(stat.sharing_uuid) - - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to eq('Shared stats not found or no longer available') - end - end - - context 'with disabled sharing' do - let(:stat) { create(:stat, :with_sharing_disabled, user:, year: 2024, month: 6) } - - it 'redirects to root with alert' do - get public_stat_url(stat.sharing_uuid) - - expect(response).to redirect_to(root_path) - expect(flash[:alert]).to eq('Shared stats not found or no longer available') - end - end - - context 'when stat has no points' do - it 'renders successfully' do - get public_stat_url(stat.sharing_uuid) - - expect(response).to have_http_status(:success) - expect(response.body).to include('Monthly Digest') - end - end - end - - describe 'PATCH /update_sharing' do - context 'when user is signed in' do - let!(:stat_to_share) { create(:stat, user:, year: 2024, month: 6) } - - before { sign_in user } - - context 'enabling sharing' do - it 'enables sharing and returns success' do - patch sharing_stats_path(year: 2024, month: 6), - params: { enabled: '1' }, - as: :json - - expect(response).to have_http_status(:success) - - json_response = JSON.parse(response.body) - expect(json_response['success']).to be(true) - expect(json_response['sharing_url']).to be_present - expect(json_response['message']).to eq('Sharing enabled successfully') - - stat_to_share.reload - expect(stat_to_share.sharing_enabled?).to be(true) - expect(stat_to_share.sharing_uuid).to be_present - end - - it 'sets custom expiration when provided' do - patch sharing_stats_path(year: 2024, month: 6), - params: { enabled: '1', expiration: '1_week' }, - as: :json - - expect(response).to have_http_status(:success) - stat_to_share.reload - expect(stat_to_share.sharing_enabled?).to be(true) - end - end - - context 'disabling sharing' do - let!(:enabled_stat) { create(:stat, :with_sharing_enabled, user:, year: 2024, month: 7) } - - it 'disables sharing and returns success' do - patch sharing_stats_path(year: 2024, month: 7), - params: { enabled: '0' }, - as: :json - - expect(response).to have_http_status(:success) - - json_response = JSON.parse(response.body) - expect(json_response['success']).to be(true) - expect(json_response['message']).to eq('Sharing disabled successfully') - - enabled_stat.reload - expect(enabled_stat.sharing_enabled?).to be(false) - end - end - - context 'when stat does not exist' do - it 'returns not found' do - patch sharing_stats_path(year: 2024, month: 12), - params: { enabled: '1' }, - as: :json - - expect(response).to have_http_status(:not_found) - end - end - end - - context 'when user is not signed in' do - it 'returns unauthorized' do - patch sharing_stats_path(year: 2024, month: 6), - params: { enabled: '1' }, - as: :json - - expect(response).to have_http_status(:unauthorized) - end - end - end - end end diff --git a/spec/requests/trips_spec.rb b/spec/requests/trips_spec.rb index af654048..53549905 100644 --- a/spec/requests/trips_spec.rb +++ b/spec/requests/trips_spec.rb @@ -114,7 +114,7 @@ RSpec.describe '/trips', type: :request do it "renders a response with 422 status (i.e. to display the 'new' template)" do post trips_url, params: { trip: invalid_attributes } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end @@ -151,7 +151,7 @@ RSpec.describe '/trips', type: :request do it 'renders a response with 422 status' do patch trip_url(trip), params: { trip: invalid_attributes } - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_http_status(:unprocessable_content) end end end diff --git a/spec/services/users/export_import_integration_spec.rb b/spec/services/users/export_import_integration_spec.rb index 66e3c6a0..d30f62fe 100644 --- a/spec/services/users/export_import_integration_spec.rb +++ b/spec/services/users/export_import_integration_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' RSpec.describe 'Users Export-Import Integration', type: :service do let(:original_user) { create(:user, email: 'original@example.com') } let(:target_user) { create(:user, email: 'target@example.com') } - let(:temp_archive_path) { Rails.root.join('tmp', 'test_export.zip') } + let(:temp_archive_path) { Rails.root.join('tmp/test_export.zip') } after do File.delete(temp_archive_path) if File.exist?(temp_archive_path) @@ -40,17 +40,12 @@ RSpec.describe 'Users Export-Import Integration', type: :service do Rails.logger.level = original_log_level end - puts "Import stats: #{import_stats.inspect}" - user_notifications_count = original_user.notifications.where.not( title: ['Data import completed', 'Data import failed', 'Export completed', 'Export failed'] ).count target_counts = calculate_user_entity_counts(target_user) - puts "Original counts: #{original_counts.inspect}" - puts "Target counts: #{target_counts.inspect}" - expect(target_counts[:areas]).to eq(original_counts[:areas]) expect(target_counts[:imports]).to eq(original_counts[:imports]) expect(target_counts[:exports]).to eq(original_counts[:exports]) @@ -185,18 +180,18 @@ RSpec.describe 'Users Export-Import Integration', type: :service do # Verify all entities were imported correctly expect(import_stats[:places_created]).to eq(original_places_count), - "Expected #{original_places_count} places to be created, got #{import_stats[:places_created]}" + "Expected #{original_places_count} places to be created, got #{import_stats[:places_created]}" expect(import_stats[:visits_created]).to eq(original_visits_count), - "Expected #{original_visits_count} visits to be created, got #{import_stats[:visits_created]}" + "Expected #{original_visits_count} visits to be created, got #{import_stats[:visits_created]}" # Verify the imported user has access to all their data imported_places_count = import_user.places.distinct.count imported_visits_count = import_user.visits.count expect(imported_places_count).to eq(original_places_count), - "Expected user to have access to #{original_places_count} places, got #{imported_places_count}" + "Expected user to have access to #{original_places_count} places, got #{imported_places_count}" expect(imported_visits_count).to eq(original_visits_count), - "Expected user to have #{original_visits_count} visits, got #{imported_visits_count}" + "Expected user to have #{original_visits_count} visits, got #{imported_visits_count}" # Verify specific visits have their place associations imported_visits = import_user.visits.includes(:place) @@ -205,7 +200,7 @@ RSpec.describe 'Users Export-Import Integration', type: :service do # Verify place names are preserved place_names = visits_with_places.map { |v| v.place.name }.sort - expect(place_names).to eq(['Gym', 'Home', 'Office']) + expect(place_names).to eq(%w[Gym Home Office]) # Cleanup temp_export_file.unlink @@ -217,11 +212,11 @@ RSpec.describe 'Users Export-Import Integration', type: :service do def create_full_user_dataset(user) user.update!(settings: { - 'distance_unit' => 'km', + 'distance_unit' => 'km', 'timezone' => 'America/New_York', 'immich_url' => 'https://immich.example.com', 'immich_api_key' => 'test-api-key' - }) + }) usa = create(:country, name: 'United States', iso_a2: 'US', iso_a3: 'USA') canada = create(:country, name: 'Canada', iso_a2: 'CA', iso_a3: 'CAN') @@ -271,37 +266,32 @@ RSpec.describe 'Users Export-Import Integration', type: :service do visit3 = create(:visit, user: user, place: nil, name: 'Unknown Location') create_list(:point, 5, - user: user, - import: import1, - country: usa, - visit: visit1, - latitude: 40.7589, - longitude: -73.9851 - ) + user: user, + import: import1, + country: usa, + visit: visit1, + latitude: 40.7589, + longitude: -73.9851) create_list(:point, 3, - user: user, - import: import2, - country: canada, - visit: visit2, - latitude: 40.7128, - longitude: -74.0060 - ) + user: user, + import: import2, + country: canada, + visit: visit2, + latitude: 40.7128, + longitude: -74.0060) create_list(:point, 2, - user: user, - import: nil, - country: nil, - visit: nil - ) + user: user, + import: nil, + country: nil, + visit: nil) create_list(:point, 2, - user: user, - import: import1, - country: usa, - visit: visit3 - ) - + user: user, + import: import1, + country: usa, + visit: visit3) end def calculate_user_entity_counts(user) @@ -342,11 +332,13 @@ RSpec.describe 'Users Export-Import Integration', type: :service do latitude: 40.7589, longitude: -73.9851 ).first - if original_office_points && target_office_points - expect(target_office_points.import.name).to eq(original_office_points.import.name) if original_office_points.import - expect(target_office_points.country.name).to eq(original_office_points.country.name) if original_office_points.country - expect(target_office_points.visit.name).to eq(original_office_points.visit.name) if original_office_points.visit + return unless original_office_points && target_office_points + + expect(target_office_points.import.name).to eq(original_office_points.import.name) if original_office_points.import + if original_office_points.country + expect(target_office_points.country.name).to eq(original_office_points.country.name) end + expect(target_office_points.visit.name).to eq(original_office_points.visit.name) if original_office_points.visit end def verify_settings_preserved(original_user, target_user) @@ -375,9 +367,9 @@ RSpec.describe 'Users Export-Import Integration', type: :service do original_export = original_user.exports.find_by(name: 'Q1 2024 Export') target_export = target_user.exports.find_by(name: 'Q1 2024 Export') - if original_export&.file&.attached? - expect(target_export).to be_present - expect(target_export.file).to be_attached - end + return unless original_export&.file&.attached? + + expect(target_export).to be_present + expect(target_export.file).to be_attached end end