MDL-59159 upgrade: remove unused functions/settings from upgradelib

All these functions were used only by deleted upgrade steps
so it's safe to proceed with straight deletion, considering
them internal. Deletion has been documented in corresponding
upgrade.txt files:

- mod_feedback_upgrade_delete_duplicate_values()
- mod_feedback_upgrade_courseid()

These have been kept because they continue being used:

- @ install: make_competence_scale()
- @ restore: upgrade_course_letter_boundary()

mod/feedback/db/upgrade.php

@@ -38,7 +38,6 @@ defined('MOODLE_INTERNAL') || die();
 
 function xmldb_feedback_upgrade($oldversion) {
     global $CFG, $DB;
-    require_once($CFG->dirroot . '/mod/feedback/db/upgradelib.php');
 
     $dbman = $DB->get_manager(); // Loads ddl manager and xmldb classes.
 

mod/feedback/db/upgradelib.php

-<?php
-// This file is part of Moodle - http://moodle.org/
-//
-// Moodle is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// Moodle is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
-
-/**
- * Upgrade helper functions
- *
- * @package   mod_feedback
- * @copyright 2016 Marina Glancy
- * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-
-defined('MOODLE_INTERNAL') || die();
-
-/**
- * Fill new field courseid in tables feedback_completed or feedback_completedtmp
- *
- * @param bool $tmp use for temporary table
- */
-function mod_feedback_upgrade_courseid($tmp = false) {
-    global $DB;
-    $suffix = $tmp ? 'tmp' : '';
-
-    // Part 1. Ensure that each completed record has associated values with only one courseid.
-    $sql = "SELECT c.id
-        FROM {feedback_completed$suffix} c, {feedback_value$suffix} v
-        WHERE c.id = v.completed
-        GROUP by c.id
-        having count(DISTINCT v.course_id) > 1";
-    $problems = $DB->get_fieldset_sql($sql);
-    foreach ($problems as $problem) {
-        $courses = $DB->get_fieldset_sql("SELECT DISTINCT course_id "
-                . "FROM {feedback_value$suffix} WHERE completed = ?", array($problem));
-        $firstcourse = array_shift($courses);
-        $record = $DB->get_record('feedback_completed'.$suffix, array('id' => $problem));
-        unset($record->id);
-        $DB->update_record('feedback_completed'.$suffix, ['id' => $problem, 'courseid' => $firstcourse]);
-        foreach ($courses as $courseid) {
-            $record->courseid = $courseid;
-            $completedid = $DB->insert_record('feedback_completed'.$suffix, $record);
-            $DB->execute("UPDATE {feedback_value$suffix} SET completed = ? WHERE completed = ? AND course_id = ?",
-                    array($completedid, $problem, $courseid));
-        }
-    }
-
-    // Part 2. Update courseid in the completed table.
-    if ($DB->get_dbfamily() !== 'mysql') {
-        $sql = "UPDATE {feedback_completed$suffix} "
-            . "SET courseid = (SELECT COALESCE(MIN(v.course_id), 0) "
-            . "FROM {feedback_value$suffix} v "
-            . "WHERE v.completed = {feedback_completed$suffix}.id)";
-        $DB->execute($sql);
-    } else {
-        $sql = "UPDATE {feedback_completed$suffix} c, {feedback_value$suffix} v "
-            . "SET c.courseid = v.course_id "
-            . "WHERE v.completed = c.id AND v.course_id <> 0";
-        $DB->execute($sql);
-    }
-}
-
-/**
- * Ensure tables feedback_value and feedback_valuetmp have unique entries for each pair (completed,item).
- *
- * @param bool $tmp use for temporary table
- */
-function mod_feedback_upgrade_delete_duplicate_values($tmp = false) {
-    global $DB;
-    $suffix = $tmp ? 'tmp' : '';
-
-    $sql = "SELECT MIN(id) AS id, completed, item, course_id " .
-            "FROM {feedback_value$suffix} GROUP BY completed, item, course_id HAVING count(id)>1";
-    $records = $DB->get_records_sql($sql);
-    foreach ($records as $record) {
-        $DB->delete_records_select("feedback_value$suffix",
-            "completed = :completed AND item = :item AND course_id = :course_id AND id > :id", (array)$record);
-    }
-}

mod/feedback/tests/upgradelib_test.php

-<?php
-// This file is part of Moodle - http://moodle.org/
-//
-// Moodle is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// Moodle is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
-
-/**
- * Tests for functions in db/upgradelib.php
- *
- * @package   mod_feedback
- * @copyright 2016 Marina Glancy
- * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-
-defined('MOODLE_INTERNAL') || die();
-
-global $CFG;
-require_once($CFG->dirroot . '/mod/feedback/db/upgradelib.php');
-
-/**
- * Tests for functions in db/upgradelib.php
- *
- * @package    mod_feedback
- * @copyright  2016 Marina Glancy
- * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-class mod_feedback_upgradelib_testcase extends advanced_testcase {
-
-    /** @var string  */
-    protected $testsql = "SELECT COUNT(v.id) FROM {feedback_completed} c, {feedback_value} v
-            WHERE c.id = v.completed AND c.courseid <> v.course_id";
-    /** @var string  */
-    protected $testsqltmp = "SELECT COUNT(v.id) FROM {feedback_completedtmp} c, {feedback_valuetmp} v
-            WHERE c.id = v.completed AND c.courseid <> v.course_id";
-    /** @var int */
-    protected $course1;
-    /** @var int */
-    protected $course2;
-    /** @var stdClass */
-    protected $feedback;
-    /** @var stdClass */
-    protected $user;
-
-    /**
-     * Sets up the fixture
-     * This method is called before a test is executed.
-     */
-    public function setUp() {
-        parent::setUp();
-        $this->resetAfterTest(true);
-
-        $this->course1 = $this->getDataGenerator()->create_course();
-        $this->course2 = $this->getDataGenerator()->create_course();
-        $this->feedback = $this->getDataGenerator()->create_module('feedback', array('course' => SITEID));
-
-        $this->user = $this->getDataGenerator()->create_user();
-    }
-
-    public function test_upgrade_courseid_completed() {
-        global $DB;
-
-        // Case 1. No errors in the data.
-        $completed1 = $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 2, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsql)); // We have errors!
-        mod_feedback_upgrade_courseid(true); // Running script for temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsql)); // Nothing changed.
-        mod_feedback_upgrade_courseid();
-        $this->assertCount(1, $DB->get_records('feedback_completed')); // Number of records is the same.
-        $this->assertEquals(0, $DB->count_records_sql($this->testsql)); // All errors are fixed!
-    }
-
-    public function test_upgrade_courseid_completed_with_errors() {
-        global $DB;
-
-        // Case 2. Errors in data (same feedback_completed has values for different courses).
-        $completed1 = $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-            'item' => 1, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsql)); // We have errors!
-        mod_feedback_upgrade_courseid(true); // Running script for temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsql)); // Nothing changed.
-        mod_feedback_upgrade_courseid();
-        $this->assertCount(2, $DB->get_records('feedback_completed')); // Extra record inserted.
-        $this->assertEquals(0, $DB->count_records_sql($this->testsql)); // All errors are fixed!
-    }
-
-    public function test_upgrade_courseid_completedtmp() {
-        global $DB;
-
-        // Case 1. No errors in the data.
-        $completed1 = $DB->insert_record('feedback_completedtmp',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 2, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsqltmp)); // We have errors!
-        mod_feedback_upgrade_courseid(); // Running script for non-temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsqltmp)); // Nothing changed.
-        mod_feedback_upgrade_courseid(true);
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp')); // Number of records is the same.
-        $this->assertEquals(0, $DB->count_records_sql($this->testsqltmp)); // All errors are fixed!
-    }
-
-    public function test_upgrade_courseid_completedtmp_with_errors() {
-        global $DB;
-
-        // Case 2. Errors in data (same feedback_completed has values for different courses).
-        $completed1 = $DB->insert_record('feedback_completedtmp',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-            'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-            'item' => 1, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsqltmp)); // We have errors!
-        mod_feedback_upgrade_courseid(); // Running script for non-temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(2, $DB->count_records_sql($this->testsqltmp)); // Nothing changed.
-        mod_feedback_upgrade_courseid(true);
-        $this->assertCount(2, $DB->get_records('feedback_completedtmp')); // Extra record inserted.
-        $this->assertEquals(0, $DB->count_records_sql($this->testsqltmp)); // All errors are fixed!
-    }
-
-    public function test_upgrade_courseid_empty_completed() {
-        global $DB;
-
-        // Record in 'feedback_completed' does not have corresponding values.
-        $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $record1 = $DB->get_record('feedback_completed', []);
-        mod_feedback_upgrade_courseid();
-        $this->assertCount(1, $DB->get_records('feedback_completed')); // Number of records is the same.
-        $record2 = $DB->get_record('feedback_completed', []);
-        $this->assertEquals($record1, $record2);
-    }
-
-    public function test_upgrade_remove_duplicates_no_duplicates() {
-        global $DB;
-
-        $completed1 = $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 2, 'value' => 2]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 3, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-                'item' => 3, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(4, $DB->count_records('feedback_value'));
-        mod_feedback_upgrade_delete_duplicate_values();
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(4, $DB->count_records('feedback_value')); // Same number of records, no changes made.
-    }
-
-    public function test_upgrade_remove_duplicates() {
-        global $DB;
-
-        // Remove the index that was added in the upgrade.php AFTER running mod_feedback_upgrade_delete_duplicate_values().
-        $dbman = $DB->get_manager();
-        $table = new xmldb_table('feedback_value');
-        $index = new xmldb_index('completed_item', XMLDB_INDEX_UNIQUE, array('completed', 'item', 'course_id'));
-        $dbman->drop_index($table, $index);
-
-        // Insert duplicated values.
-        $completed1 = $DB->insert_record('feedback_completed',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 2]); // This is a duplicate with another value.
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 3, 'value' => 1]);
-        $DB->insert_record('feedback_value',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-                'item' => 3, 'value' => 2]); // This is not a duplicate because course id is different.
-
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(4, $DB->count_records('feedback_value'));
-        mod_feedback_upgrade_delete_duplicate_values(true); // Running script for temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completed'));
-        $this->assertEquals(4, $DB->count_records('feedback_value')); // Nothing changed.
-        mod_feedback_upgrade_delete_duplicate_values();
-        $this->assertCount(1, $DB->get_records('feedback_completed')); // Number of records is the same.
-        $this->assertEquals(3, $DB->count_records('feedback_value')); // Duplicate was deleted.
-        $this->assertEquals(1, $DB->get_field('feedback_value', 'value', ['item' => 1]));
-
-        $dbman->add_index($table, $index);
-    }
-
-    public function test_upgrade_remove_duplicates_no_duplicates_tmp() {
-        global $DB;
-
-        $completed1 = $DB->insert_record('feedback_completedtmp',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 2, 'value' => 2]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 3, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-                'item' => 3, 'value' => 2]);
-
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(4, $DB->count_records('feedback_valuetmp'));
-        mod_feedback_upgrade_delete_duplicate_values(true);
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(4, $DB->count_records('feedback_valuetmp')); // Same number of records, no changes made.
-    }
-
-    public function test_upgrade_remove_duplicates_tmp() {
-        global $DB;
-
-        // Remove the index that was added in the upgrade.php AFTER running mod_feedback_upgrade_delete_duplicate_values().
-        $dbman = $DB->get_manager();
-        $table = new xmldb_table('feedback_valuetmp');
-        $index = new xmldb_index('completed_item', XMLDB_INDEX_UNIQUE, array('completed', 'item', 'course_id'));
-        $dbman->drop_index($table, $index);
-
-        // Insert duplicated values.
-        $completed1 = $DB->insert_record('feedback_completedtmp',
-            ['feedback' => $this->feedback->id, 'userid' => $this->user->id]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 1, 'value' => 2]); // This is a duplicate with another value.
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course1->id,
-                'item' => 3, 'value' => 1]);
-        $DB->insert_record('feedback_valuetmp',
-            ['completed' => $completed1, 'course_id' => $this->course2->id,
-                'item' => 3, 'value' => 2]); // This is not a duplicate because course id is different.
-
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(4, $DB->count_records('feedback_valuetmp'));
-        mod_feedback_upgrade_delete_duplicate_values(); // Running script for non-temp tables.
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp'));
-        $this->assertEquals(4, $DB->count_records('feedback_valuetmp')); // Nothing changed.
-        mod_feedback_upgrade_delete_duplicate_values(true);
-        $this->assertCount(1, $DB->get_records('feedback_completedtmp')); // Number of records is the same.
-        $this->assertEquals(3, $DB->count_records('feedback_valuetmp')); // Duplicate was deleted.
-        $this->assertEquals(1, $DB->get_field('feedback_valuetmp', 'value', ['item' => 1]));
-
-        $dbman->add_index($table, $index);
-    }
-}
\ No newline at end of file

mod/feedback/upgrade.txt

+=== 3.5 ===
+
+* The following functions, previously used (exclusively) by upgrade steps are not available
+  anymore because of the upgrade cleanup performed for this version. See MDL-59159 for more info:
+    - mod_feedback_upgrade_delete_duplicate_values()
+    - mod_feedback_upgrade_courseid()
+
 === 3.3.2 ===
 
 * feedback_refresh_events() Now takes two additional parameters to refine the update to a specific instance. This function