From b1b804e844df6bafee27a7db272625cca0b08f0f Mon Sep 17 00:00:00 2001 From: Stuart Owen Date: Fri, 14 Apr 2023 10:41:35 +0100 Subject: [PATCH] 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. --- app/controllers/programmes_controller.rb | 16 +++--- test/functional/programmes_controller_test.rb | 51 +++++++++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/app/controllers/programmes_controller.rb b/app/controllers/programmes_controller.rb index ebf0527327..279e61c9a9 100644 --- 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 index c3d19a900d..9bbfc6b892 100644 --- 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)