Make the parent org access callout on the team page collapsible

app/controllers/events_controller.rb

@@ -299,17 +299,26 @@ def team
     @positions = Kaminari.paginate_array(@all_positions).page(params[:page]).per(params[:per] || (@view == "list" ? 20 : 10))
 
     if @event.parent
-      ops = @event.ancestor_organizer_positions.includes(:user)
-      users = ops.map(&:user).uniq
-
-      access_levels = users.filter_map do |user|
-        access_level = user.access_level_for(@event, ops)
-        next if access_level[:access_level] == :direct
-
-        [user, access_level[:role]]
-      end.sort_by { |_, role| role }.to_h
-
-      @indirect_access = access_levels
+      # `ancestor_organizer_positions` covers this organization as well as its
+      # ancestors, so both halves come from the one query. The avatars are
+      # preloaded because every user here is rendered as a `user_mention`.
+      ops = @event.ancestor_organizer_positions.includes(user: { profile_picture_attachment: :blob })
+      direct_ops, ancestor_ops = ops.partition { |op| op.event_id == @event.id }
+      direct_roles = direct_ops.to_h { |op| [op.user_id, op.role] }
+
+      @indirect_access = ancestor_ops.group_by(&:user).filter_map do |user, user_ops|
+        # Inheriting from an ancestor is all-or-nothing (see
+        # OrganizerPosition.role_at_least?): managers inherit their full role,
+        # everyone else only inherits read access.
+        inherited_role = user_ops.any?(&:manager?) ? "manager" : "reader"
+
+        # Someone with their own position here is already in the team list
+        # below, so only mention them when what they inherit outranks it.
+        direct_role = direct_roles[user.id]
+        next if direct_role && OrganizerPosition.roles[direct_role] >= OrganizerPosition.roles[inherited_role]
+
+        [user, inherited_role]
+      end.sort_by { |user, role| [-OrganizerPosition.roles[role], user.name.to_s] }.to_h
     end
 
     @invites = @event.organizer_position_invites.pending.includes(:sender)

app/models/user.rb

@@ -583,22 +583,6 @@ def disable_backup_codes!
     BackupCodeMailer.with(user_id: id).backup_codes_disabled.deliver_now
   end
 
-  def access_level_for(event, organizer_positions)
-    role = nil
-    access_level = nil
-    user_ops = organizer_positions.select { |op| op.user == self }
-    return nil if user_ops.empty?
-
-    user_ops.each do |op|
-      if role.nil? || OrganizerPosition.roles[op.role] > OrganizerPosition.roles[role]
-        role = op.role
-        access_level = op.event == event ? :direct : :indirect
-      end
-    end
-
-    { role:, access_level: }
-  end
-
   def needs_to_enable_2fa?
     admin_override_pretend? && !use_two_factor_authentication
   end

app/views/events/team.html.erb

@@ -33,21 +33,31 @@
 </h1>
 
 <% if @event.parent.present? %>
-  <%= render "application/callout", title: "The team behind #{@event.parent.name} also has access to #{@event.name}", type: "info", icon: "leader" do %>
-    <p>Because <%= @event.name %> is a sub-organization of <%= @event.parent.name %>, all team members of <%= @event.parent.name %> can access and view this organization.</p>
+  <%# The icons are wrapped rather than placed directly in the <summary> because
+      `summary > svg` picks up padding from the dock styles. %>
+  <details id="parent_organization_access" class="card border b--info p-0 mt3 mb3">
+    <summary class="px-4 py-3 list-none">
+      <div class="flex items-center gap-2">
+        <span class="info line-height-0"><%= inline_icon "leader", size: 24 %></span>
+        <p class="bold m0 flex-auto">The team behind <%= @event.parent.name %> also has access to <%= @event.name %></p>
+        <span class="muted line-height-0"><%= inline_icon "down-caret", size: 24, class: "flip-when-open" %></span>
+      </div>
+    </summary>
 
-    <% if @indirect_access.present? %>
-      <p>These team members can access this organization:</p>
+    <div class="border-top b--muted px-4 py-3">
+      <p class="mt0">Because <%= @event.name %> is a sub-organization of <%= @event.parent.name %>, the <%= @event.parent.name %> team has access here too. Their managers can manage this organization, while their members and readers can only view it.</p>
 
-      <div class="grid">
-        <% @indirect_access.each do |user, role| %>
-          <span class="flex">
-            <%= user_mention(user) %><%= role == "manager" ? "can manage" : "can view" %>
-          </span>
-        <% end %>
-      </div>
-    <% end %>
-  <% end %>
+      <% if @indirect_access.present? %>
+        <div class="grid">
+          <% @indirect_access.each do |user, role| %>
+            <span class="flex">
+              <%= user_mention(user) %><%= role == "manager" ? "can manage" : "can view" %>
+            </span>
+          <% end %>
+        </div>
+      <% end %>
+    </div>
+  </details>
 <% end %>
 
 <div class="mb-4 flex flex-row justify-between items-center width-100 gap-2">

spec/controllers/events_controller_spec.rb

@@ -315,6 +315,99 @@ def sign_in_organizer_of(event)
     end
   end
 
+  describe "#team" do
+    render_views
+
+    let(:parent) { create(:event, name: "Parent Organization") }
+    let(:event) { create(:event, parent:, name: "Sub Organization") }
+
+    before { sign_in_organizer_of(event) }
+
+    # The callout's list, as { user's displayed name => the role it credits them with }.
+    def indirect_access
+      get(:team, params: { event_id: event.slug })
+
+      Nokogiri::HTML5(response.body).css("#parent_organization_access .grid > span").to_h do |row|
+        [row.at_css(".mention").text.squish, row.text.include?("can manage") ? "manager" : "reader"]
+      end
+    end
+
+    it "collapses the parent organization callout by default", :aggregate_failures do
+      get(:team, params: { event_id: event.slug })
+
+      callout = Nokogiri::HTML5(response.body).at_css("details#parent_organization_access")
+      expect(callout.text).to include("The team behind Parent Organization also has access to Sub Organization")
+      expect(callout.attributes).not_to have_key("open")
+    end
+
+    it "grants a reader on the parent read access here" do
+      reader = create(:user)
+      create(:organizer_position, user: reader, event: parent, role: :reader)
+
+      expect(indirect_access).to eq({ reader.initial_name => "reader" })
+    end
+
+    # A member of the parent only inherits read access here, so their own
+    # member position is the higher of the two and already appears in the
+    # team list.
+    it "grants a member on the parent only read access here" do
+      member = create(:user)
+      create(:organizer_position, user: member, event: parent, role: :member)
+
+      expect(indirect_access).to eq({ member.initial_name => "reader" })
+    end
+
+    it "grants a manager on the parent full management here" do
+      manager = create(:user)
+      create(:organizer_position, user: manager, event: parent, role: :manager)
+
+      expect(indirect_access).to eq({ manager.initial_name => "manager" })
+    end
+
+    it "takes the highest role when the user holds positions on several ancestors" do
+      grandparent = create(:event)
+      parent.update!(parent: grandparent)
+      user = create(:user)
+      create(:organizer_position, user:, event: parent, role: :reader)
+      create(:organizer_position, user:, event: grandparent, role: :manager)
+
+      expect(indirect_access).to eq({ user.initial_name => "manager" })
+    end
+
+    it "omits a user whose position here already matches what they inherit" do
+      user = create(:user)
+      create(:organizer_position, user:, event: parent, role: :reader)
+      create(:organizer_position, user:, event:, role: :reader)
+
+      expect(indirect_access).to eq({})
+    end
+
+    it "omits a user whose position here outranks what they inherit" do
+      user = create(:user)
+      create(:organizer_position, user:, event: parent, role: :member)
+      create(:organizer_position, user:, event:, role: :member)
+
+      expect(indirect_access).to eq({})
+    end
+
+    it "keeps a user whose inherited role outranks their position here" do
+      user = create(:user)
+      create(:organizer_position, user:, event: parent, role: :manager)
+      create(:organizer_position, user:, event:, role: :member)
+
+      expect(indirect_access).to eq({ user.initial_name => "manager" })
+    end
+
+    it "lists managers before readers" do
+      reader = create(:user, full_name: "Aaron Reader")
+      manager = create(:user, full_name: "Zoe Manager")
+      create(:organizer_position, user: reader, event: parent, role: :reader)
+      create(:organizer_position, user: manager, event: parent, role: :manager)
+
+      expect(indirect_access.keys).to eq([manager.initial_name, reader.initial_name])
+    end
+  end
+
   describe "#async_sub_organization_balance" do
     render_views