Issue #3561 - Fix for bug "Discrepancy in Org Admin view of the

CHANGELOG.md

@@ -15,6 +15,7 @@
 - Bump rexml from 3.4.1 to 3.4.2 [#3562](https://github.com/DMPRoadmap/roadmap/pull/3562)
 - Bump tmp from 0.2.3 to 0.2.4 [#3548](https://github.com/DMPRoadmap/roadmap/pull/3548)
 - Update Gems and Address New Rubocop Offences [#3572](https://github.com/DMPRoadmap/roadmap/pull/3572)
+- Fix for a discrepancy in Org Admin view of the Organisation Plans and the related CSV Download. [#3561] (https://github.com/DMPRoadmap/roadmap/issues/3561)
 
 ## v5.0.1
 - Updated seeds.rb file for identifier_schemes to include context value and removed logo_url and idenitifier_prefix for Shibboleth (as it was causing issues with SSO). [#3525](https://github.com/DMPRoadmap/roadmap/pull/3525)

app/controllers/paginable/plans_controller.rb

@@ -45,8 +45,9 @@ def org_admin
       # check if current user if super_admin
       @super_admin = current_user.can_super_admin?
       @clicked_through = params[:click_through].present?
-      plans = @super_admin ? Plan.all : current_user.org.org_admin_plans
-      plans = plans.joins(:template, roles: [user: :org]).where(Role.creator_condition)
+
+      plans = @super_admin ? Plan.where(Role.creator_condition) : current_user.org.org_admin_plans
+      plans = plans.joins(:template, roles: [user: :org])
 
       paginable_renderise(
         partial: 'org_admin',

app/models/org.rb

@@ -288,17 +288,17 @@ def org_admins
   # This replaces the old plans method. We now use the native plans method and this.
   # rubocop:disable Metrics/AbcSize
   def org_admin_plans
-    combined_plan_ids = (native_plan_ids + affiliated_plan_ids).flatten.uniq
-
-    if Rails.configuration.x.plans.org_admins_read_all
-      Plan.includes(:template, :phases, :roles, :users).where(id: combined_plan_ids)
-          .where(roles: { active: true })
-    else
-      Plan.includes(:template, :phases, :roles, :users).where(id: combined_plan_ids)
-          .where.not(visibility: Plan.visibilities[:privately_visible])
-          .where.not(visibility: Plan.visibilities[:is_test])
-          .where(roles: { active: true })
+    scope = Plan.includes(:template, :phases, :roles, :users)
+                .where(id: (native_plan_ids + affiliated_plan_ids).uniq)
+                .where(roles: { active: true })
+                .where(Role.creator_condition)
+
+    unless Rails.configuration.x.plans.org_admins_read_all
+      scope = scope.where.not(visibility: Plan.visibilities[:privately_visible])
+                   .where.not(visibility: Plan.visibilities[:is_test])
     end
+
+    scope
   end
   # rubocop:enable Metrics/AbcSize
 

spec/factories/plans.rb

@@ -66,11 +66,26 @@
         obj.roles << create(:role, :creator, user: create(:user, org: create(:org)))
       end
     end
+    trait :administrator do
+      after(:create) do |obj|
+        obj.roles << create(:role, :administrator, user: create(:user, org: create(:org)))
+      end
+    end
+    trait :editor do
+      after(:create) do |obj|
+        obj.roles << create(:role, :editor, user: create(:user, org: create(:org)))
+      end
+    end
     trait :commenter do
       after(:create) do |obj|
         obj.roles << create(:role, :commenter, user: create(:user, org: create(:org)))
       end
     end
+    trait :reviewer do
+      after(:create) do |obj|
+        obj.roles << create(:role, :reviewer, user: create(:user, org: create(:org)))
+      end
+    end
     trait :organisationally_visible do
       after(:create) do |plan|
         plan.update(visibility: Plan.visibilities[:organisationally_visible])

spec/models/org_spec.rb

@@ -344,79 +344,125 @@
 
   describe '#org_admin_plans' do
     Rails.configuration.x.plans.org_admins_read_all = true
-    let!(:org) { create(:org) }
-    let!(:plan) { create(:plan, org: org, visibility: 'publicly_visible') }
-    let!(:user) { create(:user, org: org) }
+    # Two Orgs
+    let!(:org1) { create(:org) }
+    let!(:org2) { create(:org) }
+
+    # Plans for org1
+    let!(:org1plan1) { create(:plan, :creator, :organisationally_visible, org: org1) }
+    let!(:org1plan2) { create(:plan, :creator, :privately_visible, org: org1) }
+    let!(:org1plan3) { create(:plan, :creator, :publicly_visible, org: org1) }
+    let!(:org1plan4) { create(:plan, :creator, :organisationally_visible, org: org1) }
 
-    subject { org.org_admin_plans }
+    # Plans for org2
+    let!(:org2plan1) { create(:plan, :creator, :organisationally_visible, org: org2) }
+
+    subject { org1.org_admin_plans }
 
     context 'when user belongs to Org and plan owner with role :creator' do
       before do
-        create(:role, :creator, user: user, plan: plan)
-        plan.add_user!(user.id, :creator)
+        Rails.configuration.x.plans.org_admins_read_all = true
       end
+      it { is_expected.to include(org1plan1, org1plan2, org1plan3, org1plan4) }
+      it { is_expected.not_to include(org2plan1) }
+    end
 
-      it { is_expected.to include(plan) }
+    context 'when user belongs to Org and a plan removed by creator assuming there are no coowners' do
+      before do
+        Rails.configuration.x.plans.org_admins_read_all = true
+        org1plan4.roles.map { |r| r.update(active: false) if r.user_id == org1plan4.owner.id }
+      end
+
+      it { is_expected.to include(org1plan1, org1plan2, org1plan3) }
+      it { is_expected.not_to include(org1plan4) }
     end
 
-    context 'when user belongs to Org and plan user with role :administrator' do
+    context 'when user belongs to Org and a plan removed by creator, but coowner still active.' do
       before do
-        plan.add_user!(user.id, :administrator)
+        Rails.configuration.x.plans.org_admins_read_all = true
+        coowner = create(:user, org: org1)
+        #  Add coowner to the plan by giving user the role administrator
+        org1plan4.add_user!(coowner.id, :administrator)
+        owner_id = org1plan4.owner.id
+        org1plan4.roles.map { |r| r.update(active: false) if r.user_id == owner_id }
+      end
+
+      it { is_expected.to include(org1plan1, org1plan2, org1plan3) }
+      it { is_expected.not_to include(org1plan4) }
+    end
+
+    context 'when user belongs to Org, plan user with role :administrator, but plan creator from a different Org' do
+      before do
+        Rails.configuration.x.plans.org_admins_read_all = true
+        # Creator belongs to different org
+        @plan = create(:plan, :creator, :organisationally_visible, org: create(:org))
+        @plan.add_user!(create(:user, org: org1).id, :administrator)
       end
 
       it {
-        is_expected.to include(plan)
+        is_expected.to include(org1plan1, org1plan2, org1plan3, org1plan4, @plan)
       }
     end
 
     context 'user belongs to Org and plan user with role :editor, but not :creator and :admin' do
       before do
-        plan.add_user!(user.id, :editor)
+        Rails.configuration.x.plans.org_admins_read_all = true
+        # Creator and admin belongs to different orgs
+        @plan = create(:plan, :creator, :organisationally_visible, org: create(:org))
+        @plan.add_user!(create(:org).id, :administrator)
+        # Editor belongs to org1
+        @plan.add_user!(create(:user, org: org1).id, :editor)
       end
 
-      it { is_expected.to include(plan) }
+      it { is_expected.not_to include(@plan) }
     end
 
     context 'user belongs to Org and plan user with role :commenter, but not :creator and :admin' do
       before do
-        plan.add_user!(user.id, :commenter)
+        Rails.configuration.x.plans.org_admins_read_all = true
+        # Creator and admin belongs to different orgs
+        @plan = create(:plan, :creator, :organisationally_visible, org: create(:org))
+        @plan.add_user!(create(:org).id, :administrator)
+        # Commenter belongs to org1
+        @plan.add_user!(create(:user, org: org1).id, :commentor)
       end
 
-      it { is_expected.to include(plan) }
+      it { is_expected.not_to include(@plan) }
     end
 
     context 'user belongs to Org and plan user with role :reviewer, but not :creator and :admin' do
       before do
-        plan.add_user!(user.id, :reviewer)
+        Rails.configuration.x.plans.org_admins_read_all = true
+        # Creator and admin belongs to different orgs
+        @plan = create(:plan, :creator, :organisationally_visible, org: create(:org))
+        @plan.add_user!(create(:org).id, :administrator)
+        # Reviewer belongs to org1
+        @plan.add_user!(create(:user, org: org1).id, :reviewer)
       end
 
-      it { is_expected.to include(plan) }
+      it { is_expected.not_to include(@plan) }
     end
 
     context 'read_all is false, visibility private and user org_admin' do
       before do
         Rails.configuration.x.plans.org_admins_read_all = false
-        @perm = build(:perm)
-        @perm.name = 'grant_permissions'
-        user.perms << @perm
-        plan.add_user!(user.id, :reviewer)
-        plan.privately_visible!
+        @user = create(:user, :org_admin, org: org1)
+        @plan = create(:plan, :creator, :privately_visible, org: org1)
+        @plan.add_user!(@user.id, :reviewer)
       end
 
-      it { is_expected.not_to include(plan) }
+      it { is_expected.not_to include(@plan) }
     end
 
     context 'read_all is false, visibility public and user org_admin' do
       before do
         Rails.configuration.x.plans.org_admins_read_all = false
-        @perm = build(:perm)
-        @perm.name = 'grant_permissions'
-        user.perms << @perm
-        plan.add_user!(user.id, :reviewer)
-        plan.publicly_visible!
+        @user = create(:user, :org_admin, org: org1)
+        @plan = create(:plan, :creator, :publicly_visible, org: org1)
+        @plan.add_user!(@user.id, :reviewer)
       end
 
-      it { is_expected.to include(plan) }
+      it { is_expected.to include(@plan) }
     end
   end