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

Do not alter current object when fetching its search options #18911

Merged

Conversation

cedric-anne
Copy link
Member

@cedric-anne cedric-anne commented Feb 4, 2025

Checklist before requesting a review

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.
  • This change requires a documentation update.

Description

When the CommonDBTM::isField() method is used on a "non-hydrated" object, the CommonDBTM::getEmpty() is triggerred, calling itself the CommonDBTM::post_getEmpty() method and the item_empty hook.

One of the side-effect of this is that $this->fields['entities_id'] field is automatically set with the $_SESSION['glpiactive_entity'] value for any CommonDBTM::add() operation. For all items created from the generic GLPI form, it will not change much, as the entities_id will be overriden by the form input, but for all objects created from anywhere else, it could hide the fact that the information is missing in the input. For instance, the following commit fixes an issue that has not been detected earlier due to this default entities_id value: 8e69433

Another side effect is that calling CommonDBTM::rawSearchOptions() may be altered the current object, because it often uses CommonDBTM::isField() to populate generic search options.

This is probably an unexpected behaviour. I am pretty sure that nobody expect the current object to be altered when using the CommonDBTM::isField() or the CommonDBTM::searchOptions() methods.

@cedric-anne cedric-anne self-assigned this Feb 4, 2025
@cedric-anne cedric-anne force-pushed the 11.0/fix-search-options-get-empty branch 4 times, most recently from 719f7cb to daed69c Compare February 6, 2025 10:00
@cedric-anne cedric-anne marked this pull request as ready for review February 6, 2025 10:00
@cedric-anne cedric-anne added this to the 11.0.0 milestone Feb 6, 2025
@cedric-anne cedric-anne force-pushed the 11.0/fix-search-options-get-empty branch from daed69c to 462df36 Compare February 6, 2025 11:57
@trasher trasher merged commit 2a84937 into glpi-project:main Feb 10, 2025
9 checks passed
@cedric-anne cedric-anne deleted the 11.0/fix-search-options-get-empty branch February 10, 2025 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants