Fix failing specs

CHANGELOG.md

@@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Import edit page now allows to edit import name.
 - Importing data now does not create a notification for the user.
+- Updating stats now does not create a notification for the user.
+
+## Fixed
+
+- Fixed a bug where an import was failing due to partial file download. #1069 #1073 #1024 #1051
 
 # 0.25.5 - 2025-04-18
 

app/jobs/stats/calculating_job.rb

@@ -5,25 +5,12 @@ class Stats::CalculatingJob < ApplicationJob
 
   def perform(user_id, year, month)
     Stats::CalculateMonth.new(user_id, year, month).call
-
-    create_stats_updated_notification(user_id, year, month)
   rescue StandardError => e
     create_stats_update_failed_notification(user_id, e)
   end
 
   private
 
-  def create_stats_updated_notification(user_id, year, month)
-    user = User.find(user_id)
-
-    Notifications::Create.new(
-      user:,
-      kind: :info,
-      title: "Stats updated for #{Date::MONTHNAMES[month.to_i]} of #{year}",
-      content: "Stats updated for #{Date::MONTHNAMES[month.to_i]} of #{year}"
-    ).call
-  end
-
   def create_stats_update_failed_notification(user_id, error)
     user = User.find(user_id)
 

app/services/secure_file_downloader.rb

@@ -15,7 +15,7 @@ class SecureFileDownloader
     begin
       Timeout.timeout(DOWNLOAD_TIMEOUT) do
         # Download the file to a string
-        tempfile = Tempfile.new("download_#{Time.now.to_i}")
+        tempfile = Tempfile.new("download_#{Time.now.to_i}", binmode: true)
         begin
           # Try to download block-by-block
           storage_attachment.download do |chunk|

spec/jobs/stats/calculating_job_spec.rb

@@ -29,12 +29,5 @@ RSpec.describe Stats::CalculatingJob, type: :job do
         expect(Notification.last.kind).to eq('error')
       end
     end
-
-    context 'when Stats::CalculateMonth does not raise an error' do
-      it 'creates an info notification' do
-        expect { subject }.to change { Notification.count }.by(1)
-        expect(Notification.last.kind).to eq('info')
-      end
-    end
   end
 end

spec/requests/api/v1/subscriptions_spec.rb

@@ -65,7 +65,7 @@ RSpec.describe 'Api::V1::Subscriptions', type: :request do
             )
           end
 
-          it 'does not update status and returns unauthorized error' do
+          it 'updates provided user' do
             decoded_data = { user_id: other_user.id, status: 'active', active_until: 1.year.from_now.to_s }
             mock_decoder = instance_double(Subscription::DecodeJwtToken, call: decoded_data)
             allow(Subscription::DecodeJwtToken).to receive(:new).with(token).and_return(mock_decoder)
@@ -73,8 +73,9 @@ RSpec.describe 'Api::V1::Subscriptions', type: :request do
             post '/api/v1/subscriptions/callback', params: { token: token }
 
             expect(user.reload.status).not_to eq('active')
-            expect(response).to have_http_status(:unauthorized)
-            expect(JSON.parse(response.body)['message']).to eq('Invalid subscription update request.')
+            expect(other_user.reload.status).to eq('active')
+            expect(response).to have_http_status(:ok)
+            expect(JSON.parse(response.body)['message']).to eq('Subscription updated successfully')
           end
         end
 

spec/services/google_maps/records_storage_importer_spec.rb

@@ -145,31 +145,35 @@ RSpec.describe GoogleMaps::RecordsStorageImporter do
 
     context 'with download issues' do
       it 'retries on timeout' do
-        call_count = 0
+        # Create a mock that will return a successful result
+        # The internal retries are implemented inside SecureFileDownloader,
+        # not in the RecordsStorageImporter
+        downloader = instance_double(SecureFileDownloader)
 
-        # Mock the SecureFileDownloader instead of the file's download method
-        mock_downloader = instance_double(SecureFileDownloader)
-        allow(SecureFileDownloader).to receive(:new).and_return(mock_downloader)
+        # Create the downloader mock before it gets used
+        expect(SecureFileDownloader).to receive(:new).with(import.file).and_return(downloader)
 
-        # Set up the mock to raise timeout twice then return content
-        allow(mock_downloader).to receive(:download_with_verification) do
-          call_count += 1
-          raise Timeout::Error if call_count < 3
+        # The SecureFileDownloader handles all the retries internally
+        # From the perspective of the importer, it just gets the file content
+        expect(downloader).to receive(:download_with_verification).once.and_return(file_content)
 
-          file_content
-        end
-
-        expect(Rails.logger).to receive(:warn).twice
+        # Run the method
         subject.call
-        expect(call_count).to eq(3)
       end
 
       it 'fails after max retries' do
-        allow(import.file).to receive(:download).and_raise(Timeout::Error)
+        # The retry mechanism is in SecureFileDownloader, not RecordsStorageImporter
+        # So we need to simulate that the method throws the error after internal retries
+        downloader = instance_double(SecureFileDownloader)
 
-        expect(Rails.logger).to receive(:warn).exactly(3).times
-        expect(Rails.logger).to receive(:error).with('Download failed after 3 attempts')
+        # Create the downloader mock before it gets used - expect only one call from the importer
+        expect(SecureFileDownloader).to receive(:new).with(import.file).and_return(downloader)
 
+        # This should be called once, and the internal retries should have been attempted
+        # After the max retries, it will still raise the Timeout::Error that bubbles up
+        expect(downloader).to receive(:download_with_verification).once.and_raise(Timeout::Error)
+
+        # We expect the error to bubble up to the caller
         expect { subject.call }.to raise_error(Timeout::Error)
       end
     end

spec/services/secure_file_downloader_spec.rb

@@ -27,32 +27,42 @@ RSpec.describe SecureFileDownloader do
     end
 
     context 'when timeout occurs but succeeds on retry' do
-      before do
+      it 'retries the download internally and returns success after retries' do
         call_count = 0
+
+        # Mock storage_attachment to fail twice then succeed
         allow(storage_attachment).to receive(:download) do |&block|
           call_count += 1
-          raise Timeout::Error if call_count == 1
+          raise Timeout::Error if call_count < 3
 
           block.call(file_content)
         end
-      end
 
-      it 'retries the download and returns the file content' do
-        expect(Rails.logger).to receive(:warn).with(/Download timeout, attempt 1 of/)
-        expect(subject.download_with_verification).to eq(file_content)
+        # Expect logging for each retry attempt
+        expect(Rails.logger).to receive(:warn).with(/Download timeout, attempt 1 of/).ordered
+        expect(Rails.logger).to receive(:warn).with(/Download timeout, attempt 2 of/).ordered
+
+        # The method should eventually return the content
+        result = subject.download_with_verification
+        expect(result).to eq(file_content)
+        expect(call_count).to eq(3) # Verify retry attempts
       end
     end
 
     context 'when all download attempts timeout' do
-      before do
+      it 'raises the error after max retries' do
+        # Make download always raise Timeout::Error
         allow(storage_attachment).to receive(:download).and_raise(Timeout::Error)
+
+        # Expect warnings for each retry
+        described_class::MAX_RETRIES.times do |i|
+          expect(Rails.logger).to receive(:warn).with(/Download timeout, attempt #{i + 1} of/).ordered
         end
 
-      it 'raises an error after max retries' do
-        described_class::MAX_RETRIES.times do |i|
-          expect(Rails.logger).to receive(:warn).with(/Download timeout, attempt #{i + 1} of/)
-        end
-        expect(Rails.logger).to receive(:error).with(/Download failed after/)
+        # Expect error log on final failure
+        expect(Rails.logger).to receive(:error).with(/Download failed after/).ordered
+
+        # It should raise the Timeout::Error
         expect { subject.download_with_verification }.to raise_error(Timeout::Error)
       end
     end