ILIAS icon indicating copy to clipboard operation
ILIAS copied to clipboard

T&A Bugfix #0029216: Missing Participant Results in Test Export/Import

Open fhelfer opened this issue 1 year ago • 5 comments

This bug was initially reported in https://mantis.ilias.de/view.php?id=29216. During the bug-fixing process, however, we discovered in #6335 that the issue was more complex than we originally thought.

As a result, we began working on a solution to ensure that the feature would not be abandoned. Before I arrived at a solution involving significant refactoring or a new implementation, I came across this bug fix, which appears to resolve the issue satisfactorily.

Further analysis is ongoing, and we are exploring additional ways to address the problem. However, this bug fix provides a quick solution that prevents the feature from being removed in version 10 and allows it to function properly in other versions as well.

Unfortunately, the solution impacts ilExportGUI, meaning this is not a localized bug fix in T&A. The core issue was that the import of the questions was not handled correctly because a shortcut through ilImport and ilExport was chosen. As a result, the export did not include a manifest file in the zip, leading to an exception in ilImport::doImportObject during the import process, which requires the manifest to function correctly.

This step is crucial to ensure that the questions are not lost. I have removed the shortcut, so the Test Import/Export now runs the entire way through ilImport/ilExport.

This is open for discussion and will be addressed further in our next T&A meeting. After we've decided on the best solution, I will implement fixes for all other versions.

Best @fhelfer

fhelfer avatar Aug 13 '24 09:08 fhelfer

I spoke with @mbecker-databay at the ILIAS DevConf in Graz last week and he and @fhelfer will investigate the dev-guide regarding the media files to provide a modern export solution for trunk which also refactors the code and helps the T&A for the future. The bug fix could be applied to ILIAS 8 and ILIAS 9 if @kergomard agrees.

dsstrassner avatar Sep 09 '24 12:09 dsstrassner

@fhelfer Please proceed with the one line solution for ILIAS 8 and 9. ILIAS 10 needs the further analysis as discussed in the TechSquad.

dsstrassner avatar Sep 19 '24 08:09 dsstrassner

Wasn't quite the one-liner, I could not figure out how I managed to get it working the first time. But with the changes copied from ilTestImporter I managed to get it running. Not quite the best solution, but as this is not the goal, it will do for now until my analysis provides a better way of fixing.

Best @fhelfer

fhelfer avatar Sep 20 '24 11:09 fhelfer

Thank you very much @fhelfer !

We are getting there!

Just one more thing:

  • Couldn't we instead of copying the code from ilTestImporter, move the first part of the code to a function public function addTexonomyAndQuestionsMapping(array $question_id_mapping, ilImportMapping $mapping): ilImportMapping;, make ilTestImporter::importRandomQuestionSetConfig() public and move $newObj->questions = [] inside it? This would avoid some duplicate code and the only thing we need to do is to instantiate an ilTestImporter which should be rather cheap.

What do you think?

Thanks again and best, @kergomard

kergomard avatar Sep 27 '24 15:09 kergomard

Ty for your review @kergomard,

I reduced the duplicated code a bit more, as you can see. Please let me know if you are satisfied with this solution.

Best @fhelfer

fhelfer avatar Oct 17 '24 09:10 fhelfer

Thank you very much @fhelfer for the update ! Merged and picked to release_9 and ported to ILIAS 10 and trunk.

kergomard avatar Nov 07 '24 09:11 kergomard