diff --git a/AGENTS.md b/AGENTS.md index 1c3a76085b..6b00076c82 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 | @@ -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 | diff --git a/app/decorators/address_decorator.rb b/app/decorators/address_decorator.rb index a046b3a4f4..98a0190238 100644 --- a/app/decorators/address_decorator.rb +++ b/app/decorators/address_decorator.rb @@ -22,4 +22,44 @@ def detail(length: nil) def url Rails.application.routes.url_helpers.polymorphic_path(addressable) end + + def state_as_text? + AddressLocality.outside_usa?(default_state, default_country) + end + + # 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? + + saved_address&.country.presence || Setting.default_address_country + end + + def default_state + return state if persisted? || state.present? + + saved_address&.state + end + + # 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? + + 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.min_by { |address| [ address.inactive? ? 1 : 0, -address.updated_at.to_f ] } + end end 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 aead846fcc..25628c31a8 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -13,6 +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 @@ -45,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/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/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/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/app/views/addresses/_address_fields.html.erb b/app/views/addresses/_address_fields.html.erb index fcba7b54fd..1ef6a33a64 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, @@ -102,14 +101,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: { @@ -135,7 +127,7 @@ 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.", 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..cb13c182d4 --- /dev/null +++ b/app/views/addresses/_state_field.html.erb @@ -0,0 +1,19 @@ +<% 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(address.default_state), + include_blank: true, + selected: address.default_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..f629c5a281 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, @@ -101,16 +100,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: { @@ -136,7 +126,7 @@ 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.", 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..e2ac4c7b62 --- /dev/null +++ b/db/seeds/features/2026-10-25-state-text-box-for-non-usa-addresses.yml @@ -0,0 +1,13 @@ +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, a non-USA address shows State as a + 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: + - "With nothing saved, a new address starts in the country of the return address in Settings." + - "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 0766c3aa53..7314bd2db5 100644 --- a/spec/decorators/address_decorator_spec.rb +++ b/spec/decorators/address_decorator_spec.rb @@ -28,4 +28,94 @@ expect(address.decorate.picker_label).to include("invoice address") end end + + 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 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 "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 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") + expect(new_address.state_as_text?).to be(true) + end + + 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 + + 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 "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 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 + 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 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 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/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..014f89400b --- /dev/null +++ b/spec/requests/address_state_field_spec.rb @@ -0,0 +1,91 @@ +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 "starts an added address in the saved address's country, locality, and state text box" do + create(:address, addressable: person, state: "AB", country: "Canada", locality: "Outside USA") + + 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") + + 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