Skip to content

[Feature] Notification target ticket #943

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

JeremieMercier
Copy link

Checklist before requesting a review

Please delete options that are not relevant.

  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

This PR adds support for injecting dynamic custom field values into GLPI's ticket notification templates.

    Created a new method findContainers() (based on findContainer()) that returns all 'dom' containers for an item based on its entity (with parent entity handling via getAncestorsOf()).

    Adapted hooks (pre_item_add, pre_item_update, post_item_add, post_item_update) to manage multiple containers using the _plugin_fields_data_multi array.

    Updated the populateData() function to extract input values by stripping the prefix, ensuring that data is saved into the correct columns of the injection table.

    Modified the container.form.php file to "clean" the form data (by removing the prefix) before calling updateFieldsValues(), thereby enabling the saving of domtab containers.

This PR provides the ability to define multiple "Insertion in the form" blocks for the same item based on its entity by leveraging the new findContainers() method and adapting the save process.
…oving the prefix) before calling updateFieldsValues(), thereby enabling the saving of domtab containers.
- Inject ##fields.<container_name>.<field_name>## placeholders into ticket notifications
@stonebuzz stonebuzz requested review from stonebuzz and Rom1-B April 14, 2025 06:57
- Added missing @var annotation for global $DB
- Replaced empty() on $entityIds with count()
- Removed redundant is_array() check on $entityRestriction
- Simplified isset() condition by removing unnecessary !== null check
@Rom1-B
Copy link
Contributor

Rom1-B commented Apr 17, 2025

@JeremieMercier,
To automatically fix issues detected by php-cs-fixer, first install the dependencies using Composer (if not already done):

composer install

Then run the fixer:

vendor/bin/php-cs-fixer fix .

As for PHPStan, there is no automatic fixer. However, you can analyze the code locally and inspect the reported issues using:

vendor/bin/phpstan analyze

Note: PHPStan still reports a type error on DB::request() (expects array<string>|string),
but this issue already exists elsewhere in the file and was not introduced by this change.
@JeremieMercier
Copy link
Author

I have updated the PR with the latest changes from PR 941

@stonebuzz
Copy link
Contributor

Hi @JeremieMercier

The feature you are proposing is very interesting and introduces a significant feature. . To ensure its smooth integration and functionality, we must be extremely vigilant. This means we need to "lock down" our testing to guarantee that everything works as expected.

All types of fields must be tested from a tag on a notification to ensure the rendering of values from the plugin (as well as the label, which depends on the user's language, such as FR/EN, etc.).

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

Successfully merging this pull request may close these issues.

3 participants