From 665a5b57abfc7f2890ff963a7440ec1806f7b046 Mon Sep 17 00:00:00 2001
From: Angel Perez <iAngel.p93@gmail.com>
Date: Mon, 8 Jan 2018 11:46:32 -0400
Subject: [PATCH] Empty instances of MapLocation won't result in a new DB
 record (#2220)

---
 .../budgets/investments_controller.rb         |  8 +++++++
 app/controllers/proposals_controller.rb       |  8 +++++++
 app/models/concerns/mappable.rb               |  2 +-
 app/models/map_location.rb                    |  2 ++
 lib/tasks/map_locations.rake                  |  8 +++++++
 spec/lib/tasks/map_location_spec.rb           | 22 +++++++++++++++++++
 spec/models/map_location_spec.rb              |  8 +++++++
 7 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 lib/tasks/map_locations.rake
 create mode 100644 spec/lib/tasks/map_location_spec.rb

diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb
index 2913ea260..4a2b2d3d9 100644
--- a/app/controllers/budgets/investments_controller.rb
+++ b/app/controllers/budgets/investments_controller.rb
@@ -15,6 +15,7 @@ module Budgets
     before_action :set_random_seed, only: :index
     before_action :load_categories, only: [:index, :new, :create]
     before_action :set_default_budget_filter, only: :index
+    before_action :destroy_map_location_association, only: :update
 
     feature_flag :budgets
 
@@ -141,6 +142,13 @@ module Budgets
         end
       end
 
+      def destroy_map_location_association
+        map_location = params[:budget_investment][:map_location_attributes]
+        if map_location && (map_location[:longitude] && map_location[:latitude]).blank? && !map_location[:id].blank?
+          MapLocation.destroy(map_location[:id])
+        end
+      end
+
   end
 
 end
diff --git a/app/controllers/proposals_controller.rb b/app/controllers/proposals_controller.rb
index 400887046..ff200fe9a 100644
--- a/app/controllers/proposals_controller.rb
+++ b/app/controllers/proposals_controller.rb
@@ -7,6 +7,7 @@ class ProposalsController < ApplicationController
   before_action :load_categories, only: [:index, :new, :create, :edit, :map, :summary]
   before_action :load_geozones, only: [:edit, :map, :summary]
   before_action :authenticate_user!, except: [:index, :show, :map, :summary]
+  before_action :destroy_map_location_association, only: :update
 
   feature_flag :proposals
 
@@ -130,4 +131,11 @@ class ProposalsController < ApplicationController
       @proposal_successful_exists = Proposal.successful.exists?
     end
 
+    def destroy_map_location_association
+      map_location = params[:proposal][:map_location_attributes]
+      if map_location && (map_location[:longitude] && map_location[:latitude]).blank? && !map_location[:id].blank?
+        MapLocation.destroy(map_location[:id])
+      end
+    end
+
 end
diff --git a/app/models/concerns/mappable.rb b/app/models/concerns/mappable.rb
index eb0414fe2..cf85f37e7 100644
--- a/app/models/concerns/mappable.rb
+++ b/app/models/concerns/mappable.rb
@@ -5,7 +5,7 @@ module Mappable
     attr_accessor :skip_map
 
     has_one :map_location, dependent: :destroy
-    accepts_nested_attributes_for :map_location, allow_destroy: true
+    accepts_nested_attributes_for :map_location, allow_destroy: true, reject_if: :all_blank
 
     validate :map_must_be_valid, on: :create, if: :feature_maps?
 
diff --git a/app/models/map_location.rb b/app/models/map_location.rb
index 42fb8ab2f..445b00aad 100644
--- a/app/models/map_location.rb
+++ b/app/models/map_location.rb
@@ -3,6 +3,8 @@ class MapLocation < ActiveRecord::Base
   belongs_to :proposal, touch: true
   belongs_to :investment, class_name: Budget::Investment, touch: true
 
+  validates :longitude, :latitude, :zoom, presence: true
+
   def available?
     latitude.present? && longitude.present? && zoom.present?
   end
diff --git a/lib/tasks/map_locations.rake b/lib/tasks/map_locations.rake
new file mode 100644
index 000000000..b204be2c7
--- /dev/null
+++ b/lib/tasks/map_locations.rake
@@ -0,0 +1,8 @@
+namespace :map_locations do
+  desc 'Destroy all empty MapLocation instances found in the database'
+  task destroy: :environment do
+    MapLocation.where(longitude: nil, latitude: nil, zoom: nil).each do |map_location|
+      map_location.destroy
+    end
+  end
+end
diff --git a/spec/lib/tasks/map_location_spec.rb b/spec/lib/tasks/map_location_spec.rb
new file mode 100644
index 000000000..8ef004bda
--- /dev/null
+++ b/spec/lib/tasks/map_location_spec.rb
@@ -0,0 +1,22 @@
+require 'rake'
+require 'rails_helper'
+Rails.application.load_tasks
+
+describe 'rake map_locations:destroy' do
+  before do
+    create(:map_location, :proposal_map_location)
+    empty_location = create(:map_location, :proposal_map_location)
+    empty_location.attributes = { longitude: nil, latitude: nil, zoom: nil }
+    empty_location.save(validate: false)
+  end
+
+  let :run_rake_task do
+    Rake.application.invoke_task('map_locations:destroy')
+  end
+
+  it 'destroys empty locations' do
+    expect(MapLocation.all.size).to eq(2)
+    run_rake_task
+    expect(MapLocation.all.size).to eq(1)
+  end
+end
diff --git a/spec/models/map_location_spec.rb b/spec/models/map_location_spec.rb
index 8c5367768..0bfb71212 100644
--- a/spec/models/map_location_spec.rb
+++ b/spec/models/map_location_spec.rb
@@ -8,6 +8,14 @@ describe MapLocation do
     expect(map_location).to be_valid
   end
 
+  it "fails if longitude, latitude or zoom attributes are blank" do
+    map_location.longitude = nil
+    map_location.latitude = nil
+
+    expect(map_location).to_not be_valid
+    expect(map_location.errors.size).to eq(2)
+  end
+
   context "#available?" do
 
     it "returns true when latitude, longitude and zoom defined" do