Ensures that empty files and nil user IDs for WorkActivity objects ra…

…ise ArgumentErrors

app/models/background_upload_snapshot.rb

@@ -2,9 +2,12 @@
 class BackgroundUploadSnapshot < UploadSnapshot
   def store_files(uploaded_files, pre_existing_files: [], current_user: nil)
     save # needed so I can point to this snapshot in the files to distinguish new files from existing ones froma past snapshot
+
+    raise(ArgumentError, "Failed to resolve the user ID from #{self}") if current_user.nil?
+    user_id = current_user&.id
     self.files = uploaded_files.map do |file|
       { "filename" => prefix_filename(file.original_filename),
-        "upload_status" => "started", user_id: current_user&.id, snapshot_id: id }
+        "upload_status" => "started", user_id:, snapshot_id: id }
     end
     files.concat pre_existing_files if pre_existing_files.present?
   end
@@ -38,7 +41,14 @@ def finalize_upload
     new_files.each do |file|
       work.track_change(:added, file["filename"])
     end
-    work.log_file_changes(new_files.first["user_id"])
+
+    raise(ArgumentError, "Upload failed with empty files.") if new_files.empty?
+    first_new_file = new_files.first
+
+    user_id = first_new_file["user_id"]
+    raise(ArgumentError, "Failed to resolve the user ID from #{first_new_file['filename']}") if user_id.nil?
+
+    work.log_file_changes(user_id)
   end
 
   def prefix_filename(filename)

app/models/work.rb

@@ -265,7 +265,6 @@ def add_message(message, current_user_id)
 
   def add_provenance_note(date, note, current_user_id, change_label = "")
     WorkActivity.add_work_activity(id, { note:, change_label: }.to_json, current_user_id, activity_type: WorkActivity::PROVENANCE_NOTES, created_at: date)
-    # WorkActivity.add_work_activity(id, note, current_user_id, activity_type: WorkActivity::PROVENANCE_NOTES, created_at: date)
   end
 
   def log_changes(resource_compare, current_user_id)

app/models/work_activity.rb

@@ -267,6 +267,7 @@ def user_refernces(text_in)
             "<a class='message-user-link' title='#{user_info}' href='#{@work_activity.users_path}/#{uid}'>#{at_uid}</a>"
           end
         else
+          Rails.logger.warn("Failed to extract the user ID from #{uid}")
           UNKNOWN_USER
         end
       end

app/models/work_state_transition_notification.rb

@@ -23,6 +23,8 @@ def initialize(work, current_user_id)
     @work_title = work.title
     @notification = notification_for_transition
     @id = work.id
+
+    raise(NotImplementedError, "Invalid user ID provided.") if current_user_id.nil?
     @current_user_id = current_user_id
   end
 

spec/models/background_upload_snapshot_spec.rb

@@ -6,6 +6,7 @@
   let(:work) { FactoryBot.create(:approved_work) }
   let(:uploaded_file1) { fixture_file_upload("us_covid_2019.csv", "text/csv") }
   let(:uploaded_file2) { fixture_file_upload("us_covid_2020.csv", "text/csv") }
+  let(:current_user) { work.created_by_user }
 
   describe "#count" do
     it "only counts the bacground uploads" do
@@ -16,10 +17,12 @@
   end
 
   describe "#store_files" do
+    let(:current_user) { work.created_by_user }
+
     it "lists filenames associated with the snapshot" do
-      background_upload_snapshot.store_files([uploaded_file1, uploaded_file2])
-      expect(background_upload_snapshot.files).to eq([{ "filename" => "#{work.prefix}us_covid_2019.csv", "upload_status" => "started", "user_id" => nil, "snapshot_id" => 123 },
-                                                      { "filename" => "#{work.prefix}us_covid_2020.csv", "upload_status" => "started", "user_id" => nil, "snapshot_id" => 123 }])
+      background_upload_snapshot.store_files([uploaded_file1, uploaded_file2], current_user:)
+      expect(background_upload_snapshot.files).to eq([{ "filename" => "#{work.prefix}us_covid_2019.csv", "upload_status" => "started", "snapshot_id" => 123, "user_id" => current_user.id },
+                                                      { "filename" => "#{work.prefix}us_covid_2020.csv", "upload_status" => "started", "user_id" => current_user.id, "snapshot_id" => 123 }])
       expect(background_upload_snapshot.existing_files).to eq([])
       expect(background_upload_snapshot.upload_complete?).to be_falsey
     end
@@ -39,26 +42,50 @@
   describe "#mark_complete" do
     it "changes the status" do
       allow(Honeybadger).to receive(:notify)
-      background_upload_snapshot.store_files([uploaded_file1, uploaded_file2])
+      background_upload_snapshot.store_files([uploaded_file1, uploaded_file2], current_user:)
       expect(work.work_activity.count).to eq(0)
       background_upload_snapshot.mark_complete(uploaded_file2.original_filename, "checksumabc123")
       expect(work.work_activity.count).to eq(0)
-      expect(background_upload_snapshot.files).to eq([{ "filename" => "#{work.prefix}us_covid_2019.csv", "upload_status" => "started", "user_id" => nil, "snapshot_id" => 123 },
-                                                      { "filename" => "#{work.prefix}us_covid_2020.csv", "upload_status" => "complete", "user_id" => nil, "checksum" => "checksumabc123",
+      expect(background_upload_snapshot.files).to eq([{ "filename" => "#{work.prefix}us_covid_2019.csv", "upload_status" => "started", "user_id" => current_user.id, "snapshot_id" => 123 },
+                                                      { "filename" => "#{work.prefix}us_covid_2020.csv", "upload_status" => "complete", "user_id" => current_user.id, "checksum" => "checksumabc123",
                                                         "snapshot_id" => 123 }])
       expect(background_upload_snapshot.upload_complete?).to be_falsey
-      expect(background_upload_snapshot.existing_files).to eq([{ "filename" => "#{work.prefix}us_covid_2020.csv", "upload_status" => "complete", "user_id" => nil, "checksum" => "checksumabc123",
+      # rubocop:disable Layout/LineLength
+      expect(background_upload_snapshot.existing_files).to eq([{ "filename" => "#{work.prefix}us_covid_2020.csv", "upload_status" => "complete", "user_id" => current_user.id, "checksum" => "checksumabc123",
                                                                  "snapshot_id" => 123 }])
       background_upload_snapshot.mark_complete(uploaded_file1.original_filename, "checksumdef456")
       expect(background_upload_snapshot.upload_complete?).to be_truthy
       expect(background_upload_snapshot.existing_files).to eq([{ "filename" => "#{work.prefix}us_covid_2019.csv", "upload_status" => "complete",
-                                                                 "user_id" => nil, "checksum" => "checksumdef456", "snapshot_id" => 123 },
-                                                               { "filename" => "#{work.prefix}us_covid_2020.csv", "upload_status" => "complete", "user_id" => nil, "checksum" => "checksumabc123",
+                                                                 "user_id" => current_user.id, "checksum" => "checksumdef456", "snapshot_id" => 123 },
+                                                               { "filename" => "#{work.prefix}us_covid_2020.csv", "upload_status" => "complete", "user_id" => current_user.id, "checksum" => "checksumabc123",
                                                                  "snapshot_id" => 123 }])
+      # rubocop:enable Layout/LineLength
       expect(work.work_activity.count).to eq(1)
       expect(work.work_activity.first.message).to eq("[{\"action\":\"added\",\"filename\":\"#{work.prefix}us_covid_2019.csv\"}"\
       ",{\"action\":\"added\",\"filename\":\"#{work.prefix}us_covid_2020.csv\"}]")
-      expect(work.work_activity.first.created_by_user_id).to eq(nil)
+      expect(work.work_activity.first.created_by_user_id).to eq(current_user.id)
+    end
+  end
+
+  describe "#finalize_upload" do
+    context "when no files are associated with the work" do
+      it "raises an exception" do
+        expect { background_upload_snapshot.finalize_upload }.to raise_error(ArgumentError, "Upload failed with empty files.")
+      end
+    end
+
+    context "when the file is not associated with a user" do
+      let(:uploaded_file1) { fixture_file_upload("us_covid_2019.csv", "text/csv") }
+      let(:files) do
+        [
+          uploaded_file1
+        ]
+      end
+
+      it "raises an error" do
+        pattern = Regexp.escape("Failed to resolve the user ID from ")
+        expect { background_upload_snapshot.store_files(files) }.to raise_error(ArgumentError, /#{pattern}/)
+      end
     end
   end
 end