Add some file type error handling

app/controllers/bulk_upload_controller.rb

@@ -1,175 +1,22 @@
 class BulkUploadController < ApplicationController
-  SPREADSHEET_CONTENT_TYPES = %w[
-    application/vnd.ms-excel
-    application/vnd.openxmlformats-officedocument.spreadsheetml.sheet
-  ].freeze
-
-  FIRST_DATA_ROW = 7
-
   def show
+    @bulk_upload = BulkUpload.new(nil, nil)
     render "case_logs/bulk_upload"
   end
 
-  def process_bulk_upload
+  def bulk_upload
-    if SPREADSHEET_CONTENT_TYPES.include?(params["case_log_bulk_upload"].content_type)
+    file = upload_params.tempfile
-      xlsx = Roo::Spreadsheet.open(params["case_log_bulk_upload"].tempfile, extension: :xlsx)
+    content_type = upload_params.content_type
-      sheet = xlsx.sheet(0)
+    @bulk_upload = BulkUpload.new(file, content_type)
-      last_row = sheet.last_row
+    @bulk_upload.process
-      if last_row < FIRST_DATA_ROW
+    if @bulk_upload.errors.present?
-        head :no_content
+      render "case_logs/bulk_upload", status: :unprocessable_entity
     else
-        data_range = FIRST_DATA_ROW..last_row
-        data_range.map do |row_num|
-          row = sheet.row(row_num)
-          CaseLog.create!(
-            lettype: row[1],
-            landlord: row[2],
-            # reg_num_la_core_code: row[3],
-            # managementgroup: row[4],
-            # schemecode: row[5],
-            # firstletting: row[6],
-            tenant_code: row[7],
-            startertenancy: row[8],
-            tenancy: row[9],
-            tenancyother: row[10],
-            # tenancyduration: row[11],
-            age1: row[12],
-            age2: row[13],
-            age3: row[14],
-            age4: row[15],
-            age5: row[16],
-            age6: row[17],
-            age7: row[18],
-            age8: row[19],
-            sex1: row[20],
-            sex2: row[21],
-            sex3: row[22],
-            sex4: row[23],
-            sex5: row[24],
-            sex6: row[25],
-            sex7: row[26],
-            sex8: row[27],
-            relat2: row[28],
-            relat3: row[29],
-            relat4: row[30],
-            relat5: row[31],
-            relat6: row[32],
-            relat7: row[33],
-            relat8: row[34],
-            ecstat1: row[35],
-            ecstat2: row[36],
-            ecstat3: row[37],
-            ecstat4: row[38],
-            ecstat5: row[39],
-            ecstat6: row[40],
-            ecstat7: row[41],
-            ecstat8: row[42],
-            ethnic: row[43],
-            national: row[44],
-            armed_forces: row[45],
-            reservist: row[46],
-            preg_occ: row[47],
-            housing_benefit: row[48],
-            benefits: row[49],
-            net_income_known: row[50].present? ? 1 : nil,
-            earnings: row[50],
-            # increfused: row[51],
-            reason_for_leaving_last_settled_home: row[52],
-            other_reason_for_leaving_last_settled_home: row[53],
-            underoccupation_benefitcap: row[54],
-            housingneeds_a: row[55],
-            housingneeds_b: row[56],
-            housingneeds_c: row[57],
-            housingneeds_f: row[58],
-            housingneeds_g: row[59],
-            housingneeds_h: row[60],
-            prevten: row[61],
-            previous_la: row[62],
-            # ppostc1: row[63],
-            # ppostc2: row[64],
-            # prevpco_unknown: row[65],
-            layear: row[66],
-            lawaitlist: row[67],
-            homeless: row[68],
-            reasonpref: row[69],
-            rp_homeless: row[70],
-            rp_insan_unsat: row[71],
-            rp_medwel: row[72],
-            rp_hardship: row[73],
-            rp_dontknow: row[74],
-            cbl: row[75],
-            chr: row[76],
-            cap: row[77],
-            # referral_source: row[78],
-            period: row[79],
-            brent: row[80],
-            scharge: row[81],
-            pscharge: row[82],
-            supcharg: row[83],
-            tcharge: row[84],
-            # tcharge_care_homes: row[85],
-            # no_rent_or_charge: row[86],
-            outstanding_rent_or_charges: row[87],
-            outstanding_amount: row[88],
-            property_void_date: row[89].to_s + row[90].to_s + row[91].to_s,
-            # property_void_date_day: row[89],
-            # property_void_date_month: row[90],
-            # property_void_date_year: row[91],
-            property_major_repairs: row[92].present? ? "1" : nil,
-            property_major_repairs_date: row[92].to_s + row[93].to_s + row[94].to_s,
-            # property_major_repairs_date_day: row[92],
-            # property_major_repairs_date_month: row[93],
-            # property_major_repairs_date_year: row[94],
-            # supported_scheme: row[95],
-            startdate: row[96].to_s + row[97].to_s + row[98].to_s,
-            # startdate_day: row[96],
-            # startdate_month: row[97],
-            # startdate_year: row[98],
-            offered: row[99],
-            property_reference: row[100],
-            beds: row[101],
-            unittype_gn: row[102],
-            property_building_type: row[103],
-            wchair: row[104],
-            property_relet: row[105],
-            rsnvac: row[106],
-            property_location: row[107],
-            # postcode: row[108],
-            # postcod2: row[109],
-            # row[110] removed
-            property_owner_organisation: row[111],
-            # username: row[112],
-            property_manager_organisation: row[113],
-            leftreg: row[114],
-            # uprn: row[115],
-            incfreq: row[116],
-            # sheltered_accom: row[117],
-            illness: row[118],
-            illness_type_1: row[119],
-            illness_type_2: row[120],
-            illness_type_3: row[121],
-            illness_type_4: row[122],
-            illness_type_8: row[123],
-            illness_type_5: row[124],
-            illness_type_6: row[125],
-            illness_type_7: row[126],
-            illness_type_9: row[127],
-            illness_type_10: row[128],
-            # london_affordable: row[129],
-            rent_type: row[130],
-            intermediate_rent_product_name: row[131],
-            # data_protection: row[132],
-            sale_or_letting: "letting",
-            gdpr_acceptance: 1,
-            gdpr_declined: 0
-            # armed_forces_partner: "",
-            # tenant_same_property_renewal: "",
-            # needs_type: "",
-            # purchaser_code: "",
-          )
-        end
       redirect_to(case_logs_path)
     end
   end
+
+  def upload_params
+    params.require("bulk_upload")["case_log_bulk_upload"]
   end
 end

app/models/bulk_upload.rb

@@ -0,0 +1,191 @@
+class BulkUpload
+  include ActiveModel::Model
+  include ActiveModel::Validations
+  include ActiveModel::Conversion
+
+  SPREADSHEET_CONTENT_TYPES = %w[
+    application/vnd.ms-excel
+    application/vnd.openxmlformats-officedocument.spreadsheetml.sheet
+  ].freeze
+
+  FIRST_DATA_ROW = 7
+
+  def initialize(file, content_type)
+    @file = file
+    @content_type = content_type
+  end
+
+  def process
+    return unless valid_content_type?
+
+    xlsx = Roo::Spreadsheet.open(@file, extension: :xlsx)
+    sheet = xlsx.sheet(0)
+    last_row = sheet.last_row
+    if last_row < FIRST_DATA_ROW
+      errors.add(:case_log_bulk_upload, "No data found")
+    else
+      data_range = FIRST_DATA_ROW..last_row
+      data_range.map do |row_num|
+        CaseLog.create!(map_row(sheet.row(row_num)))
+      end
+    end
+  end
+
+  def valid_content_type?
+    if SPREADSHEET_CONTENT_TYPES.include?(@content_type)
+      return true
+    else
+      errors.add(:case_log_bulk_upload, "Invalid file type")
+      return false
+    end
+  end
+
+  def map_row(row)
+    {
+      lettype: row[1],
+      landlord: row[2],
+      # reg_num_la_core_code: row[3],
+      # managementgroup: row[4],
+      # schemecode: row[5],
+      # firstletting: row[6],
+      tenant_code: row[7],
+      startertenancy: row[8],
+      tenancy: row[9],
+      tenancyother: row[10],
+      # tenancyduration: row[11],
+      age1: row[12],
+      age2: row[13],
+      age3: row[14],
+      age4: row[15],
+      age5: row[16],
+      age6: row[17],
+      age7: row[18],
+      age8: row[19],
+      sex1: row[20],
+      sex2: row[21],
+      sex3: row[22],
+      sex4: row[23],
+      sex5: row[24],
+      sex6: row[25],
+      sex7: row[26],
+      sex8: row[27],
+      relat2: row[28],
+      relat3: row[29],
+      relat4: row[30],
+      relat5: row[31],
+      relat6: row[32],
+      relat7: row[33],
+      relat8: row[34],
+      ecstat1: row[35],
+      ecstat2: row[36],
+      ecstat3: row[37],
+      ecstat4: row[38],
+      ecstat5: row[39],
+      ecstat6: row[40],
+      ecstat7: row[41],
+      ecstat8: row[42],
+      ethnic: row[43],
+      national: row[44],
+      armed_forces: row[45],
+      reservist: row[46],
+      preg_occ: row[47],
+      hb: row[48],
+      benefits: row[49],
+      net_income_known: row[50].present? ? 1 : nil,
+      earnings: row[50],
+      # increfused: row[51],
+      reason: row[52],
+      other_reason_for_leaving_last_settled_home: row[53],
+      underoccupation_benefitcap: row[54],
+      housingneeds_a: row[55],
+      housingneeds_b: row[56],
+      housingneeds_c: row[57],
+      housingneeds_f: row[58],
+      housingneeds_g: row[59],
+      housingneeds_h: row[60],
+      prevten: row[61],
+      prevloc: row[62],
+      # ppostc1: row[63],
+      # ppostc2: row[64],
+      # prevpco_unknown: row[65],
+      layear: row[66],
+      lawaitlist: row[67],
+      homeless: row[68],
+      reasonpref: row[69],
+      rp_homeless: row[70],
+      rp_insan_unsat: row[71],
+      rp_medwel: row[72],
+      rp_hardship: row[73],
+      rp_dontknow: row[74],
+      cbl: row[75],
+      chr: row[76],
+      cap: row[77],
+      # referral_source: row[78],
+      period: row[79],
+      brent: row[80],
+      scharge: row[81],
+      pscharge: row[82],
+      supcharg: row[83],
+      tcharge: row[84],
+      # tcharge_care_homes: row[85],
+      # no_rent_or_charge: row[86],
+      hbrentshortfall: row[87],
+      tshortfall: row[88],
+      property_void_date: row[89].to_s + row[90].to_s + row[91].to_s,
+      # property_void_date_day: row[89],
+      # property_void_date_month: row[90],
+      # property_void_date_year: row[91],
+      majorrepairs: row[92].present? ? "1" : nil,
+      property_major_repairs_date: row[92].to_s + row[93].to_s + row[94].to_s,
+      # property_major_repairs_date_day: row[92],
+      # property_major_repairs_date_month: row[93],
+      # property_major_repairs_date_year: row[94],
+      # supported_scheme: row[95],
+      startdate: row[96].to_s + row[97].to_s + row[98].to_s,
+      # startdate_day: row[96],
+      # startdate_month: row[97],
+      # startdate_year: row[98],
+      offered: row[99],
+      # property_reference: row[100],
+      beds: row[101],
+      unittype_gn: row[102],
+      property_building_type: row[103],
+      wchair: row[104],
+      property_relet: row[105],
+      rsnvac: row[106],
+      la: row[107],
+      # postcode: row[108],
+      # postcod2: row[109],
+      # row[110] removed
+      property_owner_organisation: row[111],
+      # username: row[112],
+      property_manager_organisation: row[113],
+      leftreg: row[114],
+      # uprn: row[115],
+      incfreq: row[116],
+      # sheltered_accom: row[117],
+      illness: row[118],
+      illness_type_1: row[119],
+      illness_type_2: row[120],
+      illness_type_3: row[121],
+      illness_type_4: row[122],
+      illness_type_8: row[123],
+      illness_type_5: row[124],
+      illness_type_6: row[125],
+      illness_type_7: row[126],
+      illness_type_9: row[127],
+      illness_type_10: row[128],
+      # london_affordable: row[129],
+      rent_type: row[130],
+      intermediate_rent_product_name: row[131],
+      # data_protection: row[132],
+      sale_or_letting: "letting",
+      gdpr_acceptance: 1,
+      gdpr_declined: 0
+      # armed_forces_partner: "",
+      # tenant_same_property_renewal: "",
+      # needs_type: "",
+      # purchaser_code: "",
+    }
+  end
+end

app/views/case_logs/bulk_upload.html.erb

@@ -1,5 +1,6 @@
 <div class="govuk-form-group">
-  <%= form_with url: "/case_logs/bulk_upload", method: "post", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %>
+  <%= form_for @bulk_upload, method: "post", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %>
+    <%= f.govuk_error_summary %>
     <%= f.govuk_file_field :case_log_bulk_upload,
         label: { text: "Bulk Upload", size: "l" },
         hint: { text: "Upload a spreadsheet using the template" }

config/routes.rb

@@ -5,8 +5,11 @@ Rails.application.routes.draw do
   get "about", to: "about#index"
 
   post "/case_logs/:id", to: "case_logs#submit_form", constraints: { id: /\d.+/ }
-  post "/case_logs/bulk_upload", to: "bulk_upload#process_bulk_upload"
+
-  get "/case_logs/bulk_upload", to: "bulk_upload#show"
+  scope :case_logs do
+    post "/bulk_uploads", to: "bulk_upload#bulk_upload"
+    get "/bulk_uploads", to: "bulk_upload#show"
+  end
 
   form_handler = FormHandler.instance
   form = form_handler.get_form("2021_2022")

spec/requests/bulk_upload_controller_spec.rb

@@ -1,31 +1,31 @@
 require "rails_helper"
 
 RSpec.describe BulkUploadController, type: :request do
-  let(:url) { "/case_logs/bulk_upload" }
+  let(:url) { "/case_logs/bulk_uploads" }
 
+  describe "GET #show" do
     before do
       get url, params: {}
     end
 
-  describe "GET #show" do
     it "returns a success response" do
       expect(response).to be_successful
     end
 
     it "returns a page with a file upload form" do
-      expect(response.body).to match(/<input id="case-log-bulk-upload-field" class="govuk-file-upload"/)
+      expect(response.body).to match(/<input id="bulk-upload-case-log-bulk-upload-field" class="govuk-file-upload"/)
       expect(response.body).to match(/<button type="submit" formnovalidate="formnovalidate" class="govuk-button"/)
     end
   end
 
   describe "POST #bulk upload" do
+    subject { post url, params: { bulk_upload: { case_log_bulk_upload: @file } } }
+
+    context "given a valid file based on the upload template" do
       before do
         @file = fixture_file_upload("2021_22_lettings_bulk_upload.xlsx", "application/vnd.ms-excel")
       end
 
-    subject { post url, params: { case_log_bulk_upload: @file } }
-
-    context "given a valid file based on the upload template" do
       it "creates case logs for each row in the template" do
         expect { subject }.to change(CaseLog, :count).by(2)
       end
@@ -34,5 +34,16 @@ RSpec.describe BulkUploadController, type: :request do
         expect(subject).to redirect_to(case_logs_path)
       end
     end
+
+    context "given an invalid file type" do
+      before do
+        @file = fixture_file_upload("random.txt", "text/plain")
+        subject
+      end
+
+      it "displays an error message" do
+        expect(response.body).to match(/Invalid file type/)
+      end
+    end
   end
 end