diff --git a/CHANGELOG.md b/CHANGELOG.md index b83e1492..ec5f7092 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Now you can use http protocol for the Photon API host if you don't have SSL certificate for it +- For stats, total distance per month might have been not equal to the sum of distances per day. Now it's fixed and values are equal + +### Added + +- `GET /api/v1/points` can now accept optional `?order=asc` query parameter to return points in ascending order by timestamp. `?order=desc` is still available to return points in descending order by timestamp. # [0.14.6] - 2024-29-30 diff --git a/app/controllers/api/v1/points_controller.rb b/app/controllers/api/v1/points_controller.rb index a7d1a622..e80d769a 100644 --- a/app/controllers/api/v1/points_controller.rb +++ b/app/controllers/api/v1/points_controller.rb @@ -3,12 +3,13 @@ class Api::V1::PointsController < ApiController def index start_at = params[:start_at]&.to_datetime&.to_i - end_at = params[:end_at]&.to_datetime&.to_i || Time.zone.now.to_i + end_at = params[:end_at]&.to_datetime&.to_i || Time.zone.now.to_i + order = params[:order] || 'desc' points = current_api_user .tracked_points .where(timestamp: start_at..end_at) - .order(:timestamp) + .order(timestamp: order) .page(params[:page]) .per(params[:per_page] || 100) diff --git a/app/controllers/map_controller.rb b/app/controllers/map_controller.rb index 8c481cb6..b2954f73 100644 --- a/app/controllers/map_controller.rb +++ b/app/controllers/map_controller.rb @@ -5,9 +5,9 @@ class MapController < ApplicationController def index @points = points - .without_raw_data - .where('timestamp >= ? AND timestamp <= ?', start_at, end_at) - .order(timestamp: :asc) + .without_raw_data + .where('timestamp >= ? AND timestamp <= ?', start_at, end_at) + .order(timestamp: :asc) @countries_and_cities = CountriesAndCities.new(@points).call @coordinates = @@ -38,7 +38,7 @@ class MapController < ApplicationController @coordinates.each_cons(2) do @distance += Geocoder::Calculations.distance_between( - [_1[0], _1[1]], [_2[0], _2[1]], units: DISTANCE_UNIT.to_sym + [_1[0], _1[1]], [_2[0], _2[1]], units: DISTANCE_UNIT ) end diff --git a/app/models/stat.rb b/app/models/stat.rb index 36353863..9316378c 100644 --- a/app/models/stat.rb +++ b/app/models/stat.rb @@ -17,7 +17,7 @@ class Stat < ApplicationRecord points.each_cons(2) do |point1, point2| distance = Geocoder::Calculations.distance_between( - [point1.latitude, point1.longitude], [point2.latitude, point2.longitude] + point1.to_coordinates, point2.to_coordinates, units: ::DISTANCE_UNIT ) data[:distance] += distance diff --git a/app/serializers/point_serializer.rb b/app/serializers/point_serializer.rb index 270e3e25..3bab9501 100644 --- a/app/serializers/point_serializer.rb +++ b/app/serializers/point_serializer.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class PointSerializer - EXCLUDED_ATTRIBUTES = %w[created_at updated_at visit_id id import_id user_id raw_data].freeze + EXCLUDED_ATTRIBUTES = %w[created_at updated_at visit_id import_id user_id raw_data].freeze def initialize(point) @point = point diff --git a/app/services/areas/visits/create.rb b/app/services/areas/visits/create.rb index 82bc863a..f58bf4b7 100644 --- a/app/services/areas/visits/create.rb +++ b/app/services/areas/visits/create.rb @@ -31,14 +31,14 @@ class Areas::Visits::Create def area_points(area) area_radius = - if ::DISTANCE_UNIT.to_sym == :km + if ::DISTANCE_UNIT == :km area.radius / 1000.0 else area.radius / 1609.344 end points = Point.where(user_id: user.id) - .near([area.latitude, area.longitude], area_radius, units: DISTANCE_UNIT.to_sym) + .near([area.latitude, area.longitude], area_radius, units: DISTANCE_UNIT) .order(timestamp: :asc) # check if all points within the area are assigned to a visit diff --git a/app/services/create_stats.rb b/app/services/create_stats.rb index fc086d69..eacaeccf 100644 --- a/app/services/create_stats.rb +++ b/app/services/create_stats.rb @@ -41,22 +41,15 @@ class CreateStats return if points.empty? stat = Stat.find_or_initialize_by(year:, month:, user:) - stat.distance = distance(points) + distance_by_day = stat.distance_by_day + stat.daily_distance = distance_by_day + stat.distance = distance(distance_by_day) stat.toponyms = toponyms(points) - stat.daily_distance = stat.distance_by_day stat.save end - def distance(points) - distance = 0 - - points.each_cons(2) do - distance += Geocoder::Calculations.distance_between( - [_1.latitude, _1.longitude], [_2.latitude, _2.longitude], units: DISTANCE_UNIT.to_sym - ) - end - - distance + def distance(distance_by_day) + distance_by_day.sum { |d| d[1] } end def toponyms(points) diff --git a/config/initializers/00_constants.rb b/config/initializers/00_constants.rb index 198a7472..22424ada 100644 --- a/config/initializers/00_constants.rb +++ b/config/initializers/00_constants.rb @@ -3,4 +3,4 @@ MIN_MINUTES_SPENT_IN_CITY = ENV.fetch('MIN_MINUTES_SPENT_IN_CITY', 60).to_i REVERSE_GEOCODING_ENABLED = ENV.fetch('REVERSE_GEOCODING_ENABLED', 'true') == 'true' PHOTON_API_HOST = ENV.fetch('PHOTON_API_HOST', nil) -DISTANCE_UNIT = ENV.fetch('DISTANCE_UNIT', 'km') +DISTANCE_UNIT = ENV.fetch('DISTANCE_UNIT', 'km').to_sym diff --git a/config/initializers/geocoder.rb b/config/initializers/geocoder.rb index 7dedd07a..0d0c1660 100644 --- a/config/initializers/geocoder.rb +++ b/config/initializers/geocoder.rb @@ -2,7 +2,7 @@ settings = { timeout: 5, - units: DISTANCE_UNIT.to_sym, + units: DISTANCE_UNIT, cache: Redis.new, always_raise: :all, cache_options: { diff --git a/spec/factories/points.rb b/spec/factories/points.rb index 7bc0e2bd..968e6f80 100644 --- a/spec/factories/points.rb +++ b/spec/factories/points.rb @@ -15,7 +15,7 @@ FactoryBot.define do connection { 1 } vertical_accuracy { 1 } accuracy { 1 } - timestamp { 1.year.ago.to_i } + timestamp { DateTime.new(2024, 5, 1).to_i + rand(1_000).minutes } latitude { FFaker::Geolocation.lat } mode { 1 } inrids { 'MyString' } diff --git a/spec/requests/api/v1/points_spec.rb b/spec/requests/api/v1/points_spec.rb index d8ac15c4..5120e5ce 100644 --- a/spec/requests/api/v1/points_spec.rb +++ b/spec/requests/api/v1/points_spec.rb @@ -88,9 +88,31 @@ RSpec.describe 'Api::V1::Points', type: :request do json_response = JSON.parse(response.body) json_response.each do |point| - expect(point.keys).to eq(%w[latitude longitude timestamp]) + expect(point.keys).to eq(%w[id latitude longitude timestamp]) end end end + + context 'when order param is provided' do + it 'returns points in ascending order' do + get api_v1_points_url(api_key: user.api_key, order: 'asc') + + expect(response).to have_http_status(:ok) + + json_response = JSON.parse(response.body) + + expect(json_response.first['timestamp']).to be < json_response.last['timestamp'] + end + + it 'returns points in descending order' do + get api_v1_points_url(api_key: user.api_key, order: 'desc') + + expect(response).to have_http_status(:ok) + + json_response = JSON.parse(response.body) + + expect(json_response.first['timestamp']).to be > json_response.last['timestamp'] + end + end end end diff --git a/spec/services/create_stats_spec.rb b/spec/services/create_stats_spec.rb index 9bd46728..bb1603bc 100644 --- a/spec/services/create_stats_spec.rb +++ b/spec/services/create_stats_spec.rb @@ -22,6 +22,8 @@ RSpec.describe CreateStats do let!(:point3) { create(:point, user:, import:, latitude: 3, longitude: 4) } context 'when units are kilometers' do + before { stub_const('DISTANCE_UNIT', :km) } + it 'creates stats' do expect { create_stats }.to change { Stat.count }.by(1) end @@ -29,7 +31,7 @@ RSpec.describe CreateStats do it 'calculates distance' do create_stats - expect(Stat.last.distance).to eq(563) + expect(user.stats.last.distance).to eq(563) end it 'created notifications' do @@ -52,7 +54,7 @@ RSpec.describe CreateStats do end context 'when units are miles' do - before { stub_const('DISTANCE_UNIT', 'mi') } + before { stub_const('DISTANCE_UNIT', :mi) } it 'creates stats' do expect { create_stats }.to change { Stat.count }.by(1) @@ -61,7 +63,7 @@ RSpec.describe CreateStats do it 'calculates distance' do create_stats - expect(Stat.last.distance).to eq(349) + expect(user.stats.last.distance).to eq(349) end it 'created notifications' do diff --git a/spec/swagger/api/v1/points_controller_spec.rb b/spec/swagger/api/v1/points_controller_spec.rb index 7136e4a5..cbc31e6d 100644 --- a/spec/swagger/api/v1/points_controller_spec.rb +++ b/spec/swagger/api/v1/points_controller_spec.rb @@ -14,6 +14,8 @@ describe 'Points API', type: :request do description: 'End date (i.e. 2024-02-03T13:00:03Z or 2024-02-03)' parameter name: :page, in: :query, type: :integer, required: false, description: 'Page number' parameter name: :per_page, in: :query, type: :integer, required: false, description: 'Number of points per page' + parameter name: :order, in: :query, type: :string, required: false, + description: 'Order of points, valid values are `asc` or `desc`' response '200', 'points found' do schema type: :array, items: { diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml index 3b175419..77a58f8d 100644 --- a/swagger/v1/swagger.yaml +++ b/swagger/v1/swagger.yaml @@ -346,6 +346,12 @@ paths: description: Number of points per page schema: type: integer + - name: order + in: query + required: false + description: Order of points, valid values are `asc` or `desc` + schema: + type: string responses: '200': description: points found