fix showing related programmes in a nested route #1421

related_programmes no longer a active record relation, so scope fails. instead loop over and filter out non-activated, whilst showing if the current user can administer it. Converting into a activerecord relation would have lost the ordering, and would have required database specific solutions.
seek4science · Apr 14, 2023 · b1b804e · b1b804e
1 parent f36891b
commit b1b804e
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 6 deletions.
diff --git a/app/controllers/programmes_controller.rb b/app/controllers/programmes_controller.rb
@@ -135,13 +135,17 @@ def inactive_view_allowed?
 
   def fetch_assets
     @programmes = super
-    return @programmes if User.admin_logged_in?
-
-    if User.programme_administrator_logged_in?
-       @programmes = @programmes.all if @programmes.eql?(Programme) #necessary because the super can return the class to defer calling .all, here it is ok to do so
-       @programmes = @programmes.activated | (@programmes & current_person.administered_programmes)
+    @programmes = @programmes.all if @programmes.eql?(Programme) #necessary because the super can return the class to defer calling .all, here it is ok to do so
+
+    # filter out non-activated, unless user can administer it
+    if User.admin_logged_in?
+      @programmes
+    elsif User.programme_administrator_logged_in?
+      @programmes.select do |programme|
+        programme.is_activated? || current_person.is_programme_administrator?(programme)
+      end
     else
-       @programmes = @programmes.activated
+      @programmes.select(&:is_activated?)
     end
   end
 

diff --git a/test/functional/programmes_controller_test.rb b/test/functional/programmes_controller_test.rb
@@ -837,6 +837,57 @@ def rdf_test_object
     end
   end
 
+  test 'people programmes through nested routing' do
+    assert_routing 'people/2/programmes', controller: 'programmes', action: 'index', person_id: '2'
+    admin = Factory(:admin)
+    person = Factory(:programme_administrator)
+    programme = person.programmes.first
+    project = Factory(:project)
+    person.add_to_project_and_institution(project, Factory(:institution))
+    programme2 = Factory(:programme, projects: [project])
+    programme3 = Factory(:programme)
+    person.save!
+    person.reload
+    assert_equal [programme, programme2], person.related_programmes
+
+    get :index, params: { person_id: person.id }
+    assert_response :success
+    assert_select 'div.list_item_title' do
+      assert_select 'a[href=?]', programme_path(programme), text: programme.title
+      assert_select 'a[href=?]', programme_path(programme2), text: programme2.title
+      assert_select 'a[href=?]', programme_path(programme3), text: programme3.title, count: 0
+    end
+
+    # inactive should be hidden from non admins
+    programme.update_column(:is_activated, false)
+
+    get :index, params: { person_id: person.id }
+    assert_response :success
+    assert_select 'div.list_item_title' do
+      assert_select 'a[href=?]', programme_path(programme), text: programme.title, count: 0
+      assert_select 'a[href=?]', programme_path(programme2), text: programme2.title
+      assert_select 'a[href=?]', programme_path(programme3), text: programme3.title, count: 0
+    end
+
+    login_as(person)
+    get :index, params: { person_id: person.id }
+    assert_response :success
+    assert_select 'div.list_item_title' do
+      assert_select 'a[href=?]', programme_path(programme), text: programme.title
+      assert_select 'a[href=?]', programme_path(programme2), text: programme2.title
+      assert_select 'a[href=?]', programme_path(programme3), text: programme3.title, count: 0
+    end
+
+    login_as(admin)
+    get :index, params: { person_id: person.id }
+    assert_response :success
+    assert_select 'div.list_item_title' do
+      assert_select 'a[href=?]', programme_path(programme), text: programme.title
+      assert_select 'a[href=?]', programme_path(programme2), text: programme2.title
+      assert_select 'a[href=?]', programme_path(programme3), text: programme3.title, count: 0
+    end
+  end
+
   test 'Empty programmes should show programme administrators as related people' do
     person1 = Factory(:programme_administrator_not_in_project)
     person2 = Factory(:programme_administrator_not_in_project)