Skip to content

T&A Bugfix #0042396: Fix Test Scoring Settings for Import/Export #8412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 5, 2024
Merged

T&A Bugfix #0042396: Fix Test Scoring Settings for Import/Export #8412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
81e0a88
Fix Test Scoring Settings for Import/Export
fhelfer Nov 12, 2024
2891483
Fix Show Pass Details defaulting to true in Import
fhelfer Nov 12, 2024
7a895bc
Remove passDetails unit test
fhelfer Nov 12, 2024
802f662
Fix Copyright ilObjTestTest
fhelfer Nov 13, 2024
928b4f2
Remove local ilObjTest setter score_reporting & results_presentation
fhelfer Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 9 additions & 83 deletions Modules/Test/classes/class.ilObjTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class ilObjTest extends ilObject implements ilMarkSchemaAware, ilEctsGradesEnabl
*/
public $mark_schema;
public int $sequence_settings;
public int $score_reporting;
public int $instant_verification;
public int $answer_feedback_points;
public $reporting_date;
Expand Down Expand Up @@ -303,7 +302,6 @@ public function __construct($a_id = 0, bool $a_call_by_reference = true)
$this->introduction = "";
$this->questions = array();
$this->sequence_settings = TEST_FIXED_SEQUENCE;
$this->score_reporting = self::SCORE_REPORTING_FINISHED;
$this->instant_verification = 0;
$this->answer_feedback_points = 0;
$this->reporting_date = "";
Expand Down Expand Up @@ -1351,7 +1349,6 @@ public function loadFromDb(): void
$this->setCustomStyle($data->customstyle);
$this->setShowFinalStatement($data->showfinalstatement);
$this->setSequenceSettings($data->sequence_settings);
$this->setScoreReporting($data->score_reporting);
$this->setInstantFeedbackSolution($data->instant_verification);
$this->setAnswerFeedbackPoints($data->answer_feedback_points);
$this->setAnswerFeedback($data->answer_feedback);
Expand All @@ -1371,7 +1368,6 @@ public function loadFromDb(): void
$this->setResetProcessingTime($data->reset_processing_time);
$this->setReportingDate($data->reporting_date);
$this->setShuffleQuestions($data->shuffle_questions);
$this->setResultsPresentation($data->results_presentation);
$this->setStartingTimeEnabled($data->starting_time_enabled);
$this->setStartingTime($data->starting_time);
$this->setEndingTimeEnabled($data->ending_time_enabled);
Expand Down Expand Up @@ -1695,18 +1691,6 @@ public function setPostponingEnabled($postponingEnabled): void
$this->setSequenceSettings((int) $postponingEnabled);
}

/**
* Sets the score reporting of the ilObjTest object
*
* @param int|string $score_reporting The score reporting
* @see $score_reporting
* @deprecated
*/
public function setScoreReporting($score_reporting = 0): void
{
$this->score_reporting = (int) $score_reporting;
}

/**
* Sets the instant feedback for the solution
*
Expand Down Expand Up @@ -1793,16 +1777,12 @@ public function setReportingDate($reporting_date): void
public const SCORE_REPORTING_DATE = 3;
public const SCORE_REPORTING_AFTER_PASSED = 4;

/**
* Gets the score reporting of the ilObjTest object
*
* @return integer The score reporting of the test
* @access public
* @see $score_reporting
*/
public function getScoreReporting(): int
{
return ($this->score_reporting) ? $this->score_reporting : 0;
if ($this->getTestId() !== -1) {
return $this->getScoreSettings()->getResultSummarySettings()->getScoreReporting();
}
return 0;
}

public function isScoreReportingEnabled(): bool
Expand Down Expand Up @@ -4950,7 +4930,6 @@ public function fromXML(ilQTIAssessment $assessment)
// online exam
$this->setFixedParticipants(1);
$this->setListOfQuestionsSettings(7);
$this->setShowSolutionPrintview(1);
break;
case 5:
// varying random test
Expand Down Expand Up @@ -5095,7 +5074,7 @@ public function fromXML(ilQTIAssessment $assessment)
$this->setFixedParticipants($metadata["entry"]);
break;
case "score_reporting":
$this->setScoreReporting((int) $metadata["entry"]);
$result_summary_settings = $result_summary_settings->withScoreReporting((int) $metadata["entry"]);
break;
case "shuffle_questions":
$this->setShuffleQuestions($metadata["entry"]);
Expand Down Expand Up @@ -5254,6 +5233,7 @@ public function fromXML(ilQTIAssessment $assessment)
}

$this->saveToDb();
$result_summary_settings = $result_summary_settings->withShowPassDetails($result_details_settings->getShowPassDetails());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, the value of results_presentation in the tst_tests table is set to 3 if it is not specified. This results in showPassDetails() always being true.

Subsequently, result_presentation will be set to the corresponding import value (in my case, 0). However, due to this line

ILIAS/Modules/Test/classes/ScoreReporting/class.ilObjTestScoreSettingsDatabaseRepository.php

Lines 126 to 128 in a54812f

$settings->getResultDetailsSettings()
->withShowPassDetails($settings->getResultSummarySettings()->getShowPassDetails())
->toStorage(),

, it will always be set to 1. The reason for this is that PassDetails is true because it inherits the still-existing default value of 3 from the database.

@nhaagen, you seem to be the author of this feature. Please take a look at this and see if there is a better solution.

Best,
@fhelfer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to also be an issue for newer releases

$score_settings = $score_settings
->withGamificationSettings($gamification_settings)
->withScoringSettings($scoring_settings)
Expand Down Expand Up @@ -7946,7 +7926,7 @@ public function setListOfQuestionsDescription($a_value = true)
*/
public function getResultsPresentation(): int
{
return ($this->results_presentation) ? $this->results_presentation : 0;
return $this->getScoreSettings()->getResultDetailsSettings()->getResultsPresentation();
}

/**
Expand Down Expand Up @@ -8021,67 +8001,21 @@ public function getShowSolutionListComparison(): bool
return $this->getScoreSettings()->getResultDetailsSettings()->getShowSolutionListComparison();
}

/**
* Sets the combined results presentation value
*
* @param integer $a_results_presentation The combined results presentation value
* @access public
* @deprecated
*/
public function setResultsPresentation($a_results_presentation = 3)
{
$this->results_presentation = $a_results_presentation;
}

public function getShowSolutionListOwnAnswers($user_id = null): bool
{
return $this->getScoreSettings()->getResultDetailsSettings()->getShowSolutionListOwnAnswers();
}

/**
* -------------------------------------------------------------------------
* Those setters are @deprecated!
* They modify $results_presentation based on some parameter and fall back to
* repo-settings when called without.
* Calls are a.o. in class.ilObjTestGUI.php and here during import;
* I left it in place to not to break too many things now, but they should go.
* Soon.
**/

/**
* Sets if the the solution details should be presented to the user or not
*
* @param integer $a_details 1 if the solution details should be presented, 0 otherwise
* @access public
* @deprecated
*/
public function setShowSolutionDetails($a_details = 1)
{
if ($a_details) {
$this->results_presentation = $this->results_presentation | 2;
} else {
if ($this->getShowSolutionDetails()) {
$this->results_presentation = $this->results_presentation ^ 2;
}
}
}

/**
* Sets if the the solution printview should be presented to the user or not
*
* @param boolean $a_details TRUE if the solution printview should be presented, FALSE otherwise
* @access public
* @deprecated
*/
public function setShowSolutionPrintview($a_printview = 1)
public function setShowSolutionPrintview(int $a_printview = 1): void
{
if ($a_printview) {
$this->results_presentation = $this->results_presentation | 4;
} else {
if ($this->getShowSolutionPrintview()) {
$this->results_presentation = $this->results_presentation ^ 4;
}
}
$this->getScoreSettings()->getResultDetailsSettings()->withShowSolutionPrintview($a_printview === 1);
}

/**
Expand Down Expand Up @@ -8803,10 +8737,6 @@ public function applyDefaults($test_defaults): bool
$this->setSequenceSettings($testsettings['SequenceSettings']);
}

if (array_key_exists('ScoreReporting', $testsettings)) {
$this->setScoreReporting($testsettings['ScoreReporting']);
}

if (array_key_exists('SpecificAnswerFeedback', $testsettings)) {
$this->setSpecificAnswerFeedback($testsettings['SpecificAnswerFeedback']);
}
Expand All @@ -8823,10 +8753,6 @@ public function applyDefaults($test_defaults): bool
$this->setAnswerFeedbackPoints($testsettings['AnswerFeedbackPoints']);
}

if (array_key_exists('ResultsPresentation', $testsettings)) {
$this->setResultsPresentation($testsettings['ResultsPresentation']);
}

if (array_key_exists('Anonymity', $testsettings)) {
$this->setAnonymity($testsettings['Anonymity']);
}
Expand Down
46 changes: 2 additions & 44 deletions Modules/Test/test/ilObjTestTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
<?php

declare(strict_types=1);

/**
* This file is part of ILIAS, a powerful learning management system
* published by ILIAS open source e-Learning e.V.
Expand All @@ -18,6 +16,8 @@
*
*********************************************************************/

declare(strict_types=1);

/**
* Class ilObjTestTest
* @author Marvin Beym <[email protected]>
Expand Down Expand Up @@ -165,15 +165,6 @@ public function testSetPostponingEnabled(): void
$this->assertTrue($this->testObj->isPostponingEnabled());
}

public function testScoreReporting(): void
{
$this->testObj->setScoreReporting(0);
$this->assertEquals(0, $this->testObj->getScoreReporting());

$this->testObj->setScoreReporting(1);
$this->assertEquals(1, $this->testObj->getScoreReporting());
}

public function testInstantFeedbackSolution(): void
{
$this->testObj->setInstantFeedbackSolution(0);
Expand Down Expand Up @@ -210,27 +201,6 @@ public function testAnswerFeedbackPoints(): void
$this->assertEquals(1, $this->testObj->getAnswerFeedbackPoints());
}

public function testIsScoreReportingEnabled(): void
{
$this->testObj->setScoreReporting(ilObjTest::SCORE_REPORTING_FINISHED);
$this->assertTrue($this->testObj->isScoreReportingEnabled());

$this->testObj->setScoreReporting(ilObjTest::SCORE_REPORTING_IMMIDIATLY);
$this->assertTrue($this->testObj->isScoreReportingEnabled());

$this->testObj->setScoreReporting(ilObjTest::SCORE_REPORTING_DATE);
$this->assertTrue($this->testObj->isScoreReportingEnabled());

$this->testObj->setScoreReporting(ilObjTest::SCORE_REPORTING_AFTER_PASSED);
$this->assertTrue($this->testObj->isScoreReportingEnabled());

$this->testObj->setScoreReporting(ilObjTest::SCORE_REPORTING_DISABLED);
$this->assertFalse($this->testObj->isScoreReportingEnabled());

$this->testObj->setScoreReporting(999);
$this->assertFalse($this->testObj->isScoreReportingEnabled());
}

public function testBlockPassesAfterPassedEnabled(): void
{
$this->testObj->setBlockPassesAfterPassedEnabled(false);
Expand Down Expand Up @@ -425,16 +395,4 @@ public function testListOfQuestions(): void
$this->testObj->setListOfQuestions(1);
$this->assertTrue($this->testObj->getListOfQuestions());
}

public function testResultsPresentation(): void
{
$this->testObj->setResultsPresentation(0);
$this->assertEquals(0, $this->testObj->getResultsPresentation());

$this->testObj->setResultsPresentation(1);
$this->assertEquals(1, $this->testObj->getResultsPresentation());

$this->testObj->setResultsPresentation(22);
$this->assertEquals(22, $this->testObj->getResultsPresentation());
}
}
Loading