Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions app/forms/hub/zip_code_routing_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,9 @@ def vita_partner_id
end

def unused_zipcode
existing = VitaPartnerZipCode.includes(:vita_partner).find_by(zip_code: @params[:zip_code])
existing = VitaPartnerZipCode.find_by(zip_code: @params[:zip_code], vita_partner_id: vita_partner.id)
if existing.present?
if existing.vita_partner == vita_partner
errors.add(:zip_code, I18n.t("hub.zip_codes.already_applied", zip_code: existing.zip_code))
else
params = { anchor: "zip-code-routing-form", id: existing.vita_partner.id }
path = existing.vita_partner.organization? ? edit_hub_organization_path(params) : edit_hub_site_path(params)
errors.add(:zip_code, I18n.t("hub.zip_codes.already_taken", vita_partner_name: existing.vita_partner.name, vita_partner_path: path, zip_code: existing.zip_code))
end
errors.add(:zip_code, I18n.t("hub.zip_codes.already_applied", zip_code: existing.zip_code))
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/models/vita_partner_zip_code.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# Indexes
#
# index_vita_partner_zip_codes_on_vita_partner_id (vita_partner_id)
# index_vita_partner_zip_codes_on_zip_code (zip_code) UNIQUE
# index_vita_partner_zip_codes_on_zip_code (zip_code)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed to take out unique constraint

#
# Foreign Keys
#
Expand All @@ -22,7 +22,7 @@ class VitaPartnerZipCode < ApplicationRecord
belongs_to :organization, optional: true, foreign_key: 'vita_partner_id', class_name: 'Organization'
belongs_to :site, optional: true, foreign_key: 'vita_partner_id', class_name: 'Site'
validate :record_of_zip_code
validates :zip_code, uniqueness: true
validates :zip_code, uniqueness: { scope: :vita_partner_id }

def city_state
ZipCodes.details(zip_code)[:name]
Expand Down
3 changes: 2 additions & 1 deletion app/services/partner_routing_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ def vita_partner_from_zip_code

eligible_with_capacity = Organization.with_capacity.joins(:serviced_zip_codes).
where(vita_partner_zip_codes: { zip_code: @zip_code })
vita_partner = eligible_with_capacity.first

vita_partner = eligible_with_capacity.order(Arel.sql('RANDOM()')).first

if vita_partner.present?
@routing_method = :zip_code
Expand Down
1 change: 0 additions & 1 deletion config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1396,7 +1396,6 @@ en:
waiting_on: Waiting on
zip_codes:
already_applied: "%{zip_code} is already routed to this partner."
already_taken: "%{zip_code} is already routed to <a href=%{vita_partner_path}>%{vita_partner_name}</a>."
confirm: Are you sure you want to delete %{code} from %{name}?
success: Added %{code} to %{name}.
invitations:
Expand Down
1 change: 0 additions & 1 deletion config/locales/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,6 @@ es:
waiting_on: En espera
zip_codes:
already_applied: "%{zip_code} ya está enrutado a este socio."
already_taken: "%{zip_code} ya está enrutado a <a href=%{vita_partner_path}>%{vita_partner_name}</a>."
confirm: "¿Estás segura de que quieres eliminar? %{code} de %{name}?"
success: Se agregó %{code} a %{name}.
invitations:
Expand Down
13 changes: 13 additions & 0 deletions db/migrate/20251119181821_remove_zipcode_indexing_to_partner.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class RemoveZipcodeIndexingToPartner < ActiveRecord::Migration[7.1]
disable_ddl_transaction!

def up
remove_index :vita_partner_zip_codes, :zip_code
add_index :vita_partner_zip_codes, :zip_code, algorithm: :concurrently
end

def down
remove_index :vita_partner_zip_codes, :zip_code
add_index :vita_partner_zip_codes, :zip_code, unique: true, algorithm: :concurrently
end
end
4 changes: 2 additions & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2025_09_26_151436) do
ActiveRecord::Schema[7.1].define(version: 2025_11_19_181821) do
# These are extensions that must be enabled in order to support this database
enable_extension "citext"
enable_extension "plpgsql"
Expand Down Expand Up @@ -2906,7 +2906,7 @@
t.bigint "vita_partner_id", null: false
t.string "zip_code", null: false
t.index ["vita_partner_id"], name: "index_vita_partner_zip_codes_on_vita_partner_id"
t.index ["zip_code"], name: "index_vita_partner_zip_codes_on_zip_code", unique: true
t.index ["zip_code"], name: "index_vita_partner_zip_codes_on_zip_code"
end

create_table "vita_partners", force: :cascade do |t|
Expand Down
27 changes: 25 additions & 2 deletions spec/controllers/hub/zip_codes_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,39 @@
end
end

context "when the code already exists" do
context "when the code already exists for a different partner" do
before do
create :vita_partner_zip_code, zip_code: zip_code, vita_partner: create(:organization)
end

it "does not make a new source parameter" do
it "creates a new vita partner zip code for this partner" do
expect {
post :create, params: params, format: :js, xhr: true
}.to change(vita_partner.serviced_zip_codes, :count).by 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is format: :js, xhr: true doing in this context?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to look this up because i copied it from the rest of existing tests, but I think its to retrieve data from a URL without a fill page refresh

end

it "increases the total count of VitaPartnerZipCodes" do
expect {
post :create, params: params, format: :js, xhr: true
}.to change(VitaPartnerZipCode, :count).by 1
end
end

context "when the code already exists for the same partner" do
before do
create :vita_partner_zip_code, zip_code: zip_code, vita_partner: vita_partner
end

it "does not create a duplicate zip code for the same partner" do
expect {
post :create, params: params, format: :js, xhr: true
}.not_to change(VitaPartnerZipCode, :count)
end

it "shows an error" do
post :create, params: params, format: :js, xhr: true
expect(flash.now[:alert]).to be_present
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/factories/vita_partner_zip_codes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# Indexes
#
# index_vita_partner_zip_codes_on_vita_partner_id (vita_partner_id)
# index_vita_partner_zip_codes_on_zip_code (zip_code) UNIQUE
# index_vita_partner_zip_codes_on_zip_code (zip_code)
#
# Foreign Keys
#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@

expect(page).to have_text "All Clients"

expect(page).to have_css('.client-table')
expect(page).to have_css('.client-row', count: 4)

# Default sort order
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a flaky test, added a wait condition to help things load first before checking the content

expected_rows = [
{
Expand Down
13 changes: 0 additions & 13 deletions spec/forms/hub/zip_code_routing_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,6 @@
expect(subject.errors[:zip_code]).to include "94606 is already routed to this partner."
end
end

context "when it belongs to a different vita partner" do
let(:other_vita_partner) { create :organization, name: "Oregano Org" }
before do
create :vita_partner_zip_code, vita_partner: other_vita_partner, zip_code: zip_code
end

it "is invalid with appropriate message" do
expect(subject.valid?).to eq false
expect(subject.errors[:zip_code]).to include "94606 is already routed to <a href=/en/hub/organizations/#{other_vita_partner.id}/edit#zip-code-routing-form>Oregano Org</a>."
end
end

end

context "when the zip code is not valid" do
Expand Down
4 changes: 2 additions & 2 deletions spec/models/vita_partner_zip_code_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# Indexes
#
# index_vita_partner_zip_codes_on_vita_partner_id (vita_partner_id)
# index_vita_partner_zip_codes_on_zip_code (zip_code) UNIQUE
# index_vita_partner_zip_codes_on_zip_code (zip_code)
#
# Foreign Keys
#
Expand Down Expand Up @@ -61,7 +61,7 @@
it "is not valid" do
new_record = described_class.new(zip_code: existing_record.zip_code, vita_partner: create(:organization))

expect(new_record).not_to be_valid
expect(new_record).to be_valid
end
end

Expand Down
27 changes: 27 additions & 0 deletions spec/services/partner_routing_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,5 +359,32 @@
end
end
end

context "when multiple Vita Partners serve the same zip code" do
let(:vita_partner_2) { create :organization, name: "Partner 2" }
before do
create :vita_partner_zip_code, zip_code: "94606", vita_partner: vita_partner_2
end

subject { PartnerRoutingService.new(zip_code: "94606") }

it "routes to one of the partners with capacity" do
result = subject.determine_partner

expect([vita_partner, vita_partner_2]).to include(result)
expect(subject.routing_method).to eq :zip_code
end

context "when one partner is at capacity" do
before do
vita_partner.update(capacity_limit: 0)
end

it "routes only to partners with capacity" do
expect(subject.determine_partner).to eq vita_partner_2
expect(subject.routing_method).to eq :zip_code
end
end
end
end
end
Loading