diff --git a/CHANGELOG.md b/CHANGELOG.md index ea343a2a..422775c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +# [UNRELEASED] + +## Added + +- `POST /api/v1/visits` endpoint. +- User now can create visits manually on the map. +- Import failure now throws an internal server error. + # [0.30.9] - 2025-08-19 diff --git a/app/controllers/api/v1/visits_controller.rb b/app/controllers/api/v1/visits_controller.rb index 727cf849..b1d44b36 100644 --- a/app/controllers/api/v1/visits_controller.rb +++ b/app/controllers/api/v1/visits_controller.rb @@ -11,34 +11,16 @@ class Api::V1::VisitsController < ApiController end def create - visit = current_api_user.visits.build(visit_params.except(:latitude, :longitude)) - - # If coordinates are provided but no place_id, create a place - if visit_params[:latitude].present? && visit_params[:longitude].present? && visit.place_id.blank? - place = create_place_from_coordinates(visit_params[:latitude], visit_params[:longitude], visit_params[:name]) - if place - visit.place = place - else - return render json: { error: 'Failed to create place for visit' }, status: :unprocessable_entity - end - end - - # Validate that visit has a place - if visit.place.blank? - return render json: { error: 'Visit must have a valid place' }, status: :unprocessable_entity - end - - # Set visit times and calculate duration - visit.started_at = DateTime.parse(visit_params[:started_at]) - visit.ended_at = DateTime.parse(visit_params[:ended_at]) - visit.duration = (visit.ended_at - visit.started_at) * 24 * 60 # duration in minutes + service = Visits::Create.new(current_api_user, visit_params) - # Set status to confirmed for manually created visits - visit.status = :confirmed - - visit.save! - - render json: Api::VisitSerializer.new(visit).call + if service.call + render json: Api::VisitSerializer.new(service.visit).call + else + render json: { + error: 'Failed to create visit', + errors: service.errors + }, status: :unprocessable_entity + end end def update @@ -107,51 +89,6 @@ class Api::V1::VisitsController < ApiController params.permit(:status, visit_ids: []) end - def create_place_from_coordinates(latitude, longitude, name) - Rails.logger.info "Creating place from coordinates: lat=#{latitude}, lon=#{longitude}, name=#{name}" - - # Create a place at the specified coordinates - place_name = name.presence || Place::DEFAULT_NAME - - # Validate coordinates - lat_f = latitude.to_f - lon_f = longitude.to_f - - if lat_f.abs > 90 || lon_f.abs > 180 - Rails.logger.error "Invalid coordinates: lat=#{lat_f}, lon=#{lon_f}" - return nil - end - - # Check if a place already exists very close to these coordinates (within 10 meters) - existing_place = Place.joins("JOIN visits ON places.id = visits.place_id") - .where(visits: { user: current_api_user }) - .where( - "ST_DWithin(lonlat, ST_SetSRID(ST_MakePoint(?, ?), 4326), ?)", - lon_f, lat_f, 0.0001 # approximately 10 meters - ).first - - if existing_place - Rails.logger.info "Found existing place: #{existing_place.id}" - return existing_place - end - - # Create new place with both coordinate formats - place = Place.create!( - name: place_name, - latitude: lat_f, - longitude: lon_f, - lonlat: "POINT(#{lon_f} #{lat_f})", - source: :manual - ) - - Rails.logger.info "Created new place: #{place.id} at #{place.lonlat}" - place - rescue StandardError => e - Rails.logger.error "Failed to create place: #{e.class} - #{e.message}" - Rails.logger.error e.backtrace.join("\n") if Rails.env.development? - nil - end - def update_visit(visit) visit_params.each do |key, value| next if %w[latitude longitude].include?(key.to_s) diff --git a/app/models/visit.rb b/app/models/visit.rb index 794cfe06..936fdc7d 100644 --- a/app/models/visit.rb +++ b/app/models/visit.rb @@ -10,6 +10,8 @@ class Visit < ApplicationRecord validates :started_at, :ended_at, :duration, :name, :status, presence: true + validates :ended_at, comparison: { greater_than: :started_at } + enum :status, { suggested: 0, confirmed: 1, declined: 2 } def coordinates diff --git a/app/services/visits/create.rb b/app/services/visits/create.rb new file mode 100644 index 00000000..7f521f35 --- /dev/null +++ b/app/services/visits/create.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Visits + class Create + attr_reader :user, :params, :errors, :visit + + def initialize(user, params) + @user = user + @params = params.respond_to?(:with_indifferent_access) ? params.with_indifferent_access : params + @errors = [] + @visit = nil + end + + def call + ActiveRecord::Base.transaction do + place = find_or_create_place + return false unless place + + create_visit(place) + end + rescue StandardError => e + ExceptionReporter.call(e, 'Failed to create visit') + + false + end + + private + + def find_or_create_place + existing_place = find_existing_place + + return existing_place if existing_place + + create_new_place + end + + def find_existing_place + Place.joins("JOIN visits ON places.id = visits.place_id") + .where(visits: { user: user }) + .where( + "ST_DWithin(lonlat, ST_SetSRID(ST_MakePoint(?, ?), 4326), ?)", + params[:longitude].to_f, params[:latitude].to_f, 0.0001 # approximately 10 meters + ).first + end + + def create_new_place + place_name = params[:name] + lat_f = params[:latitude].to_f + lon_f = params[:longitude].to_f + + place = Place.create!( + name: place_name, + latitude: lat_f, + longitude: lon_f, + lonlat: "POINT(#{lon_f} #{lat_f})", + source: :manual + ) + + place + rescue StandardError => e + ExceptionReporter.call(e, 'Failed to create place') + + nil + end + + def create_visit(place) + started_at = DateTime.parse(params[:started_at]) + ended_at = DateTime.parse(params[:ended_at]) + duration_minutes = (ended_at - started_at) * 24 * 60 + + @visit = user.visits.create!( + name: params[:name], + place: place, + started_at: started_at, + ended_at: ended_at, + duration: duration_minutes.to_i, + status: :confirmed + ) + + @visit + end + end +end diff --git a/spec/models/visit_spec.rb b/spec/models/visit_spec.rb index 563d4370..edff03a3 100644 --- a/spec/models/visit_spec.rb +++ b/spec/models/visit_spec.rb @@ -10,6 +10,21 @@ RSpec.describe Visit, type: :model do it { is_expected.to have_many(:points).dependent(:nullify) } end + describe 'validations' do + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_presence_of(:started_at) } + it { is_expected.to validate_presence_of(:ended_at) } + it { is_expected.to validate_presence_of(:duration) } + it { is_expected.to validate_presence_of(:status) } + + it 'validates ended_at is greater than started_at' do + visit = build(:visit, started_at: Time.zone.now, ended_at: Time.zone.now - 1.hour) + + expect(visit).not_to be_valid + expect(visit.errors[:ended_at]).to include("must be greater than #{visit.started_at}") + end + end + describe 'factory' do it { expect(build(:visit)).to be_valid } end diff --git a/spec/services/visits/create_spec.rb b/spec/services/visits/create_spec.rb new file mode 100644 index 00000000..bddfaf54 --- /dev/null +++ b/spec/services/visits/create_spec.rb @@ -0,0 +1,176 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Visits::Create do + let(:user) { create(:user) } + let(:valid_params) do + { + name: 'Test Visit', + latitude: 52.52, + longitude: 13.405, + started_at: '2023-12-01T10:00:00Z', + ended_at: '2023-12-01T12:00:00Z' + } + end + + describe '#call' do + context 'when all parameters are valid' do + subject(:service) { described_class.new(user, valid_params) } + + it 'creates a visit successfully' do + expect { service.call }.to change { user.visits.count }.by(1) + expect(service.call).to be_truthy + expect(service.visit).to be_persisted + end + + it 'creates a visit with correct attributes' do + service.call + visit = service.visit + + expect(visit.name).to eq('Test Visit') + expect(visit.user).to eq(user) + expect(visit.status).to eq('confirmed') + expect(visit.started_at).to eq(DateTime.parse('2023-12-01T10:00:00Z')) + expect(visit.ended_at).to eq(DateTime.parse('2023-12-01T12:00:00Z')) + expect(visit.duration).to eq(120) # 2 hours in minutes + end + + it 'creates a place with correct coordinates' do + service.call + place = service.visit.place + + expect(place).to be_persisted + expect(place.name).to eq('Test Visit') + expect(place.latitude).to eq(52.52) + expect(place.longitude).to eq(13.405) + expect(place.source).to eq('manual') + end + + it 'has no errors' do + service.call + expect(service.errors).to be_empty + end + end + + context 'when reusing existing place' do + let!(:existing_place) do + create(:place, + latitude: 52.52, + longitude: 13.405, + lonlat: 'POINT(13.405 52.52)') + end + let!(:existing_visit) { create(:visit, user: user, place: existing_place) } + + subject(:service) { described_class.new(user, valid_params) } + + it 'reuses the existing place' do + expect { service.call }.not_to change { Place.count } + expect(service.visit.place).to eq(existing_place) + end + + it 'creates a new visit with the existing place' do + expect { service.call }.to change { user.visits.count }.by(1) + expect(service.visit.place).to eq(existing_place) + end + end + + context 'when place creation fails' do + subject(:service) { described_class.new(user, valid_params) } + + before do + allow(Place).to receive(:create!).and_raise(ActiveRecord::RecordInvalid.new(Place.new)) + end + + it 'returns false' do + expect(service.call).to be(false) + end + + it 'calls ExceptionReporter' do + expect(ExceptionReporter).to receive(:call) + + service.call + end + + it 'does not create a visit' do + expect { service.call }.not_to change { Visit.count } + end + end + + context 'when visit creation fails' do + subject(:service) { described_class.new(user, valid_params) } + + before do + allow_any_instance_of(User).to receive_message_chain(:visits, :create!).and_raise(ActiveRecord::RecordInvalid.new(Visit.new)) + end + + it 'returns false' do + expect(service.call).to be(false) + end + + it 'calls ExceptionReporter' do + expect(ExceptionReporter).to receive(:call) + + service.call + end + end + + context 'edge cases' do + context 'when name is not provided but defaults are used' do + let(:params) { valid_params.merge(name: '') } + subject(:service) { described_class.new(user, params) } + + it 'returns false due to validation' do + expect(service.call).to be(false) + end + end + + context 'when coordinates are strings' do + let(:params) do + valid_params.merge( + latitude: '52.52', + longitude: '13.405' + ) + end + subject(:service) { described_class.new(user, params) } + + it 'converts them to floats and creates visit successfully' do + expect(service.call).to be_truthy + place = service.visit.place + expect(place.latitude).to eq(52.52) + expect(place.longitude).to eq(13.405) + end + end + + context 'when visit duration is very short' do + let(:params) do + valid_params.merge( + started_at: '2023-12-01T12:00:00Z', + ended_at: '2023-12-01T12:01:00Z' # 1 minute + ) + end + subject(:service) { described_class.new(user, params) } + + it 'creates visit with correct duration' do + service.call + expect(service.visit.duration).to eq(1) + end + end + + context 'when visit duration is very long' do + let(:params) do + valid_params.merge( + started_at: '2023-12-01T08:00:00Z', + ended_at: '2023-12-02T20:00:00Z' # 36 hours + ) + end + subject(:service) { described_class.new(user, params) } + + it 'creates visit with correct duration' do + service.call + expect(service.visit.duration).to eq(36 * 60) # 36 hours in minutes + end + end + end + end +end