Skip to content

Commit

Permalink
only render quiz restriction checks when trying to take the quiz
Browse files Browse the repository at this point in the history
fixes CNVS-22655
fixes CNVS-22833

the can_take_quiz? method renders an access_code restriction template or
an ip restriction template if those things are required for the quiz.
previously, those were presented when trying to take the quiz, but
a recent change made it so that they were presented when simply trying
to view the quiz.  this reverts to the previous behavior.

test plan:
- as a teacher, create an access code or ip restricted quiz, publish it
- as a student, you should be able to view the quiz without being
  prompted for an access code (or from an invalid ip)
- as a student, when trying to take the quiz, the restrictions should
  apply correctly
- as a student who meets the restrictions, you should be able to take
  the quiz

Change-Id: Iedc78c3728501da56710e00857527a7323633eeb
Reviewed-on: https://gerrit.instructure.com/62089
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Adam Stone <astone@instructure.com>
QA-Review: Pedro Fajardo <pfajardo@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
Tested-by: Rob Orton <rob@instructure.com>
  • Loading branch information
simonista authored and roor0 committed Aug 30, 2015
1 parent 4f85020 commit caf5d63
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 7 deletions.
10 changes: 7 additions & 3 deletions app/controllers/quizzes/quizzes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -869,9 +869,13 @@ def can_take_quiz?
remote_ip: request.remote_ip,
access_code: params[:access_code])

reason = can_take.declined_reason_renders
render reason if reason
can_take.eligible?
if params[:take]
reason = can_take.declined_reason_renders
render reason if reason
can_take.eligible?
else
can_take.potentially_eligible?
end
end

def quiz_submission_active?
Expand Down
13 changes: 10 additions & 3 deletions app/models/quizzes/quiz_eligibility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,17 @@ def initialize(args = {})
store_session_access_code(args[:access_code]) if args[:access_code]
end

def eligible?
def potentially_eligible?
return true if quiz.grants_right?(user, session, :manage)
return false unless course
return false if inactive_student_with_private_course?
!(course_restrictions_apply? || user_restrictions_apply?)
end

def eligible?
potentially_eligible? && !quiz_restrictions_apply?
end

def declined_reason_renders
return :access_code if need_access_code?
return :invalid_ip if invalid_ip?
Expand Down Expand Up @@ -66,8 +70,11 @@ def course_restrictions_apply?
end

def user_restrictions_apply?
return true if inactive_non_admin? || need_access_code? || invalid_ip?
!quiz.grants_right?(user, session, :submit)
inactive_non_admin? || !quiz.grants_right?(user, session, :submit)
end

def quiz_restrictions_apply?
need_access_code? || invalid_ip?
end

def restricted_by_date?
Expand Down
6 changes: 5 additions & 1 deletion spec/controllers/quizzes/quizzes_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1479,7 +1479,10 @@ def logged_out_survey_with_submission(user, questions, &block)
context "when access code is required but does not match" do
before do
@quiz.stubs(:access_code).returns("trust me. *winks*")
subject.stubs(:params).returns({:access_code => "Don't trust me. *tips hat*"})
subject.stubs(:params).returns({
:access_code => "Don't trust me. *tips hat*",
:take => 1
})
end

it "renders access_code template" do
Expand All @@ -1503,6 +1506,7 @@ def logged_out_survey_with_submission(user, questions, &block)
before do
@quiz.stubs(:ip_filter).returns(true)
@quiz.stubs(:valid_ip?).returns(false)
subject.stubs(:params).returns({:take => 1})
end

it "renders invalid_ip" do
Expand Down
33 changes: 33 additions & 0 deletions spec/models/quizzes/quiz_eligibility_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,43 @@
quiz.stubs(:grants_right?)
.with(anything, anything, :manage).returns(true)
expect(eligibility.eligible?).to be_truthy
expect(eligibility.potentially_eligible?).to be_truthy
end

it "returns false if no course is provided" do
eligibility.stubs(:course).returns(nil)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end

it "returns false if the student is inactive" do
user.stubs(:workflow_state).returns("deleted")
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end

it "returns false if a user cannot read as an admin" do
user.stubs(:new_record?).returns(false)
course.stubs(:grants_right?).returns(false)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end

it "returns false if a quiz is access code restricted (but is still potentially_eligible)" do
quiz.access_code = 'x'
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_truthy
end

it "returns false if a quiz is ip restricted (but is still potentially_eligible)" do
quiz.ip_filter = '1.1.1.1'
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_truthy
end

it "otherwise returns true" do
expect(eligibility.eligible?).to be_truthy
expect(eligibility.potentially_eligible?).to be_truthy
end

describe "date-based overrides" do
Expand All @@ -86,6 +103,7 @@
eligibility.stubs(:course).returns(concluded_course)
eligibility.stubs(:course_section).returns(concluded_section)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end

it "returns false if active course because term > course" do
Expand All @@ -94,6 +112,7 @@
restricted_active_course.stubs(:enrollment_term).returns(concluded_term)
eligibility.stubs(:course_section).returns(concluded_section)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end

it "returns true if active course overrides" do
Expand All @@ -102,20 +121,23 @@
restricted_active_course.stubs(:enrollment_term).returns(concluded_term)
eligibility.stubs(:course_section).returns(concluded_section)
expect(eligibility.eligible?).to be_truthy
expect(eligibility.potentially_eligible?).to be_truthy
end

it "returns false and ignores section" do
eligibility.stubs(:term).returns(concluded_term)
eligibility.stubs(:course).returns(concluded_course)
eligibility.stubs(:course_section).returns(active_section)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end

it "returns false and ignores section" do
eligibility.stubs(:term).returns(concluded_term)
eligibility.stubs(:course).returns(concluded_course)
eligibility.stubs(:course_section).returns(active_section)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end

it "returns true if active section overrides" do
Expand All @@ -124,6 +146,7 @@
concluded_course.stubs(:enrollment_term).returns(concluded_term)
eligibility.stubs(:course_section).returns(restricted_active_section)
expect(eligibility.eligible?).to be_truthy
expect(eligibility.potentially_eligible?).to be_truthy
end
end

Expand All @@ -134,13 +157,15 @@
restricted_concluded_course.stubs(:enrollment_term).returns(active_term)
eligibility.stubs(:course_section).returns(active_section)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end

it "returns true if active section overrides" do
eligibility.stubs(:term).returns(concluded_term)
eligibility.stubs(:course).returns(restricted_concluded_course)
eligibility.stubs(:course_section).returns(restricted_active_section)
expect(eligibility.eligible?).to be_truthy
expect(eligibility.potentially_eligible?).to be_truthy
end
end

Expand All @@ -150,22 +175,27 @@
eligibility.stubs(:course).returns(concluded_course)
eligibility.stubs(:course_section).returns(concluded_section)
expect(eligibility.eligible?).to be_truthy
expect(eligibility.potentially_eligible?).to be_truthy
end

it "returns false if term is closed with no respect for section" do
eligibility.stubs(:term).returns(concluded_term)
eligibility.stubs(:course).returns(concluded_course)
eligibility.stubs(:course_section).returns(active_section)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey

eligibility.stubs(:course_section).returns(concluded_section)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end

it "returns true if active section overrides" do
eligibility.stubs(:term).returns(concluded_term)
eligibility.stubs(:course).returns(concluded_course)
eligibility.stubs(:course_section).returns(restricted_active_section)
expect(eligibility.eligible?).to be_truthy
expect(eligibility.potentially_eligible?).to be_truthy
end
end

Expand All @@ -175,13 +205,15 @@
eligibility.stubs(:course).returns(active_course)
eligibility.stubs(:course_section).returns(concluded_section)
expect(eligibility.eligible?).to be_truthy
expect(eligibility.potentially_eligible?).to be_truthy
end

it "returns false if section overrides" do
eligibility.stubs(:term).returns(active_term)
eligibility.stubs(:course).returns(active_course)
eligibility.stubs(:course_section).returns(restricted_concluded_section)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end
end

Expand All @@ -190,6 +222,7 @@
eligibility.stubs(:course).returns(restricted_active_course)
eligibility.stubs(:course_section).returns(restricted_concluded_section)
expect(eligibility.eligible?).to be_falsey
expect(eligibility.potentially_eligible?).to be_falsey
end
end
end
Expand Down

0 comments on commit caf5d63

Please sign in to comment.