From 8c6bc7988a86f89994b49db079f6681af112e8ba Mon Sep 17 00:00:00 2001 From: Lars Kaltefleiter Date: Fri, 29 May 2026 13:19:36 +0200 Subject: [PATCH] fix(core): make IsBulkModifyCompatible non-static, gate LinkedSetIndirect on display_style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since N°3190 (commit fb1ceeba), AttributeLinkedSetIndirect::IsBulkModifyCompatible() returned true unconditionally. For the default (tab) display style this caused DisplayBulkModifyForm() to eagerly materialise and render the full link set of every selected object, leading to timeouts when bulk modifying records with thousands of links each. Bulk modify of an indirect link set is only meaningful for the property-style (tagset-like) widget introduced by N°3190 — the tab-style widget cannot be rendered inside the bulk modify form at all. IsBulkModifyCompatible() is changed from static to a regular instance method across the AttributeDefinition hierarchy so it can depend on the per-instance display_style XML property. All four call sites in cmdbabstract.class.inc.php already use instance dispatch, so no caller changes are required. Breaking change for extensions: overrides of IsBulkModifyCompatible() must drop the \`static\` modifier to avoid a PHP fatal error. Adds a regression test covering the four relevant paths (scalar, AttributeLinkedSet, AttributeLinkedSetIndirect with default tab style, AttributeLinkedSetIndirect with display_style=property). --- .../AttributeDefinition.php | 6 ++- .../AttributeLinkedSet.php | 2 +- .../AttributeLinkedSetIndirect.php | 14 +++++-- .../core/AttributeDefinitionTest.php | 41 +++++++++++++++++++ 4 files changed, 57 insertions(+), 6 deletions(-) diff --git a/sources/Core/AttributeDefinition/AttributeDefinition.php b/sources/Core/AttributeDefinition/AttributeDefinition.php index 359d5c7de0..d8dfc3cad8 100644 --- a/sources/Core/AttributeDefinition/AttributeDefinition.php +++ b/sources/Core/AttributeDefinition/AttributeDefinition.php @@ -373,9 +373,11 @@ public static function IsScalar() * * @return bool * @since 3.1.0 N°3190 - * + * @since 3.3.x Changed from static to non-static so that subclasses can decide + * based on instance-level configuration (e.g. display_style on LinkedSets). + * Overriding implementations in extensions must drop the `static` modifier. */ - public static function IsBulkModifyCompatible(): bool + public function IsBulkModifyCompatible(): bool { return static::IsScalar(); } diff --git a/sources/Core/AttributeDefinition/AttributeLinkedSet.php b/sources/Core/AttributeDefinition/AttributeLinkedSet.php index e0b99b5a1f..77bea5af40 100644 --- a/sources/Core/AttributeDefinition/AttributeLinkedSet.php +++ b/sources/Core/AttributeDefinition/AttributeLinkedSet.php @@ -64,7 +64,7 @@ public function GetEditClass() } /** @inheritDoc */ - public static function IsBulkModifyCompatible(): bool + public function IsBulkModifyCompatible(): bool { return false; } diff --git a/sources/Core/AttributeDefinition/AttributeLinkedSetIndirect.php b/sources/Core/AttributeDefinition/AttributeLinkedSetIndirect.php index 40933f61f4..fe25eaef67 100644 --- a/sources/Core/AttributeDefinition/AttributeLinkedSetIndirect.php +++ b/sources/Core/AttributeDefinition/AttributeLinkedSetIndirect.php @@ -100,10 +100,18 @@ public function GetMirrorLinkAttribute() return $oRet; } - /** @inheritDoc */ - public static function IsBulkModifyCompatible(): bool + /** + * Bulk modify is only meaningful for the property-style (tagset-like) widget. + * The tab-style widget cannot be rendered inside the bulk modify form, and + * eagerly materialising large indirect link sets for every selected object + * caused timeouts. + * + * @inheritDoc + * @since 3.3.x + */ + public function IsBulkModifyCompatible(): bool { - return true; + return $this->GetDisplayStyle() === LINKSET_DISPLAY_STYLE_PROPERTY; } } diff --git a/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php b/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php index 5c4e620a41..650cbfc29f 100644 --- a/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php +++ b/tests/php-unit-tests/unitary-tests/core/AttributeDefinitionTest.php @@ -340,4 +340,45 @@ public function GivenAttribute(string $sHostClass, string $sAttCode, string $sAt return $oAttribute; } + /** + * @dataProvider IsBulkModifyCompatibleProvider + * @covers \AttributeDefinition::IsBulkModifyCompatible + * @covers \Combodo\iTop\Core\AttributeDefinition\AttributeLinkedSet::IsBulkModifyCompatible + * @covers \Combodo\iTop\Core\AttributeDefinition\AttributeLinkedSetIndirect::IsBulkModifyCompatible + * + * Regression test for the bulk modify timeout caused by N°3190: indirect link sets + * displayed as a tab (the default) used to declare themselves bulk-modify-compatible, + * triggering eager materialisation of every link for every selected object. The + * predicate must now depend on the instance-level display_style. + */ + public function testIsBulkModifyCompatible(string $sClass, string $sAttCode, bool $bExpected): void + { + $oAttDef = MetaModel::GetAttributeDef($sClass, $sAttCode); + $this->assertSame( + $bExpected, + $oAttDef->IsBulkModifyCompatible(), + "IsBulkModifyCompatible() mismatch for $sClass::$sAttCode" + ); + } + + public static function IsBulkModifyCompatibleProvider(): array + { + return [ + 'Scalar attribute (AttributeString)' => [ + 'UserRequest', 'title', true, + ], + 'AttributeLinkedSet (1:n) is never bulk compatible' => [ + 'UserRequest', 'workorders_list', false, + ], + 'AttributeLinkedSetIndirect with default (tab) display_style' => [ + // Regression: must be false to avoid the bulk modify timeout + 'UserRequest', 'contacts_list', false, + ], + 'AttributeLinkedSetIndirect with property display_style' => [ + // Preserves the N°3190 feature (tagset-like inline editor in bulk form) + 'FunctionalCI', 'groups_list', true, + ], + ]; + } + }