ItemGroup 45702: Prevent invalid item group memberships

components/ILIAS/ItemGroup/Service/class.InternalRepoService.php

@@ -1,7 +1,5 @@
 <?php
 
-declare(strict_types=1);
-
 /**
  * This file is part of ILIAS, a powerful learning management system
  * published by ILIAS open source e-Learning e.V.
@@ -18,33 +16,23 @@
  *
  *********************************************************************/
 
+declare(strict_types=1);
+
 namespace ILIAS\ItemGroup;
 
-/**
- * @author Alexander Killing <killing@leifos.de>
- */
+use ilDBInterface;
+use ILIAS\ItemGroup\Repository\ItemGroupRepository;
+
 class InternalRepoService
 {
-    protected InternalDataService $data;
-    protected \ilDBInterface $db;
-
-    public function __construct(InternalDataService $data, \ilDBInterface $db)
-    {
-        $this->data = $data;
-        $this->db = $db;
+    public function __construct(
+        protected readonly InternalDataService $data,
+        protected readonly ilDBInterface $db,
+    ) {
     }
 
-    /*
-    public function ...() : ...\RepoService
+    public function itemGroup(): ItemGroupRepository
     {
-        return new ...\RepoService(
-            $this->data,
-            $this->db
-        );
-    }*/
-
-    /*public function accessSession() : AccessSessionRepository
-    {
-        return new AccessSessionRepository();
-    }*/
+        return new ItemGroupRepository($this->db);
+    }
 }

components/ILIAS/ItemGroup/classes/class.ilItemGroupAppEventListener.php

@@ -0,0 +1,40 @@
+<?php
+
+/**
+ * This file is part of ILIAS, a powerful learning management system
+ * published by ILIAS open source e-Learning e.V.
+ *
+ * ILIAS is licensed with the GPL-3.0,
+ * see https://www.gnu.org/licenses/gpl-3.0.en.html
+ * You should have received a copy of said license along with the
+ * source code, too.
+ *
+ * If this is not the case or you just want to try ILIAS, you'll find
+ * us at:
+ * https://www.ilias.de
+ * https://github.com/ILIAS-eLearning
+ *
+ *********************************************************************/
+
+declare(strict_types=1);
+
+/**
+ * Cleans up item group references when objects are trashed or deleted.
+ */
+class ilItemGroupAppEventListener implements ilAppEventListener
+{
+    public static function handleEvent(string $a_component, string $a_event, array $a_parameter): void
+    {
+        $ref_id = (int) ($a_parameter['ref_id'] ?? 0);
+        if ($a_component !== 'components/ILIAS/ILIASObject' || $ref_id <= 0) {
+            return;
+        }
+
+        global $DIC;
+
+        match ($a_event) {
+            'toTrash', 'delete' => $DIC->itemGroup()->internal()->repo()->itemGroup()->removeItems([$ref_id]),
+            default => null,
+        };
+    }
+}

components/ILIAS/ItemGroup/classes/class.ilItemGroupItems.php

@@ -173,14 +173,28 @@ public function getAssignableItems(): array
 
     public function getValidItems(): array
     {
-        $items = $this->getItems();
-        $ass_items = $this->getAssignableItems();
-        $valid_items = array();
-        foreach ($ass_items as $aitem) {
-            if (in_array($aitem["ref_id"], $items)) {
-                $valid_items[] = $aitem["ref_id"];
+        if ($this->getItemGroupRefId() <= 0) {
+            return $this->items;
+        }
+
+        return $this->filterValidRefIds($this->items);
+    }
+
+    /**
+     * @param int[] $items
+     * @return int[]
+     */
+    protected function filterValidRefIds(array $items): array
+    {
+        $valid_items = [];
+        foreach ($this->getAssignableItems() as $assignable_item) {
+            if (!in_array($assignable_item['ref_id'], $items, true)) {
+                continue;
             }
+
+            $valid_items[] = $assignable_item['ref_id'];
         }
+
         return $valid_items;
     }
 
@@ -198,7 +212,7 @@ public function cloneItems(
         $new_items = array();
         // check: is this a ref id!?
         $source_ig = new ilItemGroupItems($a_source_id);
-        foreach ($source_ig->getItems() as $item_ref_id) {
+        foreach ($source_ig->getValidItems() as $item_ref_id) {
             if (isset($mappings[$item_ref_id]) and $mappings[$item_ref_id]) {
                 $ilLog->write(__METHOD__ . ': Clone item group item nr. ' . $item_ref_id);
                 $new_items[] = $mappings[$item_ref_id];
@@ -232,6 +246,12 @@ public static function _getItemsOfContainer(int $a_ref_id): array
         while ($row = $res->fetchRow(ilDBConstants::FETCHMODE_OBJECT)) {
             $items[] = $row->item_ref_id;
         }
-        return $items;
+
+        $container_child_ref_ids = [];
+        foreach ($tree->getChilds($a_ref_id) as $node) {
+            $container_child_ref_ids[] = (int) ($node['ref_id'] ?? $node['child']);
+        }
+
+        return array_values(array_intersect($items, $container_child_ref_ids));
     }
 }

components/ILIAS/ItemGroup/classes/class.ilItemGroupItemsTableGUI.php

@@ -26,7 +26,7 @@
 class ilItemGroupItemsTableGUI extends ilTable2GUI
 {
     protected InternalGUIService $gui;
-    protected array $items;
+    protected array $valid_items;
     protected ilItemGroupItems $item_group_items;
     protected ilTree $tree;
     protected ilObjectDefinition $obj_def;
@@ -50,7 +50,7 @@ public function __construct(
         $this->obj_def = $objDefinition;
 
         $this->item_group_items = new ilItemGroupItems($a_parent_obj->getObject()->getRefId());
-        $this->items = $this->item_group_items->getItems();
+        $this->valid_items = $this->item_group_items->getValidItems();
 
         parent::__construct($a_parent_obj, $a_parent_cmd);
         $this->setLimit(9999);
@@ -75,7 +75,7 @@ public function getMaterials(): void
         $items = $this->item_group_items->getAssignableItems();
 
         foreach ($items as $item) {
-            $item["sorthash"] = (int) (!in_array($item['ref_id'], $this->items)) . $item["title"];
+            $item["sorthash"] = (int) (!in_array($item['ref_id'], $this->valid_items)) . $item["title"];
             $materials[] = $item;
         }
 
@@ -101,7 +101,7 @@ protected function fillRow(array $a_set): void
             "ilIcon"
         ));
 
-        if (in_array($a_set["child"], $this->items)) {
+        if (in_array($a_set["child"], $this->valid_items)) {
             $i = $f->symbol()->icon()->custom(
                 ilUtil::getImagePath("standard/icon_ok.svg"),
                 $this->lng->txt("yes")

components/ILIAS/ItemGroup/module.xml

@@ -14,4 +14,7 @@
 			<parent id="root">root</parent>		
 		</object>	
 	</objects>
+	<events>
+		<event type="listen" id="components/ILIAS/ILIASObject" />
+	</events>
 </module>

components/ILIAS/ItemGroup/src/Repository/ItemGroupRepository.php

@@ -0,0 +1,50 @@
+<?php
+
+/**
+ * This file is part of ILIAS, a powerful learning management system
+ * published by ILIAS open source e-Learning e.V.
+ *
+ * ILIAS is licensed with the GPL-3.0,
+ * see https://www.gnu.org/licenses/gpl-3.0.en.html
+ * You should have received a copy of said license along with the
+ * source code, too.
+ *
+ * If this is not the case or you just want to try ILIAS, you'll find
+ * us at:
+ * https://www.ilias.de
+ * https://github.com/ILIAS-eLearning
+ *
+ *********************************************************************/
+
+declare(strict_types=1);
+
+namespace ILIAS\ItemGroup\Repository;
+
+use ilDBConstants;
+use ilDBInterface;
+
+class ItemGroupRepository
+{
+    public function __construct(
+        private readonly ilDBInterface $db,
+    ) {
+    }
+
+    /**
+     * @param int[] $item_ref_ids
+     */
+    public function removeItems(array $item_ref_ids, ?int $item_group_id = null): void
+    {
+        if ($item_ref_ids === []) {
+            return;
+        }
+
+        $query = "DELETE FROM item_group_item WHERE ({$this->db->in('item_ref_id', $item_ref_ids, false, ilDBConstants::T_INTEGER)})";
+
+        if (is_int($item_group_id) && $item_group_id > 0) {
+            $query .= " AND item_group_id = {$this->db->quote($item_group_id, ilDBConstants::T_INTEGER)}";
+        }
+
+        $this->db->manipulate($query);
+    }
+}