Implement error message for failed imports

Freika · Freika · commit b6c2d2e20f6b · 2026-02-22T23:03:37.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ This release adds a dedicated `motion_data` column for transportation-relevant f
 - Tracks are exported with their `original_path` serialized as WKT and `track_segments` embedded as a nested array, preserving transportation mode detection data across export/import cycles.
 - Digests get a fresh `sharing_uuid` on import for security — old share links from the original user won't work for the importing user.
 - Raw Data Archives are exported with their attached gzip files, enabling full data restoration.
+- Failed imports now will have an error message shown to the user.
 
 ### Changed
 
@@ -40,6 +41,7 @@ This release adds a dedicated `motion_data` column for transportation-relevant f
 
 - Stats queries (daily distance, time of day) now correctly handle timezone conversion without double-converting from UTC.
 - Timezone validation in stats queries now properly resolves Rails timezone names to IANA identifiers.
+- Clicking on [Map] on Stats page now correctly respects the user's preferred map version (v1 or v2) instead of always linking to Map v1. #2281
 
 
 ## [1.2.0] - 2026-02-15
diff --git a/app/assets/builds/tailwind.css b/app/assets/builds/tailwind.css
diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb
@@ -13,7 +13,7 @@ class ImportsController < ApplicationController
 
   def index
     @imports = policy_scope(Import)
-               .select(:id, :name, :source, :created_at, :processed, :status)
+               .select(:id, :name, :source, :created_at, :processed, :status, :error_message)
                .with_attached_file
                .order(created_at: :desc)
                .page(params[:page])
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
@@ -133,7 +133,7 @@ def trial_button_class(user)
 
   def trial_days_remaining_compact(user)
     expiry = user.active_until
-    return "Expired" if expiry.blank? || expiry.past?
+    return 'Expired' if expiry.blank? || expiry.past?
 
     days_left = [(expiry.to_date - Time.zone.today).to_i, 0].max
     "#{days_left}d left"
@@ -159,11 +159,11 @@ def email_password_login_enabled?
     ALLOW_EMAIL_PASSWORD_REGISTRATION
   end
 
-  def preferred_map_path
-    return map_v2_path unless user_signed_in?
+  def preferred_map_path(params = {})
+    return map_v2_path(params) unless user_signed_in?
 
     preferred_version = current_user.safe_settings.maps&.dig('preferred_version')
-    preferred_version == 'v1' ? map_v1_path : map_v2_path
+    preferred_version == 'v1' ? map_v1_path(params) : map_v2_path(params)
   end
 
   def format_duration_short(seconds)
diff --git a/app/helpers/stats_helper.rb b/app/helpers/stats_helper.rb
@@ -122,7 +122,7 @@ def peak_day(stat)
     distance_value = Stat.convert_distance(peak[1], distance_unit).round
     text = "#{date.strftime('%B %d')} (#{distance_value} #{distance_unit})"
 
-    link_to text, map_url(start_at: date.beginning_of_day, end_at: date.end_of_day), class: 'underline'
+    link_to text, preferred_map_path(start_at: date.beginning_of_day, end_at: date.end_of_day), class: 'underline'
   end
 
   def quietest_week(stat)
diff --git a/app/jobs/stale_jobs_recovery_job.rb b/app/jobs/stale_jobs_recovery_job.rb
@@ -31,7 +31,7 @@ def recover_stale_exports
 
   def recover_stale_imports
     Import.processing.where(processing_started_at: ...IMPORT_TIMEOUT.ago).find_each do |import|
-      import.update!(status: :failed)
+      import.update!(status: :failed, error_message: 'Import timed out after being stuck in processing')
 
       Notifications::Create.new(
         user: import.user,
diff --git a/app/jobs/users/import_data_job.rb b/app/jobs/users/import_data_job.rb
@@ -28,6 +28,7 @@ def perform(import_id)
     user_id = user&.id || import&.user_id || 'unknown'
     ExceptionReporter.call(e, "Import job failed for user #{user_id}")
 
+    import&.update!(status: :failed, error_message: e.message)
     create_import_failed_notification(user, e)
 
     raise e
diff --git a/app/services/imports/create.rb b/app/services/imports/create.rb
@@ -30,7 +30,7 @@ def call
     update_import_points_count(import)
     User.reset_counters(user.id, :points)
   rescue StandardError => e
-    import.update!(status: :failed)
+    import.update!(status: :failed, error_message: e.message)
     broadcast_status_update
 
     ExceptionReporter.call(e, 'Import failed')
diff --git a/app/services/supporter/verify_email.rb b/app/services/supporter/verify_email.rb
@@ -33,7 +33,7 @@ def fetch_supporter_status
       )
 
       response.success? ? response.parsed_response.symbolize_keys : { supporter: false }
-    rescue HTTParty::Error, Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, SocketError => e
+    rescue StandardError => e
       Rails.logger.warn("Supporter verification failed: #{e.message}")
       { supporter: false }
     end
diff --git a/app/views/imports/_table_row.html.erb b/app/views/imports/_table_row.html.erb
@@ -14,7 +14,14 @@
   <td data-points-count>
     <%= number_with_delimiter import.processed %>
   </td>
-  <td data-status-display><%= import.status %></td>
+  <td data-status-display>
+    <%= import.status %>
+    <% if import.failed? %>
+      <span class="tooltip tooltip-left cursor-help" data-tip="<%= import.error_message.presence || 'Import failed' %>">
+        ⚠
+      </span>
+    <% end %>
+  </td>
   <td><%= human_datetime(import.created_at) %></td>
   <td class="whitespace-nowrap">
     <% if import.deleting? %>
diff --git a/app/views/stats/_year.html.erb b/app/views/stats/_year.html.erb
@@ -1,6 +1,6 @@
 <h2 class='text-3xl font-bold mt-10'>
   <%= link_to year, "/stats/#{year}", class: "underline hover:no-underline text-#{header_colors.sample}" %>
-  <%= link_to '[Map]', map_url(year_timespan(year)), class: 'underline hover:no-underline' %>
+  <%= link_to '[Map]', preferred_map_path(year_timespan(year)), class: 'underline hover:no-underline' %>
 </h2>
 <div class='my-10'>
   <%= column_chart(
diff --git a/app/views/stats/index.html.erb b/app/views/stats/index.html.erb
@@ -50,7 +50,7 @@
           <h2 class="card-title justify-between text-<%= header_colors[year % header_colors.size] %>">
             <div>
               <%= link_to year, "/stats/#{year}", class: 'underline hover:no-underline' %>
-              <%= link_to '[Map]', map_url(year_timespan(year)), class: 'underline hover:no-underline' %>
+              <%= link_to '[Map]', preferred_map_path(year_timespan(year)), class: 'underline hover:no-underline' %>
             </div>
             <div class="flex items-center gap-2">
               <span class='text-xs text-gray-500'>Last update: <%= human_date(stats.first.updated_at) %></span>
diff --git a/db/migrate/20260222215414_add_error_message_to_imports.rb b/db/migrate/20260222215414_add_error_message_to_imports.rb
@@ -0,0 +1,5 @@
+class AddErrorMessageToImports < ActiveRecord::Migration[8.0]
+  def change
+    add_column :imports, :error_message, :text
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
diff --git a/e2e/v2/map/layers/areas.spec.js b/e2e/v2/map/layers/areas.spec.js
@@ -215,38 +215,35 @@ test.describe('Areas Layer', () => {
         await dialog.accept()
       })
 
-      // Wait for API response
+      // Wait for Turbo form submission response (posts to /areas, not /api/v1/areas)
       const [response] = await Promise.all([
         page.waitForResponse(
-          response => response.url().includes('/api/v1/areas') && response.request().method() === 'POST',
+          response => response.url().endsWith('/areas') && response.request().method() === 'POST',
           { timeout: 10000 }
         ),
-        page.locator('button[type="submit"]:has-text("Create Area")').click()
+        page.locator('[data-area-creation-v2-target="submitButton"]').click()
       ])
 
       const status = response.status()
       console.log('API response status:', status)
 
-      if (status >= 200 && status < 300) {
-        // Success - verify modal closes (modal-open class is removed)
-        await expect(areaModal).not.toHaveClass(/modal-open/, { timeout: 5000 })
-
-        // Wait for area:created event to be processed
-        await page.waitForTimeout(1000)
-
-        // Verify areas layer is now enabled
-        await page.locator('[data-action="click->maps--maplibre#toggleSettings"]').first().click()
-        await page.waitForTimeout(200)
-        await page.locator('button[data-tab="layers"]').click()
-        await page.waitForTimeout(200)
-
-        const areasToggle = page.locator('label:has-text("Areas")').first().locator('input.toggle')
-        await expect(areasToggle).toBeChecked({ timeout: 3000 })
-      } else {
-        // API failed - log the error and fail the test with helpful info
-        const responseBody = await response.text()
-        throw new Error(`API call failed with status ${status}: ${responseBody}`)
-      }
+      expect(status).toBeGreaterThanOrEqual(200)
+      expect(status).toBeLessThan(300)
+
+      // Verify modal closes (modal-open class is removed)
+      await expect(areaModal).not.toHaveClass(/modal-open/, { timeout: 5000 })
+
+      // Wait for area:created event to be processed
+      await page.waitForTimeout(1000)
+
+      // Verify areas layer is now enabled
+      await page.locator('[data-action="click->maps--maplibre#toggleSettings"]').first().click()
+      await page.waitForTimeout(200)
+      await page.locator('button[data-tab="layers"]').click()
+      await page.waitForTimeout(200)
+
+      const areasToggle = page.locator('label:has-text("Areas")').first().locator('input.toggle')
+      await expect(areasToggle).toBeChecked({ timeout: 3000 })
     })
   })
 
@@ -358,8 +355,8 @@ test.describe('Areas Layer', () => {
       // Click the submit button specifically in the area creation modal
       await page.locator('[data-area-creation-v2-target="submitButton"]').click()
 
-      // Wait for creation success
-      await expect(page.locator('.toast:has-text("successfully")')).toBeVisible({ timeout: 10000 })
+      // Wait for creation success (Turbo form uses server-side flash, not client-side Toast)
+      await expect(page.locator('#flash-messages .alert:has-text("successfully")')).toBeVisible({ timeout: 10000 })
       await page.waitForTimeout(2000)
 
       // Get the created area ID
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
@@ -193,6 +193,32 @@
           expect(helper.preferred_map_path).to eq(helper.map_v2_path)
         end
       end
+
+      context 'when called with query params' do
+        let(:params) { { start_at: '2025-01-01T00:00', end_at: '2025-12-31T23:59' } }
+
+        context 'when preferred version is v1' do
+          before do
+            user.settings['maps'] = { 'preferred_version' => 'v1' }
+            user.save
+          end
+
+          it 'returns map_v1_path with query params' do
+            expect(helper.preferred_map_path(params)).to eq(helper.map_v1_path(params))
+          end
+        end
+
+        context 'when preferred version is v2' do
+          before do
+            user.settings['maps'] = { 'preferred_version' => 'v2' }
+            user.save
+          end
+
+          it 'returns map_v2_path with query params' do
+            expect(helper.preferred_map_path(params)).to eq(helper.map_v2_path(params))
+          end
+        end
+      end
     end
   end
 end
diff --git a/spec/jobs/stale_jobs_recovery_job_spec.rb b/spec/jobs/stale_jobs_recovery_job_spec.rb
@@ -63,6 +63,12 @@
         expect(stale_import.reload.status).to eq('failed')
       end
 
+      it 'sets error_message on stale imports' do
+        described_class.new.perform
+
+        expect(stale_import.reload.error_message).to include('stuck in processing')
+      end
+
       it 'does not affect recent imports' do
         described_class.new.perform
 
diff --git a/spec/services/imports/create_spec.rb b/spec/services/imports/create_spec.rb
@@ -52,6 +52,11 @@
           service.call
           expect(import.reload.status).to eq('failed')
         end
+
+        it 'sets the error message' do
+          service.call
+          expect(import.reload.error_message).to eq('StandardError')
+        end
       end
     end
 

Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ def fetch_supporter_status`
`33`	`33`	`)`
`34`	`34`
`35`	`35`	`response.success? ? response.parsed_response.symbolize_keys : { supporter: false }`
`36`		`- rescue HTTParty::Error, Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, SocketError => e`
	`36`	`+ rescue StandardError => e`
`37`	`37`	`Rails.logger.warn("Supporter verification failed: #{e.message}")`
`38`	`38`	`{ supporter: false }`
`39`	`39`	`end`