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

Fix FAQ/KB #12922

Merged
merged 4 commits into from
Oct 12, 2022
Merged

Fix FAQ/KB #12922

merged 4 commits into from
Oct 12, 2022

Conversation

AdrienClairembault
Copy link
Contributor

@AdrienClairembault AdrienClairembault commented Oct 7, 2022

The FAQ / KB view does not seem to work properly.
You can see on the following images that the category tree indicate that some articles should be visible, however when you click on the category you find out that there is no valid articles:

image
image
image

The code handling this list was hard to read because it was handling two different cases (FAQ and KB) at the same time.

I've:

  • Split and moved the code around to have proper KB / FAQ "code paths" which are easier to read and understand.
  • Added a great number of tests which should handle every possible KB configuration.

Regarding the specific error in the images above, I think the is_faq => 1 condition was not properly applied for self-service users (it was thrown in an OR clause with other criteria that should only be used for the KB side, not the FAQ).
The new code handle it correctly as it separate KB and FAQ more cleanly, thus only applying the correct condition.

More details on the FAQ / KB expected restrictions can be found here : https://glpi-user-documentation.readthedocs.io/fr/latest/modules/tools/knowledgebase.html

Only public FAQ items are visible to users of simplified interface.
FAQ items are public to everyone (within their entity) that has access to read FAQs.
The additional visibility restrictions will have no affect in this case

Other elements are visible only to technicians via standard interface

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets !25026

@AdrienClairembault AdrienClairembault added this to the 10.0.4 milestone Oct 10, 2022
@AdrienClairembault AdrienClairembault changed the title WIP: fix kb Fix FAQ/KB Oct 10, 2022
@AdrienClairembault AdrienClairembault marked this pull request as ready for review October 10, 2022 09:10
tests/functionnal/KnowbaseItem.php Outdated Show resolved Hide resolved
tests/functionnal/KnowbaseItem.php Outdated Show resolved Hide resolved
tests/functionnal/KnowbaseItem.php Outdated Show resolved Hide resolved
Co-authored-by: Cédric Anne <cedric.anne@gmail.com>
src/KnowbaseItem.php Show resolved Hide resolved
src/KnowbaseItem.php Show resolved Hide resolved
@cedric-anne cedric-anne requested a review from Rom1-B October 11, 2022 06:58
@AdrienClairembault
Copy link
Contributor Author

Wait before merge, I've got one final check to do following internal ref feedback

@AdrienClairembault
Copy link
Contributor Author

Everything seems to be OK, the issue found in the internal ref was related to another issue directly in formcreator this time (see PR mentioned above).
Ready for merge :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants