Skip to content

Fixes random question order in participant sheets of XLS export.#1052

Closed
lieblb wants to merge 1 commit intoILIAS-eLearning:trunkfrom
lieblb:sortxls
Closed

Fixes random question order in participant sheets of XLS export.#1052
lieblb wants to merge 1 commit intoILIAS-eLearning:trunkfrom
lieblb:sortxls

Conversation

@lieblb
Copy link
Contributor

@lieblb lieblb commented May 9, 2018

Momentan werden beim Excel-Export der Ergebnisse die Fragen auf den Detail-Sheets der Teilnehmer in der Reihenfolge gespeichert, die der Präsentationsreihenfolge für den Teilnehmer entspricht.

Das ist aus unserer Sicht etwas ungünstig, da hierdurch für Nachkorrektur u.ä. dieselben Fragen verschiedener Teilnehmer an verschiedenen Positionen stehen (bei randomisierter Fragenreihenfolge).

Dieser PR sortiert die Fragen beim Export immer in die kanonische Erstellungsreihenfolge, so dass bei jedem Teilnehmer dieselbe Reihenfolge exportiert und, unabhängig von der Präsentationsreihenfolge.

Meine Implementierung ist wohl etwas fragwürdig (insbes. die Nutzung der sonst noch nicht genutzten Methode getQuestionsOfPass), ich hoffe aber, dass der PR im Zweifel als detaillierter Bug Report/Anregung verstanden werden kann.

$order[$record["question_fi"]] = $record["sequence"];
}

$questions = array_merge($userdata->getQuestions($pass));
Copy link
Contributor

Choose a reason for hiding this comment

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

Diese Zeile ist tatsächlich etwas fragwürdig ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Die Idee war, das von ilTestEvaluationUserData::getQuestions() gelieferte Original-Array nicht zu verändern, und via array_merge eine Kopie zu ziehen, um so ohne Bedenken den usort machen zu können. Dort war ich mir übrigens nicht sicher, ob das direkt übergebene function-lambda mit den ILIAS-PHP-requirements ok geht.

{
foreach($userdata->getQuestions($pass) as $question)
$order = array();
foreach ($this->test_obj->getQuestionsOfPass($active_id, $pass) as $record) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Es mag sein, dass hier ein anderer Weg verfügbar ist, ich schaue das nochmals nach und kommentiere hier nochmals.

@bheyser
Copy link
Contributor

bheyser commented May 9, 2018

Hallo Herr Liebl,

ich habe zwei Zeilenkommentare hinterlassen. Grundsätzlich aber vielen Dank. Eine kleine Bitte habe ich. Wäre es Ihnen möglich, die Methode exportToCSV entsprechend auch anzupassen? Dann brauche ich das ganze lediglich in die stable Branches zu verteilen.

Viele Grüße, BJörn

@lieblb
Copy link
Contributor Author

lieblb commented May 15, 2018

Bei exportToCSV tritt der Effekt gar nicht auf, weil dort die betroffene Exportdarstellung nicht enthalten ist (sondern nur jene, die im Excel im Haupt-Sheet "Testergebnisse" zu finden ist, und die ist ja in Ordnung). Zur Klärung hier nochmal visuell die von diesem PR betroffene Ausgabereihenfolge:

export

@lieblb
Copy link
Contributor Author

lieblb commented May 16, 2018

Mir ist vorhin gekommen, dass man wohl für eine solch definierte Reihenfolge auch garantieren sollte, dass bei MC-Fragen die Antwortoptionen immer in der Definitionsreihenfolge erscheinen. Das sehe ich mir nochmal an, ob das hier der Fall ist.

@bheyser
Copy link
Contributor

bheyser commented May 17, 2018

Okay, darauf hätte ich eigentlich selbst kommen können -.- Okay, CSV nicht das Problem. Der Hinweis mit den Antwort Optionen der Choice Typen ist aber in jedem Fall auch völlig korrekt (!) Soll ich mit einem Merge noch warten? Wir können gerne hier einen Merge machen und das Problem - sofern es besteht - auch noch angehen. Vorteil: Die Review Menge im PR bleibt jeweils klein. Bei dem konstruktiven Maß an Unterstützung bleib ich aber gerne mal flexibel ;-) (!!!)

@Amstutz
Copy link
Contributor

Amstutz commented Dec 11, 2018

TB: @bheyser since you did not get any feedback, we would prefer to close this PR to keep the list of open PR's as clean as possible. Do you want to close this?

@bheyser
Copy link
Contributor

bheyser commented Dec 12, 2018

@lieblb Die PR Liste soll nicht lang werden, ich schlage vor, dass wir in dem anvisierten Telefonat mal alle Probleme durchsprechen und einen Plan machen.

Für den Moment schließen wir diesen PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants