Merge pull request #2295 from wairbut-m2c/aperez-mappable-objects

Improvements for Mappable objects

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

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

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?
 

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

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

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

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