Merge pull request #298 from oat-sa/fix/TR-1241/attempt-taken-for-non…

…-attempted-items fix: Only take a new attempt on `beginAttempt` for items which `endAttempt` was invoked to allow skipped items to be navigated freely
oat-sa · Oct 20, 2021 · b851599 · b851599
2 parents 8af0f5d + ad82bc1
commit b851599
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 18 deletions.
diff --git a/src/qtism/runtime/tests/AssessmentItemSession.php b/src/qtism/runtime/tests/AssessmentItemSession.php
@@ -736,8 +736,15 @@ public function beginAttempt()
             $this[$endAttemptIdentifier] = new QtiBoolean(false);
         }
 
-        // Increment the built-in variable 'numAttempts' by one.
-        $this['numAttempts']->setValue($numAttempts + 1);
+        if (
+            $this['numAttempts']->getValue() === 0
+            || $this['completionStatus']->getValue() === self::COMPLETION_STATUS_COMPLETED
+        ) {
+            // Increment the built-in variable 'numAttempts' by one.
+            $this['numAttempts']->setValue($numAttempts + 1);
+
+            $this['completionStatus']->setValue(self::COMPLETION_STATUS_UNKNOWN);
+        }
 
         // The session get the INTERACTING state.
         $this->setState(AssessmentItemSessionState::INTERACTING);
@@ -960,7 +967,6 @@ public function endCandidateSession()
             $code = AssessmentItemSessionException::STATE_VIOLATION;
             throw new AssessmentItemSessionException($msg, $this, $code);
         } else {
-            $this->endAttempt(null, false);
             $this->setState(AssessmentItemSessionState::SUSPENDED);
         }
     }

diff --git a/test/qtismtest/runtime/tests/AssessmentItemSessionTimingTest.php b/test/qtismtest/runtime/tests/AssessmentItemSessionTimingTest.php
@@ -288,7 +288,7 @@ public function testEvolutionBasicTimeLimitsUnderflowOverflow()
 
         // Try again by waiting too much to respect max time at endAttempt time.
         $itemSession->beginAttempt();
-        $this::assertEquals(0, $itemSession->getRemainingAttempts());
+        $this::assertEquals(1, $itemSession->getRemainingAttempts());
         $itemSession->setTime(self::createDate('2014-07-14 13:00:03'));
 
         try {
@@ -298,7 +298,7 @@ public function testEvolutionBasicTimeLimitsUnderflowOverflow()
             $this::assertEquals(AssessmentItemSessionException::DURATION_OVERFLOW, $e->getCode());
         }
 
-        $this::assertEquals(2, $itemSession['numAttempts']->getValue());
+        $this::assertEquals(1, $itemSession['numAttempts']->getValue());
         $this::assertEquals(AssessmentItemSessionState::CLOSED, $itemSession->getState());
         $this::assertInstanceOf(QtiFloat::class, $itemSession['SCORE']);
         $this::assertEquals(0.0, $itemSession['SCORE']->getValue());

diff --git a/test/qtismtest/runtime/tests/AssessmentTestSessionAttemptsTest.php b/test/qtismtest/runtime/tests/AssessmentTestSessionAttemptsTest.php
@@ -8,34 +8,83 @@
 use qtism\runtime\common\ResponseVariable;
 use qtism\runtime\common\State;
 use qtism\runtime\tests\AssessmentItemSession;
+use qtism\runtime\tests\AssessmentTestSession;
+use qtism\runtime\tests\AssessmentTestSessionException;
 use qtismtest\QtiSmAssessmentTestSessionTestCase;
 
 /**
  * Class AssessmentTestSessionAttemptsTest
  */
 class AssessmentTestSessionAttemptsTest extends QtiSmAssessmentTestSessionTestCase
 {
+    /** @var AssessmentTestSession */
+    private $session;
+
+    public function setUp(): void
+    {
+        $this->session = self::instantiate(self::samplesDir() . 'custom/runtime/attempts/max_3_attempts_nonlinear.xml');
+    }
+
     public function testMultipleAttempts()
     {
-        $session = self::instantiate(self::samplesDir() . 'custom/runtime/attempts/max_3_attempts_nonlinear.xml');
-        $session->beginTestSession();
+        $this->session->beginTestSession();
 
         // Q01 - first attempt.
-        $session->beginAttempt();
-        $session->endAttempt(new State([new ResponseVariable('RESPONSE', Cardinality::SINGLE, BaseType::IDENTIFIER, new QtiIdentifier('ChoiceA'))]));
+        $this->session->beginAttempt();
+        $this->session->endAttempt(new State([new ResponseVariable('RESPONSE', Cardinality::SINGLE, BaseType::IDENTIFIER, new QtiIdentifier('ChoiceA'))]));
+
+        $this::assertEquals(AssessmentItemSession::COMPLETION_STATUS_COMPLETED, $this->session['Q01.completionStatus']);
+
+        // Q01 - second attempt.
+        $this->session->beginAttempt();
+        $this->session->endAttempt(new State([new ResponseVariable('RESPONSE', Cardinality::SINGLE, BaseType::IDENTIFIER, new QtiIdentifier('ChoiceC'))]));
 
-        $this::assertEquals(AssessmentItemSession::COMPLETION_STATUS_COMPLETED, $session['Q01.completionStatus']);
+        $this::assertEquals(AssessmentItemSession::COMPLETION_STATUS_COMPLETED, $this->session['Q01.completionStatus']);
+
+        // Q01 - third attempt. The completion status is now completed.
+        $this->session->beginAttempt();
+        $this->session->endAttempt(new State([new ResponseVariable('RESPONSE', Cardinality::SINGLE, BaseType::IDENTIFIER, new QtiIdentifier('ChoiceA'))]));
+
+        $this::assertEquals(AssessmentItemSession::COMPLETION_STATUS_COMPLETED, $this->session['Q01.completionStatus']);
+    }
+
+    public function testDoesNotTakeAnAttemptWhenInvokingBeginAttemptConsecutivelyWithoutEndingTheAttempt()
+    {
+        $this->session->beginTestSession();
 
-        // Q02 - second attempt.
-        $session->beginAttempt();
-        $session->endAttempt(new State([new ResponseVariable('RESPONSE', Cardinality::SINGLE, BaseType::IDENTIFIER, new QtiIdentifier('ChoiceC'))]));
+        // Q01 - first attempt.
+        $this->session->beginAttempt();
+
+        $this::assertEquals(AssessmentItemSession::COMPLETION_STATUS_UNKNOWN, $this->session['Q01.completionStatus']);
+        $this::assertEquals(1, $this->session['Q01.numAttempts']->getValue());
+
+        // Q01 - same attempt.
+        $this->session->beginAttempt();
+
+        $this::assertEquals(AssessmentItemSession::COMPLETION_STATUS_UNKNOWN, $this->session['Q01.completionStatus']);
+        $this::assertEquals(1, $this->session['Q01.numAttempts']->getValue());
+    }
+
+    public function testThrowsWhenMaxAttemptsIsReached()
+    {
+        $this::expectException(AssessmentTestSessionException::class);
+        $this::expectExceptionMessage('Maximum number of attempts of Item Session \'Q01.0\' reached.');
+
+        $this->session->beginTestSession();
+
+        // Q01 - first attempt.
+        $this->session->beginAttempt();
+        $this->session->endAttempt(new State([new ResponseVariable('RESPONSE', Cardinality::SINGLE, BaseType::IDENTIFIER, new QtiIdentifier('ChoiceA'))]));
 
-        $this::assertEquals(AssessmentItemSession::COMPLETION_STATUS_COMPLETED, $session['Q01.completionStatus']);
+        // Q01 - second attempt.
+        $this->session->beginAttempt();
+        $this->session->endAttempt(new State([new ResponseVariable('RESPONSE', Cardinality::SINGLE, BaseType::IDENTIFIER, new QtiIdentifier('ChoiceC'))]));
 
-        // Q03 - third attempt. The completion status is now completed.
-        $session->beginAttempt();
-        $session->endAttempt(new State([new ResponseVariable('RESPONSE', Cardinality::SINGLE, BaseType::IDENTIFIER, new QtiIdentifier('ChoiceA'))]));
+        // Q01 - third attempt. The completion status is now completed.
+        $this->session->beginAttempt();
+        $this->session->endAttempt(new State([new ResponseVariable('RESPONSE', Cardinality::SINGLE, BaseType::IDENTIFIER, new QtiIdentifier('ChoiceA'))]));
 
-        $this::assertEquals(AssessmentItemSession::COMPLETION_STATUS_COMPLETED, $session['Q01.completionStatus']);
+        // Must throw an exception
+        $this->session->beginAttempt();
     }
 }