Merged @wesley PR for WIP #31

just3ws · just3ws · commit 1eb203e30887 · 2014-06-30T15:15:53.000-05:00
diff --git a/app/models/opportunity.rb b/app/models/opportunity.rb
@@ -305,21 +305,24 @@ def assign_random_id
 
   protected
   def set_location_city
-    self.location_city = begin
-      locations = []
-      begin
-        locations = self.team.cities.compact.select { |city| self.location.include?(city) }
-      end while locations.blank? && add_opportunity_locations_to_team && set_location_city
-      self.team.cities.join("|")
-    end unless self.location.nil?
-    errors.add(:location, "is not valid, please specify one or more cities separated by | (e.g. Miami, FL | San Francisco). put 'anywhere' if location doesn't matter") if !self.location.nil? and !valid_location_city
-    self.location_city
+    add_opportunity_locations_to_team
+    locations = self.team.cities.compact.select { |city| self.location.include?(city) }
+
+    return if locations.blank? && anywhere?(self.location)
+
+    self.location_city = locations.join("|")
   end
 
   def add_opportunity_locations_to_team
     geocoded_all = true
     self.location.split('|').each do |location_string|
-      geocoded_all &&= self.team.team_locations.where(conditions: ['address LIKE ?', "%#{location_string}%"]).exists? or anywhere?(location_string) ? false : self.team.team_locations.build(address: location_string, name: location_string).geocode
+      # skip if location is anywhere or already exists
+      if anywhere?(location_string) || self.team.team_locations.where(address: /.*#{location_string}.*/).count > 0
+        geocoded_all = false
+        next
+      end
+
+      geocoded_all &&= self.team.team_locations.build(address: location_string, name: location_string).geocode
     end
     geocoded_all || nil
   end
diff --git a/app/views/opportunities/_form.html.haml b/app/views/opportunities/_form.html.haml
@@ -19,7 +19,7 @@
     -if @team.team_locations.any?
       =j.label :location do
         == Select one or more locations where the candidate must be located or #{link_to('add/manage team locations', edit_team_locations_path(@team))}
-      =j.select(:location, @team.cities+["anywhere"], {:selected => @job.locations}, {:multiple => true})
+      =j.select(:location, @team.cities+["anywhere"], {:selected => (@job.location.blank? ? [] : @job.location.split("|"))}, {:multiple => true})
     -else
       =j.label :location, 'Specify the city/location where the candidate must be located'
       =j.text_field :location
diff --git a/spec/models/opportunity_spec.rb b/spec/models/opportunity_spec.rb
@@ -101,4 +101,37 @@
       job.has_application_from?(user).should be_true
     end
   end
+
+  describe "changing job location" do
+    it "should set location_city" do
+      job = Fabricate(:job)
+      job.location = "Amsterdam|San Francisco"
+      job.save
+      (job.location_city.split("|") - ["Amsterdam", "San Francisco"]).should == []
+    end
+
+    it "should not add anywhere to location_city" do
+      job = Fabricate(:job)
+      job.location = "Amsterdam|San Francisco|anywhere"
+      job.save
+      (job.location_city.split("|") - ["Amsterdam", "San Francisco"]).should == []
+    end
+
+    it "should update location_city with changes" do
+      job = Fabricate(:job)
+      job.location = "Amsterdam|San Francisco"
+      job.save
+      (job.location_city.split("|") - ["Amsterdam", "San Francisco"]).should == []
+      job.location = "Amsterdam"
+      job.save
+      job.location_city.should == "Amsterdam"
+    end
+
+    it "should not add existing locations to the team" do
+      job = Fabricate(:job)
+      job.location = "San Francisco"
+      job.save
+      job.team.team_locations.count.should === 1
+    end
+  end
 end
