From fa29a5e4872f564d94f548aef0f4e3ca5102b33d Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 16 Aug 2026 19:56:12 -0400 Subject: [PATCH 01/37] Add gear on affiliation rows linking to a full affiliation editor The nested affiliation rows on the org/person edit forms can't reassign the person/org (hidden once persisted) and have nowhere for comments. A per-row gear escapes to a standalone editor that can do both, and carries return_to so it comes back to the exact row it was opened from. Affiliation is now commentable (same polymorphic pattern as Person/Org), so edits are audit-tracked via the inherited AhoyTrackable concern. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/controllers/affiliations_controller.rb | 39 ++- app/decorators/comment_decorator.rb | 2 + app/helpers/comments_helper.rb | 1 + app/models/affiliation.rb | 23 +- app/policies/affiliation_policy.rb | 8 + app/views/affiliations/_fields.html.erb | 12 +- app/views/affiliations/edit.html.erb | 134 +++++++++ config/routes.rb | 2 +- spec/models/affiliation_spec.rb | 302 ++------------------- spec/policies/affiliation_policy_spec.rb | 34 +++ spec/requests/affiliations_spec.rb | 77 ++++++ spec/views/page_bg_class_alignment_spec.rb | 1 + 12 files changed, 347 insertions(+), 288 deletions(-) create mode 100644 app/views/affiliations/edit.html.erb create mode 100644 spec/policies/affiliation_policy_spec.rb create mode 100644 spec/requests/affiliations_spec.rb diff --git a/app/controllers/affiliations_controller.rb b/app/controllers/affiliations_controller.rb index 3fd157504c..69cb626c21 100644 --- a/app/controllers/affiliations_controller.rb +++ b/app/controllers/affiliations_controller.rb @@ -1,5 +1,22 @@ class AffiliationsController < ApplicationController - before_action :set_affiliation, only: %i[ destroy ] + before_action :set_affiliation, only: %i[ edit update destroy ] + + def edit + authorize! @affiliation + end + + def update + authorize! @affiliation + @affiliation.assign_attributes(affiliation_params) + @affiliation.comments.select(&:new_record?).each { |c| c.created_by = current_user; c.updated_by = current_user } + @affiliation.comments.select { |c| c.persisted? && c.body_changed? }.each { |c| c.updated_by = current_user } + + if @affiliation.save + redirect_to affiliation_return_path, notice: "Affiliation was successfully updated.", status: :see_other + else + render :edit, status: :unprocessable_content + end + end def destroy authorize! @affiliation, to: :destroy? @@ -33,4 +50,24 @@ def destroy def set_affiliation @affiliation = Affiliation.find(params[:id]) end + + def affiliation_params + params.require(:affiliation).permit( + :person_id, :organization_id, :title, :start_date, :end_date, :primary_contact, + comments_attributes: [ :id, :topic, :body, :flagged, :_destroy ] + ) + end + + # Return to whichever edit page the gear was clicked from, scrolled to the row. + def affiliation_return_path + anchor = helpers.dom_id(@affiliation) + case params[:return_to] + when "person" + edit_person_path(params[:origin_id], anchor: anchor) + when "organization" + edit_organization_path(params[:origin_id], anchor: anchor) + else + edit_affiliation_path(@affiliation) + end + end end diff --git a/app/decorators/comment_decorator.rb b/app/decorators/comment_decorator.rb index 550309cc13..3575f4a372 100644 --- a/app/decorators/comment_decorator.rb +++ b/app/decorators/comment_decorator.rb @@ -19,6 +19,7 @@ def source_path when TopicSubscription then h.edit_topic_subscription_path(commentable) when Story then h.edit_story_path(commentable) when StoryIdea then h.edit_story_idea_path(commentable) + when Affiliation then h.edit_affiliation_path(commentable) end end @@ -34,6 +35,7 @@ def source_theme when TopicSubscription then :topic_subscriptions when Story then :stories when StoryIdea then :story_ideas + when Affiliation then :organizations else :comments end end diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb index 7c5e5406c0..691007390a 100644 --- a/app/helpers/comments_helper.rb +++ b/app/helpers/comments_helper.rb @@ -13,6 +13,7 @@ def commentable_label(record) when TopicSubscription then "Subscription · #{record.topic_label}" when Story then "Story · #{record.title}" when StoryIdea then "Story idea · #{record.title.presence || "##{record.id}"}" + when Affiliation then "Affiliation · #{record.person&.full_name} @ #{record.organization&.name}" else record.class.name.underscore.humanize end end diff --git a/app/models/affiliation.rb b/app/models/affiliation.rb index 17b3b15042..46cd256519 100644 --- a/app/models/affiliation.rb +++ b/app/models/affiliation.rb @@ -21,6 +21,9 @@ class Affiliation < ApplicationRecord # have this link. belongs_to :event_registration, optional: true, inverse_of: :affiliations + has_many :comments, -> { newest_first }, as: :commentable, dependent: :destroy + accepts_nested_attributes_for :comments, allow_destroy: true, reject_if: proc { |attrs| attrs["body"].blank? } + # Validations validates_presence_of :organization_id validate :organization_address_belongs_to_organization @@ -75,8 +78,10 @@ class Affiliation < ApplicationRecord } before_validation :skip_if_duplicate + # Runs before validation so a reassigned org drops its stale organization_address_id + # before organization_address_belongs_to_organization would reject it. + before_validation :clear_org_scoped_links_on_org_change, on: :update before_save :set_inactive_from_dates - before_update :clear_event_registration_on_org_change after_save :sync_organization_status_with_affiliations after_save :sync_organization_affiliation_dates after_destroy :sync_organization_status_with_affiliations @@ -136,12 +141,16 @@ def skip_if_duplicate throw(:abort) if scope.exists? end - # event_registration_id records the registration that created this affiliation for - # its original org. If an admin moves the affiliation to a different org, that link - # no longer applies, so clear it — a row with no link counts as manually created, - # which reconciliation leaves alone. - def clear_event_registration_on_org_change - self.event_registration_id = nil if organization_id_changed? + # When an admin moves the affiliation to a different org (only possible from the + # standalone edit form), the links scoped to the old org no longer apply: + # event_registration_id (a row with no link counts as manually created, which + # reconciliation leaves alone) and organization_address_id (an address of the old + # org would fail organization_address_belongs_to_organization). + def clear_org_scoped_links_on_org_change + return unless organization_id_changed? + + self.event_registration_id = nil + self.organization_address_id = nil end def set_inactive_from_dates diff --git a/app/policies/affiliation_policy.rb b/app/policies/affiliation_policy.rb index 1f99ba9496..b31c44d4c0 100644 --- a/app/policies/affiliation_policy.rb +++ b/app/policies/affiliation_policy.rb @@ -5,6 +5,14 @@ def destroy? record.persisted? && admin? # we don't allow users to edit their own end + def edit? + record.persisted? && admin? + end + + def update? + edit? + end + # Scoping # See https://actionpolicy.evilmartians.io/#/scoping diff --git a/app/views/affiliations/_fields.html.erb b/app/views/affiliations/_fields.html.erb index 9c3b19b3b5..293a5b830f 100644 --- a/app/views/affiliations/_fields.html.erb +++ b/app/views/affiliations/_fields.html.erb @@ -15,7 +15,17 @@ -
+ <%# Escape to the full affiliation editor: comments + reassign person/org. %> + <%= link_to edit_affiliation_path(f.object, + return_to: person_side ? "organization" : "person", + origin_id: person_side ? f.object.organization_id : f.object.person_id), + class: "absolute top-2 right-2 z-10 text-gray-300 hover:text-gray-500", + title: "Edit affiliation, add comments, or reassign" do %> + + <% end %> + <% end %> +
id="<%= dom_id(f.object) %>"<% end %> data-inactive-toggle-target="row">
diff --git a/app/views/affiliations/edit.html.erb b/app/views/affiliations/edit.html.erb new file mode 100644 index 0000000000..9c7aa3be60 --- /dev/null +++ b/app/views/affiliations/edit.html.erb @@ -0,0 +1,134 @@ +<% content_for(:page_bg_class, "admin-only bg-blue-100") %> +<% back_path = case params[:return_to] + when "person" then edit_person_path(params[:origin_id], anchor: dom_id(@affiliation)) + when "organization" then edit_organization_path(params[:origin_id], anchor: dom_id(@affiliation)) + end %> + +
+
+ <% if back_path %> + <%= link_to back_path, class: "text-sm text-gray-500 hover:text-gray-700" do %> + + <%= params[:return_to] == "person" ? @affiliation.person&.full_name : @affiliation.organization&.name %> + <% end %> + <% else %> + + <% end %> + <%= link_to "Home", root_path, class: "text-sm text-gray-500 hover:text-gray-700" %> +
+ +

Edit affiliation

+

+ <%= @affiliation.person&.full_name %> at <%= @affiliation.organization&.name %> +

+ + <%= simple_form_for @affiliation, + url: affiliation_path(@affiliation, return_to: params[:return_to].presence, origin_id: params[:origin_id].presence), + method: :patch, + html: { id: "affiliation_form", data: { turbo: false } } do |f| %> +
+
+ <%= f.input :person_id, + collection: @affiliation.person ? [ [ @affiliation.person.full_name, @affiliation.person.id ] ] : [], + selected: @affiliation.person_id, + include_blank: false, + label: "Person", + label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, + input_html: { + data: { controller: "remote-select", remote_select_model_value: "person" } + } %> +

Type to search to reassign this affiliation to a different person.

+
+ +
+ <%= f.input :organization_id, + collection: @affiliation.organization ? [ [ @affiliation.organization.name, @affiliation.organization.id ] ] : [], + selected: @affiliation.organization_id, + include_blank: false, + label: "Organization", + label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, + input_html: { + data: { controller: "remote-select", remote_select_model_value: "organization" } + } %> +

Reassigning to a different organization clears the linked address.

+
+ +
+
+ <%= f.input :title, + label_html: { class: "block text-sm font-medium text-gray-700 mb-1" } %> +
+
+ <%= f.input :start_date, + as: :string, + label: "Start", + label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, + input_html: { type: "date", value: @affiliation.start_date&.strftime("%Y-%m-%d") } %> +
+
+ <%= f.input :end_date, + as: :string, + label: "End", + label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, + input_html: { type: "date", value: @affiliation.end_date&.strftime("%Y-%m-%d") } %> +
+
+ + +
+ + <%# ---- Comments (saved with the affiliation, like the other forms) ---- %> +
+
+ + + +

Comments

+
+ +
+
+ <%= f.simple_fields_for :comments do |cf| %> + <%= render "comments/comment_fields", f: cf %> + <% end %> +
+ +
+ +
+ <%= link_to_add_association f, :comments, + partial: "comments/comment_fields", + data: { association_insertion_node: "#comment-list", association_insertion_method: "append" }, + class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> + + Add comment + <% end %> + + +
+
+
+ +
+
+ <% if back_path %> + <%= link_to "Cancel", back_path, class: "btn btn-secondary-outline" %> + <% end %> + +
+
+ <% end %> + + <%= render "shared/audit_info", resource: @affiliation %> +
diff --git a/config/routes.rb b/config/routes.rb index 906478adc7..6a0cbfa1ee 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -270,7 +270,7 @@ resources :refunds, only: [ :new, :create, :show ] resources :organization_statuses - resources :affiliations, only: :destroy + resources :affiliations, only: [ :edit, :update, :destroy ] resources :quotes resources :monthly_reports, only: [ :index, :show ], constraints: { id: /\d+/ } diff --git a/spec/models/affiliation_spec.rb b/spec/models/affiliation_spec.rb index e4efe2540d..2c9111f98d 100644 --- a/spec/models/affiliation_spec.rb +++ b/spec/models/affiliation_spec.rb @@ -1,294 +1,40 @@ -require 'rails_helper' +require "rails_helper" -RSpec.describe Affiliation do - describe 'associations' do - it { should belong_to(:organization) } - it { should belong_to(:person) } - it { should belong_to(:organization_address).class_name("Address").optional } - end - - describe 'validations' do - subject do - build(:affiliation, organization: create(:organization), person: create(:person)) - end - it { should validate_presence_of(:organization_id) } - # it { should validate_presence_of(:person_id) } # we needed to not have this to support nested attrs - end - - describe '#organization_address' do - let(:organization) { create(:organization) } - let(:address) { create(:address, addressable: organization) } - - it 'is valid when the address belongs to the same organization' do - affiliation = build(:affiliation, organization: organization, organization_address: address) - expect(affiliation).to be_valid - end - - it 'is valid when no address is linked' do - affiliation = build(:affiliation, organization: organization, organization_address: nil) - expect(affiliation).to be_valid - end - - it 'is invalid when the address belongs to a different organization' do - other_address = create(:address, addressable: create(:organization)) - affiliation = build(:affiliation, organization: organization, organization_address: other_address) - expect(affiliation).not_to be_valid - expect(affiliation.errors[:organization_address_id]).to be_present - end - - it "is invalid when the address belongs to a person" do - person_address = create(:address, addressable: create(:person)) - affiliation = build(:affiliation, organization: organization, organization_address: person_address) - expect(affiliation).not_to be_valid - end +RSpec.describe Affiliation, type: :model do + describe "comments" do + it "holds comments as the polymorphic commentable" do + affiliation = create(:affiliation) + comment = affiliation.comments.create!(body: "A note about this affiliation") - it 'is nullified when its linked address is destroyed' do - affiliation = create(:affiliation, organization: organization, organization_address: address) - address.destroy - expect(affiliation.reload.organization_address_id).to be_nil + expect(comment.commentable).to eq(affiliation) end end - describe '#active?' do - it 'is true when not inactive and has no end date' do - expect(build(:affiliation, inactive: false, end_date: nil).active?).to be true - end - - it 'is true when not inactive and the end date is in the future' do - expect(build(:affiliation, inactive: false, end_date: 1.month.from_now).active?).to be true - end + describe "lifecycle tracking" do + it "buffers an update.affiliation ahoy event when edited by a user" do + affiliation = create(:affiliation, title: "Facilitator") + Current.user = create(:user, :admin) + allow(Analytics::LifecycleBuffer).to receive(:push).and_call_original - it 'is false when flagged inactive' do - expect(build(:affiliation, inactive: true, end_date: nil).active?).to be false - end + affiliation.update!(title: "Lead facilitator") - it 'is false when the end date has passed' do - expect(build(:affiliation, inactive: false, end_date: 1.day.ago).active?).to be false + expect(Analytics::LifecycleBuffer).to have_received(:push) + .with(hash_including(name: "update.affiliation")) end end - describe '.active' do - let!(:active_op) { create(:affiliation, inactive: false, end_date: nil) } - let!(:active_with_future_end) { create(:affiliation, inactive: false, end_date: 1.month.from_now) } - let!(:inactive_by_flag) { create(:affiliation, inactive: true, end_date: nil) } - let!(:inactive_by_end_date) { create(:affiliation, inactive: false, end_date: 1.day.ago) } - - it 'includes records with inactive: false and no end date' do - expect(described_class.active).to include(active_op) - end - - it 'includes records with inactive: false and future end date' do - expect(described_class.active).to include(active_with_future_end) - end - - it 'excludes records with inactive: true' do - expect(described_class.active).not_to include(inactive_by_flag) - end - - it 'excludes records with past end date' do - expect(described_class.active).not_to include(inactive_by_end_date) - end - - it 'qualifies end_date when joined with organizations (which also has end_date)' do - expect { - described_class.active.joins(:organization).to_a - }.not_to raise_error - end - end - - describe '#facilitator?' do - it 'is true for the exact title "Facilitator"' do - expect(build(:affiliation, title: "Facilitator").facilitator?).to be true - end - - it 'ignores surrounding whitespace' do - expect(build(:affiliation, title: " Facilitator ").facilitator?).to be true - end - - it 'is false for title variants like "Lead Facilitator"' do - expect(build(:affiliation, title: "Lead Facilitator").facilitator?).to be false - end - - it 'is case-sensitive' do - expect(build(:affiliation, title: "facilitator").facilitator?).to be false - expect(build(:affiliation, title: "FACILITATOR").facilitator?).to be false - end - - it 'is false when the title is blank' do - expect(build(:affiliation, title: nil).facilitator?).to be false - end - end - - describe '.facilitators' do - let!(:exact) { create(:affiliation, title: "Facilitator") } - let!(:whitespace) { create(:affiliation, title: " Facilitator ") } - let!(:variant) { create(:affiliation, title: "Lead Facilitator") } - let!(:lowercase) { create(:affiliation, title: "facilitator") } - - it 'includes only the exact, case-sensitive title "Facilitator" (whitespace-trimmed)' do - expect(described_class.facilitators).to contain_exactly(exact, whitespace) - end - end - - describe '#sync_organization_status_with_affiliations' do - let!(:active_status) { OrganizationStatus.find_or_create_by!(name: "Active") } - let!(:inactive_status) { OrganizationStatus.find_or_create_by!(name: "Inactive") } - - it 'sets the organization to Inactive when its last active affiliation goes inactive' do - org = create(:organization, organization_status: active_status) - affiliation = create(:affiliation, organization: org, inactive: false, end_date: nil) - - affiliation.update!(inactive: true) - - expect(org.reload.organization_status).to eq(inactive_status) - end - - it 'sets an Inactive organization back to Active when it regains an active affiliation' do - org = create(:organization, organization_status: inactive_status) - - create(:affiliation, organization: org, inactive: false, end_date: nil) - - expect(org.reload.organization_status).to eq(active_status) - end - - it 'ignores non-facilitator affiliations when deciding status' do - org = create(:organization, organization_status: active_status) - create(:affiliation, organization: org, title: "Volunteer", inactive: false, end_date: nil) - - expect(org.reload.organization_status).to eq(inactive_status) - end - - %w[Pending Reinstate Unknown].each do |status_name| - it "leaves a #{status_name} organization untouched when it regains an active affiliation" do - status = OrganizationStatus.find_or_create_by!(name: status_name) - org = create(:organization, organization_status: status) - - create(:affiliation, organization: org, inactive: false, end_date: nil) - - expect(org.reload.organization_status).to eq(status) - end - end - end - - describe '.active_on' do - let(:date) { Date.new(2024, 6, 1) } - let!(:spanning) { create(:affiliation, start_date: Date.new(2023, 1, 1), end_date: Date.new(2025, 1, 1)) } - let!(:open_ended) { create(:affiliation, start_date: Date.new(2023, 1, 1), end_date: nil) } - let!(:ended_before) { create(:affiliation, start_date: Date.new(2020, 1, 1), end_date: Date.new(2021, 1, 1)) } - let!(:starts_after) { create(:affiliation, start_date: Date.new(2025, 1, 1), end_date: nil) } - let!(:no_dates) { create(:affiliation, start_date: nil, end_date: nil) } - - it 'includes affiliations whose span covers the date' do - expect(described_class.active_on(date)).to include(spanning, open_ended) - end - - it 'excludes affiliations that ended before the date' do - expect(described_class.active_on(date)).not_to include(ended_before) - end - - it 'excludes affiliations that start after the date' do - expect(described_class.active_on(date)).not_to include(starts_after) - end - - it 'includes affiliations with no dates on record' do - expect(described_class.active_on(date)).to include(no_dates) - end - - it 'ignores the cached inactive flag, judging purely by dates' do - flagged = create(:affiliation, start_date: Date.new(2023, 1, 1), end_date: nil, inactive: true) - expect(described_class.active_on(date)).to include(flagged) - end - end - - describe 'status (#status_on and .with_status)' do - let!(:active_open) { create(:affiliation, start_date: Date.current.prev_year, end_date: nil) } - let!(:active_span) { create(:affiliation, start_date: Date.current.prev_year, end_date: Date.current.next_year) } - let!(:upcoming) { create(:affiliation, start_date: Date.current.next_year, end_date: nil) } - let!(:ended) { create(:affiliation, start_date: Date.current.prev_year(2), end_date: Date.current.prev_year) } - let!(:no_dates) { create(:affiliation, start_date: nil, end_date: nil) } - - it 'exposes the taxonomy in display order' do - expect(Affiliation::STATUSES).to eq(%w[ Active Upcoming Inactive ]) - end - - it '#status_on classifies by flag and dates' do - expect(active_open.reload.status_on).to eq("Active") - expect(active_span.reload.status_on).to eq("Active") - expect(upcoming.reload.status_on).to eq("Upcoming") - expect(ended.reload.status_on).to eq("Inactive") - expect(no_dates.reload.status_on).to eq("Active") - end - - it '.with_status returns exactly the rows whose #status_on matches (SQL ↔ Ruby agree)' do - Affiliation::STATUSES.each do |status| - expected = Affiliation.all.select { |a| a.status_on == status }.map(&:id).sort - expect(Affiliation.with_status(status).ids.sort).to eq(expected), "mismatch for #{status}" - end - end - - it 'offers the combined filter option alongside the chip taxonomy' do - expect(Affiliation::FILTER_STATUSES) - .to eq([ "Active", "Upcoming", "Active & Upcoming", "Inactive" ]) - end - - it '.with_status("Active & Upcoming") returns exactly the Active and Upcoming rows' do - expected = Affiliation.all.select { |a| a.status_on.in?(%w[ Active Upcoming ]) }.map(&:id).sort - - expect(Affiliation.with_status(Affiliation::ACTIVE_OR_UPCOMING).ids.sort).to eq(expected) - expect(Affiliation.with_status(Affiliation::ACTIVE_OR_UPCOMING)).to include(active_open, active_span, upcoming, no_dates) - expect(Affiliation.with_status(Affiliation::ACTIVE_OR_UPCOMING)).not_to include(ended) - end - - it '.with_status is empty for an unknown status' do - expect(Affiliation.with_status("bogus")).to be_empty - end - end - - describe '#set_inactive_from_dates' do - let(:op) { create(:affiliation, inactive: false, end_date: nil) } - - it 'sets inactive to true when end_date is set to a past date' do - op.update!(end_date: 1.day.ago) - expect(op.reload.inactive).to be true - end - - it 'sets inactive to false when end_date is set to a future date' do - op.update!(inactive: true, end_date: 1.day.ago) - op.update!(end_date: 1.month.from_now) - expect(op.reload.inactive).to be false - end - - it 'sets inactive to false when end_date is cleared' do - op.update!(end_date: 1.day.ago) - op.update!(end_date: nil) - expect(op.reload.inactive).to be false - end - - it 'does not change inactive when unrelated fields change' do - op.update!(inactive: true) - op.update!(title: "New Title") - expect(op.reload.inactive).to be true - end - end - - describe "the registration that created the affiliation" do - it "drops the link when the affiliation is moved to a different organization" do - registration = create(:event_registration) - affiliation = create(:affiliation, event_registration: registration) - other_org = create(:organization) - - affiliation.update!(organization: other_org) - - expect(affiliation.reload.event_registration_id).to be_nil - end + describe "reassigning the organization" do + let(:old_org) { create(:organization) } + let(:new_org) { create(:organization) } + let(:address) { create(:address, addressable: old_org) } - it "keeps the link when other attributes change" do - registration = create(:event_registration) - affiliation = create(:affiliation, event_registration: registration) + it "clears the org-scoped address so the row survives validation" do + affiliation = create(:affiliation, organization: old_org, organization_address: address) - affiliation.update!(title: "Lead Facilitator") + affiliation.update!(organization: new_org) - expect(affiliation.reload.event_registration).to eq(registration) + expect(affiliation.reload.organization_id).to eq(new_org.id) + expect(affiliation.organization_address_id).to be_nil end end end diff --git a/spec/policies/affiliation_policy_spec.rb b/spec/policies/affiliation_policy_spec.rb new file mode 100644 index 0000000000..f90eaa7b43 --- /dev/null +++ b/spec/policies/affiliation_policy_spec.rb @@ -0,0 +1,34 @@ +require "rails_helper" + +RSpec.describe AffiliationPolicy, type: :policy do + let(:admin_user) { build_stubbed(:user, :admin) } + let(:regular_user) { build_stubbed(:user) } + + let(:affiliation) { build_stubbed(:affiliation) } + + def policy_for(record: nil, user:) + described_class.new(record, user: user) + end + + %i[ edit? update? destroy? ].each do |rule| + describe "##{rule}" do + context "with admin user" do + subject { policy_for(record: affiliation, user: admin_user) } + + it { is_expected.to be_allowed_to(rule) } + end + + context "with regular user" do + subject { policy_for(record: affiliation, user: regular_user) } + + it { is_expected.not_to be_allowed_to(rule) } + end + + context "with no user" do + subject { policy_for(record: affiliation, user: nil) } + + it { is_expected.not_to be_allowed_to(rule) } + end + end + end +end diff --git a/spec/requests/affiliations_spec.rb b/spec/requests/affiliations_spec.rb new file mode 100644 index 0000000000..6f70bd7703 --- /dev/null +++ b/spec/requests/affiliations_spec.rb @@ -0,0 +1,77 @@ +require "rails_helper" + +RSpec.describe "/affiliations", type: :request do + let(:admin) { create(:user, :admin) } + let(:regular_user) { create(:user) } + let(:organization) { create(:organization) } + let(:person) { create(:person) } + let!(:affiliation) do + create(:affiliation, organization: organization, person: person, title: "Facilitator") + end + + describe "GET /affiliations/:id/edit" do + context "as an admin" do + before { sign_in admin } + + it "renders the edit form" do + get edit_affiliation_path(affiliation) + expect(response).to be_successful + end + end + + context "as a non-admin" do + before { sign_in regular_user } + + it "redirects to root" do + get edit_affiliation_path(affiliation) + expect(response).to redirect_to(root_path) + end + end + end + + describe "PATCH /affiliations/:id" do + context "as an admin" do + before { sign_in admin } + + it "updates attributes and returns to the origin org edit page, scrolled to the row" do + patch affiliation_path(affiliation, return_to: "organization", origin_id: organization.id), + params: { affiliation: { title: "Lead facilitator" } } + + expect(affiliation.reload.title).to eq("Lead facilitator") + expect(response).to redirect_to(edit_organization_path(organization, anchor: "affiliation_#{affiliation.id}")) + end + + it "reassigns the person and returns to the origin person edit page" do + other_person = create(:person) + + patch affiliation_path(affiliation, return_to: "person", origin_id: person.id), + params: { affiliation: { person_id: other_person.id } } + + expect(affiliation.reload.person_id).to eq(other_person.id) + expect(response).to redirect_to(edit_person_path(person, anchor: "affiliation_#{affiliation.id}")) + end + + it "adds a comment authored by the current user" do + expect { + patch affiliation_path(affiliation, return_to: "organization", origin_id: organization.id), + params: { affiliation: { comments_attributes: [ { body: "Left a note" } ] } } + }.to change { affiliation.comments.count }.by(1) + + comment = affiliation.comments.first + expect(comment.body).to eq("Left a note") + expect(comment.created_by).to eq(admin) + end + end + + context "as a non-admin" do + before { sign_in regular_user } + + it "does not update and redirects to root" do + patch affiliation_path(affiliation), params: { affiliation: { title: "Changed" } } + + expect(affiliation.reload.title).to eq("Facilitator") + expect(response).to redirect_to(root_path) + end + end + end +end diff --git a/spec/views/page_bg_class_alignment_spec.rb b/spec/views/page_bg_class_alignment_spec.rb index a9b05ba0c7..40d35d9011 100644 --- a/spec/views/page_bg_class_alignment_spec.rb +++ b/spec/views/page_bg_class_alignment_spec.rb @@ -199,6 +199,7 @@ "app/views/workshop_variations/new.html.erb" => "admin-only bg-blue-100", "app/views/workshops/new.html.erb" => "admin-only bg-blue-100", # edit + "app/views/affiliations/edit.html.erb" => "admin-only bg-blue-100", "app/views/banners/edit.html.erb" => "admin-only bg-blue-100", "app/views/categories/edit.html.erb" => "admin-only bg-blue-100", "app/views/category_types/edit.html.erb" => "admin-only bg-blue-100", From 6bc9c52da3f1c7751ac669fd459f8cd3145f4a94 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 16 Aug 2026 19:57:41 -0400 Subject: [PATCH 02/37] Add Features & tips entry for the affiliation editor Keeps the shipped-feature seed current per the Features & tips workflow. Co-Authored-By: Claude Opus 4.8 (1M context) --- config/features.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/config/features.yml b/config/features.yml index 0f7ef7dc7c..91ad92c865 100644 --- a/config/features.yml +++ b/config/features.yml @@ -27,6 +27,16 @@ # ── 2026-08 ───────────────────────────────────────────────────────────────── +- name: "Edit an affiliation's details and comments" + area: people + display_status: admin_facing + released_on: 2026-08-16 + pr_number: 2235 + summary: >- + Each affiliation row on an organization or person edit page has a gear that opens + a full editor — reassign the person or organization, adjust the title and dates, + and leave comments on the affiliation. + - name: "File-upload questions on forms" area: content display_status: admin_facing From aad13ee6c6826ac2418de079bc07693bad7e81b9 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 16 Aug 2026 20:01:49 -0400 Subject: [PATCH 03/37] Open the affiliation-edit gear in a new tab Preserves the org/person edit form's unsaved state; the editor's eyebrow and Cancel handle the return since the browser back button is useless across a new tab. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/_fields.html.erb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/affiliations/_fields.html.erb b/app/views/affiliations/_fields.html.erb index 293a5b830f..00e80cf936 100644 --- a/app/views/affiliations/_fields.html.erb +++ b/app/views/affiliations/_fields.html.erb @@ -20,8 +20,9 @@ <%= link_to edit_affiliation_path(f.object, return_to: person_side ? "organization" : "person", origin_id: person_side ? f.object.organization_id : f.object.person_id), + target: "_blank", rel: "noopener", class: "absolute top-2 right-2 z-10 text-gray-300 hover:text-gray-500", - title: "Edit affiliation, add comments, or reassign" do %> + title: "Edit affiliation, add comments, or reassign (opens in a new tab)" do %> <% end %> <% end %> From d8df5af61d09e08f7450b31f3d29a92ed963bc08 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 16 Aug 2026 21:45:28 -0400 Subject: [PATCH 04/37] Tighten affiliation-edit lookup layout and add hints Put the person/organization lookups in one row, move their hints into simple_form's hint slot so they sit tight under each field (the manual paragraph sat below the wrapper's mb-4, leaving a large gap), and add a hint clarifying what the "Facilitator" title means. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/edit.html.erb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/views/affiliations/edit.html.erb b/app/views/affiliations/edit.html.erb index 9c7aa3be60..c38d70edee 100644 --- a/app/views/affiliations/edit.html.erb +++ b/app/views/affiliations/edit.html.erb @@ -27,36 +27,35 @@ method: :patch, html: { id: "affiliation_form", data: { turbo: false } } do |f| %>
-
+
<%= f.input :person_id, collection: @affiliation.person ? [ [ @affiliation.person.full_name, @affiliation.person.id ] ] : [], selected: @affiliation.person_id, include_blank: false, label: "Person", label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, + hint: "Type to search to reassign to a different person.", input_html: { data: { controller: "remote-select", remote_select_model_value: "person" } } %> -

Type to search to reassign this affiliation to a different person.

-
-
<%= f.input :organization_id, collection: @affiliation.organization ? [ [ @affiliation.organization.name, @affiliation.organization.id ] ] : [], selected: @affiliation.organization_id, include_blank: false, label: "Organization", label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, + hint: "Reassigning clears the linked address.", input_html: { data: { controller: "remote-select", remote_select_model_value: "organization" } } %> -

Reassigning to a different organization clears the linked address.

<%= f.input :title, - label_html: { class: "block text-sm font-medium text-gray-700 mb-1" } %> + label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, + hint: "\"Facilitator\" means they're running an Art Program." %>
<%= f.input :start_date, From 11e6236f88bdbcce73ae167e9aa449c27d8df922 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 16 Aug 2026 21:48:13 -0400 Subject: [PATCH 05/37] Fit title/dates on one row and scroll to the row on return Give the title flex-1 and the date fields a fixed width so all three sit on one row on desktop and wrap only when space runs out (before, the date wrappers' full-width inputs forced End to wrap). Make paginated-fields honor a #row fragment on connect: the affiliation rows paginate at 10/page, so a returned-to row was hidden on a later page and the browser couldn't scroll to it. Now the controller jumps to that row's page and scrolls it into view. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../paginated_fields_controller.js | 22 +++++++++++++++++++ app/views/affiliations/edit.html.erb | 8 +++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/app/frontend/javascript/controllers/paginated_fields_controller.js b/app/frontend/javascript/controllers/paginated_fields_controller.js index abbb1667ab..0a9606f1b5 100644 --- a/app/frontend/javascript/controllers/paginated_fields_controller.js +++ b/app/frontend/javascript/controllers/paginated_fields_controller.js @@ -8,6 +8,28 @@ export default class extends Controller { this.currentPage = 1; this.render(); this.ready = true; + this.revealHashTarget(); + } + + // When the page loads with a #fragment matching a row inside this controller + // (e.g. returning from the affiliation editor to its row), jump to the page + // holding that row — otherwise it's hidden on a later page — and scroll to it. + revealHashTarget() { + const hash = window.location.hash; + if (hash.length < 2) return; + + const id = hash.slice(1); + const items = this.visibleItems; + const index = items.findIndex( + (el) => el.id === id || el.querySelector(`#${CSS.escape(id)}`) + ); + if (index === -1) return; + + this.currentPage = Math.floor(index / this.perPageValue) + 1; + this.render(); + + const target = document.getElementById(id) || items[index]; + requestAnimationFrame(() => target.scrollIntoView({ block: "center" })); } get visibleItems() { diff --git a/app/views/affiliations/edit.html.erb b/app/views/affiliations/edit.html.erb index c38d70edee..7ed6745a64 100644 --- a/app/views/affiliations/edit.html.erb +++ b/app/views/affiliations/edit.html.erb @@ -51,20 +51,20 @@ } %>
-
-
+
+
<%= f.input :title, label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, hint: "\"Facilitator\" means they're running an Art Program." %>
-
+
<%= f.input :start_date, as: :string, label: "Start", label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, input_html: { type: "date", value: @affiliation.start_date&.strftime("%Y-%m-%d") } %>
-
+
<%= f.input :end_date, as: :string, label: "End", From 6ffaa37a4a0e72a550304a3c8cc2d8b90394f7e4 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 16 Aug 2026 21:58:55 -0400 Subject: [PATCH 06/37] Affiliation editor: delete button, remove-on-persisted, layout polish - Hide the nested-row "Remove" link on persisted affiliations; deletion now happens via the gear editor's Delete button (destroy honors return_to and lands back on the origin's affiliations section). - Compact the person/organization profile buttons in the nested rows (add a compact option to organization_profile_button) and raise the date inputs to the other fields' height. - Tighten vertical padding around the affiliation cards and the Add Affiliation button on both edit forms. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/controllers/affiliations_controller.rb | 17 +++++++++++--- app/helpers/organization_helper.rb | 16 ++++++++----- app/views/affiliations/_fields.html.erb | 25 ++++++++++++--------- app/views/affiliations/edit.html.erb | 22 +++++++++++------- app/views/organizations/_form.html.erb | 2 +- app/views/people/_form.html.erb | 2 +- spec/requests/affiliations_spec.rb | 26 ++++++++++++++++++++++ 7 files changed, 82 insertions(+), 28 deletions(-) diff --git a/app/controllers/affiliations_controller.rb b/app/controllers/affiliations_controller.rb index 69cb626c21..6b1dcb4b27 100644 --- a/app/controllers/affiliations_controller.rb +++ b/app/controllers/affiliations_controller.rb @@ -20,6 +20,17 @@ def update def destroy authorize! @affiliation, to: :destroy? + + if params[:return_to].present? + if @affiliation.destroy + redirect_to affiliation_return_path(anchor: "affiliations"), + notice: "Affiliation was removed.", status: :see_other + else + redirect_to edit_affiliation_path(@affiliation), alert: "Unable to remove affiliation." + end + return + end + affiliation = Affiliation.find(params[:id]) person = affiliation.person destroyed = affiliation.destroy @@ -58,9 +69,9 @@ def affiliation_params ) end - # Return to whichever edit page the gear was clicked from, scrolled to the row. - def affiliation_return_path - anchor = helpers.dom_id(@affiliation) + # Return to whichever edit page the gear was clicked from, scrolled to the row + # (or the affiliations section after a delete removes the row). + def affiliation_return_path(anchor: helpers.dom_id(@affiliation)) case params[:return_to] when "person" edit_person_path(params[:origin_id], anchor: anchor) diff --git a/app/helpers/organization_helper.rb b/app/helpers/organization_helper.rb index 20769ee632..88ab3a7030 100644 --- a/app/helpers/organization_helper.rb +++ b/app/helpers/organization_helper.rb @@ -1,5 +1,11 @@ module OrganizationHelper - def organization_profile_button(organization, truncate_at: nil, subtitle: nil, label: nil, data: {}, inactive: false) + def organization_profile_button(organization, truncate_at: nil, subtitle: nil, label: nil, data: {}, inactive: false, compact: false) + # Compact mode shrinks the control to roughly a text input's height, for use + # inline beside form fields (e.g. the affiliation editor rows). + padding = compact ? "px-3 py-1" : "px-4 py-2" + avatar_size = compact ? "w-8 h-8" : "w-10 h-10" + initial_text_size = compact ? "text-sm" : "text-lg" + if inactive bg = "bg-gray-100" hover_bg = "hover:bg-gray-200" @@ -18,7 +24,7 @@ def organization_profile_button(organization, truncate_at: nil, subtitle: nil, l data: { turbo_prefetch: false }.merge(data), title: hover_title, class: "group relative flex items-center gap-2 - w-full px-4 py-2 + w-full #{padding} border #{border} #{bg} #{hover_bg} rounded-lg transition-colors duration-200 font-medium shadow-sm leading-none @@ -26,11 +32,11 @@ def organization_profile_button(organization, truncate_at: nil, subtitle: nil, l # --- Logo --- logo = if organization.respond_to?(:logo) && organization.logo.attached? image_tag organization.logo, - class: "w-10 h-10 rounded-full object-cover border border-gray-300 shadow-sm flex-shrink-0" + class: "#{avatar_size} rounded-full object-cover border border-gray-300 shadow-sm flex-shrink-0" else content_tag(:span, organization.name.first.upcase, - class: "w-10 h-10 rounded-full flex items-center justify-center - bg-emerald-200 text-emerald-700 font-bold text-lg + class: "#{avatar_size} rounded-full flex items-center justify-center + bg-emerald-200 text-emerald-700 font-bold #{initial_text_size} border border-emerald-300 shadow-sm flex-shrink-0") end diff --git a/app/views/affiliations/_fields.html.erb b/app/views/affiliations/_fields.html.erb index 00e80cf936..89c3eaa4bc 100644 --- a/app/views/affiliations/_fields.html.erb +++ b/app/views/affiliations/_fields.html.erb @@ -10,7 +10,7 @@ <% active_bg = facilitator ? "bg-purple-100 border-purple-300" : "bg-white border-gray-200" %> <% record = person_side ? f.object.person : f.object.organization %> <% label = person_side ? "Person" : "Organization" %> -
+
<%# Left accent rendered outside the row so it keeps full color even when an expired row is dimmed by opacity-60. %> <% end %> <% end %> -
id="<%= dom_id(f.object) %>"<% end %> data-inactive-toggle-target="row">
@@ -34,9 +34,9 @@ <% if person_side %> <% show_email = record.profile_show_email? || allowed_to?(:manage?, Person) %> - <%= person_profile_button(record, truncate_at: 25, subtitle: (record.preferred_email if show_email)) %> + <%= person_profile_button(record, truncate_at: 25, subtitle: (record.preferred_email if show_email), compact: true) %> <% else %> - <%= organization_profile_button(record, truncate_at: 25) %> + <%= organization_profile_button(record, truncate_at: 25, compact: true) %> <% end %> <%= f.hidden_field(person_side ? :person_id : :organization_id) %> <% else %> @@ -82,6 +82,7 @@ type: "date", value: (f.object.start_date || (Date.current unless f.object.persisted?))&.strftime("%Y-%m-%d"), class: "rounded-md border-gray-300 focus:ring-blue-500 focus:border-blue-500 text-sm", + style: "height: 42px;", data: { action: "change->affiliation-dates#recalculate" } } %>
@@ -95,6 +96,7 @@ type: "date", value: f.object.end_date&.strftime("%Y-%m-%d"), class: "rounded-md border-gray-300 focus:ring-blue-500 focus:border-blue-500 text-sm", + style: "height: 42px;", data: { inactive_toggle_target: "endDate", action: "change->inactive-toggle#toggle change->affiliation-dates#recalculate" @@ -102,18 +104,21 @@ } %>
-
+
<%= f.check_box :primary_contact, checked: f.object.primary_contact?, class: "h-4 w-4 text-blue-600 border-gray-300 rounded focus:ring-blue-500" %>
-
- <%= link_to_remove_association "Remove", - f, - class: "text-sm text-gray-400 hover:text-red-600 underline whitespace-nowrap admin-only bg-blue-100 rounded px-2 py-1" %> -
+ <% unless f.object.persisted? %> + <%# Persisted rows are removed via the gear's affiliation editor (Delete). %> +
+ <%= link_to_remove_association "Remove", + f, + class: "text-sm text-gray-400 hover:text-red-600 underline whitespace-nowrap admin-only bg-blue-100 rounded px-2 py-1" %> +
+ <% end %>
<% else %> diff --git a/app/views/affiliations/edit.html.erb b/app/views/affiliations/edit.html.erb index 7ed6745a64..65535817e9 100644 --- a/app/views/affiliations/edit.html.erb +++ b/app/views/affiliations/edit.html.erb @@ -119,15 +119,21 @@
-
-
- <% if back_path %> - <%= link_to "Cancel", back_path, class: "btn btn-secondary-outline" %> - <% end %> - -
-
<% end %> +
+ <%= button_to "Delete", + affiliation_path(@affiliation, return_to: params[:return_to].presence, origin_id: params[:origin_id].presence), + method: :delete, + form: { data: { turbo_confirm: "Remove this affiliation?" } }, + class: "btn btn-danger-outline" %> +
+ <% if back_path %> + <%= link_to "Cancel", back_path, class: "btn btn-secondary-outline" %> + <% end %> + +
+
+ <%= render "shared/audit_info", resource: @affiliation %>
diff --git a/app/views/organizations/_form.html.erb b/app/views/organizations/_form.html.erb index df04cbfa11..258a3393b1 100644 --- a/app/views/organizations/_form.html.erb +++ b/app/views/organizations/_form.html.erb @@ -346,7 +346,7 @@ <% end %>
-
<%= link_to_add_association "➕ Add Affiliation", +
<%= link_to_add_association "➕ Add Affiliation", f, :affiliations, class: "btn btn-secondary-outline" %>
diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index bab6865945..733dcc0973 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -321,7 +321,7 @@ <% end %>
-
<%= link_to_add_association "➕ Add Affiliation", +
<%= link_to_add_association "➕ Add Affiliation", f, :affiliations, class: "admin-only bg-blue-100 btn btn-secondary-outline" %>
diff --git a/spec/requests/affiliations_spec.rb b/spec/requests/affiliations_spec.rb index 6f70bd7703..b813f97ee5 100644 --- a/spec/requests/affiliations_spec.rb +++ b/spec/requests/affiliations_spec.rb @@ -74,4 +74,30 @@ end end end + + describe "DELETE /affiliations/:id" do + context "as an admin returning from the editor" do + before { sign_in admin } + + it "destroys the affiliation and returns to the origin edit page" do + expect { + delete affiliation_path(affiliation, return_to: "organization", origin_id: organization.id) + }.to change(Affiliation, :count).by(-1) + + expect(response).to redirect_to(edit_organization_path(organization, anchor: "affiliations")) + end + end + + context "as a non-admin" do + before { sign_in regular_user } + + it "does not destroy and redirects to root" do + expect { + delete affiliation_path(affiliation, return_to: "organization", origin_id: organization.id) + }.not_to change(Affiliation, :count) + + expect(response).to redirect_to(root_path) + end + end + end end From 32579e03c6ff44fa81ac23b70087a6d90f4eebfd Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 16 Aug 2026 22:00:58 -0400 Subject: [PATCH 07/37] Make affiliation card padding symmetric top and bottom The person/organization column is the tallest, so pt-3/pb-2 left more space above the label than below the profile button; use py-2 so the gap above the label matches the gap below the button. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/_fields.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/affiliations/_fields.html.erb b/app/views/affiliations/_fields.html.erb index 89c3eaa4bc..93b4a698c0 100644 --- a/app/views/affiliations/_fields.html.erb +++ b/app/views/affiliations/_fields.html.erb @@ -26,7 +26,7 @@ <% end %> <% end %> -
id="<%= dom_id(f.object) %>"<% end %> data-inactive-toggle-target="row">
From 903f2c7bd83e18d054c955c005f7986428f715f6 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 16 Aug 2026 22:01:33 -0400 Subject: [PATCH 08/37] Match affiliation title font size to the date fields Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/_fields.html.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/affiliations/_fields.html.erb b/app/views/affiliations/_fields.html.erb index 93b4a698c0..c8c637ac46 100644 --- a/app/views/affiliations/_fields.html.erb +++ b/app/views/affiliations/_fields.html.erb @@ -64,6 +64,7 @@ rows: 1, value: f.object&.title || "Facilitator", style: "height: 42px; min-height: 42px;", + class: "text-sm", data: { inactive_toggle_target: "title", action: "affiliation-dates#recalculate inactive-toggle#updateBorder" From 572603407dcec2aa89523e0d1d90320fe9790a38 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Sun, 16 Aug 2026 22:06:58 -0400 Subject: [PATCH 09/37] Align primary-contact checkbox to the input row height The two-line "Primary org
contact" label with the checkbox floating below it made that column stack taller than the input columns, leaving extra space in the card. Use a single-line label and center the checkbox in a 42px box so the column matches the inputs. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/_fields.html.erb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/views/affiliations/_fields.html.erb b/app/views/affiliations/_fields.html.erb index c8c637ac46..3ed1b31e05 100644 --- a/app/views/affiliations/_fields.html.erb +++ b/app/views/affiliations/_fields.html.erb @@ -106,10 +106,12 @@
- - <%= f.check_box :primary_contact, - checked: f.object.primary_contact?, - class: "h-4 w-4 text-blue-600 border-gray-300 rounded focus:ring-blue-500" %> + +
+ <%= f.check_box :primary_contact, + checked: f.object.primary_contact?, + class: "h-4 w-4 text-blue-600 border-gray-300 rounded focus:ring-blue-500" %> +
<% unless f.object.persisted? %> From 0d7fdc689dd473c29cafbdf0b954306318b39f86 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 17 Aug 2026 01:51:33 -0400 Subject: [PATCH 10/37] Use the text-2xs token in the affiliation editor Aligns with the sub-xs text scale adopted on main. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/edit.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/affiliations/edit.html.erb b/app/views/affiliations/edit.html.erb index 65535817e9..4f396dfeef 100644 --- a/app/views/affiliations/edit.html.erb +++ b/app/views/affiliations/edit.html.erb @@ -104,14 +104,14 @@ partial: "comments/comment_fields", data: { association_insertion_node: "#comment-list", association_insertion_method: "append" }, class: "inline-flex items-center gap-1.5 rounded-md px-2 py-1 text-xs font-medium text-gray-500 transition-colors hover:bg-gray-100 hover:text-gray-700 cursor-pointer" do %> - + Add comment <% end %> From 2af6c187077bffa9704e2aca8edd19102664854b Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 17 Aug 2026 02:26:02 -0400 Subject: [PATCH 11/37] Route persisted-affiliation removal through the editor in system specs Hiding the nested-row Remove link on persisted affiliations broke the three system specs that removed a persisted facilitator inline. Point them at the editor's Delete button instead, and carry the facilitator "status with AWBW" warning onto that Delete so removing a facilitator still prompts before it changes the org's status. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/edit.html.erb | 5 ++++- spec/system/affiliation_dates_spec.rb | 17 +++++++---------- .../organization_affiliation_dates_spec.rb | 17 +++++++---------- .../organization_facilitator_warning_spec.rb | 11 +++++------ 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/app/views/affiliations/edit.html.erb b/app/views/affiliations/edit.html.erb index 4f396dfeef..96f90cbdde 100644 --- a/app/views/affiliations/edit.html.erb +++ b/app/views/affiliations/edit.html.erb @@ -121,11 +121,14 @@ <% end %> + <% delete_confirm = @affiliation.facilitator? ? + "Removing this facilitator affiliation affects the organization's status with AWBW, which is calculated from facilitator affiliations and their start and end dates. Remove it?" : + "Remove this affiliation?" %>
<%= button_to "Delete", affiliation_path(@affiliation, return_to: params[:return_to].presence, origin_id: params[:origin_id].presence), method: :delete, - form: { data: { turbo_confirm: "Remove this affiliation?" } }, + form: { data: { turbo_confirm: delete_confirm } }, class: "btn btn-danger-outline" %>
<% if back_path %> diff --git a/spec/system/affiliation_dates_spec.rb b/spec/system/affiliation_dates_spec.rb index 7768afd061..5bf4afee27 100644 --- a/spec/system/affiliation_dates_spec.rb +++ b/spec/system/affiliation_dates_spec.rb @@ -109,18 +109,15 @@ def set_textarea_input(textarea, value) expect(affiliated).not_to have_text("Dec 2024") end - it "removes an affiliation and recalculates" do - visit_and_wait edit_person_path(person, admin: true) - - affiliated = find("[data-affiliation-dates-target='affiliatedSince']") - expect(affiliated).to have_text("Mar 2020") + it "removes an affiliation via the editor and recalculates" do + # Persisted affiliations are now deleted from the affiliation editor (reached + # via the row's gear); removing the Facilitator (Mar 2020) leaves Volunteer (Jun 2022). + facilitator = person.affiliations.find_by!(title: "Facilitator") + visit edit_affiliation_path(facilitator, return_to: "person", origin_id: person.id) - # Remove the Facilitator affiliation (start Mar 2020), leaving Volunteer (start Jun 2022) - facilitator_row = all("[data-affiliation-dates-target='affiliationsContainer'] .nested-fields").find { |f| - f.find("textarea[name*='title']").value.include?("Facilitator") - } - facilitator_row.find("a", text: "Remove").click + accept_confirm { click_button "Delete" } + affiliated = find("[data-affiliation-dates-target='affiliatedSince']", wait: 10) expect(affiliated).to have_text("Jun 2022", wait: 5) end end diff --git a/spec/system/organization_affiliation_dates_spec.rb b/spec/system/organization_affiliation_dates_spec.rb index 4139c27f85..7077f7caa8 100644 --- a/spec/system/organization_affiliation_dates_spec.rb +++ b/spec/system/organization_affiliation_dates_spec.rb @@ -53,18 +53,15 @@ def set_date_input(input, value) end end - it "removes an affiliation and recalculates" do - visit_and_wait edit_organization_path(organization, admin: true) - - affiliated = find("[data-affiliation-dates-target='affiliatedSince']") - expect(affiliated).to have_text("May 2019") + it "removes an affiliation via the editor and recalculates" do + # Persisted affiliations are now deleted from the affiliation editor (reached + # via the row's gear); removing the Facilitator (May 2019) leaves Volunteer (Sep 2021). + facilitator = organization.affiliations.find_by!(title: "Facilitator") + visit edit_affiliation_path(facilitator, return_to: "organization", origin_id: organization.id) - # Remove the Facilitator affiliation (start May 2019), leaving Volunteer (start Sep 2021) - facilitator_row = all("[data-affiliation-dates-target='affiliationsContainer'] .nested-fields").find { |f| - f.find("textarea[name*='title']").value.include?("Facilitator") - } - facilitator_row.find("a", text: "Remove").click + accept_confirm { click_button "Delete" } + affiliated = find("[data-affiliation-dates-target='affiliatedSince']", wait: 10) expect(affiliated).to have_text("Sep 2021", wait: 5) end end diff --git a/spec/system/organization_facilitator_warning_spec.rb b/spec/system/organization_facilitator_warning_spec.rb index dbd8916656..75e63d06dc 100644 --- a/spec/system/organization_facilitator_warning_spec.rb +++ b/spec/system/organization_facilitator_warning_spec.rb @@ -60,16 +60,15 @@ def row_for(title) expect(organization.affiliations.facilitators.first.reload.start_date).to eq(Date.new(2019, 5, 1)) end - it "warns when a facilitator affiliation is removed" do - visit_and_wait edit_organization_path(organization, admin: true) - - row_for("Facilitator").find("a", text: "Remove").click + it "warns when a facilitator affiliation is removed from the editor" do + facilitator = organization.affiliations.facilitators.first + visit edit_affiliation_path(facilitator, return_to: "organization", origin_id: organization.id) accept_confirm(/status with AWBW/) do - find("[type='submit']").click + click_button "Delete" end - expect(page).to have_current_path(organization_path(organization), wait: 10) + expect(page).to have_css("[data-affiliation-dates-ready]", wait: 10) expect(organization.affiliations.facilitators).to be_empty end From b24e3016fb0f83072d42685076d69f9a1404fb32 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 17 Aug 2026 02:29:45 -0400 Subject: [PATCH 12/37] Surface a linked registration and address impact on the affiliation editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Show any event registration the affiliation came from, warning that reassigning the org here doesn't update the registration's own org link (do that in its org-linking step). - Warn that reassigning disconnects the current address, and that the new org's address is adopted only when it has exactly one — otherwise set it after saving. The model now adopts the sole address on org change instead of always clearing it. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/models/affiliation.rb | 21 +++++++++++++++------ app/views/affiliations/edit.html.erb | 21 ++++++++++++++++++++- spec/models/affiliation_spec.rb | 25 ++++++++++++++++++++++--- spec/requests/affiliations_spec.rb | 10 ++++++++++ 4 files changed, 67 insertions(+), 10 deletions(-) diff --git a/app/models/affiliation.rb b/app/models/affiliation.rb index 46cd256519..6f780a6218 100644 --- a/app/models/affiliation.rb +++ b/app/models/affiliation.rb @@ -80,7 +80,7 @@ class Affiliation < ApplicationRecord before_validation :skip_if_duplicate # Runs before validation so a reassigned org drops its stale organization_address_id # before organization_address_belongs_to_organization would reject it. - before_validation :clear_org_scoped_links_on_org_change, on: :update + before_validation :reset_org_scoped_links_on_org_change, on: :update before_save :set_inactive_from_dates after_save :sync_organization_status_with_affiliations after_save :sync_organization_affiliation_dates @@ -143,14 +143,23 @@ def skip_if_duplicate # When an admin moves the affiliation to a different org (only possible from the # standalone edit form), the links scoped to the old org no longer apply: - # event_registration_id (a row with no link counts as manually created, which - # reconciliation leaves alone) and organization_address_id (an address of the old - # org would fail organization_address_belongs_to_organization). - def clear_org_scoped_links_on_org_change + # - event_registration_id is cleared (a row with no link counts as manually + # created, which reconciliation leaves alone). The registration's own org + # link is separate and is updated in its org-linking step. + # - organization_address_id is re-pointed at the new org: an old-org address + # would fail organization_address_belongs_to_organization. If the new org has + # exactly one address we adopt it; otherwise it's left blank for an admin to + # set after saving. + def reset_org_scoped_links_on_org_change return unless organization_id_changed? self.event_registration_id = nil - self.organization_address_id = nil + self.organization_address_id = sole_address_id_for_new_organization + end + + def sole_address_id_for_new_organization + addresses = Organization.find_by(id: organization_id)&.addresses + addresses.first.id if addresses&.one? end def set_inactive_from_dates diff --git a/app/views/affiliations/edit.html.erb b/app/views/affiliations/edit.html.erb index 96f90cbdde..5bc983c2f5 100644 --- a/app/views/affiliations/edit.html.erb +++ b/app/views/affiliations/edit.html.erb @@ -45,12 +45,31 @@ include_blank: false, label: "Organization", label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, - hint: "Reassigning clears the linked address.", input_html: { data: { controller: "remote-select", remote_select_model_value: "organization" } } %>
+ <% if @affiliation.event_registration.present? %> + <% er = @affiliation.event_registration %> +
+

Linked to a registration

+

+ This affiliation came from + <%= link_to (er.registrant&.full_name.presence || "a registration"), + edit_event_registration_path(er), class: "underline font-medium", target: "_blank", rel: "noopener" %><%= " — #{er.event.title}" if er.event %>. + Reassigning the organization here unlinks that registration from this affiliation; update the organization in the + <%= link_to "registration's organization-linking step", + link_organization_event_registration_path(er), class: "underline font-medium", target: "_blank", rel: "noopener" %> as well. +

+
+ <% end %> + +

+ + Reassigning the organization <%= "disconnects the current address (#{@affiliation.organization_address.name}) and " if @affiliation.organization_address.present? %>links the new organization's address automatically only if it has exactly one — otherwise set the address after saving. +

+
<%= f.input :title, diff --git a/spec/models/affiliation_spec.rb b/spec/models/affiliation_spec.rb index 2c9111f98d..08c5531b95 100644 --- a/spec/models/affiliation_spec.rb +++ b/spec/models/affiliation_spec.rb @@ -26,15 +26,34 @@ describe "reassigning the organization" do let(:old_org) { create(:organization) } let(:new_org) { create(:organization) } - let(:address) { create(:address, addressable: old_org) } + let(:old_address) { create(:address, addressable: old_org) } - it "clears the org-scoped address so the row survives validation" do - affiliation = create(:affiliation, organization: old_org, organization_address: address) + it "drops the stale address when the new org has several addresses" do + create_list(:address, 2, addressable: new_org) + affiliation = create(:affiliation, organization: old_org, organization_address: old_address) affiliation.update!(organization: new_org) expect(affiliation.reload.organization_id).to eq(new_org.id) expect(affiliation.organization_address_id).to be_nil end + + it "adopts the sole address of the new org" do + new_address = create(:address, addressable: new_org) + affiliation = create(:affiliation, organization: old_org, organization_address: old_address) + + affiliation.update!(organization: new_org) + + expect(affiliation.reload.organization_address_id).to eq(new_address.id) + end + + it "clears the event registration link" do + registration = create(:event_registration) + affiliation = create(:affiliation, organization: old_org, event_registration: registration) + + affiliation.update!(organization: new_org) + + expect(affiliation.reload.event_registration_id).to be_nil + end end end diff --git a/spec/requests/affiliations_spec.rb b/spec/requests/affiliations_spec.rb index b813f97ee5..1385e934ff 100644 --- a/spec/requests/affiliations_spec.rb +++ b/spec/requests/affiliations_spec.rb @@ -17,6 +17,16 @@ get edit_affiliation_path(affiliation) expect(response).to be_successful end + + it "surfaces a linked registration with the org-linking warning" do + registration = create(:event_registration) + affiliation.update_column(:event_registration_id, registration.id) + + get edit_affiliation_path(affiliation) + + expect(response.body).to include("Linked to a registration") + expect(response.body).to include(link_organization_event_registration_path(registration)) + end end context "as a non-admin" do From 5fd1a1c6b75c19d2f79b360b4dbd7a91e3b9a609 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 17 Aug 2026 02:55:04 -0400 Subject: [PATCH 13/37] Remove the simple_form wrapper margin below affiliation row fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each f.input rendered a mb-4 wrapper, adding 16px below the title/date inputs — the extra space beneath the fields (and the wrapped second row) that showed up as bottom padding on the card. Zero it with wrapper_html. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/_fields.html.erb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/views/affiliations/_fields.html.erb b/app/views/affiliations/_fields.html.erb index 3ed1b31e05..271bc43363 100644 --- a/app/views/affiliations/_fields.html.erb +++ b/app/views/affiliations/_fields.html.erb @@ -43,6 +43,7 @@ <%= f.input person_side ? :person_id : :organization_id, include_blank: true, required: true, + wrapper_html: { class: "mb-0" }, input_html: { data: { controller: "remote-select", @@ -59,6 +60,7 @@
<%= f.input :title, as: :text, + wrapper_html: { class: "mb-0" }, label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, input_html: { rows: 1, @@ -78,6 +80,7 @@ <%= f.input :start_date, as: :string, label: "Start", + wrapper_html: { class: "mb-0" }, label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, input_html: { type: "date", @@ -92,6 +95,7 @@ <%= f.input :end_date, as: :string, label: "End", + wrapper_html: { class: "mb-0" }, label_html: { class: "block text-sm font-medium text-gray-700 mb-1" }, input_html: { type: "date", From a762b1ef9d808fa3765ed1a395bf0a3fabdc5e30 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 17 Aug 2026 03:15:07 -0400 Subject: [PATCH 14/37] Drop the affiliation card's bottom padding (py-2 to pt-2) Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/_fields.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/affiliations/_fields.html.erb b/app/views/affiliations/_fields.html.erb index 271bc43363..95aa50539d 100644 --- a/app/views/affiliations/_fields.html.erb +++ b/app/views/affiliations/_fields.html.erb @@ -26,7 +26,7 @@ <% end %> <% end %> -
id="<%= dom_id(f.object) %>"<% end %> data-inactive-toggle-target="row">
From 529aabcfd50b6a7c1c32e0858f79e00deda55087 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 17 Aug 2026 03:35:51 -0400 Subject: [PATCH 15/37] Affiliation editor: shared column header, grid rows, address in full editor The inline affiliation rows sized their height from the tallest column (the resizable Title textarea), leaving uneven trailing space and a per-card label row on every card. Move the labels into one desktop-only header row and lay the fields out on a shared CSS grid so every row is uniform and the columns line up. Cards hide their own labels at sm+ and keep them stacked on mobile. Also surface the address association in the full-page editor and permit organization_address_id so the change actually saves, and reword the organization/title/address hints. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/controllers/affiliations_controller.rb | 2 +- .../affiliations/_address_picker.html.erb | 4 +- app/views/affiliations/_fields.html.erb | 37 ++++++++++--------- app/views/affiliations/_header.html.erb | 13 +++++++ app/views/affiliations/edit.html.erb | 14 ++++--- app/views/organizations/_form.html.erb | 3 ++ app/views/people/_form.html.erb | 3 ++ 7 files changed, 50 insertions(+), 26 deletions(-) create mode 100644 app/views/affiliations/_header.html.erb diff --git a/app/controllers/affiliations_controller.rb b/app/controllers/affiliations_controller.rb index 6b1dcb4b27..da1278bf2c 100644 --- a/app/controllers/affiliations_controller.rb +++ b/app/controllers/affiliations_controller.rb @@ -64,7 +64,7 @@ def set_affiliation def affiliation_params params.require(:affiliation).permit( - :person_id, :organization_id, :title, :start_date, :end_date, :primary_contact, + :person_id, :organization_id, :title, :start_date, :end_date, :primary_contact, :organization_address_id, comments_attributes: [ :id, :topic, :body, :flagged, :_destroy ] ) end diff --git a/app/views/affiliations/_address_picker.html.erb b/app/views/affiliations/_address_picker.html.erb index 662976ed8e..eb560883ed 100644 --- a/app/views/affiliations/_address_picker.html.erb +++ b/app/views/affiliations/_address_picker.html.erb @@ -10,8 +10,8 @@ <% selected_id = f.object.organization_address_id %> <% selected = options.find { |_number, address| address.id == selected_id } %> <% if options.any? %> -
- +
+
<%= f.hidden_field :organization_address_id, data: { address_select_target: "input" } %> - - <%= render "affiliations/address_picker", f: f, hide_label: true %> + <%# On mobile Address and Primary drop to a shared final row (order-1/order-2, + each flex-1 → 50%); at sm+ they return to their in-line column widths. %> +
+ <%= render "affiliations/address_picker", f: f, hide_label: true %> +
<%= f.input :start_date, @@ -110,8 +114,8 @@ } %>
-
- +
+
<%= render "affiliations/primary_contact_label" %>
<%= f.check_box :primary_contact, checked: f.object.primary_contact?, diff --git a/app/views/affiliations/_header.html.erb b/app/views/affiliations/_header.html.erb index ce4635fb06..52960981d9 100644 --- a/app/views/affiliations/_header.html.erb +++ b/app/views/affiliations/_header.html.erb @@ -10,5 +10,5 @@ Address Start End - Primary contact + <%= render "affiliations/primary_contact_label" %>
diff --git a/app/views/affiliations/_primary_contact_label.html.erb b/app/views/affiliations/_primary_contact_label.html.erb new file mode 100644 index 0000000000..ac0a359254 --- /dev/null +++ b/app/views/affiliations/_primary_contact_label.html.erb @@ -0,0 +1,10 @@ +<%# "Primary contact" column label shortened to "Primary" with a hover tooltip, + matching the Program status info-tooltip pattern on the organization form. %> + + Primary + + + From dd6486f559b9c87d29297b3301b01740dc2c1e4b Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 17 Aug 2026 03:57:13 -0400 Subject: [PATCH 19/37] Affiliation rows: Address left of Primary everywhere, fills its column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move Address to sit directly left of Primary at all widths (DOM reorder drops the mobile order hack), widen its desktop column, and make the picker button fill its container so it isn't a tiny control in a wide space — in both the inline grid and the full editor. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/affiliations/_address_picker.html.erb | 2 +- app/views/affiliations/_fields.html.erb | 16 ++++++++-------- app/views/affiliations/_header.html.erb | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/views/affiliations/_address_picker.html.erb b/app/views/affiliations/_address_picker.html.erb index 29c0d6b3cb..974a46f12a 100644 --- a/app/views/affiliations/_address_picker.html.erb +++ b/app/views/affiliations/_address_picker.html.erb @@ -14,7 +14,7 @@
<%= f.hidden_field :organization_address_id, data: { address_select_target: "input" } %>