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 list #3031

Merged
merged 3 commits into from
Oct 13, 2022
Merged

Conversation

AdrienClairembault
Copy link
Contributor

Changes description

After fixing the FAQ tree in glpi-project/glpi#12922, I've noticed that it was still incorrect in formcreator.
If you have at least one visible article then all categories will be shown (even if these categories have no visible articles themselves).

This seems caused by an incorrect SQL query:

image

The column that is supposed to count the visible items in a category actually count the number of visible articles in the whole FAQ (as you can see in the WHERE clause - no restriction on current category).

If I compare the code to the one used in the core (there is two very similar functions - maybe formcreator should reuse the core code here instead of rewriting it).
Formcreator inc.knowbase.class.php:
image

Core Src/Knowbase.php:
image

You can see that the core code have the correct WHERE clause that limit the scope to the current category, while the formcreator code seems to have an useless JOIN instead (that must be removed for the WHERE to work as it target the same table).

After copying the core code, the SQL request work as expected:

image

References

!25026
glpi-project/glpi#12922

@AdrienClairembault
Copy link
Contributor Author

AdrienClairembault commented Oct 12, 2022

I've added another commit to use the same INNER join as the core as well (not sure if it changes anything, but let's keep the same behavior in both core and plugin to be safe).

@btry btry merged commit bb0732c into pluginsGLPI:support/2.13.0 Oct 13, 2022
btry pushed a commit that referenced this pull request Oct 13, 2022
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.

2 participants