ILIAS icon indicating copy to clipboard operation
ILIAS copied to clipboard

T&A Bugfix #0039805: Create export result file empty

Open fhelfer opened this issue 1 year ago • 1 comments

https://mantis.ilias.de/view.php?id=39805

fhelfer avatar Jan 17 '24 12:01 fhelfer

Thank you very much for the PR @fhelfer !

Two things though:

  • This seems to not address the issue presented in the comment by @dsstrassner here, right?
  • For the issue to be addressed, we would also need to set the $bestonly-parameter, right? ...and this leads me directly to the second issue: Am I right assuming that $bestonly is a misnomer? What we actually mean here is $scoredonly. This might be the best pass, but it might also simply be the last one. If I'm right: Could you please change the variable name.

Thank you very much again and best, @kergomard

kergomard avatar Jan 25 '24 14:01 kergomard

Hi @fhelfer! As ILIAS 9 should be released in April, we need to prioritize this PR. Please check the comment from @kergomard

dsstrassner avatar Mar 21 '24 11:03 dsstrassner

Hi @kergomard,

Export now inludes only scored passes and also passes that are not passed Thats why the result file was empty. There were no passed runs Also PHP Type Erro Bugfix otw

pls review again Best, @fhelfer

fhelfer avatar Mar 22 '24 09:03 fhelfer

Thank you very much @fhelfer and sorry to have let you wait for so long!

I've one last issue, sorry! I'm having so many issues with wrong type-casts that I'm getting a little paranoid: Are you sure these casts here are really right? My feeling is that we are casting to avoid null-values. This would e.g. mean for the sequence value, that all values where sequence is null will then be 0. But 0 is the first question in a sequence and should thus already exist, right? And further the field is NOT NULLABLE in the db, so if it is null, the question should actually not exist, right? The same goes for question_fi. original_id is NULLABLE and right now I think in this case 0 is equal to null. Personally I think this is wrong, but the cast might still be correct for now. I also suspect that the method signature of ilTestEvaluationUserData::addQuestion(int $original_id, int $question_id, float $max_points, int $sequence = null, int $pass = 0) is not completely correct and the type of the $sequence-paramater should actually be ?int (I know, the error would be mine, but as we are now looking into this, I would like to straighten this out if possible). My educated guess would be that we should cast the original_id and continue on question_fi, sequence, or points being null which would then mean, the method signature would hold up for the moment. I hope it becomes clear that I'm not 100% sure here what the right solution is, so if you think we should simply cast the whole shebang, it would be kind, if you could quickly explain why?

Thanks again and best, @kergomard

kergomard avatar Apr 13 '24 15:04 kergomard

Hey @kergomard, I added a check with a continue. Could you give it another review?

Best, @fhelfer

fhelfer avatar Apr 16 '24 07:04 fhelfer

Thanks again @fhelfer ! This change at least guards against my fears, so lets run with it. Merged and picked to trunk.

Best, @kergomard

kergomard avatar Apr 16 '24 07:04 kergomard