Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T&A 41629: Fix handling of deleted global templates #8338

Merged
merged 6 commits into from
Nov 7, 2024

Conversation

lukas-heinrich
Copy link
Contributor

Hi everyone,

this PR fixes an issue where the settings tab crashes when trying to access a deleted template.

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

Looking forward to a review.
Best,
@lukas-heinrich

@dsstrassner dsstrassner requested a review from thojou November 5, 2024 21:41
@dsstrassner dsstrassner added bugfix php Pull requests that update Php code labels Nov 5, 2024
@dsstrassner
Copy link
Contributor

Hi @kergomard and @thojou, please discuss which one of you review this PR.
Thanks and Best!
@dsstrassner

Copy link
Contributor

@kergomard kergomard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for the PR @lukas-heinrich !

This is partly Administration, so I ask @fneumann or @lscharmer for a review.

In the part of the test:

  • Please remove all html-tags you introduced in ilObjTestSettingsGeneralGUI::getSettingsTemplateMessageHTML and use the corresponding functions from the UI-Framework to generate the message and the link. The rest looks ok from where I stand.

Thanks again and best,
@kergomard

Copy link
Contributor

@kergomard kergomard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much @lukas-heinrich !

@kergomard
Copy link
Contributor

Could you please fix the tests @lukas-heinrich , then I will merge as we have all thumb-ups we need.

This will be ILIAS 8 only and no cherry-picks are needed.

@kergomard
Copy link
Contributor

Just in case you haven't seen it @lukas-heinrich : We now need one more little fix for the copyright ;-).

@kergomard
Copy link
Contributor

Thank you very much @lukas-heinrich !

@kergomard kergomard merged commit d99e42b into ILIAS-eLearning:release_8 Nov 7, 2024
3 checks passed
@lukas-heinrich
Copy link
Contributor Author

Thanks for the reviews!

I hope this PR gets squashed so no one ever sees this commit history again ;)

@lukas-heinrich lukas-heinrich deleted the bugfix/8/41629 branch November 7, 2024 08:40
@kergomard
Copy link
Contributor

Don't worry, it did ;-), ...and thanks again for all the work!

@rfcmaXi
Copy link

rfcmaXi commented Nov 12, 2024

Thanks for the fix. The problem has been bothering us for a while. The solution seems to make a lot of sense to me and has already worked productively ;)

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.

7 participants