Skip to content

Commit

Permalink
Merge pull request #2319 from openstax/no_update_after_course_ends
Browse files Browse the repository at this point in the history
Prevent instructors from updating task plans after course ends
  • Loading branch information
Dantemss authored May 20, 2021
2 parents 84d55d7 + 4f3f7d9 commit 25af134
Showing 2 changed files with 132 additions and 39 deletions.
16 changes: 11 additions & 5 deletions app/access_policies/task_plan_access_policy.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class TaskPlanAccessPolicy
TEACHER_ACTIONS = [ :index, :read, :create, :update, :destroy, :restore ]
READ_ONLY_ACTIONS = [ :index, :read ]
TEACHER_ACTIONS = READ_ONLY_ACTIONS + [ :create, :update, :destroy, :restore ]

def self.action_allowed?(action, requestor, task_plan)
return false if requestor.is_anonymous? || !requestor.is_human?
@@ -11,9 +12,14 @@ def self.action_allowed?(action, requestor, task_plan)
return false unless TEACHER_ACTIONS.include?(action)

course = task_plan.course
UserIsCourseTeacher[user: requestor, course: course] || (
action == :read &&
course.cloned_courses.any? { |clone| UserIsCourseTeacher[user: requestor, course: clone] }
)
is_teacher = UserIsCourseTeacher[user: requestor, course: course]

if READ_ONLY_ACTIONS.include?(action)
is_teacher || course.cloned_courses.any? do |clone|
UserIsCourseTeacher[user: requestor, course: clone]
end
else
is_teacher && !course.ended?
end
end
end
155 changes: 121 additions & 34 deletions spec/access_policies/task_plan_access_policy_spec.rb
Original file line number Diff line number Diff line change
@@ -59,70 +59,157 @@
end
end

context 'students in the original course' do
let(:requestor) { @student }
context 'course not ended' do
context 'students in the original course' do
let(:requestor) { @student }

context 'index' do
let(:action) { :index }
context 'index' do
let(:action) { :index }

it { should eq true }
it { should eq true }
end

[:read, :create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq false }
end
end
end

[:read, :create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }
context 'students in the cloned course' do
let(:requestor) { @clone_student }

it { should eq false }
context 'index' do
let(:action) { :index }

it { should eq true }
end

[:read, :create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq false }
end
end
end
end

context 'students in the cloned course' do
let(:requestor) { @clone_student }
context 'teachers in the original course' do
let(:requestor) { @teacher }

context 'index' do
let(:action) { :index }
[:index, :read, :create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq true }
it { should eq true }
end
end
end

[:read, :create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }
context 'teachers in the cloned course' do
let(:requestor) { @clone_teacher }

it { should eq false }
[:index, :read].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq true }
end
end

[:create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq false }
end
end
end
end

context 'teachers in the original course' do
let(:requestor) { @teacher }
context 'course ended' do
before(:all) do
DatabaseCleaner.start

[:index, :read, :create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }
@course.update_attribute :ends_at, Time.current - 1.hour
end
after(:all) { DatabaseCleaner.clean }

context 'students in the original course' do
let(:requestor) { @student }

context 'index' do
let(:action) { :index }

it { should eq true }
end

[:read, :create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq false }
end
end
end
end

context 'teachers in the cloned course' do
let(:requestor) { @clone_teacher }
context 'students in the cloned course' do
let(:requestor) { @clone_student }

[:index, :read].each do |test_action|
context test_action.to_s do
let(:action) { test_action }
context 'index' do
let(:action) { :index }

it { should eq true }
end

[:read, :create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq false }
end
end
end

[:create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }
context 'teachers in the original course' do
let(:requestor) { @teacher }

it { should eq false }
[:index, :read].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq true }
end
end

[:create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq false }
end
end
end

context 'teachers in the cloned course' do
let(:requestor) { @clone_teacher }

[:index, :read].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq true }
end
end

[:create, :update, :destroy, :restore].each do |test_action|
context test_action.to_s do
let(:action) { test_action }

it { should eq false }
end
end
end
end

0 comments on commit 25af134

Please sign in to comment.