Fix various issues with import script.

- Improve logic that determines whether or not a location needs to be geocoded. In addition, the location importer now checks to see if an address already exists for the location. If so, it updates that same address instead of assigning address attributes, which, unbeknown to me, actually destroyed the existing location and created a new one. The side effect of destroying the address is that it triggered the `reset_location_coordinates` callback, which in turn triggered the geocoding since the location no longer had coordinates.

- Invalid entries no longer prevent the entire CSV file from being saved to the database. Now, valid entries get saved, and error messages are displayed for invalid entries.

- If a CSV file doesn't have sequential IDs, the database will now save the entry with the same ID as in the CSV file. Previously, the database would assign sequential IDs, which would cause mismatches between CSV files that refer to a particular foreign key.

- Added a conditional for the `touch_locations` callback in organization.rb to improve performance during initial import of organizations.

- The console output that lets you know which file is being imported has been moved out of the rake task and into the EntityImporter. That way, if a file is skipped (because it's not required and is missing or empty, for example), then the console won't say that it is being imported.

Closes #336.

app/models/location.rb

@@ -110,12 +110,9 @@ def full_physical_address
   end
 
   def needs_geocoding?
-    return false if address.blank? || new_record_with_coordinates?
-    address.changed?
-  end
-
-  def new_record_with_coordinates?
-    new_record? && latitude.present? && longitude.present?
+    return false if address.blank? || address.marked_for_destruction?
+    return true if latitude.blank? && longitude.blank?
+    address.changed? && !address.new_record?
   end
 
   # See app/models/concerns/search.rb

app/models/organization.rb

@@ -37,10 +37,15 @@ def self.with_locations(ids)
   extend FriendlyId
   friendly_id :name, use: [:history]
 
-  after_save :touch_locations, if: :name_changed?
+  after_save :touch_locations, if: :needs_touch?
 
   private
 
+  def needs_touch?
+    return false if locations.count == 0
+    name_changed?
+  end
+
   def touch_locations
     locations.find_each(&:touch)
   end

lib/category_importer.rb

@@ -8,7 +8,7 @@ def errors
   end
 
   def import
-    categories.each(&:save!) if valid?
+    categories.each(&:save)
   end
 
   protected

lib/contact_importer.rb

@@ -8,7 +8,7 @@ def errors
   end
 
   def import
-    contacts.each(&:save!) if valid?
+    contacts.each(&:save)
   end
 
   protected

lib/entity_importer.rb

@@ -7,9 +7,12 @@ def self.import_file(path)
   end
 
   def self.check_and_import_file(path)
-    FileChecker.new(path, required_headers).validate
+    check = FileChecker.new(path, required_headers).validate
 
-    process_import(path)
+    if check != 'skip import'
+      Kernel.puts("\n===> Importing #{path.to_s.split('/').last}")
+      process_import(path)
+    end
   end
 
   def self.process_import(path)

lib/file_checker.rb

@@ -5,6 +5,7 @@ def initialize(path, required_headers)
   end
 
   def validate
+    return 'skip import' if missing_or_empty_but_not_required?
     if missing_or_empty?
       abort "Aborting because #{filename} is required, but is missing or empty."
     elsif invalid_headers?
@@ -21,18 +22,26 @@ def missing?
     !File.file?(@path)
   end
 
+  def empty?
+    csv_entries.all?(&:blank?)
+  end
+
   def required_but_missing?
     required? && missing?
   end
 
   def required_but_empty?
-    required? && csv_entries.blank?
+    required? && empty?
   end
 
   def missing_or_empty?
     required_but_missing? || required_but_empty?
   end
 
+  def missing_or_empty_but_not_required?
+    !required? && (missing? || empty?)
+  end
+
   def invalid_headers?
     missing_headers.present?
   end

lib/holiday_schedule_importer.rb

@@ -8,7 +8,7 @@ def errors
   end
 
   def import
-    holiday_schedules.each(&:save!) if valid?
+    holiday_schedules.each(&:save)
   end
 
   protected

lib/location_importer.rb

@@ -30,9 +30,8 @@ def errors
   end
 
   def import
-    return unless valid?
     locations.each do |location|
-      location.save!
+      location.save
       # Slows down the geocoding. See INSTALL.md for more details.
       sleep ENV['sleep'].to_i if ENV['sleep'].present?
     end
@@ -41,7 +40,7 @@ def import
   protected
 
   def locations
-    csv_entries.map(&:to_hash).map do |p|
+    @locations ||= csv_entries.map(&:to_hash).map do |p|
       LocationPresenter.new(p, addresses).to_location
     end
   end

lib/location_presenter.rb

@@ -4,21 +4,29 @@
 LocationPresenter = Struct.new(:row, :addresses) do
   def to_location
     location = Location.find_or_initialize_by(id: row[:id].to_i)
-    transform_fields(row)
+    transform_fields(location, row)
     location.attributes = row
     assign_parents_for(location, row)
     location
   end
 
-  def transform_fields(row)
+  def transform_fields(location, row)
     to_array(row, :accessibility, :admin_emails, :languages)
     row[:virtual] = false if row[:virtual].blank?
-    assign_address_attributes(row)
+    assign_address_attributes(location, row)
   end
 
-  def assign_address_attributes(row)
+  def assign_address_attributes(location, row)
     return if row[:virtual] == true || matching_address(row[:id]).blank?
-    row[:address_attributes] = address_attributes_for(row[:id])
+    if location.address.blank?
+      row[:address_attributes] = address_attributes_for(row[:id])
+    else
+      update_address_for(location)
+    end
+  end
+
+  def update_address_for(location)
+    location.address.update(address_attributes_for(row[:id]))
   end
 
   def address_attributes_for(id)

lib/mail_address_importer.rb

@@ -8,7 +8,7 @@ def errors
   end
 
   def import
-    mail_addresses.each(&:save!) if valid?
+    mail_addresses.each(&:save)
   end
 
   protected

lib/organization_importer.rb

@@ -8,13 +8,13 @@ def errors
   end
 
   def import
-    organizations.each(&:save!) if valid?
+    organizations.each(&:save)
   end
 
   protected
 
   def organizations
-    csv_entries.map(&:to_hash).map do |p|
+    @organizations ||= csv_entries.map(&:to_hash).map do |p|
       OrganizationPresenter.new(p).to_org
     end
   end

lib/organization_presenter.rb

@@ -5,6 +5,7 @@ def to_org
     org = Organization.find_or_initialize_by(id: row[:id].to_i)
     to_array(row, :accreditations, :licenses, :funding_sources)
     org.attributes = row
+    org.id = row[:id].to_i
     org
   end
 end

lib/parent_assigner.rb

@@ -9,6 +9,6 @@ def assign_parents_for(record, row)
   end
 
   def foreign_keys_in(row)
-    row.keys.select { |key| key.to_s.include?('_id') }
+    row.keys.select { |key| key.to_s == 'id' || key.to_s.include?('_id') }
   end
 end

lib/phone_importer.rb

@@ -8,7 +8,7 @@ def errors
   end
 
   def import
-    phones.each(&:save!) if valid?
+    phones.each(&:save)
   end
 
   protected

lib/program_importer.rb

@@ -8,7 +8,7 @@ def errors
   end
 
   def import
-    programs.each(&:save!) if valid?
+    programs.each(&:save)
   end
 
   protected

lib/regular_schedule_importer.rb

@@ -8,7 +8,7 @@ def errors
   end
 
   def import
-    regular_schedules.each(&:save!) if valid?
+    regular_schedules.each(&:save)
   end
 
   protected

lib/service_importer.rb

@@ -8,7 +8,7 @@ def errors
   end
 
   def import
-    services.each(&:save!) if valid?
+    services.each(&:save)
   end
 
   protected

lib/tasks/import.rake

@@ -5,21 +5,19 @@ namespace :import do
 
   desc 'Imports organizations'
   task :organizations, [:path] => :environment do |_, args|
-    Kernel.puts('...Importing your organizations...')
     args.with_defaults(path: Rails.root.join('data/organizations.csv'))
     OrganizationImporter.check_and_import_file(args[:path])
   end
 
   desc 'Imports programs'
   task :programs, [:path] => :environment do |_, args|
-    Kernel.puts('...Importing your programs...')
     args.with_defaults(path: Rails.root.join('data/programs.csv'))
     ProgramImporter.check_and_import_file(args[:path])
   end
 
   desc 'Imports locations'
   task :locations, [:path, :addresses_path] => :environment do |_, args|
-    Kernel.puts('...Importing your locations and addresses...')
+    Kernel.puts("\n===> Importing locations.csv and addresses.csv")
     args.with_defaults(
       path: Rails.root.join('data/locations.csv'),
       addresses_path: Rails.root.join('data/addresses.csv')
@@ -29,49 +27,42 @@ namespace :import do
 
   desc 'Imports taxonomy'
   task :taxonomy, [:path] => :environment do |_, args|
-    Kernel.puts('...Importing your taxonomy...')
     args.with_defaults(path: Rails.root.join('data/taxonomy.csv'))
     CategoryImporter.check_and_import_file(args[:path])
   end
 
   desc 'Imports services'
   task :services, [:path] => :environment do |_, args|
-    Kernel.puts('...Importing your services...')
     args.with_defaults(path: Rails.root.join('data/services.csv'))
     ServiceImporter.check_and_import_file(args[:path])
   end
 
   desc 'Imports mail addresses'
   task :mail_addresses, [:path] => :environment do |_, args|
-    Kernel.puts('...Importing your mail addresses...')
     args.with_defaults(path: Rails.root.join('data/mail_addresses.csv'))
     MailAddressImporter.check_and_import_file(args[:path])
   end
 
   desc 'Imports contacts'
   task :contacts, [:path] => :environment do |_, args|
-    Kernel.puts('...Importing your contacts...')
     args.with_defaults(path: Rails.root.join('data/contacts.csv'))
     ContactImporter.check_and_import_file(args[:path])
   end
 
   desc 'Imports phones'
   task :phones, [:path] => :environment do |_, args|
-    Kernel.puts('...Importing your phones...')
     args.with_defaults(path: Rails.root.join('data/phones.csv'))
     PhoneImporter.check_and_import_file(args[:path])
   end
 
   desc 'Imports regular_schedules'
   task :regular_schedules, [:path] => :environment do |_, args|
-    Kernel.puts('...Importing your regular_schedules...')
     args.with_defaults(path: Rails.root.join('data/regular_schedules.csv'))
     RegularScheduleImporter.check_and_import_file(args[:path])
   end
 
   desc 'Imports holiday_schedules'
   task :holiday_schedules, [:path] => :environment do |_, args|
-    Kernel.puts('...Importing your holiday_schedules...')
     args.with_defaults(path: Rails.root.join('data/holiday_schedules.csv'))
     HolidayScheduleImporter.check_and_import_file(args[:path])
   end

spec/lib/address_extractor_spec.rb

@@ -6,17 +6,22 @@
       it 'extracts the addresses' do
         address_hash = {
           id: '1',
-          location_id: '1',
+          location_id: '2',
           address_1: '123 Main Street',
           address_2: 'Suite 101',
           city: 'Fairfax',
           state_province: 'VA',
           postal_code: '22031',
           country: 'US'
         }
+        addresses = [
+          address_hash,
+          address_hash.merge(id: '2', location_id: '3'),
+          address_hash.merge(id: '4', location_id: '1')
+        ]
 
         path = Rails.root.join('spec/support/fixtures/valid_address.csv')
-        expect(AddressExtractor.extract_addresses(path)).to eq [address_hash]
+        expect(AddressExtractor.extract_addresses(path)).to eq addresses
       end
     end
 
@@ -34,7 +39,10 @@
         }
 
         path = Rails.root.join('spec/support/fixtures/invalid_address.csv')
-        expect(AddressExtractor.extract_addresses(path)).to eq [address_hash]
+        expect(AddressExtractor.extract_addresses(path)).
+          to eq [address_hash,
+                 address_hash.merge(id: '2', location_id: '2', city: 'Fairfax')
+                ]
       end
     end
   end