Fix importing files with empty header cells and error messages improvements [SCI-1484]

Gemfile

@@ -46,7 +46,7 @@ gem 'ajax-datatables-rails', '~> 0.3.1'
 gem 'commit_param_routing' # Enables different submit actions in the same form to route to different actions in controller
 gem 'kaminari'
 gem "i18n-js", ">= 3.0.0.rc11" # Localization in javascript files
-gem 'roo', '~> 2.1.0' # Spreadsheet parser
+gem 'roo', '~> 2.7.1' # Spreadsheet parser
 gem 'wicked_pdf'
 gem 'silencer' # Silence certain Rails logs
 gem 'wkhtmltopdf-heroku'

Gemfile.lock

@@ -190,9 +190,8 @@ GEM
       jquery-rails
       rails (>= 3.2.0)
     newrelic_rpm (4.0.0.332)
-    nokogiri (1.6.8)
+    nokogiri (1.6.8.1)
       mini_portile2 (~> 2.1.0)
-      pkg-config (~> 1.1.7)
     nokogumbo (1.4.10)
       nokogiri
     oj (2.17.4)
@@ -206,7 +205,6 @@ GEM
     parser (2.4.0.0)
       ast (~> 2.2)
     pg (0.18.4)
-    pkg-config (1.1.7)
     polyglot (0.3.5)
     powerpack (0.1.1)
     puma (2.15.3)
@@ -257,7 +255,7 @@ GEM
       algorithms (~> 0.6.1)
       stream (~> 0.5.0)
     rkelly-remix (0.0.7)
-    roo (2.1.1)
+    roo (2.7.1)
       nokogiri (~> 1)
       rubyzip (~> 1.1, < 2.0.0)
     rubocop (0.48.1)
@@ -268,7 +266,7 @@ GEM
       unicode-display_width (~> 1.0, >= 1.0.1)
     ruby-graphviz (1.2.2)
     ruby-progressbar (1.8.1)
-    rubyzip (1.1.7)
+    rubyzip (1.2.1)
     sanitize (4.4.0)
       crass (~> 1.0.2)
       nokogiri (>= 1.4.4)
@@ -390,7 +388,7 @@ DEPENDENCIES
   recaptcha
   remotipart (~> 1.2)
   rgl
-  roo (~> 2.1.0)
+  roo (~> 2.7.1)
   rubocop
   ruby-graphviz (~> 1.2)
   rubyzip

app/controllers/repositories_controller.rb

@@ -238,8 +238,9 @@ class RepositoriesController < ApplicationController
                                 number_of_rows: status[:nr_of_added])
             render json: {}, status: :ok
           else
-            flash[:alert] = t('repositories.import_records.error_flash',
-                              message: status[:errors])
+            flash[:alert] =
+              t('repositories.import_records.partial_success_flash',
+                nr: status[:nr_of_added], total_nr: status[:total_nr])
             render json: {}, status: :unprocessable_entity
           end
         else

app/controllers/teams_controller.rb

@@ -36,8 +36,7 @@ class TeamsController < ApplicationController
 
             # Get data (it will trigger any errors as well)
             @header = sheet.row(1)
-            @rows = [];
-            @rows << Hash[[@header, sheet.row(2)].transpose]
+            @columns = sheet.row(2)
 
             # Fill in fields for dropdown
             @available_fields = @team.get_available_sample_fields

app/models/repository.rb

@@ -109,9 +109,10 @@ class Repository < ActiveRecord::Base
 
   # Imports records
   def import_records(sheet, mappings, user)
-    errors = []
+    errors = false
     custom_fields = []
     name_index = -1
+    total_nr = 0
     nr_of_added = 0
 
     mappings.each.with_index do |(_k, value), index|
@@ -127,13 +128,17 @@ class Repository < ActiveRecord::Base
 
     # Now we can iterate through record data and save stuff into db
     (2..sheet.last_row).each do |i|
-      error = []
+      total_nr += 1
+      cell_error = false
       record_row = RepositoryRow.new(name: sheet.row(i)[name_index],
                                  repository: self,
                                  created_by: user,
                                  last_modified_by: user)
 
-      next unless record_row.valid?
+      unless record_row.valid?
+        errors = true
+        next
+      end
       sheet.row(i).each.with_index do |value, index|
         if custom_fields[index] && value
           rep_column = RepositoryTextValue.new(
@@ -145,10 +150,11 @@ class Repository < ActiveRecord::Base
               repository_column: custom_fields[index]
             }
           )
-          error << rep_column.errors.messages unless rep_column.save
+          cell_error = true unless rep_column.save
         end
       end
-      if error.any?
+      if cell_error
+        errors = true
         record_row.destroy
       else
         nr_of_added += 1
@@ -156,10 +162,12 @@ class Repository < ActiveRecord::Base
       end
     end
 
-    if errors.count > 0
-      return { status: :error, errors: errors, nr_of_added: nr_of_added }
+    if errors
+      return { status: :error,
+               nr_of_added: nr_of_added,
+               total_nr: total_nr }
     end
-    { status: :ok, nr_of_added: nr_of_added }
+    { status: :ok, nr_of_added: nr_of_added, total_nr: total_nr }
   end
 
   private

app/services/import_repository/parse_repository.rb

@@ -11,15 +11,14 @@ module ImportRepository
     def data
       # Get data (it will trigger any errors as well)
       header = @sheet.row(1)
-      rows = []
-      rows << Hash[[header, @sheet.row(2)].transpose]
+      columns = @sheet.row(2)
       # Fill in fields for dropdown
       @repository.available_repository_fields.transform_values! do |name|
         truncate(name, length: Constants::NAME_TRUNCATION_LENGTH_DROPDOWN)
       end
       @temp_file = TempFile.create(session_id: @session.id, file: @file)
       Data.new(header,
-               rows,
+               columns,
                @repository.available_repository_fields,
                @repository,
                @temp_file)
@@ -47,7 +46,7 @@ module ImportRepository
     end
 
     Data = Struct.new(
-      :header, :rows, :available_fields, :repository, :temp_file
+      :header, :columns, :available_fields, :repository, :temp_file
     )
   end
 end

app/views/repositories/_parse_records_modal.html.erb

@@ -30,29 +30,31 @@
                                include_blank: t('teams.parse_sheet.do_not_include_column'),
                                hide_label: true) %>
                   <br />
-                  <% if th.length > Constants::NAME_TRUNCATION_LENGTH_DROPDOWN %>
-                    <div class="modal-tooltip">
-                      <%= truncate(th, length: Constants::NAME_TRUNCATION_LENGTH_DROPDOWN) %>
-                    </div>
+                  <% if th.nil? %>
+                    <i><%= t('repositories.import_records.no_header_name') %></i>
                   <% else %>
-                    <%= th %>
+                    <% if th.length > Constants::NAME_TRUNCATION_LENGTH_DROPDOWN %>
+                      <div class="modal-tooltip">
+                        <%= truncate(th, length: Constants::NAME_TRUNCATION_LENGTH_DROPDOWN) %>
+                      </div>
+                    <% else %>
+                      <%= th %>
+                    <% end %>
                   <% end %>
                 </th>
               <% end %>
             </thead>
             <tbody>
-              <%  @import_data.rows.each do |row| %>
-                <tr>
+              <tr>
+                <td>
+                  <p><%= t('teams.parse_sheet.example_value') %></p>
+                </td>
+                <% @import_data.columns.each do |td| %>
                   <td>
-                    <p><%= t('teams.parse_sheet.example_value') %></p>
+                    <%= td %>
                   </td>
-                  <% row.each do |td| %>
-                    <td>
-                      <%= td[1] %>
-                    </td>
-                  <% end %>
-                </tr>
-              <% end %>
+                <% end %>
+              </tr>
             </tbody>
           </table>
         </div>

app/views/samples/_parse_samples_modal.html.erb

@@ -25,29 +25,31 @@
                              include_blank: t('teams.parse_sheet.do_not_include_column'),
                              hide_label: true) %>
                 <br />
-                <% if th.length > Constants::NAME_TRUNCATION_LENGTH_DROPDOWN %>
-                  <div class="modal-tooltip">
-                    <%= truncate(th, length: Constants::NAME_TRUNCATION_LENGTH_DROPDOWN) %>
-                  </div>
+                <% if th.nil? %>
+                  <i><%= t('samples.modal_import.no_header_name') %></i>
                 <% else %>
-                  <%= th %>
+                  <% if th.length > Constants::NAME_TRUNCATION_LENGTH_DROPDOWN %>
+                    <div class="modal-tooltip">
+                      <%= truncate(th, length: Constants::NAME_TRUNCATION_LENGTH_DROPDOWN) %>
+                    </div>
+                  <% else %>
+                    <%= th %>
+                  <% end %>
                 <% end %>
               </th>
             <% end %>
           </thead>
           <tbody>
-            <% @rows.each do |row| %>
-              <tr>
+            <tr>
+              <td>
+                <p><%= t('teams.parse_sheet.example_value') %></p>
+              </td>
+              <% @columns.each do |td| %>
                 <td>
-                  <p><%= t('teams.parse_sheet.example_value') %></p>
+                  <%= td %>
                 </td>
-                <% row.each do |td| %>
-                  <td>
-                    <%= td[1] %>
-                  </td>
-                <% end %>
-              </tr>
-            <% end %>
+              <% end %>
+            </tr>
           </tbody>
         </table>
       </div>

config/locales/en.yml

@@ -911,8 +911,9 @@ en:
     add_new_record: "Add new item"
     import_records:
       import: 'Import'
+      no_header_name: 'No column name'
       success_flash: "%{number_of_rows} new item(s) successfully imported."
-      error_flash: "Something went wrong: %{message}"
+      partial_success_flash: "%{nr} of %{total_nr} successfully imported. Other rows contained errors."
       error_message:
         temp_file_not_found: "This file could not be found. Your session might expire."
         session_expired: "Your session expired. Please try again."
@@ -1009,6 +1010,7 @@ en:
     modal_import:
       title: "Import samples"
       notice: "You may upload .csv file (comma separated) or tab separated file (.txt or .tdv) or Excel file (.xls, .xlsx). First row should include header names, followed by rows with sample data."
+      no_header_name: 'No column name'
       upload: "Upload file"
     modal_delete:
       title: "Delete samples"