fixes #613

Closes #613

Merge request studip/studip!1801

5 files changed, 123 insertions, 38 deletions

diff --git a/app/controllers/course/members.php b/app/controllers/course/members.php
index bcbea42..822f4a4 100644
--- a/app/controllers/course/members.php
+++ b/app/controllers/course/members.php
@@ -93,10 +93,6 @@ class Course_MembersController extends AuthenticatedController
 
     public function index_action()
     {
-        if (!$this->is_tutor && $this->config->COURSE_MEMBERS_HIDE) {
-            throw new AccessDeniedException();
-        }
-
         $course             = Course::find($this->course_id);
         $this->sort_by      = Request::option('sortby', 'nachname');
         $this->order        = Request::option('order', 'desc');
@@ -1820,7 +1816,7 @@ class Course_MembersController extends AuthenticatedController
             $options = new OptionsWidget();
             $options->addCheckbox(
                 _('Diese Seite für Studierende verbergen'),
-                $this->config->COURSE_MEMBERS_HIDE,
+                $this->getToolActivation()->getVisibilityPermission() === 'tutor',
                 $this->url_for('course/members/course_members_hide/1'),
                 $this->url_for('course/members/course_members_hide/0'),
                 ['title' => _('Über diese Option können Sie die Teilnehmendenliste für Studierende der Veranstaltung unsichtbar machen')]
@@ -1916,7 +1912,13 @@ class Course_MembersController extends AuthenticatedController
             throw new AccessDeniedException();
         }
 
-        $this->config->store('COURSE_MEMBERS_HIDE', $state);
+        $tool_activation = $this->getToolActivation();
+        if ($state) {
+            $tool_activation->setVisibilityPermission(ToolActivation::VISIBILITY_PERMISSION_TEACHERS);
+        } else {
+            $tool_activation->setVisibilityPermission(ToolActivation::VISIBILITY_PERMISSION_STUDENTS);
+        }
+        $tool_activation->store();
 
         $this->redirect($this->indexURL());
     }
@@ -2187,7 +2189,6 @@ class Course_MembersController extends AuthenticatedController
         return sprintf('%s %s', $directionString, $log_level);
     }
 
-
     /**
      * Checks whether a tutor is attempting to add or remove tutors or
      * instructors.
@@ -2206,4 +2207,15 @@ class Course_MembersController extends AuthenticatedController
         }
     }
 
+    private function getToolActivation(): ToolActivation
+    {
+        return ToolActivation::findOneBySQL(
+            "range_id = ? AND range_type = 'course' AND plugin_id IN (
+                SELECT pluginid
+                FROM plugins
+                WHERE pluginclassname = 'CoreParticipants'
+            )",
+            [$this->course_id]
+        );
+    }
 }
diff --git a/app/controllers/course/statusgroups.php b/app/controllers/course/statusgroups.php
index df8c86e..cbd80ad 100644
--- a/app/controllers/course/statusgroups.php
+++ b/app/controllers/course/statusgroups.php
@@ -38,11 +38,6 @@ class Course_StatusgroupsController extends AuthenticatedController
         $this->is_tutor  = $GLOBALS['perm']->have_studip_perm('tutor', $this->course_id);
         $this->is_autor  = $GLOBALS['perm']->have_studip_perm('autor', $this->course_id);
 
-        // Hide groups page?
-        if (!$this->is_tutor && $this->config->COURSE_MEMBERS_HIDE) {
-            throw new AccessDeniedException();
-        }
-
         // Check lock rules
         $this->is_locked = LockRules::Check($this->course_id, 'groups');
         $this->is_participants_locked = LockRules::Check($this->course_id, 'participants');
diff --git a/db/migrations/5.3.27_remove_course_members_hide_configuration.php b/db/migrations/5.3.27_remove_course_members_hide_configuration.php
new file mode 100644
index 0000000..de9a854
--- /dev/null
+++ b/db/migrations/5.3.27_remove_course_members_hide_configuration.php
@@ -0,0 +1,78 @@
+<?php
+/**
+ * @see https://gitlab.studip.de/studip/studip/-/issues/613
+ */
+final class RemoveCourseMembersHideConfiguration extends Migration
+{
+    public function description()
+    {
+        return 'Removes the configuration "COURSE_MEMBERS_HIDE" in favor of the setting in modules.';
+    }
+
+    protected function up()
+    {
+        // Migrate config settings
+        $query = "SELECT `pluginid`
+                  FROM `plugins`
+                  WHERE `pluginclassname` = 'CoreParticipants'";
+        $plugin_id = DBManager::get()->fetchColumn($query);
+
+        $query = "SELECT `range_id`
+                  FROM `config_values`
+                  JOIN `tools_activated` USING (`range_id`)
+                  WHERE `field` = 'COURSE_MEMBERS_HIDE'
+                    AND `plugin_id` = ?
+                    AND  IFNULL(`metadata`, '') NOT LIKE ?";
+        DBManager::get()->fetchFirst(
+            $query,
+            [$plugin_id, '%"visibility":"tutor"%'],
+            function ($course_id) use ($plugin_id) {
+                $metadata = $this->getMetaDataForCourseAndPlugin($course_id, $plugin_id);
+                $metadata['visibility'] = 'tutor';
+
+                $query = "UPDATE `tools_activated`
+                          SET `metadata` = ?
+                          WHERE `range_id` = ?
+                            AND `plugin_id` = ?";
+                DBManager::get()->execute($query, [
+                    json_encode($metadata),
+                    $course_id,
+                    $plugin_id
+                ]);
+            }
+        );
+
+        // Code taken from migration 1.305
+        $query = "DELETE `config`, `config_values`
+                  FROM `config`
+                  LEFT JOIN `config_values` USING (`field`)
+                  WHERE `field` = 'COURSE_MEMBERS_HIDE'";
+        DBManager::get()->exec($query);
+    }
+
+    protected function down()
+    {
+        // Code taken from migration 1.305
+        $query = "INSERT IGNORE INTO `config` (`field`, `value`, `type`, `range`, `mkdate`, `chdate`, `description`)
+                  VALUES (:name, :value, :type, :range, UNIX_TIMESTAMP(), UNIX_TIMESTAMP(), :description)";
+
+        $statement = DBManager::get()->prepare($query);
+        $statement->execute([
+            ':name'        => 'COURSE_MEMBERS_HIDE',
+            ':description' => 'Über diese Option können Sie die Teilnehmendenliste für Studierende der Veranstaltung unsichtbar machen',
+            ':range'       => 'course',
+            ':type'        => 'boolean',
+            ':value'       => '0'
+        ]);
+    }
+
+    private function getMetaDataForCourseAndPlugin(string $course_id, string $plugin_id): array
+    {
+        $query = "SELECT `metadata`
+                  FROM `tools_activated`
+                  WHERE `range_id` = ?
+                    AND `plugin_id` = ?";
+        $value = DBManager::get()->fetchColumn($query, [$course_id, $plugin_id]);
+        return json_decode($value, true) ?: [];
+    }
+}
diff --git a/lib/models/ToolActivation.php b/lib/models/ToolActivation.php
index 4209a20..7bb471c 100644
--- a/lib/models/ToolActivation.php
+++ b/lib/models/ToolActivation.php
@@ -26,6 +26,9 @@
  */
 class ToolActivation extends SimpleORMap
 {
+    public const VISIBILITY_PERMISSION_STUDENTS = 'students';
+    public const VISIBILITY_PERMISSION_TEACHERS = 'teachers';
+
     protected static function configure($config = [])
     {
         $config['db_table'] = 'tools_activated';
@@ -90,12 +93,28 @@ class ToolActivation extends SimpleORMap
         }
     }
 
-    public function getVisibilityPermission()
+    public function setVisibilityPermission(string $permission)
     {
-        if ($this->metadata['visibility'] === 'tutor') {
-            return 'tutor';
+        if (!in_array($permission, [self::VISIBILITY_PERMISSION_STUDENTS, self::VISIBILITY_PERMISSION_TEACHERS])) {
+            throw new InvalidArgumentException("Invalid permission setting {$permission}");
+        }
+
+        if ($permission === self::VISIBILITY_PERMISSION_STUDENTS) {
+            unset($this->metadata['visibility']);
         } else {
+            $this->metadata['visibility'] = 'tutors';
+        }
+    }
+
+    public function getVisibilityPermission(): string
+    {
+        if (
+            empty($this->metadata['visibility'])
+            || $this->metadata['visibility'] !== 'tutor'
+        ) {
             return 'nobody';
         }
+
+        return 'tutor';
     }
 }
diff --git a/lib/modules/CoreParticipants.php b/lib/modules/CoreParticipants.php
index b265598..1d15272 100644
--- a/lib/modules/CoreParticipants.php
+++ b/lib/modules/CoreParticipants.php
@@ -34,21 +34,6 @@ class CoreParticipants extends CorePlugin implements StudipModule
 
         $course = Course::find($course_id);
 
-        // Is the participants page hidden for students?
-        if (!$GLOBALS['perm']->have_studip_perm('tutor', $course_id, $user_id) && $course->config->COURSE_MEMBERS_HIDE) {
-            $tab_navigation = $this->getTabNavigation($course_id);
-            if ($tab_navigation && count($tab_navigation['members']->getSubNavigation()) > 0) {
-                $sub_nav = $tab_navigation['members']->getSubNavigation();
-                $first_nav = reset($sub_nav);
-
-                $navigation = new Navigation($first_nav->getTitle(), $first_nav->getURL());
-                $navigation->setImage(Icon::create('persons'));
-                return $navigation;
-
-            }
-            return null;
-        }
-
         // Determine url to redirect to
         if (!$course->getSemClass()->isGroup()) {
             $url = 'dispatch.php/course/members/index';
@@ -138,14 +123,10 @@ class CoreParticipants extends CorePlugin implements StudipModule
 
         // Only courses without children have a regular member list and statusgroups.
         if (!$course->getSemClass()->isGroup()) {
-            if ($GLOBALS['perm']->have_studip_perm('tutor', $course_id) || !$course->config->COURSE_MEMBERS_HIDE) {
-                $navigation->addSubNavigation('view', new Navigation(_('Teilnehmende'), 'dispatch.php/course/members'));
-                $navigation->addSubNavigation('statusgroups', new Navigation(_('Gruppen'), 'dispatch.php/course/statusgroups'));
-            }
-        } else {
-            if ($GLOBALS['perm']->have_studip_perm('tutor', $course_id)) {
-                $navigation->addSubNavigation('children', new Navigation(_('Teilnehmende in Unterveranstaltungen'), 'dispatch.php/course/grouping/members'));
-            }
+            $navigation->addSubNavigation('view', new Navigation(_('Teilnehmende'), 'dispatch.php/course/members'));
+            $navigation->addSubNavigation('statusgroups', new Navigation(_('Gruppen'), 'dispatch.php/course/statusgroups'));
+        } elseif ($GLOBALS['perm']->have_studip_perm('tutor', $course_id)) {
+            $navigation->addSubNavigation('children', new Navigation(_('Teilnehmende in Unterveranstaltungen'), 'dispatch.php/course/grouping/members'));
         }
 
         if ($course->aux_lock_rule) {