Fix rest of the tests

app/controllers/api/v1/places_controller.rb

@@ -18,10 +18,11 @@ module Api
       end
 
       def create
-        @place = current_api_user.places.build(place_params)
+        @place = current_api_user.places.build(place_params.except(:tag_ids))
 
         if @place.save
           add_tags if tag_ids.present?
+          @place.reload # Reload to get updated associations
           render json: serialize_place(@place), status: :created
         else
           render json: { errors: @place.errors.full_messages }, status: :unprocessable_entity
@@ -65,11 +66,13 @@ module Api
       end
 
       def place_params
-        params.require(:place).permit(:name, :latitude, :longitude, :source, :note)
+        params.require(:place).permit(:name, :latitude, :longitude, :source, :note, tag_ids: [])
       end
 
       def tag_ids
-        params.dig(:place, :tag_ids) || []
+        # tag_ids can be in params[:place][:tag_ids] or params[:tag_ids]
+        ids = params.dig(:place, :tag_ids) || params[:tag_ids]
+        Array(ids).compact
       end
 
       def add_tags

spec/factories/places.rb

@@ -41,6 +41,26 @@ FactoryBot.define do
       end
     end
 
+    # Trait for setting coordinates from lonlat geometry
+    # This is forward-compatible for when latitude/longitude are deprecated
+    trait :from_lonlat do
+      transient do
+        lonlat_wkt { nil }
+      end
+
+      after(:build) do |place, evaluator|
+        if evaluator.lonlat_wkt
+          # Parse WKT to extract coordinates
+          # Format: "POINT(longitude latitude)" or "SRID=4326;POINT(longitude latitude)"
+          coords = evaluator.lonlat_wkt.match(/POINT\(([^ ]+) ([^ ]+)\)/)
+          if coords
+            place.longitude = coords[1].to_f
+            place.latitude = coords[2].to_f
+          end
+        end
+      end
+    end
+
     # Special trait for testing with nil lonlat
     trait :without_lonlat do
       # Skip validation to create an invalid record for testing

spec/jobs/data_migrations/migrate_places_lonlat_job_spec.rb

@@ -7,15 +7,22 @@ RSpec.describe DataMigrations::MigratePlacesLonlatJob, type: :job do
     let(:user) { create(:user) }
 
     context 'when places exist for the user' do
-      let!(:place1) { create(:place, :without_lonlat, longitude: 10.0, latitude: 20.0) }
+      let!(:place1) { create(:place, user: user, longitude: 10.0, latitude: 20.0) }
-      let!(:place2) { create(:place, :without_lonlat, longitude: -73.935242, latitude: 40.730610) }
+      let!(:place2) { create(:place, user: user, longitude: -73.935242, latitude: 40.730610) }
-      let!(:other_place) { create(:place, :without_lonlat, longitude: 15.0, latitude: 25.0) }
+      let!(:other_place) { create(:place, longitude: 15.0, latitude: 25.0) }
 
       # Create visits to associate places with users
       let!(:visit1) { create(:visit, user: user, place: place1) }
       let!(:visit2) { create(:visit, user: user, place: place2) }
       let!(:other_visit) { create(:visit, place: other_place) } # associated with a different user
 
+      # Simulate old data by clearing lonlat after creation (to test migration)
+      before do
+        place1.update_column(:lonlat, nil)
+        place2.update_column(:lonlat, nil)
+        other_place.update_column(:lonlat, nil)
+      end
+
       it 'updates lonlat field for all places belonging to the user' do
         # Force a reload to ensure we have the initial state
         place1.reload

spec/services/reverse_geocoding/places/fetch_data_spec.rb

@@ -591,7 +591,7 @@ RSpec.describe ReverseGeocoding::Places::FetchData do
     end
 
     context 'when lonlat is already present on existing place' do
-      let!(:existing_place) { create(:place, :with_geodata, lonlat: 'POINT(10.0 50.0)') }
+      let!(:existing_place) { create(:place, :with_geodata, latitude: 50.0, longitude: 10.0) }
       let(:existing_data) do
         double(
           data: {
@@ -600,7 +600,9 @@ RSpec.describe ReverseGeocoding::Places::FetchData do
               'osm_id' => existing_place.geodata.dig('properties', 'osm_id'),
               'name' => 'Updated Name'
             }
-          }
+          },
+          latitude: 55.0,
+          longitude: 15.0
         )
       end
 
@@ -608,11 +610,14 @@ RSpec.describe ReverseGeocoding::Places::FetchData do
         allow(Geocoder).to receive(:search).and_return([mock_geocoded_place, existing_data])
       end
 
-      it 'does not override existing lonlat' do
+      it 'does not override existing coordinates when updating geodata' do
         service.call
 
         existing_place.reload
+        # lonlat is auto-generated from latitude/longitude, so it should remain based on original coordinates
         expect(existing_place.lonlat.to_s).to eq('POINT (10.0 50.0)')
+        expect(existing_place.latitude).to eq(50.0)
+        expect(existing_place.longitude).to eq(10.0)
       end
     end
   end

spec/services/users/export_data/places_spec.rb

@@ -16,8 +16,8 @@ RSpec.describe Users::ExportData::Places, type: :service do
     end
 
     context 'when user has places' do
-      let!(:place1) { create(:place, name: 'Home', longitude: -74.0059, latitude: 40.7128) }
+      let!(:place1) { create(:place, user: user, name: 'Home', longitude: -74.0059, latitude: 40.7128) }
-      let!(:place2) { create(:place, name: 'Office', longitude: -73.9851, latitude: 40.7589) }
+      let!(:place2) { create(:place, user: user, name: 'Office', longitude: -73.9851, latitude: 40.7589) }
       let!(:visit1) { create(:visit, user: user, place: place1) }
       let!(:visit2) { create(:visit, user: user, place: place2) }
 

spec/services/users/export_import_integration_spec.rb

@@ -128,9 +128,9 @@ RSpec.describe 'Users Export-Import Integration', type: :service do
       original_user = create(:user, email: 'original@example.com')
 
       # Create places with different characteristics
-      home_place = create(:place, name: 'Home', latitude: 40.7128, longitude: -74.0060)
+      home_place = create(:place, user: original_user, name: 'Home', latitude: 40.7128, longitude: -74.0060)
-      office_place = create(:place, name: 'Office', latitude: 40.7589, longitude: -73.9851)
+      office_place = create(:place, user: original_user, name: 'Office', latitude: 40.7589, longitude: -73.9851)
-      gym_place = create(:place, name: 'Gym', latitude: 40.7505, longitude: -73.9934)
+      gym_place = create(:place, user: original_user, name: 'Gym', latitude: 40.7505, longitude: -73.9934)
 
       # Create visits associated with those places
       create(:visit, user: original_user, place: home_place, name: 'Home Visit')

spec/services/visits/creator_spec.rb

@@ -25,7 +25,7 @@ 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(:place) { create(:place, user: user, latitude: 40.7128, longitude: -74.0060, name: 'Existing Place') }
       let!(:existing_visit) do
         create(
           :visit,
@@ -61,7 +61,7 @@ RSpec.describe Visits::Creator do
     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(:different_place) { create(:place, user: user, latitude: 41.0000, longitude: -73.9000, name: 'Different Place') }
       let!(:existing_visit) do
         create(
           :visit,

spec/support/map_layer_helpers.rb

@@ -1,70 +0,0 @@
-# frozen_string_literal: true
-
-module MapLayerHelpers
-  OVERLAY_LAYERS = [
-    'Points',
-    'Routes',
-    'Fog of War',
-    'Heatmap',
-    'Scratch map',
-    'Areas',
-    'Photos',
-    'Suggested Visits',
-    'Confirmed Visits'
-  ].freeze
-
-  def test_layer_toggle(layer_name)
-    within('.leaflet-control-layers-expanded') do
-      if page.has_content?(layer_name)
-        # Find the label that contains the layer name, then find its associated checkbox
-        layer_label = find('label', text: layer_name)
-        layer_checkbox = layer_label.find('input[type="checkbox"]', visible: false)
-
-        # Get initial state
-        initial_checked = layer_checkbox.checked?
-
-        # Toggle the layer by clicking the label (more reliable)
-        layer_label.click
-        sleep 0.5 # Small delay for layer toggle
-
-        # Verify state changed
-        expect(layer_checkbox.checked?).not_to eq(initial_checked)
-
-        # Toggle back
-        layer_label.click
-        sleep 0.5 # Small delay for layer toggle
-
-        # Verify state returned to original
-        expect(layer_checkbox.checked?).to eq(initial_checked)
-      end
-    end
-  end
-
-  def test_base_layer_switching
-    within('.leaflet-control-layers-expanded') do
-      # Check that we have base layer options (radio buttons)
-      expect(page).to have_css('input[type="radio"]')
-
-      # Verify OpenStreetMap is available
-      expect(page).to have_content('OpenStreetMap')
-
-      # Test clicking different radio buttons if available
-      radio_buttons = all('input[type="radio"]', visible: false)
-      expect(radio_buttons.length).to be >= 1
-
-      # Click the first radio button to test layer switching
-      if radio_buttons.length > 1
-        radio_buttons[1].click
-        sleep 1
-
-        # Click back to the first one
-        radio_buttons[0].click
-        sleep 1
-      end
-    end
-  end
-end
-
-RSpec.configure do |config|
-  config.include MapLayerHelpers, type: :system
-end

spec/support/point_helpers.rb

@@ -1,20 +0,0 @@
-# frozen_string_literal: true
-
-module PointHelpers
-  # Creates a list of points spaced ~100m apart northwards
-  def create_points_around(user:, count:, base_lat: 20.0, base_lon: 10.0, timestamp: nil, **attrs)
-    Array.new(count) do |i|
-      create(
-        :point,
-        user: user,
-        timestamp: (timestamp.respond_to?(:call) ? timestamp.call(i) : timestamp) || (Time.current - i.minutes).to_i,
-        lonlat: "POINT(#{base_lon} #{base_lat + i * 0.0009})",
-        **attrs
-      )
-    end
-  end
-end
-
-RSpec.configure do |config|
-  config.include PointHelpers
-end

spec/support/polyline_popup_helpers.rb

@@ -1,150 +0,0 @@
-# frozen_string_literal: true
-
-module PolylinePopupHelpers
-  def trigger_polyline_hover_and_get_popup
-    # Wait for polylines to be fully loaded
-    expect(page).to have_css('.leaflet-overlay-pane', wait: 10)
-    sleep 2 # Allow time for polylines to render
-
-    # Try multiple approaches to trigger polyline hover
-    popup_content = try_canvas_hover || try_polyline_click || try_map_interaction
-
-    popup_content
-  end
-
-  def verify_popup_content_structure(popup_content, distance_unit)
-    return false unless popup_content
-
-    # Check for required fields in popup
-    required_fields = [
-      'Start:',
-      'End:',
-      'Duration:',
-      'Total Distance:',
-      'Current Speed:'
-    ]
-
-    # Check that all required fields are present
-    fields_present = required_fields.all? { |field| popup_content.include?(field) }
-
-    # Check distance unit in Total Distance field
-    distance_unit_present = popup_content.include?(distance_unit == 'km' ? 'km' : 'mi')
-
-    # Check speed unit in Current Speed field (should match distance unit)
-    speed_unit_present = if distance_unit == 'mi'
-                          popup_content.include?('mph')
-                        else
-                          popup_content.include?('km/h')
-                        end
-
-    fields_present && distance_unit_present && speed_unit_present
-  end
-
-  def extract_popup_data(popup_content)
-    return {} unless popup_content
-
-    data = {}
-
-    # Extract start time
-    if match = popup_content.match(/Start:<\/strong>\s*([^<]+)/)
-      data[:start] = match[1].strip
-    end
-
-    # Extract end time
-    if match = popup_content.match(/End:<\/strong>\s*([^<]+)/)
-      data[:end] = match[1].strip
-    end
-
-    # Extract duration
-    if match = popup_content.match(/Duration:<\/strong>\s*([^<]+)/)
-      data[:duration] = match[1].strip
-    end
-
-    # Extract total distance
-    if match = popup_content.match(/Total Distance:<\/strong>\s*([^<]+)/)
-      data[:total_distance] = match[1].strip
-    end
-
-    # Extract current speed
-    if match = popup_content.match(/Current Speed:<\/strong>\s*([^<]+)/)
-      data[:current_speed] = match[1].strip
-    end
-
-    data
-  end
-
-  private
-
-  def try_canvas_hover
-    page.evaluate_script(<<~JS)
-      return new Promise((resolve) => {
-        const polylinesPane = document.querySelector('.leaflet-polylinesPane-pane');
-        if (polylinesPane) {
-          const canvas = polylinesPane.querySelector('canvas');
-          if (canvas) {
-            const rect = canvas.getBoundingClientRect();
-            const centerX = rect.left + rect.width / 2;
-            const centerY = rect.top + rect.height / 2;
-
-            const event = new MouseEvent('mouseover', {
-              bubbles: true,
-              cancelable: true,
-              clientX: centerX,
-              clientY: centerY
-            });
-
-            canvas.dispatchEvent(event);
-
-            setTimeout(() => {
-              const popup = document.querySelector('.leaflet-popup-content');
-              resolve(popup ? popup.innerHTML : null);
-            }, 1000);
-          } else {
-            resolve(null);
-          }
-        } else {
-          resolve(null);
-        }
-      });
-    JS
-  rescue => e
-    Rails.logger.debug "Canvas hover failed: #{e.message}"
-    nil
-  end
-
-  def try_polyline_click
-    # Try to find and click on polyline elements directly
-    if page.has_css?('path[stroke]', wait: 2)
-      polyline = first('path[stroke]')
-      polyline.click if polyline
-      sleep 1
-
-      if page.has_css?('.leaflet-popup-content')
-        return find('.leaflet-popup-content').native.inner_html
-      end
-    end
-    nil
-  rescue => e
-    Rails.logger.debug "Polyline click failed: #{e.message}"
-    nil
-  end
-
-  def try_map_interaction
-    # As a fallback, click in the center of the map
-    map_element = find('.leaflet-container')
-    map_element.click
-    sleep 1
-
-    if page.has_css?('.leaflet-popup-content', wait: 2)
-      return find('.leaflet-popup-content').native.inner_html
-    end
-    nil
-  rescue => e
-    Rails.logger.debug "Map interaction failed: #{e.message}"
-    nil
-  end
-end
-
-RSpec.configure do |config|
-  config.include PolylinePopupHelpers, type: :system
-end

spec/support/shared_examples/map_examples.rb

@@ -1,132 +0,0 @@
-# frozen_string_literal: true
-
-RSpec.shared_context 'authenticated map user' do
-  before do
-    sign_in_and_visit_map(user)
-  end
-end
-
-RSpec.shared_examples 'map basic functionality' do
-  it 'displays the leaflet map with basic elements' do
-    expect(page).to have_css('#map')
-    expect(page).to have_css('.leaflet-map-pane')
-    expect(page).to have_css('.leaflet-tile-pane')
-  end
-
-  it 'loads map data and displays route information' do
-    expect(page).to have_css('.leaflet-overlay-pane', wait: 10)
-    expect(page).to have_css('[data-maps-target="container"]')
-  end
-end
-
-RSpec.shared_examples 'map controls' do
-  it 'has zoom controls' do
-    expect(page).to have_css('.leaflet-control-zoom')
-    expect(page).to have_css('.leaflet-control-zoom-in')
-    expect(page).to have_css('.leaflet-control-zoom-out')
-  end
-
-  it 'has layer control' do
-    expect(page).to have_css('.leaflet-control-layers', wait: 10)
-  end
-
-  it 'has scale control' do
-    expect(page).to have_css('.leaflet-control-scale')
-    expect(page).to have_css('.leaflet-control-scale-line')
-  end
-
-  it 'has stats control' do
-    expect(page).to have_css('.leaflet-control-stats', wait: 10)
-  end
-
-  it 'has attribution control' do
-    expect(page).to have_css('.leaflet-control-attribution')
-  end
-end
-
-RSpec.shared_examples 'expandable layer control' do
-  let(:layer_control) { find('.leaflet-control-layers') }
-
-  def expand_layer_control
-    if page.has_css?('.leaflet-control-layers-toggle', visible: true)
-      find('.leaflet-control-layers-toggle').click
-    else
-      layer_control.click
-    end
-    expect(page).to have_css('.leaflet-control-layers-expanded', wait: 5)
-  end
-
-  def collapse_layer_control
-    if page.has_css?('.leaflet-control-layers-toggle', visible: true)
-      find('.leaflet-control-layers-toggle').click
-    else
-      find('.leaflet-container').click
-    end
-    sleep 1
-    expect(page).not_to have_css('.leaflet-control-layers-expanded')
-  end
-end
-
-RSpec.shared_examples 'polyline popup content' do |distance_unit|
-  it "displays correct popup content with #{distance_unit} units" do
-    # Wait for polylines to load
-    expect(page).to have_css('.leaflet-overlay-pane', wait: 10)
-    sleep 2 # Allow polylines to fully render
-
-    # Find and hover over a polyline to trigger popup
-    # We need to use JavaScript to trigger the mouseover event on polylines
-    popup_content = page.evaluate_script(<<~JS)
-      // Find the first polyline group and trigger mouseover
-      const polylinesPane = document.querySelector('.leaflet-polylinesPane-pane');
-      if (polylinesPane) {
-        const canvas = polylinesPane.querySelector('canvas');
-        if (canvas) {
-          // Create a mouseover event at the center of the canvas
-          const rect = canvas.getBoundingClientRect();
-          const centerX = rect.left + rect.width / 2;
-          const centerY = rect.top + rect.height / 2;
-
-          const event = new MouseEvent('mouseover', {
-            bubbles: true,
-            cancelable: true,
-            clientX: centerX,
-            clientY: centerY
-          });
-
-          canvas.dispatchEvent(event);
-
-          // Wait a moment for popup to appear
-          setTimeout(() => {
-            const popup = document.querySelector('.leaflet-popup-content');
-            return popup ? popup.innerHTML : null;
-          }, 500);
-        }
-      }
-      return null;
-    JS
-
-    # Alternative approach: try to click on the map area where polylines should be
-    if popup_content.nil?
-      # Click in the center of the map to potentially trigger polyline interaction
-      map_element = find('.leaflet-container')
-      map_element.click
-      sleep 1
-
-      # Try to find any popup that might have appeared
-      if page.has_css?('.leaflet-popup-content', wait: 2)
-        popup_content = find('.leaflet-popup-content').text
-      end
-    end
-
-    # If we still don't have popup content, let's verify the polylines exist and are interactive
-    expect(page).to have_css('.leaflet-overlay-pane')
-
-            # Check that the map has the expected data attributes for distance unit
-    map_element = find('#map')
-    expect(map_element['data-user_settings']).to include("maps")
-
-    # Verify the user settings contain the expected distance unit
-    user_settings = JSON.parse(map_element['data-user_settings'])
-    expect(user_settings.dig('maps', 'distance_unit')).to eq(distance_unit)
-  end
-end

spec/support/system_helpers.rb

@@ -1,24 +0,0 @@
-# frozen_string_literal: true
-
-module SystemHelpers
-  include Rails.application.routes.url_helpers
-
-  def sign_in_user(user, password = 'password123')
-    visit '/users/sign_in'
-    expect(page).to have_field('Email', wait: 10)
-    fill_in 'Email', with: user.email
-    fill_in 'Password', with: password
-    click_button 'Log in'
-  end
-
-  def sign_in_and_visit_map(user, password = 'password123')
-    sign_in_user(user, password)
-    expect(page).to have_current_path('/map')
-    expect(page).to have_css('.leaflet-container', wait: 10)
-  end
-end
-
-RSpec.configure do |config|
-  config.include SystemHelpers, type: :system
-  config.include Rails.application.routes.url_helpers, type: :system
-end