From 3aed37ee4db443d6987b4c5441d782510a045348 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Thu, 6 Nov 2014 16:46:50 +0800 Subject: [PATCH 1/6] MDL-48269 group: Remove the option to hide the picture of a group Plus additional amendments to Fred's original commit: 1. Updating the version numbers 2. Merging the original two-step upgrade below into one for simplicity: 1. Deleting the pictures for groups with hidepicture set to 1; and 2. Dropping the hidepicture field itself. 3. Converted array() usages to the short syntax []. --- backup/moodle2/backup_stepslib.php | 2 +- group/group_form.php | 3 -- group/upgrade.txt | 7 ++++ lib/db/install.xml | 3 +- lib/db/upgrade.php | 51 ++++++++++++++++++++++++++++++ lib/weblib.php | 5 --- mod/forum/lib.php | 2 +- version.php | 2 +- 8 files changed, 62 insertions(+), 13 deletions(-) create mode 100644 group/upgrade.txt diff --git a/backup/moodle2/backup_stepslib.php b/backup/moodle2/backup_stepslib.php index 6a66e3f7e93..4178d887429 100644 --- a/backup/moodle2/backup_stepslib.php +++ b/backup/moodle2/backup_stepslib.php @@ -1250,7 +1250,7 @@ class backup_groups_structure_step extends backup_structure_step { $group = new backup_nested_element('group', array('id'), array( 'name', 'idnumber', 'description', 'descriptionformat', 'enrolmentkey', - 'picture', 'hidepicture', 'timecreated', 'timemodified')); + 'picture', 'timecreated', 'timemodified')); $members = new backup_nested_element('group_members'); diff --git a/group/group_form.php b/group/group_form.php index b92dcec58d9..15b638870de 100644 --- a/group/group_form.php +++ b/group/group_form.php @@ -77,9 +77,6 @@ class group_form extends moodleform { $mform->addElement('checkbox', 'deletepicture', get_string('delete')); $mform->setDefault('deletepicture', 0); - $options = array(get_string('no'), get_string('yes')); - $mform->addElement('select', 'hidepicture', get_string('hidepicture'), $options); - $mform->addElement('filepicker', 'imagefile', get_string('newpicture', 'group')); $mform->addHelpButton('imagefile', 'newpicture', 'group'); diff --git a/group/upgrade.txt b/group/upgrade.txt new file mode 100644 index 00000000000..894ffc4efbc --- /dev/null +++ b/group/upgrade.txt @@ -0,0 +1,7 @@ +This files describes API changes in /group/*, +information provided here is intended especially for developers. + +=== 3.11 === + +* The groups do not support 'hidepicture' any more, and so the column 'hidepicture' + from the table {groups} has be dropped. diff --git a/lib/db/install.xml b/lib/db/install.xml index 87a8826c2fd..581661e6432 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -2245,7 +2245,6 @@ - diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 40efad053fb..e8e7877da48 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2393,5 +2393,56 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2021052500.55); } + if ($oldversion < 2021052500.59) { + + // We are going to remove the field 'hidepicture' from the groups + // so we need to remove the pictures from those groups. But we prevent + // the execution twice because this could be executed again when upgrading + // to different versions. + if ($dbman->field_exists('groups', 'hidepicture')) { + + $sql = "SELECT g.id, g.courseid, ctx.id AS contextid + FROM {groups} g + JOIN {context} ctx + ON ctx.instanceid = g.courseid + AND ctx.contextlevel = :contextlevel + WHERE g.hidepicture = 1"; + + // Selecting all the groups that have hide picture enabled, and organising them by context. + $groupctx = []; + $records = $DB->get_recordset_sql($sql, ['contextlevel' => CONTEXT_COURSE]); + foreach ($records as $record) { + if (!isset($groupctx[$record->contextid])) { + $groupctx[$record->contextid] = []; + } + $groupctx[$record->contextid][] = $record->id; + } + $records->close(); + + // Deleting the group files. + $fs = get_file_storage(); + foreach ($groupctx as $contextid => $groupids) { + list($in, $inparams) = $DB->get_in_or_equal($groupids, SQL_PARAMS_NAMED); + $fs->delete_area_files_select($contextid, 'group', 'icon', $in, $inparams); + } + + // Updating the database to remove picture from all those groups. + $sql = "UPDATE {groups} SET picture = :pic WHERE hidepicture = :hide"; + $DB->execute($sql, ['pic' => 0, 'hide' => 1]); + } + + // Define field hidepicture to be dropped from groups. + $table = new xmldb_table('groups'); + $field = new xmldb_field('hidepicture'); + + // Conditionally launch drop field hidepicture. + if ($dbman->field_exists($table, $field)) { + $dbman->drop_field($table, $field); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2021052500.59); + } + return true; } diff --git a/lib/weblib.php b/lib/weblib.php index a2ba0c34d91..34ba16b7b2c 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -2576,11 +2576,6 @@ function get_group_picture_url($group, $courseid, $large = false, $includetoken return; } - // If picture is hidden, only show to those with course:managegroups. - if ($group->hidepicture and !has_capability('moodle/course:managegroups', $context)) { - return; - } - if ($large) { $file = 'f1'; } else { diff --git a/mod/forum/lib.php b/mod/forum/lib.php index e9089555fd1..579c9414cb2 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -2450,7 +2450,7 @@ function forum_print_discussion_header(&$post, $forum, $group = -1, $datestring // Group picture if ($group !== -1) { // Groups are active - group is a group data object or NULL echo ''; - if (!empty($group->picture) and empty($group->hidepicture)) { + if (!empty($group->picture)) { if ($canviewparticipants && $COURSE->groupmode) { $picturelink = true; } else { diff --git a/version.php b/version.php index 4094cb8cf06..8a0d56b7a43 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2021052500.58; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2021052500.59; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '4.0dev (Build: 20210211)'; // Human-friendly version name From 19c8985c7e2b7ece9041e4ea9ba5cad4d978e154 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Fri, 7 Nov 2014 14:30:12 +0800 Subject: [PATCH 2/6] MDL-48269 language: Deprecating 'hidepicture' string --- lang/en/deprecated.txt | 2 ++ lang/en/group.php | 4 +++- lang/en/moodle.php | 4 +++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lang/en/deprecated.txt b/lang/en/deprecated.txt index 1028bfe8b3f..79f960dcdef 100644 --- a/lang/en/deprecated.txt +++ b/lang/en/deprecated.txt @@ -125,3 +125,5 @@ sitebackpackverify,core_badges filetypesnotwhitelisted,core_form modeloutputdirinfo,core_analytics backpackneedsupdate,core_badges +hidepicture,core_group +hidepicture,core_moodle diff --git a/lang/en/group.php b/lang/en/group.php index 2f344729f23..d290ea22436 100644 --- a/lang/en/group.php +++ b/lang/en/group.php @@ -134,7 +134,6 @@ $string['groupspreview'] = 'Groups preview'; $string['groupsseparate'] = 'Separate groups'; $string['groupsvisible'] = 'Visible groups'; $string['grouptemplate'] = 'Group @'; -$string['hidepicture'] = 'Hide picture'; $string['importgroups'] = 'Import groups'; $string['importgroups_help'] = 'Groups may be imported via text file. The format of the file should be as follows: @@ -199,3 +198,6 @@ $string['usercount'] = 'User count'; $string['usercounttotal'] = 'User count ({$a})'; $string['usergroupmembership'] = 'Selected user\'s membership:'; $string['memberofgroup'] = 'Group member of: {$a}'; + +// Deprecated since Moodle 3.11. +$string['hidepicture'] = 'Hide picture'; diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 6437a546747..b4e29fbbf6e 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -993,7 +993,6 @@ $string['hideadvancedsettings'] = 'Hide advanced settings'; $string['hidechartdata'] = 'Hide chart data'; $string['hidefromstudents'] = 'Hide from students'; $string['hideoncoursepage'] = 'Make available but not shown on course page'; -$string['hidepicture'] = 'Hide picture'; $string['hidesection'] = 'Hide section {$a}'; $string['hidesettings'] = 'Hide settings'; $string['hideshowblocks'] = 'Hide or show blocks'; @@ -2314,3 +2313,6 @@ $string['deprecatedeventname'] = '{$a} (no longer in use)'; // Deprecated since Moodle 3.9. $string['participantscount'] = 'Number of participants: {$a}'; $string['userfilterplaceholder'] = 'Search keyword or select filter'; + +// Deprecated since Moodle 3.11. +$string['hidepicture'] = 'Hide picture'; From 666cce74e5466d573032dcc34103f0a1787beee1 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Thu, 28 Jan 2021 14:08:45 +0800 Subject: [PATCH 3/6] MDL-48269 group: Remove additional occurrences of hidepicture --- group/classes/output/group_details.php | 2 +- group/overview.php | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/group/classes/output/group_details.php b/group/classes/output/group_details.php index 11d4a62e078..aaf61a94009 100644 --- a/group/classes/output/group_details.php +++ b/group/classes/output/group_details.php @@ -60,7 +60,7 @@ class group_details implements renderable, templatable { */ public function export_for_template(renderer_base $output) { - if (!empty($this->group->description) || (!empty($this->group->picture) && empty($this->group->hidepicture))) { + if (!empty($this->group->description) || (!empty($this->group->picture))) { $context = context_course::instance($this->group->courseid); $description = file_rewrite_pluginfile_urls($this->group->description, 'pluginfile.php', diff --git a/group/overview.php b/group/overview.php index 5c23992cc19..fcbf697da24 100644 --- a/group/overview.php +++ b/group/overview.php @@ -152,7 +152,6 @@ $groups[OVERVIEW_NO_GROUP] = (object)array( 'descriptionformat' => FORMAT_HTML, 'enrolmentkey' => '', 'picture' => 0, - 'hidepicture' => 0, 'timecreated' => 0, 'timemodified' => 0, ); From 32ed88ad07f4b6ef4c17bf1aab82cb1547761114 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Thu, 28 Jan 2021 14:09:23 +0800 Subject: [PATCH 4/6] MDL-48269 messageoutput_email: Remove occurrences of hidepicture --- message/output/email/classes/output/email_digest.php | 1 - message/output/email/classes/task/send_email_task.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/message/output/email/classes/output/email_digest.php b/message/output/email/classes/output/email_digest.php index d19f641f060..39d2e519b69 100644 --- a/message/output/email/classes/output/email_digest.php +++ b/message/output/email/classes/output/email_digest.php @@ -102,7 +102,6 @@ class email_digest implements \renderable, \templatable { $group = new \stdClass(); $group->id = $conversation->groupid; $group->picture = $conversation->picture; - $group->hidepicture = $conversation->hidepicture; $group->courseid = $conversation->courseid; $grouppictureurl = $renderer->image_url('g/g1')->out(false); // Default image. if ($url = get_group_picture_url($group, $group->courseid, false, true)) { diff --git a/message/output/email/classes/task/send_email_task.php b/message/output/email/classes/task/send_email_task.php index ddad9842d8a..d21f3155da4 100644 --- a/message/output/email/classes/task/send_email_task.php +++ b/message/output/email/classes/task/send_email_task.php @@ -137,7 +137,7 @@ class send_email_task extends scheduled_task { // now this will have to do before 3.7 code freeze. // See related MDL-63814. $sql = "SELECT DISTINCT mc.id, mc.name, c.id as courseid, c.fullname as coursename, g.id as groupid, - g.picture, g.hidepicture + g.picture FROM {message_conversations} mc JOIN {groups} g ON mc.itemid = g.id From f04cbfa7aba4e2f6dcbc94fa59811ab4e8e1a893 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Thu, 28 Jan 2021 14:09:49 +0800 Subject: [PATCH 5/6] MDL-48269 mod_forum: Remove occurrences of hidepicture --- mod/forum/classes/local/exporters/author.php | 6 +----- mod/forum/classes/local/exporters/discussion.php | 3 +-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/mod/forum/classes/local/exporters/author.php b/mod/forum/classes/local/exporters/author.php index 66f39dedc02..4ef65200fcc 100644 --- a/mod/forum/classes/local/exporters/author.php +++ b/mod/forum/classes/local/exporters/author.php @@ -163,12 +163,8 @@ class author extends exporter { ]; } else { $groups = array_map(function($group) use ($urlfactory, $context, $output) { - $imageurl = null; $groupurl = null; - - if (!$group->hidepicture) { - $imageurl = get_group_picture_url($group, $group->courseid, true); - } + $imageurl = get_group_picture_url($group, $group->courseid, true); if (course_can_view_participants($context)) { $groupurl = $urlfactory->get_author_group_url($group); diff --git a/mod/forum/classes/local/exporters/discussion.php b/mod/forum/classes/local/exporters/discussion.php index ede9dc1fd48..38cf2268089 100644 --- a/mod/forum/classes/local/exporters/discussion.php +++ b/mod/forum/classes/local/exporters/discussion.php @@ -179,8 +179,7 @@ class discussion extends exporter { ]; // If not hiding the group picture, and the group has a picture then use it. Fallback to generic group image. - if (!$group->hidepicture && - ($url = get_group_picture_url($group, $forum->get_course_id(), true))) { + if ($url = get_group_picture_url($group, $forum->get_course_id(), true)) { $groupdata['urls']['picture'] = $url; } else { From c8f5e8c7f362707cb583f36d7480b75c27429693 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Fri, 29 Jan 2021 17:15:40 +0800 Subject: [PATCH 6/6] MDL-48269 core_backup: Prevent unwanted group icons from being restored * Old backups might have hidepicture defined and set to 1 in which case we must make sure to exclude the group picture for these groups from being restored. --- backup/moodle2/restore_stepslib.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index 8108f6efbc4..140dacde06c 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -1191,6 +1191,13 @@ class restore_groups_structure_step extends restore_structure_step { $restorefiles = false; // Only if we end creating the group + // This is for backwards compatibility with old backups. If the backup data for a group contains a non-empty value of + // hidepicture, then we'll exclude this group's picture from being restored. + if (!empty($data->hidepicture)) { + // Exclude the group picture from being restored if hidepicture is set to 1 in the backup data. + unset($data->picture); + } + // Search if the group already exists (by name & description) in the target course $description_clause = ''; $params = array('courseid' => $this->get_courseid(), 'grname' => $data->name); @@ -1212,6 +1219,12 @@ class restore_groups_structure_step extends restore_structure_step { } // Save the id mapping $this->set_mapping('group', $oldid, $newitemid, $restorefiles); + + // Add the related group picture file if it's available at this point. + if (!empty($data->picture)) { + $this->add_related_files('group', 'icon', 'group', null, $oldid); + } + // Invalidate the course group data cache just in case. cache_helper::invalidate_by_definition('core', 'groupdata', array(), array($data->courseid)); } @@ -1271,8 +1284,7 @@ class restore_groups_structure_step extends restore_structure_step { } protected function after_execute() { - // Add group related files, matching with "group" mappings - $this->add_related_files('group', 'icon', 'group'); + // Add group related files, matching with "group" mappings. $this->add_related_files('group', 'description', 'group'); // Add grouping related files, matching with "grouping" mappings $this->add_related_files('grouping', 'description', 'grouping');