WIP Improve import process

Author: cesidio

app/controllers/imports_controller.rb

@@ -9,15 +9,25 @@ def create
     @import = Import.new(import_params)
     if @import.save
       ProcessImportJob.perform_later @import
+      # ImportOffenders.call(@import)
       redirect_to new_import_path
     else
       render :new
     end
   end
 
+  def errors_log
+    send_data Import.last.error_log, type: 'text', disposition: 'inline'
+  end
+
   private
 
   def import_params
-    params.require(:import).permit(:offenders_file)
+    params.require(:import).permit(:offenders_file_1,
+                                   :offenders_file_2,
+                                   :offenders_file_3,
+                                   :offenders_file_4,
+                                   :offenders_file_5,
+                                   :offenders_file_6)
   end
 end

app/models/import.rb

@@ -1,19 +1,74 @@
 class Import < ActiveRecord::Base
-  mount_uploader :offenders_file, FileUploader
+  mount_uploader :offenders_file_1, FileUploader
+  mount_uploader :offenders_file_2, FileUploader
+  mount_uploader :offenders_file_3, FileUploader
+  mount_uploader :offenders_file_4, FileUploader
+  mount_uploader :offenders_file_5, FileUploader
+  mount_uploader :offenders_file_6, FileUploader
 
-  validates :offenders_file, presence: true
-  validate :offenders_file_uniqueness
+  validates :offenders_file_1, presence: true
 
-  after_create :create_upload
+  validate :offenders_file_1_uniqueness
+  validate :offenders_file_2_uniqueness
+  validate :offenders_file_3_uniqueness
+  validate :offenders_file_4_uniqueness
+  validate :offenders_file_5_uniqueness
+  validate :offenders_file_6_uniqueness
+
+  after_create :create_uploads
 
   private
 
-  def create_upload
-    Upload.create(md5: offenders_file.md5)
+  def create_uploads
+    Upload.create(md5: offenders_file_1.md5)
+    Upload.create(md5: offenders_file_2.md5) if offenders_file_2
+    Upload.create(md5: offenders_file_3.md5) if offenders_file_3
+    Upload.create(md5: offenders_file_4.md5) if offenders_file_4
+    Upload.create(md5: offenders_file_5.md5) if offenders_file_5
+    Upload.create(md5: offenders_file_6.md5) if offenders_file_6
+  end
+
+  def offenders_file_1_uniqueness
+    return unless Upload.where(md5: offenders_file_1.md5).any?
+    errors[:base] << 'Offenders file 1 has already been uploaded'
+  end
+
+  def offenders_file_2_uniqueness
+    return unless offenders_file_2.present?
+    return unless Upload.where(md5: offenders_file_2.md5).any?
+    errors[:base] << 'Offenders file 2 has already been uploaded'
+  end
+
+  def offenders_file_3_uniqueness
+    return unless offenders_file_3.present?
+    return unless Upload.where(md5: offenders_file_3.md5).any?
+    errors[:base] << 'Offenders file 3 has already been uploaded'
+  end
+
+  def offenders_file_4_uniqueness
+    return unless offenders_file_4.present?
+    return unless Upload.where(md5: offenders_file_4.md5).any?
+    errors[:base] << 'Offenders file 4 has already been uploaded'
+  end
+
+  def offenders_file_5_uniqueness
+    return unless offenders_file_5.present?
+    return unless Upload.where(md5: offenders_file_5.md5).any?
+    errors[:base] << 'Offenders file 5 has already been uploaded'
+  end
+
+  def offenders_file_6_uniqueness
+    return unless offenders_file_6.present?
+    return unless Upload.where(md5: offenders_file_6.md5).any?
+    errors[:base] << 'Offenders file 6 has already been uploaded'
   end
 
-  def offenders_file_uniqueness
-    return unless Upload.where(md5: offenders_file.md5).any?
-    errors[:base] << 'This file has already been uploaded'
+  def offender_files_md5s
+    [offenders_file_1.md5,
+     offenders_file_2.md5,
+     offenders_file_3.md5,
+     offenders_file_4.md5,
+     offenders_file_5.md5,
+     offenders_file_6.md5].compact
   end
 end

app/services/import_offenders.rb

@@ -2,14 +2,36 @@ module ImportOffenders
   module_function
 
   def call(import)
-    errors = ParseOffenders.call(import.offenders_file.file.file)
-    if errors.any?
+    report_log, errors_log = report_and_errors(import)
+    if errors_log.any?
       import.update_attribute(:status, :failed)
-      import.update_attribute(:error_log, errors.to_json)
+      import.update_attribute(:error_log, errors_log.join("\n"))
       NotificationMailer.import_failed(import).deliver_later
     else
       import.update_attribute(:status, :successful)
       Import.where('id != ?', import.id).destroy_all
     end
+    import.update_attribute(:report_log, report_log.join("\n"))
+  end
+
+  private
+
+  def self.report_and_errors(import)
+    report_log = []
+    errors_log = []
+
+    1.upto(6).each do |n|
+      file = import.public_send("offenders_file_#{n}").file
+      next unless file
+      errors = ParseOffenders.call(file.file)
+      if errors.any?
+        report_log << "File #{file.filename} failed to import at #{Time.current}"
+        errors_log << "There were some errors in file #{file.filename}"
+        errors_log << errors.join("\n")
+      else
+        report_log << "File #{file.filename} imported at #{Time.current}"
+      end
+    end
+    [report_log, errors_log]
   end
 end

app/views/imports/_last_import.html.haml

@@ -10,12 +10,11 @@
     %thead
       %tr
         %th
-          File
-        %th
-          Date
+          Report
     %tbody
-      %tr
-        %td
-          = last_import.offenders_file.file.filename
-        %td
-          = last_import.created_at.strftime('%d/%m/%Y %H:%M:%S')
+      - last_import.report_log&.each_line do |file_log|
+        %tr
+          %td
+            = file_log
+  - if last_import.error_log
+    = link_to 'Download error log', errors_log_imports_path

app/views/imports/new.html.haml

@@ -7,10 +7,35 @@
   New import
 
 = form_for @import, html: { multipart: true } do |f|
-  = f.hidden_field :offenders_file
+  = f.hidden_field :offenders_file_1
+  = f.hidden_field :offenders_file_2
+  = f.hidden_field :offenders_file_3
+  = f.hidden_field :offenders_file_4
+  = f.hidden_field :offenders_file_5
+  = f.hidden_field :offenders_file_6
 
   .form-group
-    != f.label :offenders_file, class: 'form-label'
-    != f.file_field :offenders_file, class: 'form-control'
+    != f.label :offenders_file_1, class: 'form-label'
+    != f.file_field :offenders_file_1, class: 'form-control'
+
+  .form-group
+    != f.label :offenders_file_2, class: 'form-label'
+    != f.file_field :offenders_file_2, class: 'form-control'
+
+  .form-group
+    != f.label :offenders_file_3, class: 'form-label'
+    != f.file_field :offenders_file_3, class: 'form-control'
+
+  .form-group
+    != f.label :offenders_file_4, class: 'form-label'
+    != f.file_field :offenders_file_4, class: 'form-control'
+
+  .form-group
+    != f.label :offenders_file_5, class: 'form-label'
+    != f.file_field :offenders_file_5, class: 'form-control'
+
+  .form-group
+    != f.label :offenders_file_6, class: 'form-label'
+    != f.file_field :offenders_file_6, class: 'form-control'
 
   = f.submit 'Import', class: "button"

app/views/notification_mailer/import_failed.html.haml

@@ -1,5 +1,5 @@
 - if @import
   %p
-    = "An error occurred while importing offenders file: #{@import.offenders_file}, on: #{@import.created_at}"
+    = "An error occurred while importing offenders file: #{@import.offenders_file_1}, on: #{@import.created_at}"
 %p
   = @error

config/routes.rb

@@ -14,7 +14,9 @@
   root to: 'home#show'
 
   resources :services
-  resources :imports, only: [:new, :create]
+  resources :imports, only: [:new, :create] do
+    get 'errors_log', on: :collection
+  end
 
   namespace :api, format: :json do
     scope module: :v1, constraints: ApiConstraint.new(version: 1) do

db/migrate/20170810231950_add_multiple_import_files.rb

@@ -0,0 +1,12 @@
+class AddMultipleImportFiles < ActiveRecord::Migration[5.0]
+  def change
+    rename_column :imports, :offenders_file, :offenders_file_1
+    add_column :imports, :offenders_file_2, :string
+    add_column :imports, :offenders_file_3, :string
+    add_column :imports, :offenders_file_4, :string
+    add_column :imports, :offenders_file_5, :string
+    add_column :imports, :offenders_file_6, :string
+    remove_column :imports, :identities_file
+    add_column :imports, :report_log, :text
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170725104015) do
+ActiveRecord::Schema.define(version: 20170810231950) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -43,12 +43,17 @@
   end
 
   create_table "imports", force: :cascade do |t|
-    t.string   "offenders_file"
-    t.string   "identities_file"
-    t.string   "status",          default: "in_progress"
+    t.string   "offenders_file_1"
+    t.string   "status",           default: "in_progress"
     t.datetime "created_at"
     t.datetime "updated_at"
     t.text     "error_log"
+    t.string   "offenders_file_2"
+    t.string   "offenders_file_3"
+    t.string   "offenders_file_4"
+    t.string   "offenders_file_5"
+    t.string   "offenders_file_6"
+    t.text     "report_log"
   end
 
   create_table "nicknames", force: :cascade do |t|

spec/features/upload_nomis_export_spec.rb

@@ -10,7 +10,7 @@
     scenario 'upload a NOMIS export' do
       visit new_import_path
 
-      attach_file('import_offenders_file', Rails.root.join('spec', 'fixtures', 'files', 'data.csv'))
+      attach_file('import_offenders_file_1', Rails.root.join('spec', 'fixtures', 'files', 'data_1.csv'))
 
       click_button 'Import'
 
@@ -24,7 +24,7 @@
     scenario 'shows an error' do
       visit new_import_path
 
-      attach_file('import_offenders_file', Rails.root.join('spec', 'fixtures', 'files', 'data.txt'))
+      attach_file('import_offenders_file_1', Rails.root.join('spec', 'fixtures', 'files', 'data.txt'))
 
       click_button 'Import'