Address PR review findings

- Fix COLLATE NOCASE placement on both indexes (apply to abbr/name
  columns, not country_id)
- Add nil guard for country_id in StateTable#write
- Add nil guard for state_id in ZipcodeTable#write
- Strengthen fallback tests to verify correct state ID returned
- Add test for nil country input to get_state_id
- Add test verifying row mutation for downstream Kiba destinations

Author: midwire

lib/free_zipcode_data/state_table.rb

@@ -17,13 +17,13 @@ def build
 
       ndx = <<-SQL
         CREATE UNIQUE INDEX "main"."unique_state"
-        ON #{tablename} (abbr, country_id COLLATE NOCASE ASC);
+        ON #{tablename} (abbr COLLATE NOCASE ASC, country_id);
       SQL
       database.execute_batch(ndx)
 
       ndx = <<-SQL
         CREATE UNIQUE INDEX "main"."state_name"
-        ON #{tablename} (name, country_id COLLATE NOCASE ASC);
+        ON #{tablename} (name COLLATE NOCASE ASC, country_id);
       SQL
       database.execute_batch(ndx)
     end
@@ -33,6 +33,8 @@ def write(row)
 
       row[:state] = 'Marshall Islands' if row[:short_state] == 'MH' && row[:state].nil?
       country_id = get_country_id(row[:country])
+      return nil unless country_id
+
       sql = <<-SQL
         INSERT INTO states (abbr, name, country_id)
         VALUES ('#{row[:short_state]}',

lib/free_zipcode_data/zipcode_table.rb

@@ -30,6 +30,8 @@ def write(row)
       return nil unless row[:postal_code]
 
       state_id = get_state_id(row[:country], row[:short_state], row[:state])
+      return nil unless state_id
+
       city_name = escape_single_quotes(row[:city])
 
       sql = <<-SQL

spec/free_zipcode_data/db_table_spec.rb

@@ -90,25 +90,33 @@ def build; end
 
     describe 'private #get_state_id' do
       it 'finds a state by exact match (abbr + name + country)' do
+        expected_id = db.execute("SELECT id FROM states WHERE abbr = 'NY'")[0][0]
         id = table.send(:get_state_id, 'US', 'NY', 'New York')
-        expect(id).to be_a(Integer)
+        expect(id).to eq(expected_id)
       end
 
       it 'falls back to abbr + country when name does not match' do
+        expected_id = db.execute("SELECT id FROM states WHERE abbr = 'NY'")[0][0]
         id = table.send(:get_state_id, 'US', 'NY', 'Wrong Name')
-        expect(id).to be_a(Integer)
+        expect(id).to eq(expected_id)
       end
 
       it 'falls back to name + country when abbr does not match' do
+        expected_id = db.execute("SELECT id FROM states WHERE name = 'New York'")[0][0]
         id = table.send(:get_state_id, 'US', 'XX', 'New York')
-        expect(id).to be_a(Integer)
+        expect(id).to eq(expected_id)
       end
 
       it 'returns nil for an unknown state' do
         id = table.send(:get_state_id, 'US', 'ZZ', 'Nonexistent')
         expect(id).to be_nil
       end
 
+      it 'returns nil when country is nil' do
+        id = table.send(:get_state_id, nil, 'NY', 'New York')
+        expect(id).to be_nil
+      end
+
       it 'scopes lookup by country' do
         # Seed a Canadian state with the same abbr as a US state
         ca_state_table = FreeZipcodeData::StateTable.new(database: db, tablename: 'states')

spec/free_zipcode_data/state_table_spec.rb

@@ -51,10 +51,14 @@
     end
 
     it 'creates a state from country lookup when short_state is nil' do
-      table.write({ country: 'US', short_state: nil, state: 'Unknown' })
+      row = { country: 'US', short_state: nil, state: 'Unknown' }
+      table.write(row)
       rows = db.execute("SELECT abbr, name FROM states WHERE abbr = 'US'")
       expect(rows.length).to eq(1)
       expect(rows[0]).to eq(['US', 'United States of America'])
+      # Verify row mutation for downstream Kiba destinations
+      expect(row[:short_state]).to eq('US')
+      expect(row[:state]).to eq('United States of America')
     end
 
     it 'creates a state from country lookup when short_state is empty' do