Fix people index for non-admin access (#1490)

app/models/affiliation.rb

@@ -7,7 +7,7 @@ class Affiliation < ApplicationRecord
 
   scope :active, -> {
     where(inactive: false)
-      .where("end_date IS NULL OR end_date >= ?", Date.current)
+      .where("affiliations.end_date IS NULL OR affiliations.end_date >= ?", Date.current)
   }
 
   scope :facilitators, -> { where("title LIKE ?", "%facilitator%") }

app/views/people/people_results.html.erb

@@ -19,11 +19,18 @@
           </thead>
           <tbody class="divide-y divide-gray-200">
             <% @people.each do |person| %>
-              <% cache [person, current_user.super_user?] do %>
+              <% cache [ person, current_user.super_user?, current_user.person_id == person.id ] do %>
               <tr class="hover:bg-gray-50 transition-colors duration-150 <%= "admin-only bg-blue-100" unless person.published? %>">
                 <td class="px-4 py-2">
                   <% show_email = person.profile_show_email? || allowed_to?(:manage?, Person) %>
-                  <%= person_profile_button(person, subtitle: (person.preferred_email if show_email), data: { turbo_frame: "_top" }) %>
+                  <% if allowed_to?(:show?, person) %>
+                    <%= person_profile_button(person, subtitle: (person.preferred_email if show_email), data: { turbo_frame: "_top" }) %>
+                  <% else %>
+                    <span class="font-semibold"><%= person.name %></span>
+                    <% if show_email && person.preferred_email.present? %>
+                      <div class="text-xs text-gray-500"><%= person.preferred_email %></div>
+                    <% end %>
+                  <% end %>
                 </td>
                 <td class="px-4 py-2 text-center text-sm text-gray-800">
                   <%= person.member_since&.year %>
@@ -50,15 +57,25 @@
                   <% if affiliations.any? %>
                     <div class="flex flex-wrap gap-2">
                       <% affiliations.first(3).each do |affiliation| %>
-                        <%= link_to affiliation.organization.name, organization_path(affiliation.organization),
-                            data: { turbo_frame: "_top" },
-                            class: "inline-block px-3 py-1 rounded-md text-sm font-medium #{DomainTheme.bg_class_for(:organizations, intensity: 100)} #{DomainTheme.text_class_for(:organizations)} #{DomainTheme.bg_class_for(:organizations, intensity: 100, hover: true)}",
-                            style: affiliation.facilitator? ? nil : "border-left: 4px solid #d1d5db" %>
+                        <% org_classes = "inline-block px-3 py-1 rounded-md text-sm font-medium #{DomainTheme.bg_class_for(:organizations, intensity: 100)} #{DomainTheme.text_class_for(:organizations)}" %>
+                        <% org_style = affiliation.facilitator? ? nil : "border-left: 4px solid #d1d5db" %>
+                        <% if allowed_to?(:show?, affiliation.organization) %>
+                          <%= link_to affiliation.organization.name, organization_path(affiliation.organization),
+                              data: { turbo_frame: "_top" },
+                              class: "#{org_classes} #{DomainTheme.bg_class_for(:organizations, intensity: 100, hover: true)}",
+                              style: org_style %>
+                        <% else %>
+                          <span class="<%= org_classes %>" style="<%= org_style %>"><%= affiliation.organization.name %></span>
+                        <% end %>
                       <% end %>
                       <% if affiliations.size > 3 %>
-                        <%= link_to "+#{affiliations.size - 3}", person_path(person),
-                            data: { turbo_frame: "_top" },
-                            class: "inline-block px-3 py-1 rounded-md text-sm font-medium text-gray-500 hover:text-gray-700" %>
+                        <% if allowed_to?(:show?, person) %>
+                          <%= link_to "+#{affiliations.size - 3}", person_path(person),
+                              data: { turbo_frame: "_top" },
+                              class: "inline-block px-3 py-1 rounded-md text-sm font-medium text-gray-500 hover:text-gray-700" %>
+                        <% else %>
+                          <span class="inline-block px-3 py-1 rounded-md text-sm font-medium text-gray-500">+<%= affiliations.size - 3 %></span>
+                        <% end %>
                       <% end %>
                     </div>
                   <% else %>

spec/models/affiliation_spec.rb

@@ -35,6 +35,12 @@
     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 '#set_inactive_from_dates' do

spec/models/person_spec.rb

@@ -299,6 +299,12 @@
       expect(results).to include(person_alice)
       expect(results).not_to include(person_bob)
     end
+
+    it 'chains with_active_affiliations and organization_name without an ambiguous end_date error' do
+      expect {
+        Person.with_active_affiliations.search_by_params(organization_name: 'Alpha').to_a
+      }.not_to raise_error
+    end
   end
 
   describe ".published" do

spec/policies/person_policy_spec.rb

@@ -132,6 +132,22 @@ def policy_for(record: nil, user:)
         expect(sql).to include('`affiliations`.`inactive` = FALSE')
         expect(sql).to include('`users`.`locked_at` IS NULL')
       end
+
+      it "excludes people whose user account is locked" do
+        regular = create(:user)
+        searchable = create(:person, profile_is_searchable: true)
+        create(:affiliation, person: searchable, inactive: false, end_date: nil)
+        locked = create(:person, profile_is_searchable: true, user: create(:user, :locked))
+        create(:affiliation, person: locked, inactive: false, end_date: nil)
+        unlinked = create(:person, profile_is_searchable: true, user: nil)
+        create(:affiliation, person: unlinked, inactive: false, end_date: nil)
+
+        policy = described_class.new(Person, user: regular)
+        scope = policy.apply_scope(Person.all, type: :active_record_relation)
+
+        expect(scope).to include(searchable, unlinked)
+        expect(scope).not_to include(locked)
+      end
     end
   end
 end