Skip to content

new criteria and actions for groups #19986

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 4 commits into
base: main
Choose a base branch
from

Conversation

orthagh
Copy link
Contributor

@orthagh orthagh commented Jun 12, 2025

Description

Add "group" and "group in charge" in both criteria and actions for Business rules for assets.
It has been asked by our ops team to explore the multi-group feature of asset shipped in 11.0

@@ -5855,7 +5855,7 @@ private function assetBusinessRules($condition)
}

// Set the condition (add or update)
$output = $ruleasset->processAllRules($input, [], [], [
Copy link
Contributor Author

@orthagh orthagh Jun 13, 2025

Choose a reason for hiding this comment

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

Please note this is probably a big change, but I struggle to estimate properly.
It aligns the behavior with RuleTicket as you can see here

glpi/src/CommonITILObject.php

Lines 11250 to 11255 in aad5067

$input = $rules->processAllRules(
$input,
$input,
$rules_params,
$rules_options
);

Without passing the input to the &output parameter, 'append' group action behaves like an 'assign', no previous groups were passed.

@orthagh orthagh marked this pull request as ready for review June 13, 2025 10:17
Copy link
Contributor

@trasher trasher left a comment

Choose a reason for hiding this comment

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

I have no idea on the imapct this can have; but at least, existing tests are OK, so LGTM.

Please add dedicated tests cases.

@trasher
Copy link
Contributor

trasher commented Jun 20, 2025

Please add dedicated tests cases.

(Maybe wait for other feedbacks before working on this)

Copy link
Member

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

It seems OK, please add tests.

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