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

KB - Adds a check which makes sure that the template is rendering before... #1891

Closed
wants to merge 2 commits into from

Conversation

mko-sci
Copy link
Contributor

@mko-sci mko-sci commented Nov 27, 2023

...processing it further to prevent an exception

Additional Notes

Checklist

  • Code change has been tested and works locally
  • Code was formatted via IntelliJ and follows SonarLint & best practices

...processing it further to prevent an exception

Fixes: OX-10557
@mko-sci mko-sci added 🧬 Enhancement Contains new features 🐛 Bugfix Contains only a small fix for an existing bug 👶🏻 Trivial Easy to review labels Nov 27, 2023
Fixes: OX-10557
@mko-sci mko-sci requested a review from sabieber November 27, 2023 16:16
template.render(context);

if (!tryRender(template, context)) {
return;
Copy link
Member

Choose a reason for hiding this comment

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

Sollte man das net irgendwie loggen/sich merken?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ne, nur abbrechen wenn es false zurück gibt und dementsprechend nicht gerendert werden kann

Copy link
Member

Choose a reason for hiding this comment

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

Ich sehe, dass der Code das macht. 😇 Ich möchte wissen, ob das reicht. Bekommen wir ein fehlerhaftes Template anderweitig mit? Haben wir eine Chance, das zu entdecken? Oder sind wir darauf angewiesen, dass irgendwer irgendwann merkt, dass ein Artikel fehlt?

Copy link
Member

@sabieber sabieber Nov 27, 2023

Choose a reason for hiding this comment

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

Guter Punkt finde ich. Vielleicht sollten wir KBA Komponenten doch in n anderes Verzeichnis legen oder anderweitig markieren, statt den Mechanismus zu verbiegen

@mko-sci
Copy link
Contributor Author

mko-sci commented Nov 28, 2023

We decided that the approach might swallow kb articles which have a problem. Therefore we are going to introduce a new "rule". In the future we are going to ignore templates inside the "kb" directory if the templates parent-folder name is "part".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bugfix Contains only a small fix for an existing bug 🧬 Enhancement Contains new features 👶🏻 Trivial Easy to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants