TIC 4391, closes #4391

Closes #4391

Merge request studip/studip!3211

2 files changed, 142 insertions, 140 deletions

diff --git a/lib/models/Course.php b/lib/models/Course.php
index 25350db..9607b91 100644
--- a/lib/models/Course.php
+++ b/lib/models/Course.php
@@ -25,8 +25,6 @@
  * @property string|null $sonstiges database column
  * @property int $lesezugriff database column
  * @property int $schreibzugriff database column
- * @property int|null $start_time database column
- * @property int|null $duration_time database column
  * @property I18NString|null $art database column
  * @property I18NString|null $teilnehmer database column
  * @property I18NString|null $vorrausetzungen database column
@@ -82,6 +80,16 @@
 
 class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, FeedbackRange, Studip\Calendar\Owner
 {
+    /**
+     * @var Semester initial start semester.
+     */
+    protected $initial_start_semester;
+
+    /**
+     * @var Semester initial end semester.
+     */
+    protected $initial_end_semester;
+
     protected static function configure($config = [])
     {
         $config['db_table'] = 'seminare';
@@ -261,7 +269,6 @@ class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, Fe
 
         $config['default_values']['lesezugriff'] = 1;
         $config['default_values']['schreibzugriff'] = 1;
-        $config['default_values']['duration_time'] = 0;
 
         $config['additional_fields']['teachers'] = [
             'get' => 'getTeachers'
@@ -299,8 +306,7 @@ class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, Fe
         $config['i18n_fields']['leistungsnachweis'] = true;
         $config['i18n_fields']['ort'] = true;
 
-        $config['registered_callbacks']['before_update'][] = 'logStore';
-        $config['registered_callbacks']['before_store'][] = 'cbSetStartAndDurationTime';
+        $config['registered_callbacks']['before_store'][] = 'logStore';
         $config['registered_callbacks']['after_create'][] = 'setDefaultTools';
         $config['registered_callbacks']['after_delete'][] = function ($course) {
             CourseAvatar::getAvatar($course->id)->reset();
@@ -367,6 +373,31 @@ class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, Fe
         parent::configure($config);
     }
 
+    /**
+     * @param string $relation
+     */
+    public function initRelation($relation): void
+    {
+        if ($relation === 'semesters' && $this->relations[$relation] === null) {
+            parent::initRelation($relation);
+            $this->initial_start_semester = $this->getStartSemester();
+            $this->initial_end_semester = $this->getEndSemester();
+        }
+        parent::initRelation($relation);
+    }
+
+    /**
+     * Override of SimpleORMap::cbAfterInitialize for resetting the flags that indicate semester changes.
+     *
+     * @see SimpleORMap::cbAfterInitialize
+     */
+    protected function cbAfterInitialize($cb_type): void
+    {
+        parent::cbAfterInitialize($cb_type);
+        //Reset the flags for the start and end semester:
+        $this->initial_start_semester = null;
+        $this->initial_end_semester = null;
+    }
 
     /**
      * Returns the currently active course or false if none is active.
@@ -411,22 +442,6 @@ class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, Fe
         });
     }
 
-    public function getEnd_Time()
-    {
-        return $this->duration_time == -1 ? -1 : $this->start_time + $this->duration_time;
-    }
-
-    public function setEnd_Time($value)
-    {
-        if ($value == -1) {
-            $this->duration_time = -1;
-        } elseif ($this->start_time > 0 && $value > $this->start_time) {
-            $this->duration_time = $value - $this->start_time;
-        } else {
-            $this->duration_time = 0;
-        }
-    }
-
     public function _set_semester($field, $value)
     {
         $method = 'set' . ($field === 'start_semester' ? 'StartSemester' : 'EndSemester');
@@ -497,11 +512,11 @@ class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, Fe
      */
     public function getStartSemester()
     {
-        if (count($this->semesters) > 0) {
-            return $this->semesters->first();
-        } else {
-            return Semester::findCurrent();
+        //this is called by __get() and therefore using magic properties is not always safe
+        if ($this->relations['semesters'] === null) {
+            $this->initRelation('semesters');
         }
+        return $this->relations['semesters']->first() ?? Semester::findCurrent();
     }
 
     /**
@@ -512,9 +527,11 @@ class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, Fe
      */
     public function getEndSemester()
     {
-        if (count($this->semesters) > 0) {
-            return $this->semesters->last();
+        //this is called by __get() and therefore using magic properties is not always safe
+        if ($this->relations['semesters'] === null) {
+            $this->initRelation('semesters');
         }
+        return $this->relations['semesters']->last();
     }
 
     /**
@@ -2133,86 +2150,66 @@ class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, Fe
      */
     protected function logStore()
     {
-        if ($this->isFieldDirty('start_time')) {
-            //Log change of start semester:
-            StudipLog::log('SEM_SET_STARTSEMESTER', $this->id, isset($this->start_semester) ? $this->start_semester->name : _('unbegrenzt'));
-            NotificationCenter::postNotification('CourseDidChangeSchedule', $this);
-        }
-        if ($this->isFieldDirty('duration_time')) {
-            StudipLog::log('SEM_SET_ENDSEMESTER', $this->id, $this->getTextualSemester());
-            NotificationCenter::postNotification('CourseDidChangeSchedule', $this);
-        }
-
-        $log = [];
-        if ($this->isFieldDirty('admission_prelim')) {
-            $log[] = $this->admission_prelim ?  _('Neuer Anmeldemodus: Vorläufiger Eintrag') : _('Neuer Anmeldemodus: Direkter Eintrag');
-        }
+        if (!$this->isNew()) {
+            if ($this->initial_start_semester?->id !== $this->start_semester?->id) {
+                //Log change of start semester:
+                StudipLog::log('SEM_SET_STARTSEMESTER', $this->id, isset($this->start_semester) ? $this->start_semester->name : _('unbegrenzt'));
+                NotificationCenter::postNotification('CourseDidChangeSchedule', $this);
+            }
+            if ($this->initial_end_semester?->id !== $this->end_semester?->id) {
+                StudipLog::log('SEM_SET_ENDSEMESTER', $this->id, $this->getTextualSemester());
+                NotificationCenter::postNotification('CourseDidChangeSchedule', $this);
+            }
 
-        if ($this->isFieldDirty('admission_binding')) {
-            $log[] = $this->admission_binding? _('Anmeldung verbindlich') : _('Anmeldung unverbindlich');
-        }
+            $log = [];
+            if ($this->isFieldDirty('admission_prelim')) {
+                $log[] = $this->admission_prelim ? _('Neuer Anmeldemodus: Vorläufiger Eintrag') : _('Neuer Anmeldemodus: Direkter Eintrag');
+            }
 
-        if ($this->isFieldDirty('admission_turnout')) {
-            $log[] = sprintf(_('Neue Teilnehmerzahl: %s'), (int)$this->admission_turnout);
-        }
+            if ($this->isFieldDirty('admission_binding')) {
+                $log[] = $this->admission_binding ? _('Anmeldung verbindlich') : _('Anmeldung unverbindlich');
+            }
 
-        if ($this->isFieldDirty('admission_disable_waitlist')) {
-            $log[] = $this->admission_disable_waitlist ? _('Warteliste aktiviert') : _('Warteliste deaktiviert');
-        }
+            if ($this->isFieldDirty('admission_turnout')) {
+                $log[] = sprintf(_('Neue Teilnehmerzahl: %s'), (int)$this->admission_turnout);
+            }
 
-        if ($this->isFieldDirty('admission_waitlist_max')) {
-            $log[] = sprintf(_('Plätze auf der Warteliste geändert: %u'), (int)$this->admission_waitlist_max);
-        }
+            if ($this->isFieldDirty('admission_disable_waitlist')) {
+                $log[] = $this->admission_disable_waitlist ? _('Warteliste aktiviert') : _('Warteliste deaktiviert');
+            }
 
-        if ($this->isFieldDirty('admission_disable_waitlist_move')) {
-            $log[] = $this->admission_disable_waitlist ? _('Nachrücken aktiviert') : _('Nachrücken deaktiviert');
-        }
+            if ($this->isFieldDirty('admission_waitlist_max')) {
+                $log[] = sprintf(_('Plätze auf der Warteliste geändert: %u'), (int)$this->admission_waitlist_max);
+            }
 
-        if ($this->isFieldDirty('admission_prelim_txt')) {
-            if ($this->admission_prelim_txt) {
-                $log[] = sprintf(_('Neuer Hinweistext bei vorläufigen Eintragungen: %s'), strip_tags(kill_format($this->admission_prelim_txt)));
-            } else {
-                $log[] = _('Hinweistext bei vorläufigen Eintragungen wurde entfert');
+            if ($this->isFieldDirty('admission_disable_waitlist_move')) {
+                $log[] = $this->admission_disable_waitlist ? _('Nachrücken aktiviert') : _('Nachrücken deaktiviert');
             }
-        }
 
-        if (!empty($log)) {
-            StudipLog::log(
-                'SEM_CHANGED_ACCESS',
-                $this->id,
-                null,
-                '',
-                implode(' - ', $log)
-            );
-        }
+            if ($this->isFieldDirty('admission_prelim_txt')) {
+                if ($this->admission_prelim_txt) {
+                    $log[] = sprintf(_('Neuer Hinweistext bei vorläufigen Eintragungen: %s'), strip_tags(kill_format($this->admission_prelim_txt)));
+                } else {
+                    $log[] = _('Hinweistext bei vorläufigen Eintragungen wurde entfert');
+                }
+            }
 
-        if ($this->isFieldDirty('visible')) {
-            StudipLog::log($this->visible ? 'SEM_VISIBLE' : 'SEM_INVISIBLE', $this->id);
-        }
-    }
+            if (!empty($log)) {
+                StudipLog::log(
+                    'SEM_CHANGED_ACCESS',
+                    $this->id,
+                    null,
+                    '',
+                    implode(' - ', $log)
+                );
+            }
 
-    /**
-     * Called directly before storing the object to edit the columns start_time and duration_time
-     * which are both deprecated but are still in use for older plugins.
-     */
-    public function cbSetStartAndDurationTime()
-    {
-        if ($this->isFieldDirty('start_time')) {
-            $this->setStartSemester(Semester::findByTimestamp($this->start_time));
-        }
-        if ($this->isFieldDirty('duration_time')) {
-            $this->setEndSemester($this->duration_time == -1 ? null : Semester::findByTimestamp($this->start_time + $this->duration_time));
-        }
-        if ($this->isOpenEnded()) {
-            $this->start_time = $this->start_time ?: Semester::findCurrent()->beginn ?? time();
-            $this->duration_time = -1;
-        } else {
-            $this->start_time = $this->getStartSemester()->beginn;
-            $this->duration_time = $this->getEndSemester()->beginn - $this->start_time;
+            if ($this->isFieldDirty('visible')) {
+                StudipLog::log($this->visible ? 'SEM_VISIBLE' : 'SEM_INVISIBLE', $this->id);
+            }
         }
     }
 
-
     //StudipItem interface implementation:
 
     public function getItemName($long_format = true)
@@ -2342,7 +2339,7 @@ class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, Fe
             "LEFT JOIN semester_courses ON (semester_courses.course_id = seminare.Seminar_id)
              WHERE Seminar_id IN (?)
              GROUP BY seminare.Seminar_id
-             ORDER BY semester_courses.semester_id IS NULL DESC, start_time DESC, {$name_sort}",
+             ORDER BY semester_courses.semester_id IS NULL DESC, {$name_sort}",
             [$seminar_ids]
         );
     }
diff --git a/lib/models/Lvgruppe.php b/lib/models/Lvgruppe.php
index 05f3b59..bdd9f0b 100644
--- a/lib/models/Lvgruppe.php
+++ b/lib/models/Lvgruppe.php
@@ -126,14 +126,10 @@ class Lvgruppe extends ModuleManagementModelTreeItem
             $semester = Semester::find($semester_id);
             if ($semester) {
                 $filter_sql = trim($filter_sql) ? $filter_sql . ' AND' : $filter_sql . ' WHERE';
-                $filter_sql .= ' seminare.start_time <= :beginn '
-                        . 'AND (semester_courses.semester_id IS NULL OR semester_courses.semester_id = :semester_id) '
-                        . 'AND (start_sem.beginn <= :ende AND '
-                        . 'IF(ISNULL(end_sem.ende), 1, end_sem.ende >= :beginn)) ';
+                $filter_sql .= ' semester_courses.semester_id = :semester_id '
+                        . 'AND (semester_courses.semester_id IS NULL OR semester_courses.semester_id = :semester_id) ';
                 $params = [
-                    ':semester_id' => $semester->semester_id,
-                    ':beginn' => $semester->beginn,
-                    ':ende' => $semester->ende
+                    ':semester_id' => $semester->semester_id
                 ];
                 $semester_join = 'LEFT JOIN mvv_modul ON mvv_modul.modul_id = mvv_modulteil.modul_id '
                 . 'LEFT JOIN semester_data as start_sem ON start_sem.semester_id = mvv_modul.start '
@@ -197,14 +193,10 @@ class Lvgruppe extends ModuleManagementModelTreeItem
                 }
 
                 $filter_sql = trim($filter_sql) ? $filter_sql  : ' AND';
-                $filter_sql .= ' seminare.start_time <= :beginn '
-                        . 'AND (semester_courses.semester_id IS NULL OR semester_courses.semester_id = :semester_id) '
-                        . 'AND (start_sem.beginn <= :ende AND '
-                        . 'IF(ISNULL(end_sem.ende), 1, end_sem.ende >= :beginn)) ';
+                $filter_sql .= ' semester_courses.semester_id = :semester_id '
+                        . 'AND (semester_courses.semester_id IS NULL OR semester_courses.semester_id = :semester_id) ';
                 $params = [
-                    ':semester_id' => $semester->semester_id,
-                    ':beginn' => $semester->beginn,
-                    ':ende' => $semester->ende
+                    ':semester_id' => $semester->semester_id
                 ];
                 $semester_join = 'LEFT JOIN mvv_modul ON mvv_modul.modul_id = mvv_modulteil.modul_id '
                 . 'LEFT JOIN semester_data as start_sem ON start_sem.semester_id = mvv_modul.start '
@@ -517,48 +509,61 @@ class Lvgruppe extends ModuleManagementModelTreeItem
      *
      * @param bool $only_visible Return only visible courses.
      * @param string $semester_id Return only this semester.
-     * @return array All assigned courses grouped by semesters.
+     * @return Course[] All assigned courses grouped by semesters.
      */
     public function getAllAssignedCourses($only_visible = false, $semester_id = null)
     {
-        $sem_start_times = [];
-
+        $conditions_sql = '';
+        if ($only_visible) {
+            $conditions_sql .= " AND `seminare`.`visible` = '1'";
+        }
         if ($semester_id) {
             $semester = Semester::find($semester_id);
             if (!$semester) {
                 return [];
             }
-            $sem_start_times[$semester->id] = $semester->beginn;
+
+            //Find only courses that lie in that semester.
+            $courses = Course::findBySQL(
+                'LEFT JOIN `semester_courses` sc
+                 ON `seminare`.`seminar_id` = sc.`course_id`
+                 JOIN `mvv_lvgruppe_seminar` mls USING (`seminar_id`)
+                 WHERE
+                 mls.`lvgruppe_id` = :group_id
+                 AND (sc.`semester_id` = :semester_id OR sc.`semester_id` IS NULL)'
+                . $conditions_sql . '
+                ORDER BY `seminare`.`Name`',
+                [
+                    'group_id'    => $this->id,
+                    'semester_id' => $semester->id
+                ]
+            );
+            return [$semester->id => $courses];
         } else {
-            $sem_start_times = SimpleORMapCollection::createFromArray(
-                    Semester::getAll())->toGroupedArray('id', 'beginn');
-            $sem_start_times = array_map(
-                    function ($sem) { return $sem['beginn']; }
-                    , $sem_start_times);
-        }
-        $visible_sql = $only_visible ? ' AND visible = 1' : '';
-        $courses = [];
-        $stmt = DBManager::get()->prepare('SELECT seminar_id, Name, '
-                . 'VeranstaltungsNummer, visible, INTERVAL(start_time,'
-                . join(',', $sem_start_times)
-                . ') AS sem_number, '
-                . 'IF(duration_time=-1,' . count($sem_start_times)
-                . ',INTERVAL(start_time+duration_time,'
-                . join(',', $sem_start_times)
-                . ')) AS sem_number_end FROM seminare '
-                . 'INNER JOIN mvv_lvgruppe_seminar USING(seminar_id) '
-                . 'WHERE lvgruppe_id = ? ' . $visible_sql
-                . ' AND start_time <= ' . end($sem_start_times)
-                . ' ORDER BY sem_number DESC, Name');
-        $stmt->execute([$this->getId()]);
-        $sem_ids = array_keys($sem_start_times);
-        foreach ($stmt->fetchAll(PDO::FETCH_ASSOC) as $course) {
-            if ($course['sem_number'] == 0) $course['sem_number'] = 1;
-            for ($i = $course['sem_number']; $i <= $course['sem_number_end']; $i++) {
-                $courses[$sem_ids[$i-1]][] = $course;
+            //No semester specified. Find courses from all semesters.
+            $courses = Course::findBySQL(
+                'JOIN `mvv_lvgruppe_seminar` mls USING (`seminar_id`)
+                 WHERE
+                 mls.`lvgruppe_id` = :group_id '
+                . $conditions_sql . '
+                ORDER BY `seminare`.`Name`',
+                [
+                    'group_id'    => $this->id
+                ]
+            );
+            $data = [];
+            foreach ($courses as $course) {
+                if (!$course->start_semester) {
+                    //An invalid course that cannot be grouped by a semester.
+                    continue;
+                }
+                if (!array_key_exists($course->start_semester->id, $data)) {
+                    $data[$course->start_semester->id] = [];
+                }
+                $data[$course->start_semester->id][] = $course;
             }
+            return $data;
         }
-        return $courses;
     }
 
     /**