Test 46663: Inline Error for Empty Question Delete

[matheuszych]

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

Aims to add no questions selected error message directly to question deletion modal.
@thojou

[dsstrassner]

@kergomard please review!

[kergomard]

Hi @matheuszych

Thank you very much for the PR. I do not think we should change anything in the UI-Components to achieve these fixes, so please remove the changes there. You can actually keep most of what you have, as far as I can see. If you feel strongly about the changes in the UI-Component, you would have to provide them separately.
Please remove the return false; on line 223 of QuestionTableActions. We never get there and if we do, things are very, very, wrong and we should notice.

Thanks again and best,
@kergomard

[matheuszych]

Hello @kergomard ,

Thank you for your feedback!

If we remove the return false; here and something goes wrong, execution could continue. Because it wouldn’t be interrupted by a return, break, or exit, it might proceed to the next case, ACTION_DELETE_CONFIRMED. Since we wouldn’t have a modal at that point, the first if condition would most likely prevent further unintended execution. Nonetheless, I would propose keeping the return false; to definitively prevent that scenario and to keep the overall control flow clear and correct. There are many other places where we, for example, call $this->ctrl->redirect($this, '....'); followed by a return with a placeholder value purely for structural and syntax reasons.

What we could or should do is prepend the return false; with a log entry.

I tested everything without the UI changes, and it does not work without them. You get a valid response, but the modal does not open. To make it work, the HTML structure needs to match that of tpl.datatable.html (which was probably the original intention). If you want, I can split this PR into two, but the UI changes are definitely necessary.

Best regards
@matheuszych

[kergomard]

Hi @matheuszych

Thank you for the explanation. I'm fine with keeping the return false;.

I added a (temporary) fix for this issue that I already had handy with 73617e8 .

If you want to go with a more proper fix (ie. that the correct modal-type is shown), please split the UI changes out, probably with it's own Mantis-Issue and a good explanation for the UI coordinators, and update the description of this PR so that it becomes clear, that we need to merge this as soon as the other one is merged. You will also need to revert my current fix.

Thank you very much,
@kergomard

[matheuszych]

Hello @kergomard ,

I moved the UI changes to a separate PR and also added the commit reversion here.

This PR depends on #11577.

Best regards
@matheuszych