Skip to content

T&A 45579: Automatically opens back exam modal when test password is wrong#10201

Open
aaronbidzan wants to merge 5 commits intoILIAS-eLearning:release_10from
aaronbidzan:bugfix/10/45579
Open

T&A 45579: Automatically opens back exam modal when test password is wrong#10201
aaronbidzan wants to merge 5 commits intoILIAS-eLearning:release_10from
aaronbidzan:bugfix/10/45579

Conversation

@aaronbidzan
Copy link
Contributor

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

This PR aims to resolve the original issue of the wrong language variable being displayed when entering a wrong password on a test.
Per request, an addition has been made to open the modal back up when the password that has been entered is wrong.

The language variable has been changed in recent history and already correctly displays the right message.

Kind regards,
@bidzanaaron

@kergomard
Copy link
Contributor

Thank you very much @bidzanaaron for the PR!

Let me be honest: I don't like this solution at all: We are using internal functions of the launcher that we shouldn't be using and a very bad structure follows from there. We then use TestScreenGUI::shouldOpenModal() to determine if the password was wrong, which is also not the right solution.

Stupid question: Couldn't we simply check the password within a constraint? This would lead to the form being reopened automatically as far as I can see. What am I missing?

Best,
@kergomard

@dsstrassner dsstrassner added the php Pull requests that update Php code label Oct 6, 2025
@aaronbidzan
Copy link
Contributor Author

Hey @kergomard,

I have updated my changes to use constraints. I did not like the original solution either. Putting constraints alone didn't reopen the modal, so I have changed parts of the code in the Inline Launcher. This definitely seems like a better approach than before. Please do let me know how you feel about this solution and if something still needs to be changed.

Kind regards,
@bidzanaaron

@kergomard
Copy link
Contributor

Thank you very much @bidzanaaron !

This goes into the right direction, even though we are not there yet ;-):

  • You do not need to use a closure here, you can just hand the lang-var in.
  • I do not think you can do this here: The access code can be empty and then a new code will be generated and shown. What you can do in the constraint is to test that if a code is given, it is valid.
  • Please do not use double casts like here, but use something like ($value ?? false) ? '1' : '0'.
    And finally and maybe most importantly:
  • Please separate the changes in the UI-Framework into a separate PR and assign it to the UI-Coordinators with a good explanation so they see, what this is about and do not need so sift through our code. We would then merge this PR as soon as the UI-Coordinators agreed on the other one.

Thanks again and best,
@kergomard

@matheuszych
Copy link
Contributor

Hello @kergomard ,

i was kindly ask to helpt out here. I implemented your requested changes and moved the UI changes to a seperate PR.

Best regards
@matheuszych

@kergomard
Copy link
Contributor

kergomard commented Nov 7, 2025

Thanks for the update @matheuszych !

I will check this one when the other PR is through, as we cannot merge it any sooner anyway.

Best,
@kergomard

@kergomard
Copy link
Contributor

Hi @bidzanaaron

Just to have a hint here: Why did you close this?

Best,
@kergomard

@aaronbidzan aaronbidzan reopened this Nov 17, 2025
@aaronbidzan
Copy link
Contributor Author

Hey @kergomard,

I thought that everything was moved and not only the UI changes. I apologize for the confusion.

Best,
@bidzanaaron

@kergomard
Copy link
Contributor

kergomard commented Nov 17, 2025

Thank you very much @bidzanaaron !

Do not worry: The day I stop making mistakes I will throw a stone, ...but this day will also be the day I'm not able to throw stones at all anymore.

Best,
@kergomard

@thojou
Copy link
Contributor

thojou commented Feb 27, 2026

Hey @kergomard,

the changes from #10323 are now integrated. Do you wanna have another quick look before i proceed merging?

Best Regards,
@thojou

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix php Pull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants