Fix visits overlapping issue

CHANGELOG.md

@@ -40,6 +40,7 @@ If you're running your own Photon instance, you can safely set `STORE_GEODATA` t
 
 - Reverse geocoding is now working as on-demand job instead of storing the result in the database.
 - Stats cards now show the last update time. #733
+- Visit card now shows buttons to confirm or decline a visit only if it's not confirmed or declined yet.
 
 ## Fixed
 
@@ -47,6 +48,7 @@ If you're running your own Photon instance, you can safely set `STORE_GEODATA` t
 - Importing GeoJSON files now saves velocity if it was stored in either `velocity` or `speed` property.
 - `rake points:migrate_to_lonlat` should work properly now. #1083 #1161
 - PostGIS extension is now being enabled only if it's not already enabled. #1186
+- Fixed a bug where visits were returning into Suggested state after being confirmed or declined. #848
 
 
 # 0.26.0 - 2025-05-08

app/services/visits/creator.rb

@@ -11,6 +11,10 @@ module Visits
 
     def create_visits(visits)
       visits.map do |visit_data|
+        # Check for existing confirmed visits at this location
+        existing_confirmed = find_existing_confirmed_visit(visit_data)
+        next existing_confirmed if existing_confirmed
+
         # Variables to store data outside the transaction
         visit_instance = nil
         place_data = nil
@@ -46,11 +50,46 @@ module Visits
         end
 
         visit_instance
-      end
+      end.compact
     end
 
     private
 
+    # Find if there's already a confirmed visit at this location within a similar time
+    def find_existing_confirmed_visit(visit_data)
+      # Define time window to look for existing visits (slightly wider than the visit)
+      start_time = Time.zone.at(visit_data[:start_time]) - 1.hour
+      end_time = Time.zone.at(visit_data[:end_time]) + 1.hour
+
+      # Look for confirmed visits with a similar location
+      user.visits
+          .confirmed
+          .where('(started_at BETWEEN ? AND ?) OR (ended_at BETWEEN ? AND ?)',
+                 start_time, end_time, start_time, end_time)
+          .find_each do |visit|
+        # Skip if the visit doesn't have place or area coordinates
+        next unless visit.place || visit.area
+
+        # Get coordinates to compare
+        visit_lat = visit.place&.lat || visit.area&.latitude
+        visit_lon = visit.place&.lon || visit.area&.longitude
+
+        next unless visit_lat && visit_lon
+
+        # Calculate distance between centers
+        distance = Geocoder::Calculations.distance_between(
+          [visit_data[:center_lat], visit_data[:center_lon]],
+          [visit_lat, visit_lon],
+          units: :km
+        )
+
+        # If this confirmed visit is within 100 meters of the new suggestion
+        return visit if distance <= 0.1
+      end
+
+      nil
+    end
+
     # Create place_visits records directly to avoid deadlocks
     def associate_suggested_places(visit, suggested_places)
       existing_place_ids = visit.place_visits.pluck(:place_id)

app/views/visits/_buttons.html.erb

@@ -1,2 +1,6 @@
-<%= link_to 'Confirm', visit_path(visit, 'visit[status]': :confirmed), method: :patch, data: { turbo_method: :patch }, class: 'btn btn-xs btn-success' %>
+<% if !visit.confirmed? %>
-<%= link_to 'Decline', visit_path(visit, 'visit[status]': :declined), method: :patch, data: { turbo_method: :patch }, class: 'btn btn-xs btn-error mx-1' %>
+  <%= link_to 'Confirm', visit_path(visit, 'visit[status]': :confirmed), method: :patch, data: { turbo_method: :patch }, class: 'btn btn-xs btn-success' %>
+<% end %>
+<% if !visit.declined? %>
+  <%= link_to 'Decline', visit_path(visit, 'visit[status]': :declined), method: :patch, data: { turbo_method: :patch }, class: 'btn btn-xs btn-error mx-1' %>
+<% end %>

spec/services/visits/creator_spec.rb

@@ -24,6 +24,76 @@ RSpec.describe Visits::Creator do
       }
     end
 
+    context 'when a confirmed visit already exists at the same location' do
+      let(:place) { create(:place, lonlat: 'POINT(-74.0060 40.7128)', name: 'Existing Place') }
+      let!(:existing_visit) do
+        create(
+          :visit,
+          user: user,
+          place: place,
+          status: :confirmed,
+          started_at: 1.5.hours.ago,
+          ended_at: 45.minutes.ago,
+          duration: 45
+        )
+      end
+
+      it 'returns the existing confirmed visit instead of creating a duplicate suggested visit' do
+        visits = subject.create_visits([visit_data])
+
+        expect(visits.size).to eq(1)
+        expect(visits.first).to eq(existing_visit)
+        expect(visits.first.status).to eq('confirmed')
+
+        # Verify no new visits were created
+        expect(Visit.count).to eq(1)
+      end
+
+      it 'does not change points associations' do
+        original_visit_id = point1.visit_id
+
+        subject.create_visits([visit_data])
+
+        # Points should remain unassociated
+        expect(point1.reload.visit_id).to eq(original_visit_id)
+        expect(point2.reload.visit_id).to eq(nil)
+      end
+    end
+
+    context 'when a confirmed visit exists but at a different location' do
+      let(:different_place) { create(:place, lonlat: 'POINT(-73.9000 41.0000)', name: 'Different Place') }
+      let!(:existing_visit) do
+        create(
+          :visit,
+          user: user,
+          place: different_place,
+          status: :confirmed,
+          started_at: 1.5.hours.ago,
+          ended_at: 45.minutes.ago,
+          duration: 45
+        )
+      end
+      let(:place) { create(:place, lonlat: 'POINT(-74.0060 40.7128)', name: 'New Place') }
+      let(:place_finder) { instance_double(Visits::PlaceFinder) }
+
+      before do
+        allow(Visits::PlaceFinder).to receive(:new).with(user).and_return(place_finder)
+        allow(place_finder).to receive(:find_or_create_place).and_return({ main_place: place, suggested_places: [] })
+      end
+
+      it 'creates a new suggested visit' do
+        visits = subject.create_visits([visit_data])
+
+        expect(visits.size).to eq(1)
+        expect(visits.first).not_to eq(existing_visit)
+        expect(visits.first.place).to eq(place)
+        expect(visits.first.status).to eq('suggested')
+
+        # Should now have two visits
+        expect(Visit.count).to eq(2)
+      end
+    end
+
     context 'when matching an area' do
       let!(:area) { create(:area, user: user, latitude: 40.7128, longitude: -74.0060, radius: 100) }