Test, partially fix import

[Amstutz]

The test object is exhausting to test atm due to the broken copy and import/export functions. This PR partially fixes the import, so that we can at least import tests from a working 7 installation, maybe also 8. Note that this needs some further testing. Since the import is completely broken for the test atm, it might not hurt much to merge at this stage hower, there are some things we should discuss first.

Changes to be discussed:

• [ ] Partially fixed ilContObjParser to deal with unset attributes. Should they even be empty to begin with?
• [ ] Fixed getQuestionCount, so that the count is not reset if questions are in the object, but not the database (as the case if loaded from xml import). However, not sure if this is the place to do it. Maybe we want a clean count from the db here. We could also move this one layer up.
• [ ] ilObjTest::_setImportDirectory(); is called to early in ilTestImporter, setting the import directory to null, which later leads to an error. Is it now to late for some cases?

Other changes: -> fix duplicate function with null -> '' -> fixed the passing as reference broken functionality by setting back to pass as reference (was dropped for some reason). However I don't like the passing as reference. I changed the maybe most important (import_mapping) to pass as value and return it when done. -> fixed a broken unserialize class which tried to set the allowed_classes to "false" which then excepted no classes at all. -> fixed passing by reference issue with the Test object by adding a getTestObject function. -> minor type stuff.

Hope this helps.

[Amstutz]

@akill: Do you know anything concerning this question:

• [ ] Partially fixed ilContObjParser to deal with unset attributes. Should they even be empty to begin with?

Are those my proposed changes oke for you: https://github.com/ILIAS-eLearning/ILIAS/pull/5097/files#diff-1e9f6d9359e2c65247af949fe3a9b47db7f3b7915e0bb518a6dbfc8c5d389942L478 ?

Thx.

[Amstutz]

In the Tech squad today, we decided on the following:

Fixed getQuestionCount, so that the count is not reset if questions are in the object, but not the database (as the case if loaded from xml import). However, not sure if this is the place to do it. Maybe we want a clean count from the db here. We could also move this one layer up.

I will implement a new getQuestionCountWithoutReloading, making clear, that this will not reload any questions from the DB.

ilObjTest::_setImportDirectory(); is called to early in ilTestImporter, setting the import directory to null, which later leads to an error. Is it now to late for some cases?

We will leave as is, but add a comment explaining the placement.

[Amstutz]

Done :point_up:

[alex40724]

@Amstutz The Modules/Test component is the only one that is still using the deprecated ilContObjParser class. Services/COPage moved to the "new" export concepts 12 years ago (see https://docu.ilias.de/goto_docu_pg_32894_42.html). I suggest to move this class to Modules/Test, since I do not really maintain it anymore.

[Amstutz]

@alex40724 Thx a lot for the quick feedback! Very helpful. Yes, we should probably get rid of this class completely. @mbecker-databay: I would suggest that we merge this as is, but then create a follow, if possible removing the class completely or move it to test, and remove everything not needed.

[dsstrassner]

We talked about this PR at the TechSquad Meeting today and came to conclusion that we want to merge this PR and follow @amstutz suggestion to clear the other problems/questions afterwards.

@kergomard will examine the "new" export concepts with focus on the T&A specifics and report afterwards in one of the next TechSquad meetings.

[kergomard]

Thank you very much @Amstutz for taking care of this. I merge and cherry-pick to trunk and then we tackle the next steps.

[kergomard]

I looked at the whole usage of ilContObjParser in the test and then I looked at the import and export, to really understand what we are doing here. I came to the conclusion that we should have a holistic look at this in one of the next releases, but not now, as I didn't find, an easy solution to replace the current import without rewriting it and breaking backwards compatibility. I will create a PR to move ilContObjParser to the test and add this to the ROADMAP.md.