diff --git a/app/models/plan.rb b/app/models/plan.rb index a837d31..fc40d41 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -341,14 +341,23 @@ # @param user_id [Integer] the id for a user # @return [Boolean] true if the user can read the plan def readable_by?(user_id) - user = user_id.is_a?(User) ? user_id : User.find(user_id) - owner_orgs = self.owner_and_coowners.collect(&:org) + user = user_id.is_a?(User) ? user_id : User.find(user_id) + owner_orgs = owner_and_coowners.collect(&:org) + sys_permission = Branding.fetch(:service_configuration, :plans, + :org_admins_read_all) - # Super Admins can view plans read-only, Org Admins can view their Org's plans - # otherwise the user must have the commenter role - (user.can_super_admin? || - user.can_org_admin? && owner_orgs.include?(user.org) || - has_role(user.id, :commenter)) + # Super Admins can view plans read-only + if user.can_super_admin? + return true + # Org Admins can view their Org's plans if system permission allows + elsif user.can_org_admin? && owner_orgs.include?(user.org) && sys_permission + return true + # ...otherwise the user must have the commenter role. + elsif has_role(user.id, :commenter) + return true + else + return false + end end ## diff --git a/config/branding_example.yml b/config/branding_example.yml index c6c95c7..e2661c1 100644 --- a/config/branding_example.yml +++ b/config/branding_example.yml @@ -52,6 +52,10 @@ owners_and_coowners: visibility_changed: true + service_configuration: + plans: + org_admins_read_all: true + development: <<: *defaults diff --git a/test/unit/plan_test.rb b/test/unit/plan_test.rb index 9aec140..9d84917 100644 --- a/test/unit/plan_test.rb +++ b/test/unit/plan_test.rb @@ -25,6 +25,44 @@ @plan.reload end + test "readable_by? false when organisation settings is false for org admin" do + @creator.update!(org: @org) + @org_admin = User.create!(email: "org-admin@example.com", + password: "password", + org: @org) + + @org_admin.perms << Perm.grant_permissions + @org_admin.perms << Perm.modify_guidance + @org_admin.perms << Perm.modify_templates + @org_admin.perms << Perm.change_org_details + + assert @org_admin.reload.can_org_admin? + Rails.configuration.stub(:branding, { + service_configuration: { plans: { org_admins_read_all: false } } + }) do + refute @plan.readable_by?(@org_admin) + end + end + + test "readable_by? true when organisation settings is true for org admin" do + @creator.update!(org: @org) + @org_admin = User.create!(email: "org-admin@example.com", + password: "password", + org: @org) + + @org_admin.perms << Perm.grant_permissions + @org_admin.perms << Perm.modify_guidance + @org_admin.perms << Perm.modify_templates + @org_admin.perms << Perm.change_org_details + + assert @org_admin.reload.can_org_admin? + Rails.configuration.stub(:branding, { + service_configuration: { plans: { org_admins_read_all: true } } + }) do + assert @plan.readable_by?(@org_admin) + end + end + # --------------------------------------------------- test "required fields are required" do # TODO: uncomment the validation on Plan and then retest this. The validations appear to be breaking the @@ -100,10 +138,10 @@ guidance_groups = GuidanceGroup.includes(guidances: :themes).where(published: true) @plan.guidance_groups << guidance_groups @plan.save! - + phase = @template.phases.first hash = @plan.guidance_by_question_as_hash - + phase.sections.includes(questions: :themes).each do |section| section.questions.each do |question| question.themes.each do |theme| @@ -237,7 +275,7 @@ assert @plan.reviewable_by?(usr), "expected the reviewer to be able to review" assert @plan.readable_by?(usr), "expected the reviewer to be able to comment" end - + # --------------------------------------------------- test "name returns the title" do assert_equal @plan.title, @plan.name @@ -310,7 +348,7 @@ plan = Plan.new(title: 'Tester', visibility: :is_test) verify_belongs_to_relationship(plan, Template.first) end - + # --------------------------------------------------- test "owner_and_coowners returns the correct users" do usrs = @plan.owner_and_coowners @@ -319,14 +357,14 @@ assert [@creator, @administrator].include?(usr), "expected only the creator and co-owner but found #{usr.email}" end end - + # --------------------------------------------------- test "can request feedback" do scaffold_org_admin(@creator.org) - + @plan.request_feedback(@creator) assert @plan.feedback_requested, "expected the feedback flag to be set to true" - assert @plan.reviewable_by?(@user), "expected the Org Admin to be a reviewer" + assert @plan.reviewable_by?(@user), "expected the Org Admin to be a reviewer" end # --------------------------------------------------- @@ -336,9 +374,9 @@ @plan.feedback_requested = true @plan.roles << Role.new(user: @user, access: val) @plan.save! - + @plan.complete_feedback(@user) assert_not @plan.feedback_requested, "expected the feedback flag to be set to false" - assert_not @plan.reviewable_by?(@user), "expected the Org Admin to no longer be a reviewer" + assert_not @plan.reviewable_by?(@user), "expected the Org Admin to no longer be a reviewer" end end