From eccec27f081020f42f20508b88ad420c0f489907 Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Thu, 30 Jun 2022 08:16:44 +0000
Subject: [PATCH] fix errors in mvv models, fixes #1230

Closes #1230

Merge request studip/studip!741
---
 lib/classes/ModuleManagementInterface.php    | 14 ++++++
 lib/models/Lvgruppe.php                      |  2 +-
 lib/models/ModuleManagementModel.php         |  6 +--
 lib/models/ModuleManagementModelTreeItem.php | 48 ++++++++++----------
 lib/models/Modulteil.php                     |  4 +-
 lib/models/MvvCourse.php                     |  4 +-
 lib/models/MvvFileFileref.php                |  6 ++-
 7 files changed, 50 insertions(+), 34 deletions(-)
 create mode 100644 lib/classes/ModuleManagementInterface.php

diff --git a/lib/classes/ModuleManagementInterface.php b/lib/classes/ModuleManagementInterface.php
new file mode 100644
index 00000000000..b5594c200f3
--- /dev/null
+++ b/lib/classes/ModuleManagementInterface.php
@@ -0,0 +1,14 @@
+<?php
+/**
+ * This interface ensures that all objects of ModuleManagementModel have
+ * the same constructor signature. Otherwise, we can not guarantee that the
+ * use of "new static()" in ModuleManagement code will always do the right
+ * things.
+ *
+ * @author Jan-Hendrik Willms <tleilax+studip@gmail.com>
+ * @license GPL2 or any later version
+ */
+interface ModuleManagementInterface
+{
+    public function __construct($id = null);
+}
diff --git a/lib/models/Lvgruppe.php b/lib/models/Lvgruppe.php
index e3a36207659..6983e01a062 100644
--- a/lib/models/Lvgruppe.php
+++ b/lib/models/Lvgruppe.php
@@ -363,7 +363,7 @@ class Lvgruppe extends ModuleManagementModelTreeItem
      */
     public function getTrailParent()
     {
-        return Modul::findCached($this->getTrailParent_id());
+        return Modul::findCached($this->getTrailParentId());
     }
 
     /**
diff --git a/lib/models/ModuleManagementModel.php b/lib/models/ModuleManagementModel.php
index f9d7064ef74..db565b94de5 100644
--- a/lib/models/ModuleManagementModel.php
+++ b/lib/models/ModuleManagementModel.php
@@ -16,7 +16,7 @@
 
 require_once 'config/mvv_config.php';
 
-abstract class ModuleManagementModel extends SimpleORMap
+abstract class ModuleManagementModel extends SimpleORMap implements ModuleManagementInterface
 {
     /**
      * Usable as option for ModuleManagementModel::getDisplayName().
@@ -133,7 +133,7 @@ abstract class ModuleManagementModel extends SimpleORMap
     /**
      * Returns an object by given id with all relations and additional fields.
      *
-     * @param tring $id The id of the object.
+     * @param string $id The id of the object.
      * @return ModuleManagementModel
      */
     public static function getEnriched($id)
@@ -493,7 +493,7 @@ abstract class ModuleManagementModel extends SimpleORMap
      * @param array $params Array with the parameters used in query
      * @param int $row_count Number of rows to return
      * @param int $offset Offset where the result set starts
-     * @return object SimpleOrMapCollection with all found objects or empty array
+     * @return SimpleOrMapCollection with all found objects or empty array
      */
     public static function getEnrichedByQuery($query = null, $params = [],
             $row_count = null, $offset = null)
diff --git a/lib/models/ModuleManagementModelTreeItem.php b/lib/models/ModuleManagementModelTreeItem.php
index c757b5cbfdb..1c4a6deb6d1 100644
--- a/lib/models/ModuleManagementModelTreeItem.php
+++ b/lib/models/ModuleManagementModelTreeItem.php
@@ -7,20 +7,18 @@
  * modify it under the terms of the GNU General Public License as
  * published by the Free Software Foundation; either version 2 of
  * the License, or (at your option) any later version.
- * 
+ *
  * @author      Peter Thienel <thienel@data-quest.de>
  * @license     http://www.gnu.org/licenses/gpl-2.0.html GPL version 2
  * @category    Stud.IP
  * @since       3.5
  */
 
-require_once 'ModuleManagementModel.php';
-
 abstract class ModuleManagementModelTreeItem extends ModuleManagementModel implements MvvTreeItem
 {
     /**
      * The default route through the MVV object structure.
-     * 
+     *
      * @var array
      */
     public static $TRAIL_DEFAULT = [
@@ -36,15 +34,15 @@ abstract class ModuleManagementModelTreeItem extends ModuleManagementModel imple
         'Abschluss',
         'AbschlussKategorie'
         ];
-    
+
     /**
      * An array of functions to filter mvv objects during path creation.
      * The class name is the key and the filter function the value.
-     * 
+     *
      * @var array
      */
-    protected static $object_filter = []; 
-    
+    protected static $object_filter = [];
+
     /**
      * @see MvvTreeItem::getTrailParentId()
      */
@@ -52,7 +50,7 @@ abstract class ModuleManagementModelTreeItem extends ModuleManagementModel imple
     {
         return ($_SESSION['MVV/' . get_class() . '/trail_parent_id']);
     }
-    
+
     public function getTrails($types = null, $mode = null, $path = null, $in_recursion = false)
     {
         $path = $path ?: self::$TRAIL_DEFAULT;
@@ -61,7 +59,7 @@ abstract class ModuleManagementModelTreeItem extends ModuleManagementModel imple
         $class_name = get_class($this);
         $next = $path[array_search($class_name, $path) + 1];
         $parents = $this->getParents($next);
-        
+
         foreach ($parents as $parent) {
             if ($parent) {
                 if ($this->checkFilter($parent)) {
@@ -81,17 +79,17 @@ abstract class ModuleManagementModelTreeItem extends ModuleManagementModel imple
                 }
             }
         }
-        
+
         if (empty($trails) && in_array($class_name, $types)) {
             $trails = [[$class_name => $this]];
         }
-        
+
         return $trails;
     }
-    
+
     /**
      * Checks trails object filter.
-     * 
+     *
      * @param MvvTreeItem $item The item to check.
      * @return boolean True if item has passed the check.
      */
@@ -106,22 +104,22 @@ abstract class ModuleManagementModelTreeItem extends ModuleManagementModel imple
         }
         return true;
     }
-    
+
     protected static function sortTrails($trail_a, $trail_b)
     {
-        
+
     }
-    
+
     /**
      * Returns whether this object is assignable to courses.
-     * 
+     *
      * @return boolean True if the object is assignable.
      */
     public function isAssignable()
     {
         return false;
     }
-    
+
     /**
      * @see MvvTreeItem::hasChildren()
      */
@@ -129,11 +127,11 @@ abstract class ModuleManagementModelTreeItem extends ModuleManagementModel imple
     {
         return count($this->getChildren()) > 0;
     }
-    
+
     /**
      * Formats the trails to pathes. The path consists of alle names of the
      * objects of a trail glued together with the given delimiter.
-     * 
+     *
      * @param array $trails All trails as array.
      * @param string $delimiter A string used as the "glue".
      * @param int $display_options Display options set by constants defined
@@ -153,10 +151,10 @@ abstract class ModuleManagementModelTreeItem extends ModuleManagementModel imple
         sort($pathes, SORT_LOCALE_STRING);
         return $pathes;
     }
-    
+
     /**
      * Filters trails by given object types.
-     * 
+     *
      * @param array $trails An array of trails.
      * @param array $filter_objects An array of object class names.
      * @return array The filtered trails.
@@ -183,11 +181,11 @@ abstract class ModuleManagementModelTreeItem extends ModuleManagementModel imple
         }
         return $filtered_trails;
     }
-    
+
     /**
      * Stores filter function to restrict pathes only to objects fulfilling
      * all conditions defined in this function.
-     * 
+     *
      * @param string $class_name The name of the class.
      * @param Closure $filter_func The function defining the filter.
      * @param array $params Parameters used by filter function.
diff --git a/lib/models/Modulteil.php b/lib/models/Modulteil.php
index e1ee79b9f3e..deffb5e4dba 100644
--- a/lib/models/Modulteil.php
+++ b/lib/models/Modulteil.php
@@ -97,7 +97,7 @@ class Modulteil extends ModuleManagementModelTreeItem
     /**
      * Retrieves all Modulteile of the given Modul.
      *
-     * @param type $modul_id The id of a Modul.
+     * @param string $modul_id The id of a Modul.
      * @return SimpleORMapCollection A collection of Modulteile.
      */
     public static function findByModul($modul_id)
@@ -117,7 +117,7 @@ class Modulteil extends ModuleManagementModelTreeItem
     /**
      * Retrieves all Modulteile the given LV-Gruppe is assigned to.
      *
-     * @param type $lvgruppe_id The id of a LV-Gruppe.
+     * @param string $lvgruppe_id The id of a LV-Gruppe.
      * @return SimpleORMapCollection A collection of Modulteile.
      */
     public static function findByLvgruppe($lvgruppe_id)
diff --git a/lib/models/MvvCourse.php b/lib/models/MvvCourse.php
index a003351a7f3..801a08e99bb 100644
--- a/lib/models/MvvCourse.php
+++ b/lib/models/MvvCourse.php
@@ -37,7 +37,7 @@ class MvvCourse extends ModuleManagementModelTreeItem
      */
     public function getTrailParent()
     {
-        return LvGruppe::findCached($this->getTrailParent_id());
+        return LvGruppe::findCached($this->getTrailParentId());
     }
 
     /**
@@ -58,7 +58,7 @@ class MvvCourse extends ModuleManagementModelTreeItem
 
     public function getDisplayName($options = self::DISPLAY_DEFAULT)
     {
-        $this->getName();
+        return $this->name;
     }
 
     /**
diff --git a/lib/models/MvvFileFileref.php b/lib/models/MvvFileFileref.php
index 34e7d671cb6..13be9e15415 100644
--- a/lib/models/MvvFileFileref.php
+++ b/lib/models/MvvFileFileref.php
@@ -44,12 +44,14 @@ class MvvFileFileref extends ModuleManagementModel
      *
      * @return string Name of the file.
      */
-    public function getFilename()
+    public function getFilename(): string
     {
         if ($this->file_ref) {
             $filetype = $this->file_ref->getFileType();
             return $filetype->getFilename();
         }
+
+        throw new Exception("Could not load file ref for file");
     }
 
     /**
@@ -92,5 +94,7 @@ class MvvFileFileref extends ModuleManagementModel
                 }
             }
         }
+
+        return false;
     }
 }
-- 
GitLab