update errors on email template

Author: krbhavith

app/services/bulk_zombie_url_uploader.rb

@@ -12,6 +12,8 @@ def initialize(filename, filepath)
 
   def upload
     @results = BulkZombieUrls::Results.new(@file_name)
+    raise 'Results object not initialized' if @results.nil?
+
     begin
       upload_urls
     rescue => e

app/services/bulk_zombie_urls/results.rb

@@ -22,16 +22,12 @@ def delete_ok
     end
 
     def add_error(error_message, key)
-      @error_count += 1
+      self.error_count += 1
       @errors[key] << error_message
     end
 
     def total_count
       ok_count + error_count
     end
-
-    def urls_with(id)
-      @errors[id]
-    end
   end
 end

app/views/admin/bulk_zombie_url_upload/index.html.haml

@@ -4,16 +4,18 @@
   To upload a list of URLs to be deleted, follow these instructions:
   %ul.bulk-upload-instructions
     %li
-      Create a new text file containing one URL per line. An example of this format
+      Create a new CSV file containing one URL and Document id per line.
+    %li
+      Document ID is mandatorily required to delete URL from Kibana when URL is not present in Search DB. An example of this format
       is shown below.
     %li
       Save the file on your computer; you can name the file whatever you like, as
-      long as it's plain text and has a .txt extension.
+      long as it has a .txt extension.
     %li
       %b
         Do not use Microsoft Word files, or any other file type except plain text.
     %li
-      The maximum file size is #{number_to_human_size(BulkUrlUploader::MAXIMUM_FILE_SIZE)}.
+      The maximum file size is #{number_to_human_size(BulkZombieUrls::FileValidator::MAXIMUM_FILE_SIZE)}.
     %li
       Browse for the file on your computer.
     %li
@@ -25,9 +27,10 @@
   %code
     %pre
       URL,DOC_ID
-      http://www.sample.gov/1.html,sample121
-      http://www.sample.gov/2.html,sample122
-      http://www.sample.gov/3.html,sample123
+      http://www.sample.gov/1.html,docid121
+      http://www.sample.gov/2.html,docid122
+      http://www.sample.gov/3.html,docid123
+      ,docid124
   = form_tag upload_admin_bulk_zombie_url_upload_index_path, :multipart => true, :class => 'form' do
     = file_field_tag 'bulk_upload_zombie_urls', :accept => 'text/csv'
     %p

app/views/bulk_zombie_url_upload_results_mailer/results_email.html.erb

@@ -19,12 +19,9 @@
   <p>
     <%= @results.error_count %> URLs failed validation.
   </p>
-
-  <% @results.error_messages.each do |error_message| %>
-    <h3><%= error_message %></h3>
-    <% @results.urls_with(error_message).each do |url| %>
-      <%= url %>
-      <br />
-    <% end %>
+  <% @results.errors.each do |k,error_message| %>
+    <h3><%= k %></h3>
+    <%= error_message %>
+    <br />
   <% end %>
 <% end %>

app/views/bulk_zombie_url_upload_results_mailer/results_email.text.erb

@@ -4,15 +4,14 @@ finished.
 There were <%= @results.total_count %> URLs.
 
 <% if @results.error_count == 0 %>
-There were no errors.
+  There were no errors.
 <% else %>
-<%= @results.ok_count %> URLs were deleted successfully.
-<%= @results.error_count %> URLs failed validation.
+  <%= @results.ok_count %> URLs were deleted successfully.
+  <%= @results.error_count %> URLs failed validation.
 
-<% @results.error_messages.each do |error_message| %>
-  <%= error_message %>
-    <% @results.urls_with(error_message).each do |url| %>
-      <%= url %>
-    <% end %>
+  <% @results.errors.each do |k,error_message| %>
+    <h3><%= k %></h3>
+    <%= error_message %>
+    <br />
   <% end %>
-<% end %>
+<% end %>

spec/services/bulk_zombie_url_uploader_spec.rb

@@ -1,54 +1,101 @@
 # frozen_string_literal: true
 
-require 'spec_helper'
-
 describe BulkZombieUrlUploader do
-  let(:valid_file_path) { 'spec/fixtures/files/valid_zombie_urls.csv' }
-  let(:invalid_file_path) { 'spec/fixtures/files/invalid_zombie_urls.csv' }
-  let(:filename) { 'valid_zombie_urls.csv' }
-  let(:uploader) { described_class.new(filename, valid_file_path) }
-  let(:results) { instance_spy(BulkZombieUrls::Results) }
-  let(:logger) { instance_double(Logger, error: nil, info: nil, debug: nil) }
+  let(:filename) { 'test_file.csv' }
+  let(:filepath) { '/path/to/test_file.csv' }
+  let(:uploader) { described_class.new(filename, filepath) }
+  let(:results) { instance_double(BulkZombieUrls::Results) }
+  let(:csv_content) do
+    <<~CSV
+      URL,DOC_ID
+      http://example.com,doc1
+      ,doc2
+      http://missingdoc.com,
+    CSV
+  end
 
   before do
+    allow(File).to receive(:read).with(filepath).and_return(csv_content)
     allow(BulkZombieUrls::Results).to receive(:new).and_return(results)
     allow(results).to receive(:add_error)
     allow(results).to receive(:delete_ok)
     allow(results).to receive(:increment_updated)
-    allow(File).to receive(:read).with(valid_file_path).and_return("URL,DOC_ID\nhttp://example.com,123\n")
-    allow(File).to receive(:read).with(invalid_file_path).and_raise(CSV::MalformedCSVError.new('Malformed CSV', 1))
-    allow(SearchgovUrl).to receive(:find_by).and_return(instance_double('SearchgovUrl', destroy: true))
-    allow(I14yDocument).to receive(:delete).and_return(true)
-    allow(Rails).to receive(:logger).and_return(logger)
-    uploader.instance_variable_set(:@results, results)
+    uploader.instance_variable_set(:@results, results) # Ensure `@results` is initialized
+  end
+
+  describe '#initialize' do
+    it 'assigns filename and filepath' do
+      expect(uploader.instance_variable_get(:@file_name)).to eq(filename)
+      expect(uploader.instance_variable_get(:@file_path)).to eq(filepath)
+    end
   end
 
-  describe '#upload' do
-    context 'with a valid CSV' do
-      it 'processes valid CSV rows successfully' do
-        uploader.upload
-        expect(results).to have_received(:delete_ok).once
-        expect(results).to have_received(:increment_updated).once
+  describe '#upload_urls' do
+    context 'with valid CSV content' do
+      it 'processes each row in the CSV' do
+        allow(uploader).to receive(:process_row)
+        uploader.send(:upload_urls)
+        expect(uploader).to have_received(:process_row).exactly(3).times
       end
     end
 
-    context 'with an invalid CSV format' do
-      it 'handles invalid CSV format gracefully' do
-        uploader = described_class.new(filename, invalid_file_path)
-        uploader.upload
-        expect(results).to have_received(:add_error).with('Invalid CSV format', 'Entire file').once
-        expect(logger).to have_received(:error).with(/Error parsing CSV/)
+    context 'with invalid CSV content' do
+      let(:csv_error) { CSV::MalformedCSVError.new('Invalid CSV format', 'Line causing error') }
+
+      before do
+        allow(CSV).to receive(:parse).and_raise(csv_error)
+        allow(Rails.logger).to receive(:error)
+      end
+
+      it 'handles the CSV error and logs it' do
+        expect(results).to receive(:add_error).with('Invalid CSV format', 'Entire file')
+        uploader.send(:upload_urls)
+        expect(Rails.logger).to have_received(:error).with(/Error parsing CSV/)
       end
     end
   end
 
   describe '#process_row' do
-    let(:row) { { 'URL' => 'http://example.com', 'DOC_ID' => '123' } }
+    let(:row) { { 'URL' => 'http://example.com', 'DOC_ID' => 'doc1' } }
+
+    context 'when DOC_ID is blank' do
+      let(:row) { { 'URL' => 'http://example.com', 'DOC_ID' => nil } }
+
+      it 'adds an error and logs it' do
+        allow(Rails.logger).to receive(:error)
+        uploader.send(:process_row, row)
+        expect(results).to have_received(:add_error).with('Document ID is missing', 'http://example.com')
+        expect(Rails.logger).to have_received(:error).with(/Document ID is mandatory/)
+      end
+    end
+  end
+
+  describe '#process_url_with_rescue' do
+    let(:row) { { 'URL' => 'http://example.com', 'DOC_ID' => 'doc1' } }
 
-    it 'processes rows with valid data' do
-      uploader.send(:process_row, row)
-      expect(results).to have_received(:delete_ok).once
-      expect(results).to have_received(:increment_updated).once
+    before do
+      allow(uploader).to receive(:process_url)
+    end
+
+    it 'processes the URL and updates results' do
+      uploader.send(:process_url_with_rescue, 'http://example.com', 'doc1', row)
+      expect(results).to have_received(:delete_ok)
+      expect(results).to have_received(:increment_updated)
+    end
+
+    context 'when an error occurs during processing' do
+      let(:error) { StandardError.new('Processing error') }
+
+      before do
+        allow(uploader).to receive(:process_url).and_raise(error)
+        allow(Rails.logger).to receive(:error)
+      end
+
+      it 'handles the error and logs it' do
+        uploader.send(:process_url_with_rescue, 'http://example.com', 'doc1', row)
+        expect(results).to have_received(:add_error).with('Processing error', 'http://example.com')
+        expect(Rails.logger).to have_received(:error).with(/Failure to process bulk upload zombie URL row/)
+      end
     end
   end
 end

spec/services/bulk_zombie_urls/results_spec.rb

@@ -7,14 +7,10 @@
     it 'initializes with correct attributes' do
       expect(results.file_name).to eq('Test File')
       expect(results.ok_count).to eq(0)
-      expect(results.updated).to eq(0)
       expect(results.error_count).to eq(0)
-    end
-  end
-
-  describe '#delete_ok' do
-    it 'increments the ok_count' do
-      expect { results.delete_ok }.to change { results.ok_count }.by(1)
+      expect(results.updated).to eq(0)
+      expect(results.errors).to be_a(Hash)
+      expect(results.errors).to be_empty
     end
   end
 
@@ -24,6 +20,12 @@
     end
   end
 
+  describe '#delete_ok' do
+    it 'increments the ok_count' do
+      expect { results.delete_ok }.to change { results.ok_count }.by(1)
+    end
+  end
+
   describe '#add_error' do
     let(:error_message) { 'Sample error message' }
     let(:key) { 'http://example.com' }
@@ -36,25 +38,23 @@
       results.add_error(error_message, key)
       expect(results.errors[key]).to include(error_message)
     end
+
+    it 'creates a new key in the errors hash if it does not exist' do
+      expect(results.errors[key]).to be_empty
+      results.add_error(error_message, key)
+      expect(results.errors).to have_key(key)
+    end
   end
 
   describe '#total_count' do
     it 'calculates the total of ok_count and error_count' do
       results.delete_ok
-      results.add_error('Error 1', 'key1')
+      results.add_error('Error message', 'key1')
       expect(results.total_count).to eq(2)
     end
-  end
-
-  describe '#urls_with' do
-    it 'retrieves all errors associated with a specific key' do
-      results.add_error('Error 1', 'key1')
-      results.add_error('Error 2', 'key1')
-      expect(results.urls_with('key1')).to eq(['Error 1', 'Error 2'])
-    end
 
-    it 'returns an empty array if no errors exist for the key' do
-      expect(results.urls_with('nonexistent_key')).to be_empty
+    it 'returns 0 when there are no ok_count or error_count' do
+      expect(results.total_count).to eq(0)
     end
   end
 end