Fix silent CSV export failures and block deleting providers with topics

app/controllers/providers_controller.rb

@@ -39,10 +39,12 @@ def update
   end
 
   def destroy
-    @provider.destroy!
-
     respond_to do |format|
-      format.html { redirect_to providers_path, status: :see_other, notice: "Provider was successfully destroyed." }
+      if @provider.destroy
+        format.html { redirect_to providers_path, status: :see_other, notice: "Provider was successfully destroyed." }
+      else
+        format.html { redirect_to providers_path, status: :see_other, alert: @provider.errors.full_messages.to_sentence }
+      end
     end
   end
 

app/jobs/file_upload_job.rb

@@ -3,14 +3,8 @@ class FileUploadJob < ApplicationJob
   # or use a more specific key to avoid blocking all jobs for a language
   limits_concurrency to: 3, key: ->(_language_id, content_id, _content_type) { "hard-limit" }
 
-  retry_on AzureFileShares::Errors::ApiError, wait: :exponentially_longer, attempts: 3
-  retry_on Timeout::Error, wait: :exponentially_longer, attempts: 2
-
-  discard_on StandardError do |job, error|
-    Rails.logger.error "FileUploadJob failed permanently: #{error.message}"
-    Rails.logger.error "Job arguments: #{job.arguments}"
-    Rails.logger.error "Suggestion: Check provider names for invalid characters if Azure API errors"
-  end
+  retry_on AzureFileShares::Errors::ApiError, wait: :polynomially_longer, attempts: 3
+  retry_on Timeout::Error, wait: :polynomially_longer, attempts: 2
 
   def perform(language_id, content_id, content_type, share = ENV["AZURE_STORAGE_SHARE_NAME"])
     @language = Language.find(language_id)

app/models/provider.rb

@@ -20,7 +20,7 @@ class Provider < ApplicationRecord
   has_many :regions, through: :branches
   has_many :contributors
   has_many :users, through: :contributors
-  has_many :topics
+  has_many :topics, dependent: :restrict_with_error
   has_many :beacon_providers, dependent: :destroy
   has_many :beacons, through: :beacon_providers
 

spec/jobs/file_upload_job_spec.rb

@@ -24,6 +24,23 @@
       end
     end
 
+    context "when generating the file fails" do
+      before { allow(CsvGenerator::Topics).to receive(:new).and_raise(NoMethodError, "undefined method 'name' for nil") }
+
+      it "raises so the job is recorded as failed" do
+        expect { described_class.perform_now(language.id, "topics", "file") }.to raise_error(NoMethodError)
+      end
+    end
+
+    context "when Azure returns an API error" do
+      before { allow(FileWorker).to receive(:new).and_raise(AzureFileShares::Errors::ApiError.new("ServerBusy")) }
+
+      it "retries the job" do
+        expect { described_class.perform_now(language.id, "topics", "file") }
+          .to have_enqueued_job(described_class).with(language.id, "topics", "file")
+      end
+    end
+
     context "when provider specific file" do
       let(:provider) { create(:provider, name: "Test Provider") }
 

spec/requests/providers_spec.rb

@@ -139,5 +139,19 @@
 
       expect(response).to redirect_to(providers_url)
     end
+
+    context "when the provider has topics" do
+      it "keeps the provider and explains why" do
+        provider = Provider.create! valid_attributes
+        create(:topic, provider:)
+
+        expect {
+          delete provider_url(provider)
+        }.not_to change(Provider, :count)
+
+        expect(response).to redirect_to(providers_url)
+        expect(flash[:alert]).to match(/topics/i)
+      end
+    end
   end
 end