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

Organisation Plans and the related CSV Download".

Changes:
- where(Role.creator_condition) condition added to Plans retrieved in
  the Org model's org_admin_plans method. This ensures that Plans
returned are only active plans. Note: if an owner or co-owner
de-activates a plan the Role of the user is set to active: false, so any
plan with creator with role (having active: false) would mean the Plan
is removed.
- To avoid duplication we removed .where(Role.creator_condition) in
  org_admin method in app/controllers/paginable/plans_controller.rb from
  line
   plans = plans.joins(:template, roles: [user::org]).where(Role.creator_condition).
- Updated Rspect tests for Org method org_admin_plans() in
  spec/models/org_spec.rb.

Author: John Pinto

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,80 +344,120 @@
 
   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) }
 
-    subject { org.org_admin_plans }
+    # 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) }
 
-    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)
-      end
+    # Plans for org2
+    let!(:org2plan1) { create(:plan, :creator, :organisationally_visible, org: org2) }
 
-      it { is_expected.to include(plan) }
+    subject { org1.org_admin_plans }
+
+    context 'when user belongs to Org and plan owner with role :creator' do
+      it { is_expected.to include(org1plan1, org1plan2, org1plan3,org1plan4) }
+      it { is_expected.not_to include(org2plan1) }
     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 assuming there are no coowners' do
       before do
-        plan.add_user!(user.id, :administrator)
+        org1plan4.roles.map { |r| r.update(active: false) if r.user_id == org1plan4.owner.id } 
       end
 
-      it {
-        is_expected.to include(plan)
-      }
+      it { is_expected.to include(org1plan1, org1plan2, org1plan3) }
+      it { is_expected.not_to include(org1plan4) }
     end
 
-    context 'user belongs to Org and plan user with role :editor, but not :creator and :admin' do
+    context 'when user belongs to Org and a plan removed by creator, but cowner still active.' do
       before do
-        plan.add_user!(user.id, :editor)
+        coowner = create(:user, org: org1)
+        org1plan4.add_user!(coowner.id, :coowner)
+        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(plan) }
+      it { is_expected.to include(org1plan1, org1plan2, org1plan3) }
+      it { is_expected.not_to include(org1plan4) }
     end
 
-    context 'user belongs to Org and plan user with role :commenter, but not :creator and :admin' do
+    context 'when user belongs to Org, plan user with role :administrator, but plan creator from a different Org' do
       before do
-        plan.add_user!(user.id, :commenter)
+        # 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) }
+      it {
+        is_expected.to include(org1plan1, org1plan2, org1plan3, org1plan4, @plan)
+      }
     end
 
-    context 'user belongs to Org and plan user with role :reviewer, but not :creator and :admin' do
+    context 'user belongs to Org and plan user with role :editor, but not :creator and :admin' do
       before do
-        plan.add_user!(user.id, :reviewer)
+        # 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 'read_all is false, visibility private and user org_admin' do
+    context 'user belongs to Org and plan user with role :commenter, but not :creator and :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!
+        # 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.not_to include(plan) }
+      it { is_expected.not_to include(@plan) }
     end
 
-    context 'read_all is false, visibility public and user org_admin' do
+    context 'user belongs to Org and plan user with role :reviewer, but not :creator and :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!
-      end
-
-      it { is_expected.to include(plan) }
-    end
+        # 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.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!
+    #   end
+
+    #   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!
+    #   end
+
+    #   it { is_expected.to include(plan) }
+    # end
   end
 
   context '#grant_api!' do