Merge pull request #103 from MayOneUS/jt-zip-code

Only accept 5-digit zip codes

app/models/location.rb

@@ -23,7 +23,7 @@ class Location < ActiveRecord::Base
   validates :state, presence: true, unless: :zip_code
   validates :zip_code, presence: true, unless: :state
   validates :zip_code, allow_nil: true,
-    format: { with: /\A\d{5}[^\w]?(\d{4})?\z/ }
+    format: { with: /\A\d{5}\z/ }
 
   ADDRESS_FIELDS = [
     :address_1, :address_2, :city, :state_id, :zip_code, :district_id
@@ -67,7 +67,7 @@ def find_district_by_address
   end
 
   def find_zip_code
-    @_find_zip_code ||= zip_code && ZipCode.find_by_zip(zip_code)
+    @_find_zip_code ||= zip_code && ZipCode.find_by(zip_code: zip_code)
   end
 
   def serializable_hash(options)

app/models/zip_code.rb

@@ -25,14 +25,6 @@ class ZipCode < ActiveRecord::Base
   validates :zip_code, presence: true, uniqueness: { case_sensitive: false },
       format: { with: /\A\d{5}\z/ }
 
-  def self.valid_zip?(string)
-    /\A(?<zip>\d{5})[^\w]?(\d{4})?\z/ =~ string
-  end
-
-  def self.find_by_zip(zip)
-    find_by(zip_code: zip.first(5))
-  end
-
   def single_district?
     districts.size == 1
   end

app/services/person_constructor.rb

@@ -55,6 +55,7 @@ def normalize_params
     flatten_remote_fields
     normalize_keys
     strip_whitespace_from_values
+    normalize_zip_code
   end
 
   def flatten_remote_fields
@@ -72,6 +73,10 @@ def strip_whitespace_from_values
     params.merge!(params){ |k, v, _| v.try(:strip) || v }
   end
 
+  def normalize_zip_code
+    params[:zip_code].try(:gsub!, /\A(\d{5})[^\w]?(\d{4})\z/, '\1')
+  end
+
   def person_params
     params.slice(*PersonWithRemoteFields::ALL_FIELDS)
   end

spec/models/location_spec.rb

@@ -29,10 +29,8 @@
     expect(Location.new(zip_code: '00001')).not_to validate_presence_of(:state)
   end
   describe "zip_code format" do
-    ['00000', '123456789', '11111-0000'].each do |valid_zip|
-      it { should allow_value(valid_zip).for(:zip_code) }
-    end
-    ['bad', '123456', '1234567890', '11111--0000'].each do |bad_zip|
+    it { should allow_value('12345').for(:zip_code) }
+    ['bad', '123456', '123456789', '11111-0000'].each do |bad_zip|
       it { should_not allow_value(bad_zip).for(:zip_code) }
     end
   end

spec/models/zip_code_spec.rb

@@ -16,28 +16,52 @@
 require 'rails_helper'
 
 describe ZipCode do
+  describe "#single_district?" do
+    it "returns true if zip code has 1 district" do
+      zip = ZipCode.new
+      zip.districts << District.new
 
-  describe ".valid_zip_5" do
-    it "accepts valid zip codes" do
-      good_zips = %w[94703 950601234 05301-1234]
-      results = good_zips.select{|zip| ZipCode.valid_zip?(zip) }
-      expect(results).to eq %w[94703 950601234 05301-1234]
+      result = zip.single_district?
+
+      expect(result).to be true
     end
 
-    it "rejects bad zip codes" do
-      bad_zips = %w[1234 123456 12345-124]
-      results = bad_zips.select{|zip| ZipCode.valid_zip?(zip) }
-      expect(results).to eq []
+    it "returns false if zip code has no districts" do
+      zip = ZipCode.new
+
+      result = zip.single_district?
+
+      expect(result).to be false
+    end
+
+    it "returns false if zip code has multiple districts" do
+      zip = ZipCode.new
+      zip.districts << [District.new, District.new]
+
+      result = zip.single_district?
+
+      expect(result).to be false
     end
   end
 
-  describe ".find_by_zip" do
-    it "converts input to zip-5 and finds zip_code" do
-      allow(ZipCode).to receive(:find_by)
+  describe "#single_district" do
+    it "returns district if zip code has 1 district" do
+      zip = ZipCode.new
+      district = District.new
+      zip.districts << district
+
+      result = zip.single_district
+
+      expect(result).to eq district
+    end
+
+    it "returns nil if zip code has multiple districts" do
+      zip = ZipCode.new
+      zip.districts << [District.new, District.new]
 
-      ZipCode.find_by_zip('111110000')
+      result = zip.single_district
 
-      expect(ZipCode).to have_received(:find_by).with(zip_code: '11111')
+      expect(result).to be nil
     end
   end
 end

spec/services/person_constructor_spec.rb

@@ -59,6 +59,25 @@
 
       expect(person.location.city).to eq 'city'
     end
+
+    it "normalizes zip codes before assigning to location" do
+      params = { zip_code: '12345-0001' }
+      expected_params = { zip_code: '12345' }
+      stub_person_finder(expected_params)
+
+      person = PersonConstructor.build(params)
+
+      expect(person.location.zip_code).to eq '12345'
+    end
+
+    it "passes bad zips through to the model" do
+      params = { zip_code: '123456' }
+      stub_person_finder(params, found: false)
+
+      PersonConstructor.build(params)
+
+      expect(PersonFinder).to have_received(:new).with(params)
+    end
   end
 
   def stub_person_finder(params, found: true)