From 7bbe0174bcd1903897a640a7f2cbaa6774e5f3fe Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Wed, 4 Dec 2024 08:48:02 +0000
Subject: [PATCH] fixes #613
Closes #613
Merge request studip/studip!1801
---
app/controllers/course/members.php | 26 +++++--
app/controllers/course/statusgroups.php | 5 --
...move_course_members_hide_configuration.php | 78 +++++++++++++++++++
lib/models/ToolActivation.php | 25 +++++-
lib/modules/CoreParticipants.php | 27 +------
5 files changed, 123 insertions(+), 38 deletions(-)
create mode 100644 db/migrations/5.3.27_remove_course_members_hide_configuration.php
diff --git a/app/controllers/course/members.php b/app/controllers/course/members.php
index bcbea42046b..822f4a4df48 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 df8c86e7487..cbd80ad40c5 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 00000000000..de9a8542cf3
--- /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 4209a20ff23..7bb471c4263 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 b2655980894..1d1527253f0 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) {
--
GitLab