From 76f592c88df93e9b44454cd741f9ca47697a670b Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Mon, 20 Feb 2023 23:25:19 +0100 Subject: [PATCH] Consider RfC visibility to view / change RfCs --- app/policies/application_policy.rb | 2 + app/policies/request_for_comment_policy.rb | 19 +- spec/factories/request_for_comment.rb | 4 +- .../request_for_comment_policy_spec.rb | 202 ++++++++++++++++++ 4 files changed, 222 insertions(+), 5 deletions(-) create mode 100644 spec/policies/request_for_comment_policy_spec.rb diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index fb483ec2..f8c0848b 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -31,6 +31,8 @@ class ApplicationPolicy # !! Order is important !! if @record.respond_to? :study_group # e.g. submission study_groups = @record.study_group + elsif @record.respond_to? :submission # e.g. request_for_comment + study_groups = @record.submission.study_group elsif @record.respond_to? :user # e.g. exercise study_groups = @record.author.study_groups.where(study_group_memberships: {role: :teacher}) elsif @record.respond_to? :users # e.g. study_group diff --git a/app/policies/request_for_comment_policy.rb b/app/policies/request_for_comment_policy.rb index 48e41feb..9ce54c4d 100644 --- a/app/policies/request_for_comment_policy.rb +++ b/app/policies/request_for_comment_policy.rb @@ -10,7 +10,7 @@ class RequestForCommentPolicy < ApplicationPolicy end def show? - everyone + admin? || rfc_visibility end def destroy? @@ -18,11 +18,11 @@ class RequestForCommentPolicy < ApplicationPolicy end def mark_as_solved? - admin? || author? + admin? || (author? && rfc_visibility) end def set_thank_you_note? - admin? || author? + admin? || (author? && rfc_visibility) end def clear_question? @@ -49,6 +49,19 @@ class RequestForCommentPolicy < ApplicationPolicy everyone end + def rfc_visibility + case @user.consumer.rfc_visibility + when 'all' + everyone + when 'consumer' + @record.author.consumer == @user.consumer + when 'study_group' + @record.submission.study_group.present? && @record.submission.study_group.id == @user.current_study_group_id + else + raise "Unknown RfC Visibility #{current_user.consumer.rfc_visibility}" + end + end + class Scope < Scope def resolve if @user.admin? diff --git a/spec/factories/request_for_comment.rb b/spec/factories/request_for_comment.rb index f31b78e7..e6b15067 100644 --- a/spec/factories/request_for_comment.rb +++ b/spec/factories/request_for_comment.rb @@ -3,8 +3,8 @@ FactoryBot.define do factory :rfc, class: 'RequestForComment' do association :user, factory: :external_user - association :submission - association :exercise, factory: :dummy + association :exercise, factory: :math + submission { association :submission, exercise:, user: } association :file sequence :question do |n| "test question #{n}" diff --git a/spec/policies/request_for_comment_policy_spec.rb b/spec/policies/request_for_comment_policy_spec.rb new file mode 100644 index 00000000..8305a155 --- /dev/null +++ b/spec/policies/request_for_comment_policy_spec.rb @@ -0,0 +1,202 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe RequestForCommentPolicy do + subject(:policy) { described_class } + + context 'when the RfC visibility is not considered' do + let(:submission) { create(:submission, study_group: create(:study_group)) } + let(:rfc) { create(:rfc, submission:, user: submission.user) } + + %i[destroy? edit?].each do |action| + permissions(action) do + it 'grants access to admins only' do + expect(policy).to permit(build(:admin), rfc) + %i[external_user teacher].each do |factory_name| + expect(policy).not_to permit(create(factory_name), rfc) + end + end + end + end + + %i[create? index? my_comment_requests? rfcs_with_my_comments?].each do |action| + permissions(action) do + it 'grants access to everyone' do + %i[external_user teacher admin].each do |factory_name| + expect(policy).to permit(create(factory_name), rfc) + end + end + end + end + + permissions(:clear_question?) do + it 'grants access to admins' do + expect(policy).to permit(build(:admin), rfc) + end + + it 'grants access to teachers in study group' do + teacher = create(:teacher, study_groups: [rfc.submission.study_group]) + expect(policy).to permit(teacher, rfc) + end + + it 'does not grant access to all other users' do + %i[external_user teacher].each do |factory_name| + expect(policy).not_to permit(create(factory_name), rfc) + end + end + end + end + + context 'when the RfC visibility is considered' do + let(:user) { create(:learner, consumer:) } + + let(:rfc_other_consumer) do + rfc = create(:rfc, user:) + rfc.user.update(consumer: create(:consumer, name: 'Other Consumer')) + rfc + end + + let(:rfc_other_study_group) do + rfc = create(:rfc, user:) + rfc.user.update(consumer: user.consumer) + rfc.submission.update(study_group: create(:study_group)) + rfc + end + + let(:rfc_peer) do + rfc = create(:rfc, user:) + rfc.user.update(consumer: user.consumer) + rfc.submission.update(study_group: user.study_groups.first) + rfc + end + + let(:all_rfcs) { [rfc_other_consumer, rfc_other_study_group, rfc_peer] } + let(:same_consumer_rfcs) { [rfc_other_study_group, rfc_peer] } + let(:other_study_groups_rfcs) { [rfc_other_consumer, rfc_other_study_group] } + + context 'when rfc_visibility is set to all' do + let(:consumer) { create(:consumer, rfc_visibility: 'all') } + + permissions(:show?) do + it 'grants access to everyone' do + %i[external_user teacher admin].each do |factory_name| + all_rfcs.each do |specific_rfc| + expect(policy).to permit(create(factory_name, consumer:), specific_rfc) + end + end + end + end + + %i[mark_as_solved? set_thank_you_note?].each do |action| + permissions(action) do + it 'grants access to admins' do + all_rfcs.each do |specific_rfc| + expect(policy).to permit(create(:admin, consumer:), specific_rfc) + end + end + + it 'grants access to authors' do + all_rfcs.each do |specific_rfc| + expect(policy).to permit(specific_rfc.author, specific_rfc) + end + end + + it 'does not grant access to all other users' do + %i[external_user teacher].each do |factory_name| + all_rfcs.each do |specific_rfc| + expect(policy).not_to permit(create(factory_name, consumer:), specific_rfc) + end + end + end + end + end + end + + context 'when rfc_visibility is set to consumer' do + let(:consumer) { create(:consumer, rfc_visibility: 'consumer') } + + permissions(:show?) do + it 'grants access to admins' do + all_rfcs.each do |specific_rfc| + expect(policy).to permit(create(:admin, consumer:), specific_rfc) + end + end + + it 'grants access to users from the same consumer' do + %i[external_user teacher].each do |factory_name| + same_consumer_rfcs.each do |specific_rfc| + expect(policy).to permit(create(factory_name, consumer:), specific_rfc) + end + end + end + + it 'does not grant access to users from other consumers' do + %i[external_user teacher].each do |factory_name| + expect(policy).not_to permit(create(factory_name, consumer:), rfc_other_consumer) + end + end + end + + # Testing `mark_as_solved?` and `set_thank_you_note?` is not necessary here, + # because an author of an RfC can only be a user from the same consumer. + end + + context 'when rfc_visibility is set to study_group' do + let(:consumer) { create(:consumer, rfc_visibility: 'study_group') } + + permissions(:show?) do + it 'grants access to admins' do + all_rfcs.each do |specific_rfc| + expect(policy).to permit(create(:admin, consumer:), specific_rfc) + end + end + + it 'grants access to users from the same study group' do + %i[external_user teacher].each do |factory_name| + expect(policy).to permit(create(factory_name, consumer:, study_groups: [rfc_peer.submission.study_group]), rfc_peer) + end + end + + it 'does not grant access to users from other consumers' do + %i[external_user teacher].each do |factory_name| + other_study_groups_rfcs.each do |specific_rfc| + expect(policy).not_to permit(create(factory_name, consumer:), specific_rfc) + end + end + end + end + + %i[mark_as_solved? set_thank_you_note?].each do |action| + permissions(action) do + it 'grants access to admins' do + all_rfcs.each do |specific_rfc| + expect(policy).to permit(create(:admin, consumer:), specific_rfc) + end + end + + # Testing `mark_as_solved?` and `set_thank_you_note?` with another consumer is not + # necessary here, because an author of an RfC can only be a user from the same consumer. + + it 'grants access to authors of the same primary study group' do + rfc_peer.author.update(study_groups: [rfc_peer.submission.study_group]) + expect(policy).to permit(rfc_peer.author, rfc_peer) + end + + it 'does not grant access to authors of another primary study groups' do + rfc_other_study_group.author.update(study_groups: [create(:study_group)]) + expect(policy).not_to permit(rfc_other_study_group.author, rfc_other_study_group) + end + + it 'does not grant access to all other users' do + %i[external_user teacher].each do |factory_name| + all_rfcs.each do |specific_rfc| + expect(policy).not_to permit(create(factory_name, consumer:), specific_rfc) + end + end + end + end + end + end + end +end