Extract visit creation to a service

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
 

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)

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

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

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

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