From 4d3048882bfa3fb9f821d8702870bd4dafb41bdf Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 06:47:50 -0400 Subject: [PATCH 1/6] Let non-USA addresses type their state Canadian addresses stored "CA" for Canada rendered as California in the US-only dropdown, with no way to enter a province. Non-USA addresses now get a text box; admins can force one with admin=true to fix US-looking values. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/models/address.rb | 4 + app/policies/address_policy.rb | 3 + app/views/addresses/_address_fields.html.erb | 9 +-- app/views/addresses/_state_field.html.erb | 17 ++++ .../organizations/_address_fields.html.erb | 11 +-- ...5-state-text-box-for-non-usa-addresses.yml | 11 +++ spec/models/address_spec.rb | 11 +++ spec/policies/address_policy_spec.rb | 17 ++++ spec/requests/address_state_field_spec.rb | 79 +++++++++++++++++++ 9 files changed, 144 insertions(+), 18 deletions(-) create mode 100644 app/policies/address_policy.rb create mode 100644 app/views/addresses/_state_field.html.erb create mode 100644 db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml create mode 100644 spec/policies/address_policy_spec.rb create mode 100644 spec/requests/address_state_field_spec.rb diff --git a/app/models/address.rb b/app/models/address.rb index aead846fcc..70d75b902f 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -73,6 +73,10 @@ def known_type? address_type.present? && address_type != UNKNOWN_CONTACT_TYPE end + def outside_usa? + AddressLocality.outside_usa?(state, country) + end + private # Every address carries a type, so one saved without an answer says so. Rows written diff --git a/app/policies/address_policy.rb b/app/policies/address_policy.rb new file mode 100644 index 0000000000..0f8c79bcd5 --- /dev/null +++ b/app/policies/address_policy.rb @@ -0,0 +1,3 @@ +class AddressPolicy < ApplicationPolicy + def enter_any_state? = admin? +end diff --git a/app/views/addresses/_address_fields.html.erb b/app/views/addresses/_address_fields.html.erb index fcba7b54fd..61e53ca192 100644 --- a/app/views/addresses/_address_fields.html.erb +++ b/app/views/addresses/_address_fields.html.erb @@ -102,14 +102,7 @@ class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" } %> - <%= f.input :state, - as: :select, - collection: state_select_options(f.object.state), - selected: f.object.state, - input_html: { - value: f.object.state, - class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" - } %> + <%= render "addresses/state_field", f: f %> <%= f.input :zip_code, label: "ZIP Code", input_html: { diff --git a/app/views/addresses/_state_field.html.erb b/app/views/addresses/_state_field.html.erb new file mode 100644 index 0000000000..67db7f9050 --- /dev/null +++ b/app/views/addresses/_state_field.html.erb @@ -0,0 +1,17 @@ +<% if f.object.outside_usa? || (params[:admin] && allowed_to?(:enter_any_state?, Address)) %> + <%= f.input :state, + label: "State / province", + input_html: { + class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" + } %> +<% else %> + <%= f.input :state, + label: "State", + as: :select, + collection: state_select_options(f.object.state), + include_blank: true, + selected: f.object.state, + input_html: { + class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" + } %> +<% end %> diff --git a/app/views/organizations/_address_fields.html.erb b/app/views/organizations/_address_fields.html.erb index abc2d637b7..ac3f9f8d90 100644 --- a/app/views/organizations/_address_fields.html.erb +++ b/app/views/organizations/_address_fields.html.erb @@ -101,16 +101,7 @@ class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" } %> - <%= f.input :state, - label: "State", - as: :select, - collection: state_select_options(f.object.state), - include_blank: true, - selected: f.object.state, - input_html: { - value: f.object.state, - class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" - } %> + <%= render "addresses/state_field", f: f %> <%= f.input :zip_code, label: "ZIP Code", input_html: { diff --git a/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml b/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml new file mode 100644 index 0000000000..532200119f --- /dev/null +++ b/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml @@ -0,0 +1,11 @@ +name: "Type the state or province on non-USA addresses" +area: people +display_status: admin_facing +released_on: 2026-10-25 +summary: >- + On person and organization address forms, an address whose country isn't the + United States shows State as a text box instead of the US-state dropdown, so a + province like AB for Alberta can be entered. +pro_tips: + - "Admins can add admin=true to the edit page address to type any state on a US address too." + - "Set the country and save first; the text box appears on the next edit." diff --git a/spec/models/address_spec.rb b/spec/models/address_spec.rb index 04d15ac946..de9a6b89f5 100644 --- a/spec/models/address_spec.rb +++ b/spec/models/address_spec.rb @@ -91,6 +91,17 @@ end end + describe "#outside_usa?" do + it "is true for a non-US country, even when the state reads like a US code" do + expect(build(:address, state: "CA", country: "Canada")).to be_outside_usa + end + + it "is false for a US state with the US or a blank country" do + expect(build(:address, state: "CA", country: "United States")).not_to be_outside_usa + expect(build(:address, state: "CA", country: nil)).not_to be_outside_usa + end + end + describe "#display_lines" do it "renders the street line and a combined city, state zip line" do address = build(:address, street_address: "123 Main St", city: "Springfield", diff --git a/spec/policies/address_policy_spec.rb b/spec/policies/address_policy_spec.rb new file mode 100644 index 0000000000..1f25b8eece --- /dev/null +++ b/spec/policies/address_policy_spec.rb @@ -0,0 +1,17 @@ +require "rails_helper" + +RSpec.describe AddressPolicy, type: :policy do + def policy_for(user:) + described_class.new(Address, user: user) + end + + describe "#enter_any_state?" do + it "lets an admin type any state" do + expect(policy_for(user: build_stubbed(:user, super_user: true))).to be_allowed_to(:enter_any_state?) + end + + it "keeps a regular user on the dropdown" do + expect(policy_for(user: build_stubbed(:user, super_user: false))).not_to be_allowed_to(:enter_any_state?) + end + end +end diff --git a/spec/requests/address_state_field_spec.rb b/spec/requests/address_state_field_spec.rb new file mode 100644 index 0000000000..d7a753dde0 --- /dev/null +++ b/spec/requests/address_state_field_spec.rb @@ -0,0 +1,79 @@ +require "rails_helper" + +RSpec.describe "State field on address forms", type: :request do + let(:owner_user) { create(:user, :with_person) } + let(:person) { owner_user.person } + + def state_input + Nokogiri::HTML(response.body).at_css("[name$='[state]']") + end + + context "as the owner" do + before do + sign_in owner_user + allow_any_instance_of(PersonPolicy).to receive(:edit?).and_return(true) + end + + it "offers the US-state dropdown for a US address" do + create(:address, addressable: person, state: "CA", country: "United States") + + get edit_person_path(person) + + expect(state_input.name).to eq("select") + expect(state_input.at_css("option[selected]")["value"]).to eq("CA") + end + + it "offers a text box for a non-USA address" do + create(:address, addressable: person, state: "CA", country: "Canada") + + get edit_person_path(person) + + expect(state_input.name).to eq("input") + expect(state_input["value"]).to eq("CA") + end + + it "ignores admin=true" do + create(:address, addressable: person, state: "CA", country: "United States") + + get edit_person_path(person, admin: true) + + expect(state_input.name).to eq("select") + end + end + + context "as an admin" do + before { sign_in create(:user, super_user: true) } + + it "offers the dropdown for a US address" do + create(:address, addressable: person, state: "CA", country: "United States") + + get edit_person_path(person) + + expect(state_input.name).to eq("select") + end + + it "offers a text box with admin=true" do + create(:address, addressable: person, state: "CA", country: "United States") + + get edit_person_path(person, admin: true) + + expect(state_input.name).to eq("input") + expect(state_input["value"]).to eq("CA") + end + end + + context "on the organization form" do + let(:organization) { create(:organization) } + + before { sign_in create(:user, super_user: true) } + + it "offers a text box for a non-USA address" do + create(:address, addressable: organization, state: "CA", country: "Canada") + + get edit_organization_path(organization) + + expect(state_input.name).to eq("input") + expect(state_input["value"]).to eq("CA") + end + end +end From 1847c45e5c5129d502b30f7cf71e38357290bee2 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 06:53:40 -0400 Subject: [PATCH 2/6] Start new addresses from the saved address's country and require one A second address abroad shouldn't make the person re-pick their country, and the state field and locality should follow it. Country is required only on the person/org forms so imports and registration capture keep saving. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/controllers/application_controller.rb | 4 ++ app/controllers/organizations_controller.rb | 2 + app/controllers/people_controller.rb | 2 + app/decorators/address_decorator.rb | 18 +++++++ app/models/address.rb | 8 +-- app/views/addresses/_address_fields.html.erb | 8 +-- app/views/addresses/_state_field.html.erb | 2 +- .../organizations/_address_fields.html.erb | 8 +-- ...5-state-text-box-for-non-usa-addresses.yml | 10 ++-- spec/decorators/address_decorator_spec.rb | 49 +++++++++++++++++++ spec/models/address_spec.rb | 20 ++++---- spec/requests/address_state_field_spec.rb | 36 ++++++++++++++ 12 files changed, 139 insertions(+), 28 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9bc5ea05fb..4e30a3e45f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -31,6 +31,10 @@ class ApplicationController < ActionController::Base private + def require_address_countries(addressable) + addressable.addresses.each { |address| address.country_required = true } + end + def count_display(shown, total = shown) return helpers.number_with_delimiter(total) if shown == total "#{helpers.number_with_delimiter(shown)}/#{helpers.number_with_delimiter(total)}" diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index 5aac647ab7..babfb6e75b 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -114,6 +114,7 @@ def create end end + require_address_countries(@organization) if @organization.save redirect_to @organization, notice: "Organization was successfully created." else @@ -125,6 +126,7 @@ def create def update authorize! @organization @organization.assign_attributes(organization_params) + require_address_countries(@organization) if @organization.save redirect_to organization_path(@organization), notice: "Organization was successfully updated.", status: :see_other diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 13da23da18..0c67ebabb9 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -283,6 +283,7 @@ def create end end + require_address_countries(@person) respond_to do |format| if @person.save format.html { redirect_to @person, notice: "Person was successfully created." } @@ -303,6 +304,7 @@ def update attrs = person_params reject_locked_license_changes!(attrs) @person.assign_attributes(attrs) + require_address_countries(@person) if @person.save redirect_to person_update_return_path, notice: "Person was successfully updated." diff --git a/app/decorators/address_decorator.rb b/app/decorators/address_decorator.rb index a046b3a4f4..2e79d20cf8 100644 --- a/app/decorators/address_decorator.rb +++ b/app/decorators/address_decorator.rb @@ -22,4 +22,22 @@ def detail(length: nil) def url Rails.application.routes.url_helpers.polymorphic_path(addressable) end + + def state_as_text? + AddressLocality.outside_usa?(state, default_country) + end + + def default_locality + return locality if locality.present? + + Address::OUTSIDE_USA if state_as_text? + end + + # A new address starts in the country of the owner's saved address (primary first), else the US. + def default_country + return country if persisted? || country.present? + + saved = addressable&.addresses&.select { |address| address.persisted? && address.country.present? } || [] + (saved.find(&:primary?) || saved.first)&.country || Address::DEFAULT_COUNTRY + end end diff --git a/app/models/address.rb b/app/models/address.rb index 70d75b902f..5fb165cd30 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -13,6 +13,7 @@ class Address < ApplicationRecord UNKNOWN_LOCALITY = "Unknown" LOCALITIES = [ LA_CITY, LA_COUNTY, "Southern CA", "Northern CA", "Central CA", ORANGE_COUNTY, OUTSIDE_CA, OUTSIDE_USA, UNKNOWN_LOCALITY ] + DEFAULT_COUNTRY = "United States" UNKNOWN_CONTACT_TYPE = "unknown" CONTACT_TYPES = [ nil, "work", "personal", "mailing", UNKNOWN_CONTACT_TYPE ].freeze # USPS abbreviations for the 50 states, DC, and the US territories the atlas @@ -29,6 +30,8 @@ class Address < ApplicationRecord # the link rather than block deletion when an org address is removed. has_many :affiliations, foreign_key: :organization_address_id, dependent: :nullify, inverse_of: :organization_address + attr_accessor :country_required + before_validation :type_as_unknown validates :locality, presence: true @@ -40,6 +43,7 @@ class Address < ApplicationRecord validates :district, length: { maximum: 255 } validates :county, length: { maximum: 255 } validates :country, length: { maximum: 255 } + validates :country, presence: true, if: :country_required validates :phone, length: { maximum: 255 } scope :active, -> { where(inactive: false) } @@ -73,10 +77,6 @@ def known_type? address_type.present? && address_type != UNKNOWN_CONTACT_TYPE end - def outside_usa? - AddressLocality.outside_usa?(state, country) - end - private # Every address carries a type, so one saved without an answer says so. Rows written diff --git a/app/views/addresses/_address_fields.html.erb b/app/views/addresses/_address_fields.html.erb index 61e53ca192..298400799a 100644 --- a/app/views/addresses/_address_fields.html.erb +++ b/app/views/addresses/_address_fields.html.erb @@ -51,9 +51,8 @@ as: :select, collection: Address::LOCALITIES, include_blank: true, - selected: f.object.locality, + selected: f.object.decorate.default_locality, input_html: { - value: f.object.locality, class: "rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring focus:ring-blue-200" } %> <%= f.input :la_city_council_district, @@ -128,9 +127,10 @@ label: "Country", as: :select, collection: GeographyHelper::COUNTRIES, - selected: f.object.country, + selected: f.object.decorate.default_country, include_blank: "Select a country", - hint: "Not listed? Pick the closest one or leave this blank.", + required: true, + hint: "Not listed? Pick the closest one.", input_html: { class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" } %> diff --git a/app/views/addresses/_state_field.html.erb b/app/views/addresses/_state_field.html.erb index 67db7f9050..13dec217fa 100644 --- a/app/views/addresses/_state_field.html.erb +++ b/app/views/addresses/_state_field.html.erb @@ -1,4 +1,4 @@ -<% if f.object.outside_usa? || (params[:admin] && allowed_to?(:enter_any_state?, Address)) %> +<% if f.object.decorate.state_as_text? || (params[:admin] && allowed_to?(:enter_any_state?, Address)) %> <%= f.input :state, label: "State / province", input_html: { diff --git a/app/views/organizations/_address_fields.html.erb b/app/views/organizations/_address_fields.html.erb index ac3f9f8d90..912e314ead 100644 --- a/app/views/organizations/_address_fields.html.erb +++ b/app/views/organizations/_address_fields.html.erb @@ -50,9 +50,8 @@ as: :select, collection: Address::LOCALITIES, include_blank: true, - selected: f.object.locality, + selected: f.object.decorate.default_locality, input_html: { - value: f.object.locality, class: "rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring focus:ring-blue-200" } %> <%= f.input :la_city_council_district, @@ -127,9 +126,10 @@ label: "Country", as: :select, collection: GeographyHelper::COUNTRIES, - selected: f.object.country, + selected: f.object.decorate.default_country, include_blank: "Select a country", - hint: "Not listed? Pick the closest one or leave this blank.", + required: true, + hint: "Not listed? Pick the closest one.", input_html: { class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" } %> diff --git a/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml b/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml index 532200119f..ce57a1d7a1 100644 --- a/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml +++ b/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml @@ -2,10 +2,12 @@ name: "Type the state or province on non-USA addresses" area: people display_status: admin_facing released_on: 2026-10-25 +pr_number: 2843 summary: >- - On person and organization address forms, an address whose country isn't the - United States shows State as a text box instead of the US-state dropdown, so a - province like AB for Alberta can be entered. + On person and organization address forms, a non-USA address shows State as a + text box (so a province like AB can be entered) and Country is now required. + A new address starts in the country of the saved one, or the United States. pro_tips: + - "A non-USA address with no locality starts on Outside USA." - "Admins can add admin=true to the edit page address to type any state on a US address too." - - "Set the country and save first; the text box appears on the next edit." + - "Changing the country switches the state field after you save and reopen the address." diff --git a/spec/decorators/address_decorator_spec.rb b/spec/decorators/address_decorator_spec.rb index 0766c3aa53..ab90a2da97 100644 --- a/spec/decorators/address_decorator_spec.rb +++ b/spec/decorators/address_decorator_spec.rb @@ -28,4 +28,53 @@ expect(address.decorate.picker_label).to include("invoice address") end end + + describe "#default_country and #state_as_text?" do + let(:person) { create(:person) } + + def new_address + person.addresses.reload.build.decorate + end + + it "keeps a saved address's own country and state field" do + address = create(:address, addressable: person, state: "CA", country: "Canada").decorate + + expect(address.default_country).to eq("Canada") + expect(address.state_as_text?).to be(true) + end + + it "starts a new address in the saved address's country" do + create(:address, addressable: person, country: "United States") + + expect(new_address.default_country).to eq("United States") + expect(new_address.state_as_text?).to be(false) + end + + it "prefers the primary saved address" do + create(:address, addressable: person, country: "United States", primary: false) + create(:address, addressable: person, state: "AB", country: "Canada", primary: true) + + expect(new_address.default_country).to eq("Canada") + expect(new_address.state_as_text?).to be(true) + end + + it "starts a new address in the United States when nothing is saved" do + expect(new_address.default_country).to eq("United States") + expect(new_address.state_as_text?).to be(false) + end + end + + describe "#default_locality" do + it "keeps a locality that's set" do + expect(build(:address, locality: "LA City", country: "Canada").decorate.default_locality).to eq("LA City") + end + + it "fills a blank locality with Outside USA for a non-USA address" do + expect(build(:address, locality: nil, state: "AB", country: "Canada").decorate.default_locality).to eq("Outside USA") + end + + it "leaves a blank locality blank for a US address" do + expect(build(:address, locality: nil, state: "CA", country: "United States").decorate.default_locality).to be_nil + end + end end diff --git a/spec/models/address_spec.rb b/spec/models/address_spec.rb index de9a6b89f5..8cbf9a85e5 100644 --- a/spec/models/address_spec.rb +++ b/spec/models/address_spec.rb @@ -18,6 +18,15 @@ expect(address.errors[:locality]).to include("can't be blank") end + it "requires a country only when the form asks for one" do + address.country = nil + expect(address).to be_valid + + address.country_required = true + expect(address).not_to be_valid + expect(address.errors[:country]).to include("can't be blank") + end + it "requires a city" do address.city = nil expect(address).not_to be_valid @@ -91,17 +100,6 @@ end end - describe "#outside_usa?" do - it "is true for a non-US country, even when the state reads like a US code" do - expect(build(:address, state: "CA", country: "Canada")).to be_outside_usa - end - - it "is false for a US state with the US or a blank country" do - expect(build(:address, state: "CA", country: "United States")).not_to be_outside_usa - expect(build(:address, state: "CA", country: nil)).not_to be_outside_usa - end - end - describe "#display_lines" do it "renders the street line and a combined city, state zip line" do address = build(:address, street_address: "123 Main St", city: "Springfield", diff --git a/spec/requests/address_state_field_spec.rb b/spec/requests/address_state_field_spec.rb index d7a753dde0..1063e803a9 100644 --- a/spec/requests/address_state_field_spec.rb +++ b/spec/requests/address_state_field_spec.rb @@ -32,6 +32,18 @@ def state_input expect(state_input["value"]).to eq("CA") end + it "starts an added address in the saved address's country, locality, and state text box" do + create(:address, addressable: person, state: "AB", country: "Canada") + + get edit_person_path(person) + + template = Nokogiri::HTML(response.body).at_css("a.add_fields[data-association='address']")["data-association-insertion-template"] + added = Nokogiri::HTML(template) + expect(added.at_css("[name$='[state]']").name).to eq("input") + expect(added.at_css("select[name$='[country]'] option[selected]")["value"]).to eq("Canada") + expect(added.at_css("select[name$='[locality]'] option[selected]")["value"]).to eq("Outside USA") + end + it "ignores admin=true" do create(:address, addressable: person, state: "CA", country: "United States") @@ -76,4 +88,28 @@ def state_input expect(state_input["value"]).to eq("CA") end end + + describe "saving an address" do + let(:address_attributes) do + { address_type: "work", locality: "Unknown", street_address: "1 Main St", city: "Edmonton", state: "AB", zip_code: "T5J" } + end + + before do + sign_in owner_user + allow_any_instance_of(PersonPolicy).to receive(:update?).and_return(true) + end + + it "requires a country" do + patch person_path(person), params: { person: { addresses_attributes: { "0" => address_attributes } } } + + expect(response).to have_http_status(:unprocessable_content) + expect(person.addresses.reload).to be_empty + end + + it "saves with a country" do + patch person_path(person), params: { person: { addresses_attributes: { "0" => address_attributes.merge(country: "Canada") } } } + + expect(person.addresses.reload.sole.country).to eq("Canada") + end + end end From 8c061f2377d85c014b48fdac0da76c8c6b6c23da Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 06:58:51 -0400 Subject: [PATCH 3/6] Start new addresses from the saved address, defaulting to the return address's country A second address usually shares the first one's country, state, and locality, and with nothing saved the organization's own country is the better guess than a hard-coded US. Country stays optional for now. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/controllers/application_controller.rb | 4 -- app/controllers/organizations_controller.rb | 2 - app/controllers/people_controller.rb | 2 - app/decorators/address_decorator.rb | 32 ++++++++++++---- app/models/address.rb | 3 -- app/models/setting.rb | 8 ++++ app/views/addresses/_address_fields.html.erb | 1 - app/views/addresses/_state_field.html.erb | 8 ++-- .../organizations/_address_fields.html.erb | 1 - ...5-state-text-box-for-non-usa-addresses.yml | 8 ++-- spec/decorators/address_decorator_spec.rb | 38 ++++++++++++++----- spec/models/address_spec.rb | 9 ----- spec/models/setting_spec.rb | 22 +++++++++++ spec/requests/address_state_field_spec.rb | 26 +------------ 14 files changed, 93 insertions(+), 71 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4e30a3e45f..9bc5ea05fb 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -31,10 +31,6 @@ class ApplicationController < ActionController::Base private - def require_address_countries(addressable) - addressable.addresses.each { |address| address.country_required = true } - end - def count_display(shown, total = shown) return helpers.number_with_delimiter(total) if shown == total "#{helpers.number_with_delimiter(shown)}/#{helpers.number_with_delimiter(total)}" diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index babfb6e75b..5aac647ab7 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -114,7 +114,6 @@ def create end end - require_address_countries(@organization) if @organization.save redirect_to @organization, notice: "Organization was successfully created." else @@ -126,7 +125,6 @@ def create def update authorize! @organization @organization.assign_attributes(organization_params) - require_address_countries(@organization) if @organization.save redirect_to organization_path(@organization), notice: "Organization was successfully updated.", status: :see_other diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 0c67ebabb9..13da23da18 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -283,7 +283,6 @@ def create end end - require_address_countries(@person) respond_to do |format| if @person.save format.html { redirect_to @person, notice: "Person was successfully created." } @@ -304,7 +303,6 @@ def update attrs = person_params reject_locked_license_changes!(attrs) @person.assign_attributes(attrs) - require_address_countries(@person) if @person.save redirect_to person_update_return_path, notice: "Person was successfully updated." diff --git a/app/decorators/address_decorator.rb b/app/decorators/address_decorator.rb index 2e79d20cf8..eef6b8d30e 100644 --- a/app/decorators/address_decorator.rb +++ b/app/decorators/address_decorator.rb @@ -24,20 +24,38 @@ def url end def state_as_text? - AddressLocality.outside_usa?(state, default_country) + AddressLocality.outside_usa?(default_state, default_country) end + # A new address starts from the owner's saved address (primary first); its country + # falls back to the Settings return address's. + def default_country + return country if persisted? || country.present? + + saved_address&.country.presence || Setting.default_address_country + end + + def default_state + return state if persisted? || state.present? + + saved_address&.state + end + + # California outside LA and Orange counties is a person's call, so it stays blank. def default_locality return locality if locality.present? + return saved_address.locality if new_record? && saved_address&.locality.present? + return Address::OUTSIDE_USA if state_as_text? - Address::OUTSIDE_USA if state_as_text? + Address::OUTSIDE_CA if default_state.present? && AddressNormalizer.state(default_state) != AddressLocality::CALIFORNIA end - # A new address starts in the country of the owner's saved address (primary first), else the US. - def default_country - return country if persisted? || country.present? + private + + def saved_address + return @saved_address if defined?(@saved_address) - saved = addressable&.addresses&.select { |address| address.persisted? && address.country.present? } || [] - (saved.find(&:primary?) || saved.first)&.country || Address::DEFAULT_COUNTRY + saved = addressable&.addresses&.select(&:persisted?) || [] + @saved_address = saved.find(&:primary?) || saved.first end end diff --git a/app/models/address.rb b/app/models/address.rb index 5fb165cd30..acef041532 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -30,8 +30,6 @@ class Address < ApplicationRecord # the link rather than block deletion when an org address is removed. has_many :affiliations, foreign_key: :organization_address_id, dependent: :nullify, inverse_of: :organization_address - attr_accessor :country_required - before_validation :type_as_unknown validates :locality, presence: true @@ -43,7 +41,6 @@ class Address < ApplicationRecord validates :district, length: { maximum: 255 } validates :county, length: { maximum: 255 } validates :country, length: { maximum: 255 } - validates :country, presence: true, if: :country_required validates :phone, length: { maximum: 255 } scope :active, -> { where(inactive: false) } diff --git a/app/models/setting.rb b/app/models/setting.rb index bbcf681fac..130bb54d4a 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -102,6 +102,14 @@ def self.app_organization_phone end private_class_method :app_organization_phone + # The country a new address starts in: the return address's, else the US. + def self.default_address_country + country = stored_association(:return_address)&.country.presence + return Address::DEFAULT_COUNTRY if country.nil? || AddressNormalizer.country(country) == AddressNormalizer::US_COUNTRY + + country + end + # The return address printed in the invoice and receipt header: the address the # row points at, else the app organization's flagged invoice address. def self.organization_address_lines diff --git a/app/views/addresses/_address_fields.html.erb b/app/views/addresses/_address_fields.html.erb index 298400799a..9a9ff290c3 100644 --- a/app/views/addresses/_address_fields.html.erb +++ b/app/views/addresses/_address_fields.html.erb @@ -129,7 +129,6 @@ collection: GeographyHelper::COUNTRIES, selected: f.object.decorate.default_country, include_blank: "Select a country", - required: true, hint: "Not listed? Pick the closest one.", input_html: { class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" diff --git a/app/views/addresses/_state_field.html.erb b/app/views/addresses/_state_field.html.erb index 13dec217fa..cb13c182d4 100644 --- a/app/views/addresses/_state_field.html.erb +++ b/app/views/addresses/_state_field.html.erb @@ -1,16 +1,18 @@ -<% if f.object.decorate.state_as_text? || (params[:admin] && allowed_to?(:enter_any_state?, Address)) %> +<% address = f.object.decorate %> +<% if address.state_as_text? || (params[:admin] && allowed_to?(:enter_any_state?, Address)) %> <%= f.input :state, label: "State / province", input_html: { + value: address.default_state, class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" } %> <% else %> <%= f.input :state, label: "State", as: :select, - collection: state_select_options(f.object.state), + collection: state_select_options(address.default_state), include_blank: true, - selected: f.object.state, + selected: address.default_state, input_html: { class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" } %> diff --git a/app/views/organizations/_address_fields.html.erb b/app/views/organizations/_address_fields.html.erb index 912e314ead..4517bdfb45 100644 --- a/app/views/organizations/_address_fields.html.erb +++ b/app/views/organizations/_address_fields.html.erb @@ -128,7 +128,6 @@ collection: GeographyHelper::COUNTRIES, selected: f.object.decorate.default_country, include_blank: "Select a country", - required: true, hint: "Not listed? Pick the closest one.", input_html: { class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" diff --git a/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml b/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml index ce57a1d7a1..188b947553 100644 --- a/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml +++ b/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml @@ -5,9 +5,9 @@ released_on: 2026-10-25 pr_number: 2843 summary: >- On person and organization address forms, a non-USA address shows State as a - text box (so a province like AB can be entered) and Country is now required. - A new address starts in the country of the saved one, or the United States. + text box so a province like AB can be entered. A new address starts with the + country, state, and locality of the saved one. pro_tips: - - "A non-USA address with no locality starts on Outside USA." + - "With nothing saved, a new address starts in the country of the return address in Settings." + - "A blank locality starts on Outside USA for non-USA addresses and Outside CA for other US states." - "Admins can add admin=true to the edit page address to type any state on a US address too." - - "Changing the country switches the state field after you save and reopen the address." diff --git a/spec/decorators/address_decorator_spec.rb b/spec/decorators/address_decorator_spec.rb index ab90a2da97..0b6429e525 100644 --- a/spec/decorators/address_decorator_spec.rb +++ b/spec/decorators/address_decorator_spec.rb @@ -29,38 +29,45 @@ end end - describe "#default_country and #state_as_text?" do + describe "defaults for a new address" do let(:person) { create(:person) } def new_address person.addresses.reload.build.decorate end - it "keeps a saved address's own country and state field" do - address = create(:address, addressable: person, state: "CA", country: "Canada").decorate + it "keeps a saved address's own values" do + address = create(:address, addressable: person, state: "CA", country: "Canada", locality: "Outside USA").decorate expect(address.default_country).to eq("Canada") + expect(address.default_state).to eq("CA") expect(address.state_as_text?).to be(true) end - it "starts a new address in the saved address's country" do - create(:address, addressable: person, country: "United States") + it "copies the saved address's country, state, and locality" do + create(:address, addressable: person, state: "CA", country: "United States", locality: "LA County") expect(new_address.default_country).to eq("United States") + expect(new_address.default_state).to eq("CA") + expect(new_address.default_locality).to eq("LA County") expect(new_address.state_as_text?).to be(false) end it "prefers the primary saved address" do - create(:address, addressable: person, country: "United States", primary: false) + create(:address, addressable: person, state: "NY", country: "United States", primary: false) create(:address, addressable: person, state: "AB", country: "Canada", primary: true) expect(new_address.default_country).to eq("Canada") + expect(new_address.default_state).to eq("AB") expect(new_address.state_as_text?).to be(true) end - it "starts a new address in the United States when nothing is saved" do - expect(new_address.default_country).to eq("United States") - expect(new_address.state_as_text?).to be(false) + it "falls back to the Settings default country when nothing is saved" do + allow(Setting).to receive(:default_address_country).and_return("Canada") + + expect(new_address.default_country).to eq("Canada") + expect(new_address.default_state).to be_nil + expect(new_address.state_as_text?).to be(true) end end @@ -73,8 +80,19 @@ def new_address expect(build(:address, locality: nil, state: "AB", country: "Canada").decorate.default_locality).to eq("Outside USA") end - it "leaves a blank locality blank for a US address" do + it "fills a blank locality with Outside CA for a US state other than California" do + expect(build(:address, locality: nil, state: "NY", country: "United States").decorate.default_locality).to eq("Outside CA") + end + + it "leaves a blank locality blank in California" do expect(build(:address, locality: nil, state: "CA", country: "United States").decorate.default_locality).to be_nil end + + it "derives a new address's locality when the saved one has none" do + person = create(:person) + create(:address, addressable: person, state: "NY", country: "United States").update_column(:locality, nil) + + expect(person.addresses.reload.build.decorate.default_locality).to eq("Outside CA") + end end end diff --git a/spec/models/address_spec.rb b/spec/models/address_spec.rb index 8cbf9a85e5..04d15ac946 100644 --- a/spec/models/address_spec.rb +++ b/spec/models/address_spec.rb @@ -18,15 +18,6 @@ expect(address.errors[:locality]).to include("can't be blank") end - it "requires a country only when the form asks for one" do - address.country = nil - expect(address).to be_valid - - address.country_required = true - expect(address).not_to be_valid - expect(address.errors[:country]).to include("can't be blank") - end - it "requires a city" do address.city = nil expect(address).not_to be_valid diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index c51c466123..d3a11f4d9a 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -148,6 +148,28 @@ def stub_env(values) end end + describe ".default_address_country" do + let(:organization) { create(:organization) } + + it "reads the return address's country" do + Setting.create!(organization: organization, return_address: create(:address, addressable: organization, country: "Canada")) + + expect(described_class.default_address_country).to eq("Canada") + end + + it "reads a US spelling as the United States option" do + Setting.create!(organization: organization, return_address: create(:address, addressable: organization, country: "USA")) + + expect(described_class.default_address_country).to eq("United States") + end + + it "falls back to the United States without a return address" do + Setting.create!(organization: organization) + + expect(described_class.default_address_country).to eq("United States") + end + end + describe ".organization_phone" do let(:organization) { create(:organization) } diff --git a/spec/requests/address_state_field_spec.rb b/spec/requests/address_state_field_spec.rb index 1063e803a9..014f89400b 100644 --- a/spec/requests/address_state_field_spec.rb +++ b/spec/requests/address_state_field_spec.rb @@ -33,7 +33,7 @@ def state_input end it "starts an added address in the saved address's country, locality, and state text box" do - create(:address, addressable: person, state: "AB", country: "Canada") + create(:address, addressable: person, state: "AB", country: "Canada", locality: "Outside USA") get edit_person_path(person) @@ -88,28 +88,4 @@ def state_input expect(state_input["value"]).to eq("CA") end end - - describe "saving an address" do - let(:address_attributes) do - { address_type: "work", locality: "Unknown", street_address: "1 Main St", city: "Edmonton", state: "AB", zip_code: "T5J" } - end - - before do - sign_in owner_user - allow_any_instance_of(PersonPolicy).to receive(:update?).and_return(true) - end - - it "requires a country" do - patch person_path(person), params: { person: { addresses_attributes: { "0" => address_attributes } } } - - expect(response).to have_http_status(:unprocessable_content) - expect(person.addresses.reload).to be_empty - end - - it "saves with a country" do - patch person_path(person), params: { person: { addresses_attributes: { "0" => address_attributes.merge(country: "Canada") } } } - - expect(person.addresses.reload.sole.country).to eq("Canada") - end - end end From f980753d390c76d8b7120abe5fee55e018461901 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:14:59 -0400 Subject: [PATCH 4/6] Derive a blank locality from the address and seed defaults from a live address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review turned up three things the form got wrong. A blank locality reinvented the locality rules, so a state no US state answers to ("ON" with no country) was offered as Outside CA where AddressLocality deliberately declines to guess; delegating to it also recovers the LA City / LA County / Orange County answers the form was throwing away. A new address seeded itself from a retired address as readily as a live one, and the primary flag it preferred is pre-checked on every address added, so it rarely pointed at the current one — active first, then most recently updated. And the country hint still told people to pick one after the requirement was reverted. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- app/decorators/address_decorator.rb | 16 +++++---- app/views/addresses/_address_fields.html.erb | 2 +- .../organizations/_address_fields.html.erb | 2 +- ...5-state-text-box-for-non-usa-addresses.yml | 4 +-- spec/decorators/address_decorator_spec.rb | 33 ++++++++++++++++--- 6 files changed, 43 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1c3a76085b..021a930681 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,7 +60,7 @@ This codebase (Rails 8.1) | `app/controllers/` | Rails controllers (admin/, events/, home/) | ~93 files | | `app/views/` | ERB templates | ~824 files | | `app/decorators/` | Draper decorators for view logic | ~60 files | -| `app/policies/` | ActionPolicy authorization rules | ~63 files | +| `app/policies/` | ActionPolicy authorization rules | ~75 files | | `app/presenters/` | Presentation objects | 8 files | | `app/helpers/` | View helpers | ~50 files | | `app/mailers/` | ActionMailer classes | 6 files | diff --git a/app/decorators/address_decorator.rb b/app/decorators/address_decorator.rb index eef6b8d30e..98a0190238 100644 --- a/app/decorators/address_decorator.rb +++ b/app/decorators/address_decorator.rb @@ -27,8 +27,8 @@ def state_as_text? AddressLocality.outside_usa?(default_state, default_country) end - # A new address starts from the owner's saved address (primary first); its country - # falls back to the Settings return address's. + # A new address starts from the owner's saved address; its country falls back to the + # Settings return address's. def default_country return country if persisted? || country.present? @@ -41,21 +41,25 @@ def default_state saved_address&.state end - # California outside LA and Orange counties is a person's call, so it stays blank. + # Anything AddressLocality can't settle from the address itself — California outside + # LA and Orange counties, a state spelling no US state answers to — stays blank for + # a person to choose. def default_locality return locality if locality.present? return saved_address.locality if new_record? && saved_address&.locality.present? - return Address::OUTSIDE_USA if state_as_text? - Address::OUTSIDE_CA if default_state.present? && AddressNormalizer.state(default_state) != AddressLocality::CALIFORNIA + derived = AddressLocality.for(city: city, state: default_state, country: default_country, county: county) + derived unless derived == Address::UNKNOWN_LOCALITY end private + # The saved address a new one copies: an active address before a retired one, and the + # most recently updated of those. def saved_address return @saved_address if defined?(@saved_address) saved = addressable&.addresses&.select(&:persisted?) || [] - @saved_address = saved.find(&:primary?) || saved.first + @saved_address = saved.min_by { |address| [ address.inactive? ? 1 : 0, -address.updated_at.to_f ] } end end diff --git a/app/views/addresses/_address_fields.html.erb b/app/views/addresses/_address_fields.html.erb index 9a9ff290c3..1ef6a33a64 100644 --- a/app/views/addresses/_address_fields.html.erb +++ b/app/views/addresses/_address_fields.html.erb @@ -129,7 +129,7 @@ collection: GeographyHelper::COUNTRIES, selected: f.object.decorate.default_country, include_blank: "Select a country", - hint: "Not listed? Pick the closest one.", + hint: "Not listed? Pick the closest one or leave this blank.", input_html: { class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" } %> diff --git a/app/views/organizations/_address_fields.html.erb b/app/views/organizations/_address_fields.html.erb index 4517bdfb45..f629c5a281 100644 --- a/app/views/organizations/_address_fields.html.erb +++ b/app/views/organizations/_address_fields.html.erb @@ -128,7 +128,7 @@ collection: GeographyHelper::COUNTRIES, selected: f.object.decorate.default_country, include_blank: "Select a country", - hint: "Not listed? Pick the closest one.", + hint: "Not listed? Pick the closest one or leave this blank.", input_html: { class: "rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500" } %> diff --git a/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml b/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml index 188b947553..e2ac4c7b62 100644 --- a/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml +++ b/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml @@ -9,5 +9,5 @@ summary: >- country, state, and locality of the saved one. pro_tips: - "With nothing saved, a new address starts in the country of the return address in Settings." - - "A blank locality starts on Outside USA for non-USA addresses and Outside CA for other US states." - - "Admins can add admin=true to the edit page address to type any state on a US address too." + - "A blank locality starts on whatever the address itself settles — Outside USA, Outside CA, LA City, LA County, or Orange County — and stays blank when it can't be worked out." + - "On a US address, admins can add ?admin=true to the end of the edit page's web address to type any state." diff --git a/spec/decorators/address_decorator_spec.rb b/spec/decorators/address_decorator_spec.rb index 0b6429e525..7314bd2db5 100644 --- a/spec/decorators/address_decorator_spec.rb +++ b/spec/decorators/address_decorator_spec.rb @@ -53,9 +53,18 @@ def new_address expect(new_address.state_as_text?).to be(false) end - it "prefers the primary saved address" do - create(:address, addressable: person, state: "NY", country: "United States", primary: false) - create(:address, addressable: person, state: "AB", country: "Canada", primary: true) + it "prefers the most recently updated saved address" do + create(:address, addressable: person, state: "AB", country: "Canada", updated_at: 2.days.ago) + create(:address, addressable: person, state: "NY", country: "United States", updated_at: 1.hour.ago) + + expect(new_address.default_country).to eq("United States") + expect(new_address.default_state).to eq("NY") + expect(new_address.state_as_text?).to be(false) + end + + it "takes an active saved address over a more recently updated inactive one" do + create(:address, addressable: person, state: "AB", country: "Canada", updated_at: 2.days.ago) + create(:address, addressable: person, state: "NY", country: "United States", inactive: true, updated_at: 1.hour.ago) expect(new_address.default_country).to eq("Canada") expect(new_address.default_state).to eq("AB") @@ -84,8 +93,22 @@ def new_address expect(build(:address, locality: nil, state: "NY", country: "United States").decorate.default_locality).to eq("Outside CA") end - it "leaves a blank locality blank in California" do - expect(build(:address, locality: nil, state: "CA", country: "United States").decorate.default_locality).to be_nil + it "leaves a blank locality blank in California outside LA and Orange counties" do + address = build(:address, locality: nil, state: "CA", country: "United States", city: "Fresno", county: "Fresno") + + expect(address.decorate.default_locality).to be_nil + end + + it "fills a blank locality with LA City for a Los Angeles address" do + address = build(:address, locality: nil, state: "CA", country: "United States", city: "Los Angeles", county: "Los Angeles") + + expect(address.decorate.default_locality).to eq("LA City") + end + + it "leaves a blank locality blank for a state no US state answers to" do + address = build(:address, locality: nil, state: "ON", country: nil) + + expect(address.decorate.default_locality).to be_nil end it "derives a new address's locality when the saved one has none" do From 548736b196ad6190dbeda54d91f88d0602102b5a Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:37:37 -0400 Subject: [PATCH 5/6] Drop an address block nobody filled in, now that the form pre-fills it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-filling a new block's country, state and locality from the saved address made an untouched "Add address" block look filled to the nested-attributes reject_if, which slices all five of locality, city, state, street_address and zip_code. The block was built instead of dropped, and the parent save failed on "Addresses city can't be blank" — an organization form could no longer be saved after someone opened an address block and changed their mind. Reject on the three fields a person actually has to type. The other three arrive pre-filled, so they say nothing about whether anyone touched the block, and an address holding only those can never pass validation anyway. Both models carried the same proc, so the list lives on Address with the comment explaining it. While there, point GeographyHelper and AddressTranslation at Address::DEFAULT_COUNTRY so the dropdown's spelling of the US is written once — a value that drifts from the option string renders the select blank. Co-Authored-By: Claude Opus 5 (1M context) --- app/helpers/geography_helper.rb | 4 ++-- app/models/address.rb | 12 +++++++++++ app/models/organization.rb | 2 +- app/models/person.rb | 2 +- app/services/address_translation.rb | 2 +- spec/models/address_spec.rb | 31 +++++++++++++++++++++++++++++ 6 files changed, 48 insertions(+), 5 deletions(-) diff --git a/app/helpers/geography_helper.rb b/app/helpers/geography_helper.rb index ad93e1b65f..e2319a9056 100644 --- a/app/helpers/geography_helper.rb +++ b/app/helpers/geography_helper.rb @@ -79,8 +79,8 @@ def state_select_options(current = nil) # Country dropdown options: the United States first, then every other country A–Z. # The US minor outlying islands are left out — nobody picks them for an address. - COUNTRIES = ([ "United States" ] + ( + COUNTRIES = ([ Address::DEFAULT_COUNTRY ] + ( ISO3166::Country.all.map { |country| PLAIN_COUNTRY_NAMES[country.alpha2] || country.common_name } - - [ "United States", "United States Minor Outlying Islands" ] + [ Address::DEFAULT_COUNTRY, "United States Minor Outlying Islands" ] ).sort).freeze end diff --git a/app/models/address.rb b/app/models/address.rb index acef041532..25628c31a8 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -13,7 +13,13 @@ class Address < ApplicationRecord UNKNOWN_LOCALITY = "Unknown" LOCALITIES = [ LA_CITY, LA_COUNTY, "Southern CA", "Northern CA", "Central CA", ORANGE_COUNTY, OUTSIDE_CA, OUTSIDE_USA, UNKNOWN_LOCALITY ] + # The one spelling of the US the country dropdown carries, so a value compared or + # copied against it matches the option rather than rendering the select blank. DEFAULT_COUNTRY = "United States" + # The parts of an address someone has to type. Locality, state and country arrive on a + # new form block already filled from the saved address, so they say nothing about + # whether anyone touched it. + ENTERED_FIELDS = %w[city street_address zip_code].freeze UNKNOWN_CONTACT_TYPE = "unknown" CONTACT_TYPES = [ nil, "work", "personal", "mailing", UNKNOWN_CONTACT_TYPE ].freeze # USPS abbreviations for the 50 states, DC, and the US territories the atlas @@ -46,6 +52,12 @@ class Address < ApplicationRecord scope :active, -> { where(inactive: false) } scope :for_invoices, -> { where(invoice_address: true) } + # A nested-attributes block nobody filled in, which the parent drops instead of + # building an address that can only fail validation. + def self.unfilled_attributes?(attributes) + attributes.slice(*ENTERED_FIELDS).values.all?(&:blank?) + end + def name "#{street_address}, #{city}, #{state} #{zip_code}" end diff --git a/app/models/organization.rb b/app/models/organization.rb index 27285328b8..c0cbb49d43 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -75,7 +75,7 @@ def communications_email # Nested attributes accepts_nested_attributes_for :addresses, allow_destroy: true, - reject_if: proc { |attrs| attrs.slice("locality", "city", "state", "street_address", "zip_code").values.all?(&:blank?) } + reject_if: proc { |attrs| Address.unfilled_attributes?(attrs) } accepts_nested_attributes_for :affiliations, allow_destroy: true, reject_if: proc { |attrs| attrs["person_id"].blank? } accepts_nested_attributes_for :comments, allow_destroy: true, reject_if: proc { |attrs| attrs["body"].blank? } diff --git a/app/models/person.rb b/app/models/person.rb index a5d0f10cde..7447566ef4 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -156,7 +156,7 @@ def known_email_2_type? = email_2_type.in?(CONTACT_TYPES) # Nested attributes accepts_nested_attributes_for :addresses, allow_destroy: true, - reject_if: proc { |attrs| attrs.slice("locality", "city", "state", "street_address", "zip_code").values.all?(&:blank?) } + reject_if: proc { |attrs| Address.unfilled_attributes?(attrs) } accepts_nested_attributes_for :contact_methods, allow_destroy: true, reject_if: :all_blank accepts_nested_attributes_for :sectorable_items, allow_destroy: true, reject_if: proc { |attrs| attrs["sector_id"].blank? } diff --git a/app/services/address_translation.rb b/app/services/address_translation.rb index 2ed8920d00..bb0c68dff1 100644 --- a/app/services/address_translation.rb +++ b/app/services/address_translation.rb @@ -6,7 +6,7 @@ # # Built once per run and reused, so an import's report can list everything it couldn't place. class AddressTranslation - UNITED_STATES = "United States" + UNITED_STATES = Address::DEFAULT_COUNTRY # The words the archive and the forms use for a kind of address, phone, or email. CONTACT_TYPES = { "home" => "personal", "personal" => "personal", "cell" => "personal", diff --git a/spec/models/address_spec.rb b/spec/models/address_spec.rb index 04d15ac946..197c70e00c 100644 --- a/spec/models/address_spec.rb +++ b/spec/models/address_spec.rb @@ -166,4 +166,35 @@ expect(organization.addresses.for_invoices).to be_empty end end + + describe ".unfilled_attributes?" do + it "is true for a block carrying only the pre-filled country, state, and locality" do + attrs = { "country" => "Canada", "state" => "AB", "locality" => "Outside USA", + "city" => "", "street_address" => "", "zip_code" => "" }.with_indifferent_access + + expect(described_class.unfilled_attributes?(attrs)).to be(true) + end + + it "is false once a typed field is filled in" do + attrs = { "city" => "Calgary", "street_address" => "", "zip_code" => "" }.with_indifferent_access + + expect(described_class.unfilled_attributes?(attrs)).to be(false) + end + end + + describe "an unfilled nested address block" do + it "is dropped rather than failing the organization it was added to" do + organization = create(:organization) + create(:address, addressable: organization, city: "Albany", state: "NY", + country: "United States", locality: "Outside CA") + + saved = organization.update(addresses_attributes: { + "0" => { "locality" => "Outside CA", "state" => "NY", "country" => "United States", + "city" => "", "street_address" => "", "zip_code" => "" } + }) + + expect(saved).to be(true) + expect(organization.addresses.reload.count).to eq(1) + end + end end From cf70518b067cee970ee73ad9b8a1e87cf07e5f05 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 25 Oct 2026 07:38:30 -0400 Subject: [PATCH 6/6] Count the policy specs this branch adds to Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 021a930681..6b00076c82 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -519,7 +519,7 @@ Tailwind only scans the paths listed in the `@source` directives at the top of t | `spec/requests/` | ~137 | HTTP request/integration tests | | `spec/system/` | ~37 | End-to-end browser tests (Capybara) | | `spec/routing/` | ~18 | Route definition tests | -| `spec/policies/` | ~19 | Authorization policy tests | +| `spec/policies/` | ~26 | Authorization policy tests | | `spec/decorators/` | ~29 | Decorator tests | | `spec/services/` | ~109 | Service object tests | | `spec/mailers/` | ~5 | Mailer tests |