UI 45579: Automatically opens back exam modal when test password is wrong

[matheuszych]

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

We kindly request these changes as part of the issue mentioned above.

The modal inside of the launcher component also needs to get the current request when calling withRequest on the launcher itself. As the modal is an internal property of the launcher there is no other way to pass the current request to it. The request should not be assigned to the modal when calling getResult on the launcher because it leads to some side effects.

Also when the result of a launcher component leads to an error, the modal should be rendered open to display the issue to the user.

If you have any questions regarding the proposed changes please fell free to ask.

These changes are highly related to the following PR.

Best regards
@matheuszych

[oliversamoila]

Hello @matheuszych, hello everyone.
From a UX and UI perspective, I can say that it looks good for the most part. Thank you very much for your contribution.

I'm not entirely sure which parts of the solutions for Mantis 45579 are in which of the two related PRs. (here and #10201)

I think a small change is still necessary, but I'm not sure whose area of responsibility this belongs to. In screenshot 3, after entering the wrong password, an “alert-danger” message box with the text “Sie müssen alle erforderlichen Felder ausfüllen!” (dt.) is already displayed behind the modal.
This should be displayed there if it is relevant to the content in the modal, i.e., if more than just the password needs to be entered. Perhaps it could also be omitted.
In any case, it is incorrect where it is currently displayed, partly because the user has no option to take action that corresponds to the reported error.

After making the change, please reassign it. For a code review in the UI service, I would then hand it over to @thibsy again.

Best regards and many thanks,
Oliver

[oliversamoila]

See previous comment. ;)

[oliversamoila]

Hello @matheuszych ,
could you please take another look at this issue?

Best regards,
Oliver

[dsstrassner]

Hi @matheuszych is there a timeline?

[oliversamoila]

My remaining request for correction has been made at https:7498a5c. As I said, I wasn't entirely sure whether this should be addressed in #10323 or #10201.
Many thanks.

[oliversamoila]

As I understand it, it's up to you, @thojou and @kergomard .

[thojou]

Hey all,

thanks for you contributions.
I merged the changes into release_10 and cherry-picked to release_11 and trunk.

Best Regards,
@thojou

[thibsy]

Hi @thojou,

I'm sorry if @oliversamoila's last comment was misleading, but I did not sign off on these code changes yet. I am also not sure why @kergomard was assigned here, as changes are entirely inside UI.

We normally integrate PR's ourselves, especially because in UI there are sometimes hidden caveats while cherry-picking (not here though). Just so you know for next time.

I think the change of opening the modal in case of an error should be fine. What I am curious about is the use of the try-catch block here. Since you are using the null-safe operator, why would we run into an exception at all here? Isn't this hiding an actual error now? This looks like a code smell to me.

Kind regards,
@thibsy (as UI coordinator)

[thojou]

Hey @thibsy,

I apologize, I misunderstood @oliversamoila message and did not mean to overstep any boundaries here.

To provide the full context:
This PR was extracted from changes we made to fix a bug in the T&A module. To our knowledge, T&A is currently the only consumer of this UI element, which is why @kergomard and I were assigned to this PR.

Regarding the try-catch block: it made sense during development, but I'm now unable to locate the specific issue that led us to add it. It's quite possible this is a leftover from the initial bugfix that wasn't needed after extracting the changes into a separate PR.

How would you like us to proceed:

1. I can revert this PR, recreate it, and hand it over to you for proper ownership?
2. Create a follow-up PR to remove the try-catch block and address any other concerns?

Again, I'm truly sorry for this misunderstanding. I thought everything had been discussed and aligned internally based on @oliversamoila's message, and that we were good to move forward.

Best regards,
@thojou

[thibsy]

Hi @thojou,

Thx for your apology and explanation.

Don't worry, these things happen. I would appreciate a follow-up PR regarding the try-catch block. The other changes LGTM.

Kind regards,
@thibsy

[oliversamoila]

Hello @thojou and hello @thibsy,
I'm sorry!
My assumption in the handover was to ask the remaining assignments whether everything was now in accordance with the use case. And yes, I should have asked the question in such detail and not just handed over.
Then the correct procedure would have been followed.

Please excuse the confusion.
@oliversamoila