Merge pull request #473 from YaleSTC/472_import_csv_breaks

Import CSV breakage (472)

app/controllers/users_controller.rb

@@ -216,8 +216,9 @@ def verify_import
     file = params[:file]
     begin
       @results = User.import(file)
-    rescue Exception => e
-      flash[:notice] = "The following error was encountered: <br>#{e}<br><br> Please make sure the file you upload is a csv file and the columns are in the right order."
+    rescue ActiveRecord::ActiveRecordError => e
+      flash[:error] = "A database error was encountered: <br>#{e}.<br><br> Please try again. If you fail, contact your administrator."
+    ensure
       render action: 'import'
     end
   end

app/models/user.rb

@@ -19,8 +19,8 @@ class User < ActiveRecord::Base
   has_many :notices, as: :remover
   has_one  :punch_clock
   has_many :sub_requests, through: :shifts #the sub requests this user owns
-	has_many :shift_preferences
-	has_many :requested_shifts
+  has_many :shift_preferences
+  has_many :requested_shifts
   has_many :restrictions
 
 
@@ -361,8 +361,26 @@ def self.import(file)
     results = {successes: [], failures: []}
     CSV.foreach(file.path, headers: true) do |row|
       attrs = row.to_hash
-      roles = [Role.where(name: row["roles"]).first].compact
-      attrs.delete("roles")
+
+      # De-humanize row labels (if using Rails 2 CSV export which has them)
+      attrs = Hash[attrs.map {|k, v| [k.sub(' ', '').underscore, v]}]
+
+      # Allow for CSVs with either `role` and/or `roles` headers
+      unless attrs['roles'].nil?
+        attrs['role'] ||= attrs['roles']
+      end
+
+      # Get roles from database and only include those that exist
+      roles = [Role.where(name: attrs["role"]).first].compact
+
+      # Chuck the record if no valid roles are included
+      if roles.empty?
+        results[:failures] << {name: attrs['first_name'] + ' ' + attrs['last_name'], errors: ['Invalid role']}
+        next
+      end
+
+      attrs.delete("role") # Not useful for initial User creation
+
       u = User.new(attrs)
       u.set_random_password
       u.roles = roles
@@ -371,7 +389,10 @@ def self.import(file)
       if u.save
         results[:successes] << {name: "#{u.first_name} #{u.last_name}"}
       else
-        results[:failures] << {name: "#{u.first_name} #{u.last_name}", errors: u.errors}
+        # Prepare errors for rendering in a readable form
+        results[:failures] << {name: "#{u.first_name} #{u.last_name}", \
+                               errors: u.errors.messages.map { |subject, failure| \
+                                                subject.to_s.humanize + ' ' + failure.join('; ')  }}
       end
     end
     results

app/views/users/import.html.erb

@@ -1,5 +1,28 @@
 <% title "Import Users From CSV" %>
-<h3>Please upload a csv file in which the columns are thus: username, first name, nick name, last name, email, employee id number, and one role.</h3>
+<% if @results %>
+  <% unless @results[:successes].empty? %>
+  <p>
+    The following users were successfully imported:
+  </p>
+  <ul>
+    <% @results[:successes].each do |item| %>
+      <li><%= item[:name] %></li>
+    <% end %>
+  </ul>
+  <% end %>
+
+  <% unless @results[:failures].empty? %>
+  <p>
+    The following users failed to be imported:
+  </p>
+  <ul>
+    <% @results[:failures].each do |item| %>
+      <li><%= item[:name] %>: <%= item[:errors] %></li>
+    <% end %>
+  </ul>
+  <% end %>
+<% end %>
+<h3>Please upload a csv file in the correct format. (Columns: login, first_name, nick_name, last_name, email, employee_id, role.)</h3>
 <%= form_tag url_for(action: 'verify_import'), multipart: true do %>
    <p> <label for="file">File to Upload</label> <%= file_field_tag "file" %></p>
     <p><%= submit_tag "Upload" %><p>

config/routes.rb

@@ -209,6 +209,7 @@
         post :restore
         post :search
         post :save_import
+        post :verify_import
       end
     end
     resources :loc_groups do

spec/features/csv_import_spec.rb

@@ -0,0 +1,22 @@
+require 'rails_helper'
+
+describe "CSV import" do 
+  before(:each) do 
+    full_setup
+    sign_in(@admin.login)
+  end
+
+  it 'should import users in correct format' do
+    # CSV file has two users with valid roles, one with invalid role.
+    # Expected behavior: import two, fail to import the third, display errors.
+    visit users_path
+    click_link('Import from CSV')
+    attach_file('file', Rails.root + 'spec/fixtures/user_import.csv')
+    expect {
+      click_button 'Upload'
+    }.to change {
+      User.count
+    }.by(2)
+    expect(page).to have_content('Invalid role', count: 1) # exactly one error
+  end
+end

spec/fixtures/user_import.csv

@@ -0,0 +1,4 @@
+Login,First Name,Nick Name,Last Name,Email,Employee Id,Role
+kma47,Khadija,"",Abdullah,[email protected],141616,admin_role
+yoa3,Yaw,"",Abu,[email protected],150833,ordinary_role
+aaa86,Angel,"",Aguilera,[email protected],157725,CT