N°6071 - Prefill Tagset in transition displayed but not saved #751

accognet · 2025-09-15T14:12:37Z

internal

Copilot

Pull request overview

This pull request fixes issue N°6071 where prefilled TagSet values in transitions were being displayed but not saved when the form was submitted. The fix modifies the PrepareValueFromPostedForm method to properly handle the reconciliation between form-provided values (orig_value) and the actual state that should be saved.

Changes:

Modified TagSet/Set handling in PrepareValueFromPostedForm to reconcile prefilled values with database state
For new objects: moves non-removed items from orig_value into the added array
For existing objects: reconciles form's orig_value with current database values, updating added and removed arrays accordingly

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T20:24:02Z

application/cmdbabstract.class.inc.php

+					if ($this->IsNew()) {
+						if ($value['orig_value'] != '' ) {
+							foreach ($value['orig_value'] as $val) {
+								if (!in_array($val, $value['removed'])) {
+									$value['added'][] = $val;
+								}
+							}
+						}
+					} else {
+						$acurrentValues = $this->Get($sAttCode)->GetValues();
+						foreach ($value['orig_value'] as $val) {
+							if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['added'][] = $val;
+							}
+						}
+						foreach ($acurrentValues as $val) {
+							if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['removed'][] = $val;
+							}
+						}


The code accesses $value['removed'] and $value['added'] arrays without first checking if these keys exist. According to the JSON structure from the widget and AttributeSet.php (lines 114-115), these keys should always be present, but defensive coding would be better. If the JSON is malformed or comes from an unexpected source, this could cause PHP warnings or errors. Consider using isset() checks or the null coalescing operator before accessing these array elements.

Suggested change

if ($this->IsNew()) {

if ($value['orig_value'] != '' ) {

foreach ($value['orig_value'] as $val) {

if (!in_array($val, $value['removed'])) {

$value['added'][] = $val;

}

}

}

} else {

$acurrentValues = $this->Get($sAttCode)->GetValues();

foreach ($value['orig_value'] as $val) {

if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed']) && !in_array($val, $value['added']) ) {

$value['added'][] = $val;

}

}

foreach ($acurrentValues as $val) {

if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed']) && !in_array($val, $value['added']) ) {

$value['removed'][] = $val;

}

}

if (is_array($value)) {

// Ensure expected keys exist and are arrays

$origValues = [];

if (isset($value['orig_value']) && is_array($value['orig_value'])) {

$origValues = $value['orig_value'];

}

$removedValues = [];

if (isset($value['removed']) && is_array($value['removed'])) {

$removedValues = $value['removed'];

}

$addedValues = [];

if (isset($value['added']) && is_array($value['added'])) {

$addedValues = $value['added'];

}

if ($this->IsNew()) {

if (!empty($origValues)) {

foreach ($origValues as $val) {

if (!in_array($val, $removedValues)) {

$addedValues[] = $val;

}

}

}

} else {

$acurrentValues = $this->Get($sAttCode)->GetValues();

foreach ($origValues as $val) {

if (!in_array($val, $acurrentValues)

&& !in_array($val, $removedValues)

&& !in_array($val, $addedValues)

) {

$addedValues[] = $val;

}

}

foreach ($acurrentValues as $val) {

if (!in_array($val, $origValues)

&& !in_array($val, $removedValues)

&& !in_array($val, $addedValues)

) {

$removedValues[] = $val;

}

}

}

// Write normalized values back to $value for further processing

$value['orig_value'] = $origValues;

$value['removed'] = $removedValues;

$value['added'] = $addedValues;

Copilot · 2026-01-28T20:24:03Z

application/cmdbabstract.class.inc.php

+									$value['added'][] = $val;
+								}
+							}
+						}
+					} else {
+						$acurrentValues = $this->Get($sAttCode)->GetValues();
+						foreach ($value['orig_value'] as $val) {
+							if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['added'][] = $val;
+							}
+						}
+						foreach ($acurrentValues as $val) {
+							if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['removed'][] = $val;
+							}
+						}


The code directly appends to $value['added'] without initializing it if it doesn't exist. While the JSON structure from the widget should include this key, if $value['added'] is undefined, this will create a PHP notice. Use $value['added'] = $value['added'] ?? []; before the loop or initialize it at the start of each branch to ensure the array exists.

Copilot · 2026-01-28T20:24:03Z

application/cmdbabstract.class.inc.php

+							if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['added'][] = $val;
+							}
+						}
+						foreach ($acurrentValues as $val) {
+							if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {


There is inconsistent spacing in this condition. There's an extra space after the opening parenthesis (before the first !in_array) and there are double spaces before the closing parenthesis. For consistency with the rest of the codebase, format as: if (!in_array($val, $acurrentValues) && !in_array($val, $value['removed']) && !in_array($val, $value['added'])). The same issue appears on line 4284.

Suggested change

if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed']) && !in_array($val, $value['added']) ) {

$value['added'][] = $val;

}

}

foreach ($acurrentValues as $val) {

if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed']) && !in_array($val, $value['added']) ) {

if (!in_array($val, $acurrentValues) && !in_array($val, $value['removed']) && !in_array($val, $value['added'])) {

$value['added'][] = $val;

}

}

foreach ($acurrentValues as $val) {

if (!in_array($val, $value['orig_value']) && !in_array($val, $value['removed']) && !in_array($val, $value['added'])) {

Copilot · 2026-01-28T20:24:03Z

application/cmdbabstract.class.inc.php

+					if ($this->IsNew()) {
+						if ($value['orig_value'] != '' ) {
+							foreach ($value['orig_value'] as $val) {
+								if (!in_array($val, $value['removed'])) {
+									$value['added'][] = $val;
+								}
+							}
+						}
+					} else {
+						$acurrentValues = $this->Get($sAttCode)->GetValues();
+						foreach ($value['orig_value'] as $val) {
+							if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['added'][] = $val;
+							}
+						}
+						foreach ($acurrentValues as $val) {
+							if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['removed'][] = $val;


The code performs multiple in_array() calls in nested loops which could be inefficient if the arrays are large. For better performance, especially with larger tag sets, consider converting $value['removed'] and $value['added'] to associative arrays using array_flip() before the loops, and then use isset() instead of in_array(). This would reduce complexity from O(n*m) to O(n) for each loop.

Suggested change

if ($this->IsNew()) {

if ($value['orig_value'] != '' ) {

foreach ($value['orig_value'] as $val) {

if (!in_array($val, $value['removed'])) {

$value['added'][] = $val;

}

}

}

} else {

$acurrentValues = $this->Get($sAttCode)->GetValues();

foreach ($value['orig_value'] as $val) {

if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed']) && !in_array($val, $value['added']) ) {

$value['added'][] = $val;

}

}

foreach ($acurrentValues as $val) {

if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed']) && !in_array($val, $value['added']) ) {

$value['removed'][] = $val;

// Ensure expected keys exist and are arrays

if (!isset($value['orig_value']) || $value['orig_value'] === '') {

$value['orig_value'] = [];

}

if (!isset($value['added']) || $value['added'] === '') {

$value['added'] = [];

}

if (!isset($value['removed']) || $value['removed'] === '') {

$value['removed'] = [];

}

// Precompute lookup maps for faster membership tests

$removedMap = array_flip($value['removed']);

$addedMap = array_flip($value['added']);

if ($this->IsNew()) {

if (!empty($value['orig_value'])) {

foreach ($value['orig_value'] as $val) {

if (!isset($removedMap[$val])) {

$value['added'][] = $val;

$addedMap[$val] = true;

}

}

}

} else {

$acurrentValues = $this->Get($sAttCode)->GetValues();

$currentMap = array_flip($acurrentValues);

$origMap = array_flip($value['orig_value']);

foreach ($value['orig_value'] as $val) {

if (!isset($currentMap[$val]) && !isset($removedMap[$val]) && !isset($addedMap[$val])) {

$value['added'][] = $val;

$addedMap[$val] = true;

}

}

foreach ($acurrentValues as $val) {

if (!isset($origMap[$val]) && !isset($removedMap[$val]) && !isset($addedMap[$val])) {

$value['removed'][] = $val;

$removedMap[$val] = true;

Copilot · 2026-01-28T20:24:03Z

application/cmdbabstract.class.inc.php

+				if ($sTagSetJson !== null) { // bulk modify, direct linked set not handled
+					$value = json_decode($sTagSetJson, true);
+					if ($this->IsNew()) {
+						if ($value['orig_value'] != '' ) {


The check $value['orig_value'] != '' is insufficient and could cause errors. If orig_value is not set in the JSON (due to malformed data), this will generate a PHP notice. Additionally, comparing an array to an empty string is semantically incorrect. Use !empty($value['orig_value']) instead, which properly handles missing keys, null values, and empty arrays, and is more semantically appropriate for array validation.

Suggested change

if ($value['orig_value'] != '' ) {

if (!empty($value['orig_value'])) {

In this case, we rather use the \utils::StrLen($value['orig_value']) > 0 pattern.

Copilot · 2026-01-28T20:24:04Z

application/cmdbabstract.class.inc.php

+							if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['added'][] = $val;
+							}
+						}
+						foreach ($acurrentValues as $val) {
+							if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {


There is inconsistent spacing in this condition. There's an extra space after the opening parenthesis (before the first !in_array) and there are double spaces before the closing parenthesis. For consistency with the rest of the codebase, format as: if (!in_array($val, $value['orig_value']) && !in_array($val, $value['removed']) && !in_array($val, $value['added'])). The same issue appears on line 4279.

Suggested change

if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed']) && !in_array($val, $value['added']) ) {

$value['added'][] = $val;

}

}

foreach ($acurrentValues as $val) {

if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed']) && !in_array($val, $value['added']) ) {

if (!in_array($val, $acurrentValues) && !in_array($val, $value['removed']) && !in_array($val, $value['added'])) {

$value['added'][] = $val;

}

}

foreach ($acurrentValues as $val) {

if (!in_array($val, $value['orig_value']) && !in_array($val, $value['removed']) && !in_array($val, $value['added'])) {

Copilot · 2026-01-28T20:24:04Z

application/cmdbabstract.class.inc.php

+						foreach ($value['orig_value'] as $val) {
+							if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['added'][] = $val;
+							}
+						}


The code iterates over $value['orig_value'] without first checking if it exists or is an array. This could cause a PHP error if 'orig_value' is not present in the JSON data. Add a check to ensure isset($value['orig_value']) && is_array($value['orig_value']) before the foreach loop.

Copilot · 2026-01-28T20:24:04Z

application/cmdbabstract.class.inc.php

+						$acurrentValues = $this->Get($sAttCode)->GetValues();
+						foreach ($value['orig_value'] as $val) {
+							if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['added'][] = $val;
+							}
+						}
+						foreach ($acurrentValues as $val) {
+							if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
+								$value['removed'][] = $val;
+							}
+						}


Variable naming is inconsistent: $acurrentValues uses a lowercase 'c' making it look like Hungarian notation but doesn't follow standard PHP conventions. For consistency with the rest of the codebase and to improve readability, use $aCurrentValues (with capital C) to follow the pattern seen elsewhere where array variables are prefixed with 'a'.

N°6071 - Prefill Tagset in transition displayed but not saved

0ec598a

accognet self-assigned this Sep 15, 2025

accognet added the internal Work made by Combodo label Sep 15, 2025

Molkobain requested a review from Copilot January 28, 2026 20:17

Copilot started reviewing on behalf of Molkobain January 28, 2026 20:17 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

N°6071 - Prefill Tagset in transition displayed but not saved #751

N°6071 - Prefill Tagset in transition displayed but not saved #751

accognet commented Sep 15, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Molkobain Jan 28, 2026 •

edited

Loading

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-					if ($this->IsNew()) {
-						if ($value['orig_value'] != '' ) {
-							foreach ($value['orig_value'] as $val) {
-								if (!in_array($val, $value['removed'])) {
-									$value['added'][] = $val;
-								}
-							}
-						}
-					} else {
-						$acurrentValues = $this->Get($sAttCode)->GetValues();
-						foreach ($value['orig_value'] as $val) {
-							if ( !in_array($val, $acurrentValues) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
-								$value['added'][] = $val;
-							}
-						}
-						foreach ($acurrentValues as $val) {
-							if ( !in_array($val, $value['orig_value']) && !in_array($val, $value['removed'])  && !in_array($val, $value['added'])  ) {
-								$value['removed'][] = $val;
-							}
-						}
+					if (is_array($value)) {
+						// Ensure expected keys exist and are arrays
+						$origValues = [];
+						if (isset($value['orig_value']) && is_array($value['orig_value'])) {
+							$origValues = $value['orig_value'];
+						}
+						$removedValues = [];
+						if (isset($value['removed']) && is_array($value['removed'])) {
+							$removedValues = $value['removed'];
+						}
+						$addedValues = [];
+						if (isset($value['added']) && is_array($value['added'])) {
+							$addedValues = $value['added'];
+						}
+						if ($this->IsNew()) {
+							if (!empty($origValues)) {
+								foreach ($origValues as $val) {
+									if (!in_array($val, $removedValues)) {
+										$addedValues[] = $val;
+									}
+								}
+							}
+						} else {
+							$acurrentValues = $this->Get($sAttCode)->GetValues();
+							foreach ($origValues as $val) {
+								if (!in_array($val, $acurrentValues)
+									&& !in_array($val, $removedValues)
+									&& !in_array($val, $addedValues)
+								) {
+									$addedValues[] = $val;
+								}
+							}
+							foreach ($acurrentValues as $val) {
+								if (!in_array($val, $origValues)
+									&& !in_array($val, $removedValues)
+									&& !in_array($val, $addedValues)
+								) {
+									$removedValues[] = $val;
+								}
+							}
+						}
+						// Write normalized values back to $value for further processing
+						$value['orig_value'] = $origValues;
+						$value['removed'] = $removedValues;
+						$value['added'] = $addedValues;

	if ($value['orig_value'] != '' ) {
	if (!empty($value['orig_value'])) {

N°6071 - Prefill Tagset in transition displayed but not saved #751

Are you sure you want to change the base?

N°6071 - Prefill Tagset in transition displayed but not saved #751

Conversation

accognet commented Sep 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Molkobain Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Molkobain Jan 28, 2026 •

edited

Loading