Merge pull request #519 from churchio/import

CSV Import Redesign

.rubocop.yml

@@ -4,11 +4,18 @@ Metrics/AbcSize:
 Metrics/LineLength:
   Max: 120
 
+Metrics/MethodLength:
+  CountComments: false
+  Max: 15
+
 DotPosition:
   EnforcedStyle: leading
 
 StringLiterals:
   EnforcedStyle: single_quotes
 
+Style/Documentation:
+  Enabled: false
+
 Style/MultilineOperationIndentation:
   EnforcedStyle: indented

.watchr

@@ -23,7 +23,7 @@ end
 @last_test = nil
 
 def run_specs(test, force=false)
-  unless File.exist?(test) or force or not @last_test
+  if test == :last || (!File.exist?(test) && !force && @last_test)
     test = @last_test
   end
 
@@ -60,6 +60,7 @@ end
 
 watch('^spec/(.*)_spec\.rb'      ) { |m| run_specs("spec/#{m[1]}_spec.rb")                    }
 watch('^spec/factories/(.*)\.rb' ) { |m| run_specs("spec/models/#{m[1]}_spec.rb")             }
+watch('^spec/support/(.*)\.rb'   ) { |m| run_specs(:last)                                     }
 watch('^app/models/(.*)\.rb'     ) { |m| run_specs("spec/models/#{m[1]}_spec.rb")             }
 watch('^app/presenters/(.*)\.rb' ) { |m| run_specs("spec/presenters/#{m[1]}_spec.rb")         }
 watch('^app/authorizers/(.*)\.rb') { |m| run_specs("spec/models/authorizers/#{m[1]}_spec.rb") }

Gemfile

@@ -30,47 +30,50 @@ else
 end
 # END database selection
 
-gem 'rails_autolink', '~> 1.1.6'
-gem 'jquery-rails', '~> 4.0.3'
-gem 'will_paginate', '~> 3.0.7'
-gem 'highline', '~> 1.7.2'
-gem 'whenever', '~> 0.9.4'
-gem 'nokogiri', '~> 1.6.6.2'
-gem 'builder', '~> 3.2.2'
-gem 'loofah', '~> 2.0.2'
-gem 'feedjira', '~> 1.6.0'
-gem 'rubyzip', '~> 1.1.7'
-gem 'zip-zip', '~> 0.3'
-gem 'sanitize', '~> 4.0.0'
-gem 'haml', '~> 4.0.6'
-gem 'httparty', '~> 0.13.5'
-gem 'draper', '~> 2.1.0'
-gem 'paperclip', '~> 4.2.1'
-gem 'acts_as_taggable_on_steroids', github: 'seven1m/acts_as_taggable_on_steroids', ref: 'cffba03'
+gem 'activerecord-session_store', '~> 0.1.1'
 gem 'acts_as_list', '~> 0.7.2'
-gem 'pdf-writer', github: 'Hermanverschooten/pdf-writer', require: 'pdf/writer', ref: 'f57c298'
+gem 'acts_as_taggable_on_steroids', github: 'seven1m/acts_as_taggable_on_steroids', ref: 'cffba03'
 gem 'authority', '~> 3.0.0'
-gem 'load_and_authorize_resource', github: 'seven1m/load_and_authorize_resource', ref: 'a77cce9'
 gem 'bcrypt', '~> 3.1.10'
-gem 'mini_magick', '~> 4.2.7'
-gem 'activerecord-session_store', '~> 0.1.1'
-gem 'sass-rails', '~> 5.0.3'
 gem 'bootstrap-sass', '~> 3.3.4.1'
-gem 'will_paginate-bootstrap', '~> 1.0.1'
-gem 'uglifier', '~> 2.7.1'
+gem 'builder', '~> 3.2.2'
 gem 'coffee-rails', '~> 4.1.0'
+gem 'country_select', github: 'stefanpenner/country_select', ref: 'fad7c1d'
+gem 'date_validator', '~> 0.8.0'
+gem 'draper', '~> 2.1.0'
+gem 'feedjira', '~> 1.6.0'
+gem 'flag_shih_tzu', '~> 0.3.13'
 gem 'font-awesome-rails', github: 'bokmann/font-awesome-rails', ref: '2137b6b'
-gem 'truncate_html', '~> 0.9.3'
 gem 'geocoder', '~> 1.2.8'
-gem 'date_validator', '~> 0.8.0'
-gem 'country_select', github: 'stefanpenner/country_select', ref: 'fad7c1d'
-gem 'responders', '~> 2.1.0'
-gem 'dossier', '~> 2.12.2'
-gem 'mustache', '~> 1.0.1'
 gem 'github_api', '~> 0.12.3'
-gem 'sucker_punch', '~> 1.5.0'
-gem 'pusher', '~> 0.14.5'
+gem 'haml', '~> 4.0.6'
+gem 'highline', '~> 1.7.2'
+gem 'httparty', '~> 0.13.5'
+gem 'jquery-rails', '~> 4.0.3'
+gem 'load_and_authorize_resource', github: 'seven1m/load_and_authorize_resource', ref: 'a77cce9'
+gem 'loofah', '~> 2.0.2'
+gem 'mini_magick', '~> 4.2.7'
+gem 'mustache', '~> 1.0.1'
+gem 'nokogiri', '~> 1.6.6.2'
 gem 'omniauth-facebook', '~> 2.0.1'
+gem 'paperclip', '~> 4.2.1'
+gem 'pdf-writer', github: 'Hermanverschooten/pdf-writer', require: 'pdf/writer', ref: 'f57c298'
+gem 'pusher', '~> 0.14.5'
+gem 'rails_autolink', '~> 1.1.6'
+gem 'responders', '~> 2.1.0'
+gem 'rubyzip', '~> 1.1.7'
+gem 'sanitize', '~> 4.0.0'
+gem 'sass-rails', '~> 5.0.3'
+gem 'sucker_punch', '~> 1.5.0'
+gem 'truncate_html', '~> 0.9.3'
+gem 'uglifier', '~> 2.7.1'
+gem 'whenever', '~> 0.9.4'
+gem 'will_paginate', '~> 3.0.7'
+gem 'will_paginate-bootstrap', '~> 1.0.1'
+gem 'zip-zip', '~> 0.3'
+
+# this needs to be down here due to load order weirdness
+gem 'dossier', '~> 2.12.2'
 
 group :test do
   gem 'factory_girl_rails', '~> 4.5.0'
@@ -81,24 +84,24 @@ end
 group :development do
   gem 'better_errors', '~> 2.1.1'
   gem 'binding_of_caller', '~> 0.7.2'
-  gem 'observr', '~> 1.0.5'
-  gem 'terminal-notifier', '~> 1.6.3'
   gem 'capistrano', '~> 3.4.0'
-  gem 'capistrano-newrelic', '~> 0.0.9'
   gem 'capistrano-bundler', '~> 1.1.4'
+  gem 'capistrano-newrelic', '~> 0.0.9'
   gem 'capistrano-rails', '~> 1.1.3'
+  gem 'observr', '~> 1.0.5'
   gem 'quiet_assets', '~> 1.1.0'
+  gem 'terminal-notifier', '~> 1.6.3'
 end
 
 group :development, :test do
+  gem 'coveralls', '~> 0.8.1', require: false
+  gem 'guard-rspec', '~> 4.5.1', require: false
   gem 'pry', '~> 0.10.1'
-  gem 'pry-remote', '~> 0.1.8'
   gem 'pry-rails', '~> 0.3.4'
+  gem 'pry-remote', '~> 0.1.8'
   gem 'rspec-rails', '~> 3.2.1'
   gem 'spring', '~> 1.3.6'
   gem 'spring-commands-rspec', '~> 1.0.4'
-  gem 'guard-rspec', '~> 4.5.1', require: false
-  gem 'coveralls', '~> 0.8.1', require: false
   gem 'timecop', '~> 0.7.4'
 end
 

app/assets/stylesheets/app/tables.css.scss

@@ -0,0 +1,17 @@
+.spaced-table td, .spaced-table th {
+  padding-right: 10px;
+}
+
+td.strong {
+  font-weight: bold;
+}
+
+tr.row-with-actions, tr.row-with-controls {
+  @extend tr.row-with-avatar;
+}
+
+.table-no-top-border tr:first-child {
+  th, td {
+    border-top: none;
+  }
+}

app/assets/stylesheets/app/tables.scss

@@ -19,3 +19,13 @@ tr.expanding {
 td.icon, th.icon {
   width: 32px;
 }
+
+table.table {
+  tr.description td, tr.details td {
+    border-top: none;
+    padding-top: 0;
+  }
+  td.actions {
+    white-space: nowrap;
+  }
+}

app/assets/stylesheets/application.scss

@@ -74,16 +74,6 @@ th.actions, td.actions {
   }
 }
 
-table.table {
-  tr.description td, tr.details td {
-    border-top: none;
-    padding-top: 0;
-  }
-  td.actions {
-    white-space: nowrap;
-  }
-}
-
 body > .footer {
   height: 30px;
   text-align: center;

app/controllers/administration/imports_controller.rb

@@ -0,0 +1,93 @@
+class Administration::ImportsController < ApplicationController
+  before_filter :only_admins
+
+  def index
+    @imports = Import.order(created_at: :desc).page(params[:page])
+  end
+
+  def show
+    @import = Import.find(params[:id])
+    respond_to do |format|
+      format.html do
+        @rows = @import.rows.includes(:import_attributes).paginate(page: params[:page], per_page: 100)
+        redirect_to(action: :edit) if @import.parsed?
+        render :errored if @import.errored?
+      end
+      format.json do
+        render json: @import
+      end
+    end
+  end
+
+  def new
+  end
+
+  def create
+    return redirect_to(action: 'new') unless params[:file]
+    @import = Import.create(
+      person:           @logged_in,
+      filename:         params[:file].original_filename,
+      importable_type:  'Person',
+      status:           'pending',
+      mappings:         previous_import.try(:mappings) || {},
+      match_strategy:   previous_import.try(:match_strategy),
+      create_as_active: previous_import.create_as_active?
+    )
+    @import.parse_async(
+      file:          params[:file],
+      strategy_name: 'csv'
+    )
+    redirect_to administration_import_path(@import)
+  end
+
+  def edit
+    @import = Import.find(params[:id])
+    @import.update_attributes(status: 'parsed')
+    @example = build_example
+  end
+
+  def update
+    @import = Import.find(params[:id])
+    @import.attributes = import_params
+    @import.mappings = params[:import][:mappings]
+    @import.status = 'matched' if params[:status] == 'matched'
+    if @import.save
+      redirect_to administration_import_path(@import)
+    else
+      @example = build_example
+      render action :edit
+    end
+  end
+
+  def destroy
+    @import = Import.find(params[:id])
+    @import.destroy if @import.destroyable?
+    redirect_to administration_imports_path
+  end
+
+  def execute
+    @import = Import.find(params[:id])
+    @import.execute_async
+    redirect_to administration_import_path(@import)
+  end
+
+  private
+
+  def import_params
+    params.require(:import).permit(:match_strategy, :create_as_active)
+  end
+
+  def build_example
+    @import.rows.first.try(:import_attributes_as_hash, keep_invalid: true) || {}
+  end
+
+  def only_admins
+    return if @logged_in.admin?(:import_data)
+    render text: t('only_admins'), layout: true, status: 401
+    false
+  end
+
+  def previous_import
+    Import.order(:created_at).last
+  end
+end

app/controllers/application_controller.rb

@@ -177,7 +177,7 @@ def safe_redirect_path(url)
   end
 
   def add_errors_to_flash(record)
-    flash[:warning] = record.errors.full_messages.join('; ')
+    flash[:warning] = record.errors.values.join('; ')
   end
 
   def only_admins

app/controllers/comments_controller.rb

@@ -6,7 +6,7 @@ def create
       if comment.save
         flash[:notice] = t('comments.saved')
       else
-        flash[:error] = comment.errors.full_messages.join(". ")
+        flash[:error] = comment.errors.values.join(". ")
       end
       redirect_back
     else

app/controllers/families_controller.rb

@@ -82,7 +82,7 @@ def update
         format.xml  { render xml: @family.errors, status: :unprocessable_entity } if can_export?
         format.js do # only used by barcode entry right now
           render :update do |page|
-            page.replace_html :notice, t('There_were_errors') + ":<br/>#{@family.errors.full_messages.join('; ')}"
+            page.replace_html :notice, t('There_were_errors') + ":<br/>#{@family.errors.values.join('; ')}"
             page[:notice].show
           end
         end

app/controllers/groups_controller.rb

@@ -94,7 +94,7 @@ def batch
             group.attributes = vals.permit(*group_attributes)
             if group.changed?
               unless group.save
-                @errors << [group.id, group.errors.full_messages]
+                @errors << [group.id, group.errors.values]
               end
             end
           end

app/controllers/people_controller.rb

@@ -98,6 +98,7 @@ def create
 
   def edit
     @person ||= Person.find(params[:id])
+    render(text: t('people.edit.no_family_error'), layout: true) && return unless @person.family
     if @logged_in.can_update?(@person)
       @family = @person.family
       @business_categories = Person.business_categories
@@ -155,22 +156,6 @@ def destroy
     end
   end
 
-  def import
-    if @logged_in.admin?(:import_data) and Site.current.import_export_enabled?
-      if request.get?
-        @column_names = Person.importable_column_names
-      elsif request.post?
-        @records = Person.queue_import_from_csv_file(params[:file].read, params[:match_by_name], params[:attributes])
-        render action: 'import_queue'
-      elsif request.put?
-        @completed, @errored = Person.import_data(params)
-        render action: 'import_results'
-      end
-    else
-      render text: t('not_authorized'), layout: true, status: 401
-    end
-  end
-
   def hashify
     params.merge!(Hash.from_xml(request.body.read))
     if @logged_in.admin?(:import_data) and Site.current.import_export_enabled?

app/controllers/pictures_controller.rb

@@ -44,10 +44,10 @@ def create
     else
       respond_to do |format|
         format.html do
-          flash[:error] = @uploader.errors.full_messages.join('; ')
+          flash[:error] = @uploader.errors.values.join('; ')
           render action: "new"
         end
-        format.json { render json: { status: 'error', errors: @uploader.errors.full_messages } }
+        format.json { render json: { status: 'error', errors: @uploader.errors.values } }
       end
     end
   end

app/controllers/privacies_controller.rb

@@ -26,7 +26,7 @@ def update_consent
     if consent.perform
       flash[:notice] = t('privacies.agreement_saved')
     else
-      flash[:warning] = consent.errors.full_messages.join('; ')
+      flash[:warning] = consent.errors.values.join('; ')
     end
     redirect_to edit_person_privacy_path(@person)
   end