diff --git a/CHANGELOG.md b/CHANGELOG.md index e724b47f..24567bc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fixed a bug preventing the app to start if a composite index on stats table already exists. #2034 - New compiled assets will override old ones on app start to prevent serving stale assets. +- Deleting an import will no longer result in negative points count for the user. + +## Changed + +- Deleting an import will now be processed in the background to prevent request timeouts for large imports. # [0.36.3] - 2025-12-14 diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index 96049978..77b75251 100644 --- a/app/controllers/imports_controller.rb +++ b/app/controllers/imports_controller.rb @@ -78,9 +78,13 @@ class ImportsController < ApplicationController end def destroy - Imports::Destroy.new(current_user, @import).call + @import.deleting! + Imports::DestroyJob.perform_later(@import.id) - redirect_to imports_url, notice: 'Import was successfully destroyed.', status: :see_other + respond_to do |format| + format.html { redirect_to imports_url, notice: 'Import is being deleted.', status: :see_other } + format.turbo_stream + end end private diff --git a/app/javascript/controllers/imports_controller.js b/app/javascript/controllers/imports_controller.js index 4a864074..18205967 100644 --- a/app/javascript/controllers/imports_controller.js +++ b/app/javascript/controllers/imports_controller.js @@ -26,16 +26,23 @@ export default class extends BaseController { received: (data) => { const row = this.element.querySelector(`tr[data-import-id="${data.import.id}"]`); - if (row) { - const pointsCell = row.querySelector('[data-points-count]'); - if (pointsCell) { - pointsCell.textContent = new Intl.NumberFormat().format(data.import.points_count); - } + if (!row) return; - const statusCell = row.querySelector('[data-status-display]'); - if (statusCell && data.import.status) { - statusCell.textContent = data.import.status; - } + // Handle deletion complete - remove the row + if (data.action === 'delete') { + row.remove(); + return; + } + + // Handle status and points updates + const pointsCell = row.querySelector('[data-points-count]'); + if (pointsCell && data.import.points_count !== undefined) { + pointsCell.textContent = new Intl.NumberFormat().format(data.import.points_count); + } + + const statusCell = row.querySelector('[data-status-display]'); + if (statusCell && data.import.status) { + statusCell.textContent = data.import.status; } } } diff --git a/app/jobs/imports/destroy_job.rb b/app/jobs/imports/destroy_job.rb new file mode 100644 index 00000000..952f9eae --- /dev/null +++ b/app/jobs/imports/destroy_job.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +class Imports::DestroyJob < ApplicationJob + queue_as :default + + def perform(import_id) + import = Import.find_by(id: import_id) + return unless import + + import.deleting! + broadcast_status_update(import) + + Imports::Destroy.new(import.user, import).call + + broadcast_deletion_complete(import) + rescue ActiveRecord::RecordNotFound + Rails.logger.warn "Import #{import_id} not found, may have already been deleted" + end + + private + + def broadcast_status_update(import) + ImportsChannel.broadcast_to( + import.user, + { + action: 'status_update', + import: { + id: import.id, + status: import.status + } + } + ) + end + + def broadcast_deletion_complete(import) + ImportsChannel.broadcast_to( + import.user, + { + action: 'delete', + import: { + id: import.id + } + } + ) + end +end diff --git a/app/models/import.rb b/app/models/import.rb index e69e8328..204357e3 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -17,7 +17,7 @@ class Import < ApplicationRecord validate :file_size_within_limit, if: -> { user.trial? } validate :import_count_within_limit, if: -> { user.trial? } - enum :status, { created: 0, processing: 1, completed: 2, failed: 3 } + enum :status, { created: 0, processing: 1, completed: 2, failed: 3, deleting: 4 } enum :source, { google_semantic_history: 0, owntracks: 1, google_records: 2, diff --git a/app/services/imports/destroy.rb b/app/services/imports/destroy.rb index 76870fdb..c348cfb0 100644 --- a/app/services/imports/destroy.rb +++ b/app/services/imports/destroy.rb @@ -9,11 +9,17 @@ class Imports::Destroy end def call + points_count = @import.points_count + ActiveRecord::Base.transaction do - @import.points.delete_all + # Use destroy_all instead of delete_all to trigger counter_cache callbacks + # This ensures users.points_count is properly decremented + @import.points.destroy_all @import.destroy! end + Rails.logger.info "Import #{@import.id} deleted with #{points_count} points" + Stats::BulkCalculator.new(@user.id).call end end diff --git a/app/views/imports/destroy.turbo_stream.erb b/app/views/imports/destroy.turbo_stream.erb new file mode 100644 index 00000000..844a1d9c --- /dev/null +++ b/app/views/imports/destroy.turbo_stream.erb @@ -0,0 +1,24 @@ +<%= turbo_stream.replace "import-#{@import.id}" do %> + + + <%= @import.name %> (<%= @import.source %>) +   + <%= link_to 'πŸ—ΊοΈ', map_path(import_id: @import.id) %> +   + <%= link_to 'πŸ“‹', points_path(import_id: @import.id) %> + + <%= number_to_human_size(@import.file&.byte_size) || 'N/A' %> + + <%= number_with_delimiter @import.processed %> + + deleting + <%= human_datetime(@import.created_at) %> + + + Deleting... + + +<% end %> diff --git a/app/views/imports/index.html.erb b/app/views/imports/index.html.erb index 8cf60feb..6bffe415 100644 --- a/app/views/imports/index.html.erb +++ b/app/views/imports/index.html.erb @@ -72,10 +72,15 @@ <%= import.status %> <%= human_datetime(import.created_at) %> - <% if import.file.present? %> - <%= link_to 'Download', rails_blob_path(import.file, disposition: 'attachment'), class: "btn btn-outline btn-sm btn-info", download: import.name %> + <% if import.deleting? %> + + Deleting... + <% else %> + <% if import.file.present? %> + <%= link_to 'Download', rails_blob_path(import.file, disposition: 'attachment'), class: "btn btn-outline btn-sm btn-info", download: import.name %> + <% end %> + <%= link_to 'Delete', import, data: { turbo_confirm: "Are you sure?", turbo_method: :delete }, method: :delete, class: "btn btn-outline btn-sm btn-error" %> <% end %> - <%= link_to 'Delete', import, data: { turbo_confirm: "Are you sure?", turbo_method: :delete }, method: :delete, class: "btn btn-outline btn-sm btn-error" %> <% end %>