introduce StudygroupInvitation model and use it as relation for course and user and cleanup the orphaned studygroup invitations, fixes #5195

Closes #5195

Merge request studip/studip!3879

7 files changed, 109 insertions, 35 deletions

diff --git a/app/controllers/course/studygroup.php b/app/controllers/course/studygroup.php
index c95c20e..30048e8 100644
--- a/app/controllers/course/studygroup.php
+++ b/app/controllers/course/studygroup.php
@@ -288,7 +288,7 @@ class Course_StudygroupController extends AuthenticatedController
                             foreach ($tags as $tag) {
                                 $tag_ids[] = $tag->id;
                                 $relation = TagRelation::findOneBySQL(
-                                    "`range_id` = :course_id AND `range_type` = 'course' AND `tag_id` = :tag_id", 
+                                    "`range_id` = :course_id AND `range_type` = 'course' AND `tag_id` = :tag_id",
                                     [
                                         'tag_id'    => $tag->id,
                                         'course_id' => $course->id
@@ -303,7 +303,7 @@ class Course_StudygroupController extends AuthenticatedController
                                 }
                             }
                             TagRelation::deleteBySQL(
-                                "`range_id` = :course_id AND `range_type` = 'course' AND `tag_id` NOT IN (:ids)", 
+                                "`range_id` = :course_id AND `range_type` = 'course' AND `tag_id` NOT IN (:ids)",
                                 [
                                     'ids'       => $tag_ids,
                                     'course_id' => $course->id
@@ -446,7 +446,7 @@ class Course_StudygroupController extends AuthenticatedController
         $this->autors           = $sem->getMembersWithStatus('autor');
         $this->accepted         = $sem->admission_applicants->findBy('status', 'accepted');
         $this->sem_class        = $sem->getSemClass();
-        $this->invitedMembers   = StudygroupModel::getInvitations($id);
+        $this->invitedMembers   = StudygroupInvitation::findBySQL('sem_id = ? ORDER BY mkdate', $id);
         $this->rechte           = $GLOBALS['perm']->have_studip_perm('tutor', $id);
         $this->page = null;
         $this->setupMembersSidebar($sem);
diff --git a/app/views/course/studygroup/members.php b/app/views/course/studygroup/members.php
index c12d97b..503991d 100644
--- a/app/views/course/studygroup/members.php
+++ b/app/views/course/studygroup/members.php
@@ -1,3 +1,19 @@
+<?php
+/**
+ * @var Course_StudygroupController $controller
+ * @var int $anzahl
+ * @var int $page
+ * @var string $sem_id
+ * @var array $moderators
+ * @var array $tutors
+ * @var array $autors
+ * @var array $accepted
+ * @var bool $rechte
+ * @var string $view
+ * @var array $sem_class
+ * @var StudygroupInvitation[] $invitedMembers
+ */
+?>
 <?= $this->render_partial('course/studygroup/_feedback', compact('anzahl', 'page', 'sem_id')) ?>
 
 <? if (count($moderators) > 0): ?>
@@ -126,20 +142,20 @@
                 <? foreach ($invitedMembers as $p): ?>
                     <tr>
                         <td>
-                            <input type="checkbox" name="members[]" value="<?= htmlReady($p['username']) ?>">
+                            <input type="checkbox" name="members[]" value="<?= htmlReady($p->user->username) ?>">
                         </td>
                         <td>
-                            <a href="<?= URLHelper::getLink('dispatch.php/profile' , ['username' => $p['username']]) ?>">
-                                <?= Avatar::getAvatar($p['user_id'])->getImageTag(Avatar::SMALL) ?>
+                            <a href="<?= URLHelper::getLink('dispatch.php/profile' , ['username' => $p->user->username]) ?>">
+                                <?= Avatar::getAvatar($p->user_id)->getImageTag(Avatar::SMALL) ?>
                             </a>
                         </td>
                         <td>
-                            <a href="<?= URLHelper::getLink('dispatch.php/profile' , ['username' => $p['username']]) ?>">
-                                <?= htmlReady($p['fullname']) ?>
+                            <a href="<?= URLHelper::getLink('dispatch.php/profile' , ['username' => $p->user->username]) ?>">
+                                <?= htmlReady($p->user->getFullName()) ?>
                             </a>
                         </td>
                         <td class="actions">
-                            <a href="<?= $controller->edit_members('cancelInvitation', ['user' => $p['username']]) ?>" data-confirm="<?= _('Wollen Sie die Einladung wirklich löschen?') ?>">
+                            <a href="<?= $controller->edit_members('cancelInvitation', ['user' => $p->user->username]) ?>" data-confirm="<?= _('Wollen Sie die Einladung wirklich löschen?') ?>">
                                 <?= Icon::create('trash')->asImg(['title' => _('Einladung löschen')]) ?>
                             </a>
                         </td>
diff --git a/db/migrations/5.3.28_cleanup_studygroup_invitations.php b/db/migrations/5.3.28_cleanup_studygroup_invitations.php
new file mode 100644
index 0000000..9c67224
--- /dev/null
+++ b/db/migrations/5.3.28_cleanup_studygroup_invitations.php
@@ -0,0 +1,18 @@
+<?php
+return new class extends Migration {
+    public function description()
+    {
+        return 'Removes orphaned entries in table "studygroup_invitations"';
+    }
+
+    protected function up()
+    {
+        $query = "DELETE FROM `studygroup_invitations`
+                  WHERE `user_id` NOT IN (
+                      SELECT `user_id` FROM `auth_user_md5`
+                  ) OR `sem_id` NOT IN (
+                      SELECT `Seminar_id` FROM `seminare`
+                  )";
+        DBManager::get()->exec($query);
+    }
+};
diff --git a/lib/classes/StudygroupModel.php b/lib/classes/StudygroupModel.php
index 09cec16..f0d9686 100644
--- a/lib/classes/StudygroupModel.php
+++ b/lib/classes/StudygroupModel.php
@@ -431,32 +431,33 @@ class StudygroupModel
     /**
      * invites a member to a given studygroup.
      *
-     * @param string user id
-     * @param string id of a studygroup
+     * @param string $user_id
+     * @param string $sem_id id of a studygroup
      */
     public static function inviteMember($user_id, $sem_id)
     {
-        $query = "REPLACE INTO studygroup_invitations (sem_id, user_id, mkdate)
-                  VALUES (?, ?, UNIX_TIMESTAMP())";
-        $stmt = DBManager::get()->prepare($query);
-        $stmt->execute([$sem_id, $user_id]);
+        $invitation = new StudygroupInvitation([$sem_id, $user_id]);
+        $invitation->mkdate = time();
+        $invitation->store();
     }
 
     /**
      * cancels invitation.
      *
-     * @param string username
-     * @param string id of a studygroup
+     * @param string $username
+     * @param string $sem_id id of a studygroup
      */
     public static function cancelInvitation($username, $sem_id)
     {
-        $query = "DELETE FROM studygroup_invitations
-                  WHERE sem_id = ? AND user_id = ?";
-        $statement = DBManager::get()->prepare($query);
-        $statement->execute([
-            $sem_id,
-            get_userid($username)
-        ]);
+        $user = User::findOneByUsername($username);
+        if (!$user) {
+            return;
+        }
+
+        StudygroupInvitation::deleteBySQL(
+            'sem_id = ? AND user_id = ?',
+            [$sem_id, $user->id]
+        );
     }
 
     /**
@@ -464,6 +465,7 @@ class StudygroupModel
      *
      * @param string id of a studygroup
      * @return array invited members
+     * @deprecated Will be removed in Stud.IP 6.2
      */
     public static function getInvitations($sem_id)
     {
@@ -474,7 +476,7 @@ class StudygroupModel
                   LEFT JOIN auth_user_md5 USING (user_id)
                   LEFT JOIN user_info USING (user_id)
                   WHERE studygroup_invitations.sem_id = ?
-                  ORDER BY studygroup_invitations.mkdate ASC";
+                  ORDER BY studygroup_invitations.mkdate";
 
         $stmt = DBManager::get()->prepare($query);
         $stmt->execute([$sem_id]);
@@ -485,19 +487,16 @@ class StudygroupModel
     /**
      * checks if a user is already invited.
      *
-     * @param string user id
-     * @param string id of a studygroup
-     * @return array invited members
+     * @param string $user_id
+     * @param string $sem_id id of a studygroup
+     * @return bool
      */
     public static function isInvited($user_id, $sem_id)
     {
-        $query = "SELECT 1
-                  FROM studygroup_invitations
-                  WHERE user_id = ? AND sem_id = ?";
-        $stmt = DBManager::get()->prepare($query);
-        $stmt->execute([$user_id, $sem_id]);
-
-        return (bool) $stmt->fetchColumn();
+        return (bool) StudygroupInvitation::countBySql(
+            'sem_id = ? AND user_id = ?',
+            [$sem_id, $user_id]
+        );
     }
 
     /**
diff --git a/lib/models/Course.php b/lib/models/Course.php
index 4fc32e3..4fa8222 100644
--- a/lib/models/Course.php
+++ b/lib/models/Course.php
@@ -304,6 +304,12 @@ class Course extends SimpleORMap implements Range, PrivacyObject, StudipItem, Fe
             'assoc_foreign_key' => 'course_id',
             'on_delete'         => 'delete',
         ];
+        $config['has_many']['studygroup_invitations'] = [
+            'class_name' => StudygroupInvitation::class,
+            'assoc_foreign_key' => 'sem_id',
+            'on_delete'  => 'delete',
+        ];
+
 
         $config['default_values']['lesezugriff'] = 1;
         $config['default_values']['schreibzugriff'] = 1;
diff --git a/lib/models/StudygroupInvitation.php b/lib/models/StudygroupInvitation.php
new file mode 100644
index 0000000..c7d09dc
--- /dev/null
+++ b/lib/models/StudygroupInvitation.php
@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * @property array $id
+ * @property string $sem_id
+ * @property string $user_id
+ * @property int $mkdate
+ *
+ * @property User $user
+ * @property Course $course
+ */
+final class StudygroupInvitation extends SimpleORMap
+{
+    protected static function configure($config = [])
+    {
+        $config['db_table'] = 'studygroup_invitations';
+
+        $config['belongs_to']['course'] = [
+            'class_name' => Course::class,
+            'foreign_key' => 'sem_id',
+        ];
+        $config['belongs_to']['user'] = [
+            'class_name' => User::class,
+            'foreign_key' => 'user_id',
+        ];
+
+        parent::configure($config);
+    }
+}
diff --git a/lib/models/User.php b/lib/models/User.php
index 190df2b..45c3b91 100644
--- a/lib/models/User.php
+++ b/lib/models/User.php
@@ -223,6 +223,12 @@ class User extends AuthUserMd5 implements Range, PrivacyObject, Studip\Calendar\
             'on_delete'  => 'delete',
         ];
 
+        $config['has_many']['studygroup_invitations'] = [
+            'class_name' => StudygroupInvitation::class,
+            'assoc_foreign_key' => 'user_id',
+            'on_delete'  => 'delete',
+        ];
+
         $config['additional_fields']['config']['get'] = function ($user) {
             return UserConfig::get($user->id);
         };