diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index c2628ab3ae1..18b43552269 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -1468,7 +1468,8 @@ class restore_course_structure_step extends restore_structure_step { // Add the one being restored $tags[] = $data->rawname; // Send all the tags back to the course - tag_set('course', $this->get_courseid(), $tags); + tag_set('course', $this->get_courseid(), $tags, 'core', + context_course::instance($this->get_courseid())->id); } } @@ -3279,6 +3280,9 @@ abstract class restore_activity_structure_step extends restore_structure_step { */ class restore_create_categories_and_questions extends restore_structure_step { + /** @var array $cachecategory store the categories */ + protected $cachedcategory = array(); + protected function define_structure() { $category = new restore_path_element('question_category', '/question_categories/question_category'); @@ -3384,7 +3388,7 @@ class restore_create_categories_and_questions extends restore_structure_step { // step will be in charge of restoring all the question files } - protected function process_question_hint($data) { + protected function process_question_hint($data) { global $DB; $data = (object)$data; @@ -3448,7 +3452,7 @@ class restore_create_categories_and_questions extends restore_structure_step { $newquestion = $this->get_new_parentid('question'); if (!empty($CFG->usetags)) { // if enabled in server - // TODO: This is highly inneficient. Each time we add one tag + // TODO: This is highly inefficient. Each time we add one tag // we fetch all the existing because tag_set() deletes them // so everything must be reinserted on each call $tags = array(); @@ -3459,8 +3463,13 @@ class restore_create_categories_and_questions extends restore_structure_step { } // Add the one being restored $tags[] = $data->rawname; + // Get the category, so we can then later get the context. + $categoryid = $this->get_new_parentid('question_category'); + if (empty($this->cachedcategory) || $this->cachedcategory->id != $categoryid) { + $this->cachedcategory = $DB->get_record('question_categories', array('id' => $categoryid)); + } // Send all the tags back to the question - tag_set('question', $newquestion, $tags); + tag_set('question', $newquestion, $tags, 'core_question', $this->cachedcategory->contextid); } } diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php index 431dbd2e6a8..c4a60131e93 100644 --- a/backup/util/dbops/restore_dbops.class.php +++ b/backup/util/dbops/restore_dbops.class.php @@ -1215,7 +1215,7 @@ abstract class restore_dbops { $usertag = (object)$usertag; $tags[] = $usertag->rawname; } - tag_set('user', $newuserid, $tags); + tag_set('user', $newuserid, $tags, 'core', $newuserctxid); } // Process preferences diff --git a/blog/external_blog_edit.php b/blog/external_blog_edit.php index e80a8b5d58b..29dfc3b1ce1 100644 --- a/blog/external_blog_edit.php +++ b/blog/external_blog_edit.php @@ -84,7 +84,8 @@ if ($externalblogform->is_cancelled()){ $newexternal->id = $DB->insert_record('blog_external', $newexternal); blog_sync_external_entries($newexternal); - tag_set('blog_external', $newexternal->id, explode(',', $data->autotags)); + tag_set('blog_external', $newexternal->id, explode(',', $data->autotags), 'core', + context_user::instance($newexternal->userid)->id); break; @@ -102,7 +103,8 @@ if ($externalblogform->is_cancelled()){ $external->timemodified = time(); $DB->update_record('blog_external', $external); - tag_set('blog_external', $external->id, explode(',', $data->autotags)); + tag_set('blog_external', $external->id, explode(',', $data->autotags), 'core', + context_user::instance($newexternal->userid)->id); } else { print_error('wrongexternalid', 'blog'); diff --git a/blog/lib.php b/blog/lib.php index 10317c7624c..9190e465881 100644 --- a/blog/lib.php +++ b/blog/lib.php @@ -254,7 +254,7 @@ function blog_sync_external_entries($externalblog) { // Set tags if ($tags = tag_get_tags_array('blog_external', $externalblog->id)) { - tag_set('post', $id, $tags); + tag_set('post', $id, $tags, 'core', context_user::instance($externalblog->userid)->id); } } else { $newentry->id = $postid; diff --git a/blog/locallib.php b/blog/locallib.php index 38a9dae242f..556105006c5 100644 --- a/blog/locallib.php +++ b/blog/locallib.php @@ -260,7 +260,7 @@ class blog_entry implements renderable { $this->add_associations(); } - tag_set('post', $this->id, $this->tags); + tag_set('post', $this->id, $this->tags, 'core', context_user::instance($this->userid)->id); // Trigger an event for the new entry. $event = \core\event\blog_entry_created::create(array( @@ -303,7 +303,7 @@ class blog_entry implements renderable { // Update record. $DB->update_record('post', $entry); - tag_set('post', $entry->id, $entry->tags); + tag_set('post', $entry->id, $entry->tags, 'core', context_user::instance($this->userid)->id); $event = \core\event\blog_entry_updated::create(array( 'objectid' => $entry->id, @@ -327,7 +327,7 @@ class blog_entry implements renderable { // Get record to pass onto the event. $record = $DB->get_record('post', array('id' => $this->id)); $DB->delete_records('post', array('id' => $this->id)); - tag_set('post', $this->id, array()); + tag_set('post', $this->id, array(), 'core', context_user::instance($this->userid)->id); $event = \core\event\blog_entry_deleted::create(array( 'objectid' => $this->id, @@ -434,7 +434,7 @@ class blog_entry implements renderable { } } - tag_set('post', $this->id, $tags); + tag_set('post', $this->id, $tags, 'core', context_user::instance($this->userid)->id); } /** diff --git a/course/lib.php b/course/lib.php index 562ac03de9e..6d54403a3cc 100644 --- a/course/lib.php +++ b/course/lib.php @@ -1703,6 +1703,9 @@ function course_delete_module($cmid) { $DB->delete_records('course_completion_criteria', array('moduleinstance' => $cm->id, 'criteriatype' => COMPLETION_CRITERIA_TYPE_ACTIVITY)); + // Delete the tag instances. + $DB->delete_records('tag_instance', array('component' => 'mod_' . $modulename, 'contextid' => $modcontext->id)); + // Delete the context. context_helper::delete_instance(CONTEXT_MODULE, $cm->id); diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index 18e7deaf180..45c731cfe56 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -29,6 +29,7 @@ global $CFG; require_once($CFG->dirroot . '/course/lib.php'); require_once($CFG->dirroot . '/course/tests/fixtures/course_capability_assignment.php'); require_once($CFG->dirroot . '/enrol/imsenterprise/tests/imsenterprise_test.php'); +require_once($CFG->dirroot . '/tag/lib.php'); class core_course_courselib_testcase extends advanced_testcase { @@ -1366,28 +1367,40 @@ class core_course_courselib_testcase extends advanced_testcase { // Generate an assignment with due date (will generate a course event). $assign = $this->getDataGenerator()->create_module('assign', array('duedate' => time(), 'course' => $course->id)); - $cm = get_coursemodule_from_instance('assign', $assign->id); + // Get the module context. + $modcontext = context_module::instance($assign->cmid); // Verify context exists. - $this->assertInstanceOf('context_module', context_module::instance($cm->id, IGNORE_MISSING)); + $this->assertInstanceOf('context_module', $modcontext); + + // Add some tags to this assignment. + tag_set('assign', $assign->id, array('Tag 1', 'Tag 2', 'Tag 3'), 'mod_assign', $modcontext->id); + + // Confirm the tag instances were added. + $this->assertEquals(3, $DB->count_records('tag_instance', array('component' => 'mod_assign', 'contextid' => + $modcontext->id))); // Verify event assignment event has been generated. $eventcount = $DB->count_records('event', array('instance' => $assign->id, 'modulename' => 'assign')); $this->assertEquals(1, $eventcount); // Run delete.. - course_delete_module($cm->id); + course_delete_module($assign->cmid); // Verify the context has been removed. - $this->assertFalse(context_module::instance($cm->id, IGNORE_MISSING)); + $this->assertFalse(context_module::instance($assign->cmid, IGNORE_MISSING)); // Verify the course_module record has been deleted. - $cmcount = $DB->count_records('course_modules', array('id' => $cm->id)); + $cmcount = $DB->count_records('course_modules', array('id' => $assign->cmid)); $this->assertEmpty($cmcount); // Verify event assignment events have been removed. $eventcount = $DB->count_records('event', array('instance' => $assign->id, 'modulename' => 'assign')); $this->assertEmpty($eventcount); + + // Verify the tag instances were deleted. + $this->assertEquals(0, $DB->count_records('tag_instance', array('component' => 'mod_assign', 'contextid' => + $modcontext->id))); } /** diff --git a/lib/adminlib.php b/lib/adminlib.php index ae18eeb61b9..9ce6ad2cb87 100644 --- a/lib/adminlib.php +++ b/lib/adminlib.php @@ -231,6 +231,9 @@ function uninstall_plugin($type, $name) { $fs = get_file_storage(); $fs->delete_component_files($component); + // Delete all tag instances for this component. + $DB->delete_records('tag_instance', array('component' => $component)); + // Finally purge all caches. purge_all_caches(); diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 23f91233d6b..fbaf87644bc 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -4214,7 +4214,7 @@ function delete_user(stdClass $user) { // TODO: remove from cohorts using standard API here. // Remove user tags. - tag_set('user', $user->id, array()); + tag_set('user', $user->id, array(), 'core', $usercontext->id); // Unconditionally unenrol from all courses. enrol_user_delete($user); diff --git a/lib/questionlib.php b/lib/questionlib.php index 5346c25937c..21bdeb3f5ca 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -331,6 +331,9 @@ function question_delete_question($questionid) { question_bank::get_qtype($question->qtype, false)->delete_question( $questionid, $question->contextid); + // Delete all tag instances. + $DB->delete_records('tag_instance', array('component' => 'core_question', 'itemid' => $question->id)); + // Now recursively delete all child questions if ($children = $DB->get_records('question', array('parent' => $questionid), '', 'id, qtype')) { @@ -435,7 +438,7 @@ function question_delete_course_category($category, $newcategory, $feedback=true // Check to see if there were any questions that were kept because // they are still in use somehow, even though quizzes in courses - // in this category will already have been deteted. This could + // in this category will already have been deleted. This could // happen, for example, if questions are added to a course, // and then that course is moved to another category (MDL-14802). $questionids = $DB->get_records_menu('question', @@ -474,12 +477,17 @@ function question_delete_course_category($category, $newcategory, $feedback=true } } else { - // Move question categories ot the new context. + // Move question categories to the new context. if (!$newcontext = context_coursecat::instance($newcategory->id)) { return false; } - $DB->set_field('question_categories', 'contextid', $newcontext->id, - array('contextid'=>$context->id)); + + // Update the contextid for any tag instances for questions in the old context. + $DB->set_field('tag_instance', 'contextid', $newcontext->id, array('component' => 'core_question', + 'contextid' => $context->id)); + + $DB->set_field('question_categories', 'contextid', $newcontext->id, array('contextid' => $context->id)); + if ($feedback) { $a = new stdClass(); $a->oldplace = $context->get_context_name(); @@ -611,6 +619,10 @@ function question_move_questions_to_category($questionids, $newcategoryid) { $DB->set_field_select('question', 'category', $newcategoryid, "parent $questionidcondition", $params); + // Update the contextid for any tag instances that may exist for these questions. + $DB->set_field_select('tag_instance', 'contextid', $newcontextid, + "component = 'core_question' AND itemid $questionidcondition", $params); + // TODO Deal with datasets. // Purge these questions from the cache. @@ -641,6 +653,13 @@ function question_move_category_to_context($categoryid, $oldcontextid, $newconte question_bank::notify_question_edited($questionid); } + if ($questionids) { + // Update the contextid for any tag instances that may exist for these questions. + list($questionids, $params) = $DB->get_in_or_equal(array_keys($questionids)); + $DB->set_field_select('tag_instance', 'contextid', $newcontextid, + "component = 'core_question' AND itemid $questionids", $params); + } + $subcatids = $DB->get_records_menu('question_categories', array('parent' => $categoryid), '', 'id,1'); foreach ($subcatids as $subcatid => $notused) { diff --git a/lib/tests/questionlib_test.php b/lib/tests/questionlib_test.php index 5fcfe3158a9..1b0c2bcfebf 100644 --- a/lib/tests/questionlib_test.php +++ b/lib/tests/questionlib_test.php @@ -26,8 +26,13 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; -require_once($CFG->libdir . '/questionlib.php'); +require_once($CFG->libdir . '/questionlib.php'); +require_once($CFG->dirroot . '/tag/lib.php'); + +// Get the necessary files to perform backup and restore. +require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); +require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php'); /** * Unit tests for (some of) ../questionlib.php. @@ -35,7 +40,16 @@ require_once($CFG->libdir . '/questionlib.php'); * @copyright 2006 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class core_questionlib_testcase extends basic_testcase { +class core_questionlib_testcase extends advanced_testcase { + + /** + * Test set up. + * + * This is executed before running any test in this file. + */ + public function setUp() { + $this->resetAfterTest(); + } public function test_question_reorder_qtypes() { $this->assertEquals( @@ -70,4 +84,118 @@ class core_questionlib_testcase extends basic_testcase { $this->assertEquals(-0.1428571, match_grade_options($gradeoptions, -0.15, 'nearest')); } + + /** + * This function tests that the functions responsible for moving questions to + * different contexts also updates the tag instances associated with the questions. + */ + public function test_altering_tag_instance_context() { + global $CFG, $DB; + + // Set to admin user. + $this->setAdminUser(); + + // Create two course categories - we are going to delete one of these later and will expect + // all the questions belonging to the course in the deleted category to be moved. + $coursecat1 = $this->getDataGenerator()->create_category(); + $coursecat2 = $this->getDataGenerator()->create_category(); + + // Create a couple of categories and questions. + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $questioncat1 = $questiongenerator->create_question_category(array('contextid' => + context_coursecat::instance($coursecat1->id)->id)); + $questioncat2 = $questiongenerator->create_question_category(array('contextid' => + context_coursecat::instance($coursecat2->id)->id)); + $question1 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat1->id)); + $question2 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat1->id)); + $question3 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat2->id)); + $question4 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat2->id)); + + // Now lets tag these questions. + tag_set('question', $question1->id, array('tag 1', 'tag 2'), 'core_question', $questioncat1->contextid); + tag_set('question', $question2->id, array('tag 3', 'tag 4'), 'core_question', $questioncat1->contextid); + tag_set('question', $question3->id, array('tag 5', 'tag 6'), 'core_question', $questioncat2->contextid); + tag_set('question', $question4->id, array('tag 7', 'tag 8'), 'core_question', $questioncat2->contextid); + + // Test moving the questions to another category. + question_move_questions_to_category(array($question1->id, $question2->id), $questioncat2->id); + + // Test that all tag_instances belong to one context. + $this->assertEquals(8, $DB->count_records('tag_instance', array('component' => 'core_question', + 'contextid' => $questioncat2->contextid))); + + // Test moving them back. + question_move_questions_to_category(array($question1->id, $question2->id), $questioncat1->id); + + // Test that all tag_instances are now reset to how they were initially. + $this->assertEquals(4, $DB->count_records('tag_instance', array('component' => 'core_question', + 'contextid' => $questioncat1->contextid))); + $this->assertEquals(4, $DB->count_records('tag_instance', array('component' => 'core_question', + 'contextid' => $questioncat2->contextid))); + + // Now test moving a whole question category to another context. + question_move_category_to_context($questioncat1->id, $questioncat1->contextid, $questioncat2->contextid); + + // Test that all tag_instances belong to one context. + $this->assertEquals(8, $DB->count_records('tag_instance', array('component' => 'core_question', + 'contextid' => $questioncat2->contextid))); + + // Now test moving them back. + question_move_category_to_context($questioncat1->id, $questioncat2->contextid, + context_coursecat::instance($coursecat1->id)->id); + + // Test that all tag_instances are now reset to how they were initially. + $this->assertEquals(4, $DB->count_records('tag_instance', array('component' => 'core_question', + 'contextid' => $questioncat1->contextid))); + $this->assertEquals(4, $DB->count_records('tag_instance', array('component' => 'core_question', + 'contextid' => $questioncat2->contextid))); + + // Now we want to test deleting the course category and moving the questions to another category. + question_delete_course_category($coursecat1, $coursecat2, false); + + // Test that all tag_instances belong to one context. + $this->assertEquals(8, $DB->count_records('tag_instance', array('component' => 'core_question', + 'contextid' => $questioncat2->contextid))); + + // Create a course. + $course = $this->getDataGenerator()->create_course(); + + // Create some question categories and questions in this course. + $questioncat = $questiongenerator->create_question_category(array('contextid' => + context_course::instance($course->id)->id)); + $question1 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat->id)); + $question2 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat->id)); + + // Add some tags to these questions. + tag_set('question', $question1->id, array('tag 1', 'tag 2'), 'core_question', $questioncat->contextid); + tag_set('question', $question2->id, array('tag 1', 'tag 2'), 'core_question', $questioncat->contextid); + + // Create a course that we are going to restore the other course to. + $course2 = $this->getDataGenerator()->create_course(); + + // Create backup file and save it to the backup location. + $bc = new backup_controller(backup::TYPE_1COURSE, $course->id, backup::FORMAT_MOODLE, + backup::INTERACTIVE_NO, backup::MODE_GENERAL, 2); + $bc->execute_plan(); + $results = $bc->get_results(); + $file = $results['backup_destination']; + $fp = get_file_packer(); + $filepath = $CFG->dataroot . '/temp/backup/test-restore-course'; + $file->extract_to_pathname($fp, $filepath); + $bc->destroy(); + unset($bc); + + // Now restore the course. + $rc = new restore_controller('test-restore-course', $course2->id, backup::INTERACTIVE_NO, + backup::MODE_GENERAL, 2, backup::TARGET_NEW_COURSE); + $rc->execute_precheck(); + $rc->execute_plan(); + + // Get the created question category. + $restoredcategory = $DB->get_record('question_categories', array('contextid' => context_course::instance($course2->id)->id), + '*', MUST_EXIST); + + // Check that there are two questions in the restored to course's context. + $this->assertEquals(2, $DB->count_records('question', array('category' => $restoredcategory->id))); + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 2d6ddc22aee..37fad078b7b 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -44,6 +44,8 @@ JavaSript: * New "Time spent waiting for the database" performance metric displayed along with the other MDL_PERF vars; the change affects both the error logs and the vars displayed in the page footer. +* Changes in the tag API. The component and contextid are now saved when assigning tags to an item. Please see + tag/upgrade.txt for more information. === 2.6 === diff --git a/mod/wiki/backup/moodle2/restore_wiki_stepslib.php b/mod/wiki/backup/moodle2/restore_wiki_stepslib.php index d90cc52d025..d57f32cfe19 100644 --- a/mod/wiki/backup/moodle2/restore_wiki_stepslib.php +++ b/mod/wiki/backup/moodle2/restore_wiki_stepslib.php @@ -165,7 +165,10 @@ class restore_wiki_activity_structure_step extends restore_activity_structure_st $tag = $data->rawname; $itemid = $this->get_new_parentid('wiki_page'); - tag_set_add('wiki_pages', $itemid, $tag); + $wikiid = $this->get_new_parentid('wiki'); + + $cm = get_coursemodule_from_instance('wiki', $wikiid); + tag_set_add('wiki_pages', $itemid, $tag, 'mod_wiki', context_module::instance($cm->id)->id); } protected function after_execute() { diff --git a/mod/wiki/pagelib.php b/mod/wiki/pagelib.php index 1b387079f2f..7b161cb2113 100644 --- a/mod/wiki/pagelib.php +++ b/mod/wiki/pagelib.php @@ -2035,7 +2035,7 @@ class page_wiki_save extends page_wiki_edit { if ($save && $data) { if (!empty($CFG->usetags)) { - tag_set('wiki_pages', $this->page->id, $data->tags); + tag_set('wiki_pages', $this->page->id, $data->tags, 'mod_wiki', $this->modcontext->id); } $message = '

' . get_string('saving', 'wiki') . '

'; diff --git a/phpunit.xml.dist b/phpunit.xml.dist index dc94ecb8faa..d776f1f0645 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -108,6 +108,9 @@ notes/tests + + tag/tests + rating/tests diff --git a/question/format.php b/question/format.php index 93280a78288..51b46728e4a 100644 --- a/question/format.php +++ b/question/format.php @@ -424,7 +424,7 @@ class qformat_default { if (!empty($CFG->usetags) && isset($question->tags)) { require_once($CFG->dirroot . '/tag/lib.php'); - tag_set('question', $question->id, $question->tags); + tag_set('question', $question->id, $question->tags, 'core_question', $question->context); } if (!empty($result->error)) { diff --git a/question/question.php b/question/question.php index 6bbef30bcc7..45cf3e6ea16 100644 --- a/question/question.php +++ b/question/question.php @@ -238,7 +238,10 @@ if ($mform->is_cancelled()) { /// whence it came. (Where we are moving to is validated by the form.) list($newcatid, $newcontextid) = explode(',', $fromform->category); if (!empty($question->id) && $newcatid != $question->category) { + $contextid = $newcontextid; question_require_capability_on($question, 'move'); + } else { + $contextid = $category->contextid; } // Ensure we redirect back to the category the question is being saved into. @@ -248,7 +251,7 @@ if ($mform->is_cancelled()) { if (!empty($question->id)) { question_require_capability_on($question, 'edit'); } else { - require_capability('moodle/question:add', context::instance_by_id($newcontextid)); + require_capability('moodle/question:add', context::instance_by_id($contextid)); if (!empty($fromform->makecopy) && !$question->formoptions->cansaveasnew) { print_error('nopermissions', '', '', 'edit'); } @@ -258,7 +261,7 @@ if ($mform->is_cancelled()) { // A wizardpage from multipe pages questiontype like calculated may not // allow editing the question tags, hence the isset($fromform->tags) test. require_once($CFG->dirroot.'/tag/lib.php'); - tag_set('question', $question->id, $fromform->tags); + tag_set('question', $question->id, $fromform->tags, 'core_question', $contextid); } // Purge this question from the cache. diff --git a/tag/coursetagslib.php b/tag/coursetagslib.php index 53d4ff045b3..2efdae1f276 100644 --- a/tag/coursetagslib.php +++ b/tag/coursetagslib.php @@ -250,7 +250,7 @@ function coursetag_store_keywords($tags, $courseid, $userid=0, $tagtype='officia tag_type_set($tagid, $tagtype); //tag_instance entry - tag_assign('course', $courseid, $tagid, $ordering, $userid); + tag_assign('course', $courseid, $tagid, $ordering, $userid, 'core', context_course::instance($courseid)->id); //logging - note only for user added tags if ($tagtype == 'default' and $myurl != '') { diff --git a/tag/edit.php b/tag/edit.php index 433d969d7d2..005dfdc79d6 100644 --- a/tag/edit.php +++ b/tag/edit.php @@ -141,7 +141,7 @@ if ($tagnew = $tagform->get_data()) { } //updated related tags - tag_set('tag', $tagnew->id, explode(',', trim($tagnew->relatedtags))); + tag_set('tag', $tagnew->id, explode(',', trim($tagnew->relatedtags)), 'core', $systemcontext); //print_object($tagnew); die(); redirect($CFG->wwwroot.'/tag/index.php?tag='.rawurlencode($tag->name)); // must use $tag here, as the name isn't in the edit form diff --git a/tag/lib.php b/tag/lib.php index b3c3369e0ed..daedf66cd4b 100644 --- a/tag/lib.php +++ b/tag/lib.php @@ -31,10 +31,10 @@ * * BASIC INSTRUCTIONS : * - to "tag a blog post" (for example): - * tag_set('post', $blog_post->id, $array_of_tags); + * tag_set('post', $blog_post->id, $array_of_tags, 'core', $thecontext); * * - to "remove all the tags on a blog post": - * tag_set('post', $blog_post->id, array()); + * tag_set('post', $blog_post->id, array(), 'core', $thecontext); * * Tag set will create tags that need to be created. * @@ -107,17 +107,20 @@ define('TAG_RELATED_CORRELATED', 2); * * This function is meant to be fed the string coming up from the user interface, which contains all tags assigned to a record. * - * @package core_tag + * @package core_tag * @category tag - * @access public - * @param string $record_type the type of record to tag ('post' for blogs, 'user' for users, 'tag' for tags, etc.) - * @param int $record_id the id of the record to tag - * @param array $tags the array of tags to set on the record. If given an empty array, all tags will be removed. - * @return bool|null + * @access public + * @param string $record_type the type of record to tag ('post' for blogs, 'user' for users, 'tag' for tags, etc.) + * @param int $record_id the id of the record to tag + * @param array $tags the array of tags to set on the record. If given an empty array, all tags will be removed. + * @param string|null $component the component that was tagged + * @param int|null $contextid the context id of where this tag was assigned + * @return bool|null */ -function tag_set($record_type, $record_id, $tags) { +function tag_set($record_type, $record_id, $tags, $component = null, $contextid = null) { static $in_recursion_semaphore = false; // this is to prevent loops when tagging a tag + if ( $record_type == 'tag' && !$in_recursion_semaphore) { $current_tagged_tag_name = tag_get_name($record_id); } @@ -162,13 +165,13 @@ function tag_set($record_type, $record_id, $tags) { $tag_current_id = $new_tag[$clean_tag]; } - tag_assign($record_type, $record_id, $tag_current_id, $ordering); + tag_assign($record_type, $record_id, $tag_current_id, $ordering, 0, $component, $contextid); // if we are tagging a tag (adding a manually-assigned related tag), we // need to create the opposite relationship as well. if ( $record_type == 'tag' && !$in_recursion_semaphore) { $in_recursion_semaphore = true; - tag_set_add('tag', $tag_current_id, $current_tagged_tag_name); + tag_set_add('tag', $tag_current_id, $current_tagged_tag_name, $component, $contextid); $in_recursion_semaphore = false; } } @@ -177,14 +180,17 @@ function tag_set($record_type, $record_id, $tags) { /** * Adds a tag to a record, without overwriting the current tags. * - * @package core_tag + * @package core_tag * @category tag - * @access public - * @param string $record_type the type of record to tag ('post' for blogs, 'user' for users, etc.) - * @param int $record_id the id of the record to tag - * @param string $tag the tag to add + * @access public + * @param string $record_type the type of record to tag ('post' for blogs, 'user' for users, etc.) + * @param int $record_id the id of the record to tag + * @param string $tag the tag to add + * @param string|null $component the component that was tagged + * @param int|null $contextid the context id of where this tag was assigned + * @return bool|null */ -function tag_set_add($record_type, $record_id, $tag) { +function tag_set_add($record_type, $record_id, $tag, $component = null, $contextid = null) { $new_tags = array(); foreach( tag_get_tags($record_type, $record_id) as $current_tag ) { @@ -192,20 +198,23 @@ function tag_set_add($record_type, $record_id, $tag) { } $new_tags[] = $tag; - return tag_set($record_type, $record_id, $new_tags); + return tag_set($record_type, $record_id, $new_tags, $component, $contextid); } /** * Removes a tag from a record, without overwriting other current tags. * - * @package core_tag + * @package core_tag * @category tag - * @access public - * @param string $record_type the type of record to tag ('post' for blogs, 'user' for users, etc.) - * @param int $record_id the id of the record to tag - * @param string $tag the tag to delete + * @access public + * @param string $record_type the type of record to tag ('post' for blogs, 'user' for users, etc.) + * @param int $record_id the id of the record to tag + * @param string $tag the tag to delete + * @param string|null $component the component that was tagged + * @param int|null $contextid the context id of where this tag was assigned + * @return bool|null */ -function tag_set_delete($record_type, $record_id, $tag) { +function tag_set_delete($record_type, $record_id, $tag, $component = null, $contextid = null) { $new_tags = array(); foreach( tag_get_tags($record_type, $record_id) as $current_tag ) { @@ -214,7 +223,7 @@ function tag_set_delete($record_type, $record_id, $tag) { } } - return tag_set($record_type, $record_id, $new_tags); + return tag_set($record_type, $record_id, $new_tags, $component, $contextid); } /** @@ -774,17 +783,24 @@ function tag_add($tags, $type="default") { * Assigns a tag to a record; if the record already exists, the time and ordering will be updated. * * @package core_tag - * @access private - * @param string $record_type the type of the record that will be tagged - * @param int $record_id the id of the record that will be tagged - * @param string $tagid the tag id to set on the record. - * @param int $ordering the order of the instance for this record - * @param int $userid (optional) only required for course tagging - * @return bool true on success, false otherwise + * @access private + * @param string $record_type the type of the record that will be tagged + * @param int $record_id the id of the record that will be tagged + * @param string $tagid the tag id to set on the record. + * @param int $ordering the order of the instance for this record + * @param int $userid (optional) only required for course tagging + * @param string|null $component the component that was tagged + * @param int|null $contextid the context id of where this tag was assigned + * @return bool true on success, false otherwise */ -function tag_assign($record_type, $record_id, $tagid, $ordering, $userid = 0) { +function tag_assign($record_type, $record_id, $tagid, $ordering, $userid = 0, $component = null, $contextid = null) { global $DB; + if ($component === null || $contextid === null) { + debugging('You should specify the component and contextid of the item being tagged in your call to tag_assign.', + DEBUG_DEVELOPER); + } + if ( $tag_instance_object = $DB->get_record('tag_instance', array('tagid'=>$tagid, 'itemtype'=>$record_type, 'itemid'=>$record_id, 'tiuserid'=>$userid), 'id')) { $tag_instance_object->ordering = $ordering; $tag_instance_object->timemodified = time(); @@ -792,11 +808,15 @@ function tag_assign($record_type, $record_id, $tagid, $ordering, $userid = 0) { } else { $tag_instance_object = new StdClass; $tag_instance_object->tagid = $tagid; + $tag_instance_object->component = $component; $tag_instance_object->itemid = $record_id; $tag_instance_object->itemtype = $record_type; + $tag_instance_object->contextid = $contextid; $tag_instance_object->ordering = $ordering; - $tag_instance_object->timemodified = time(); + $tag_instance_object->timecreated = time(); + $tag_instance_object->timemodified = $tag_instance_object->timecreated; $tag_instance_object->tiuserid = $userid; + return $DB->insert_record('tag_instance', $tag_instance_object); } } diff --git a/tag/tests/taglib_test.php b/tag/tests/taglib_test.php new file mode 100644 index 00000000000..719d832bac8 --- /dev/null +++ b/tag/tests/taglib_test.php @@ -0,0 +1,146 @@ +. + +/** + * Tag related unit tests. + * + * @package core_tag + * @category test + * @copyright 2014 Mark Nelson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +require_once($CFG->dirroot . '/tag/lib.php'); + +class core_tag_taglib_testcase extends advanced_testcase { + + /** + * Test set up. + * + * This is executed before running any test in this file. + */ + public function setUp() { + $this->resetAfterTest(); + } + + /** + * Test the tag_set function. + */ + public function test_tag_set() { + global $DB; + + // Create a course to tag. + $course = $this->getDataGenerator()->create_course(); + + // Create the tag and tag instance. + tag_set('course', $course->id, array('A random tag'), 'core', context_course::instance($course->id)->id); + + // Get the tag instance that should have been created. + $taginstance = $DB->get_record('tag_instance', array('itemtype' => 'course', 'itemid' => $course->id), '*', MUST_EXIST); + $this->assertEquals('core', $taginstance->component); + $this->assertEquals(context_course::instance($course->id)->id, $taginstance->contextid); + + // Now call the tag_set function without specifying the component or contextid and + // ensure the function debugging is called. + tag_set('course', $course->id, array('Another tag')); + $this->assertDebuggingCalled(); + } + + /** + * Test the tag_set_add function. + */ + public function test_tag_set_add() { + global $DB; + + // Create a course to tag. + $course = $this->getDataGenerator()->create_course(); + + // Create the tag and tag instance. + tag_set_add('course', $course->id, 'A random tag', 'core', context_course::instance($course->id)->id); + + // Get the tag instance that should have been created. + $taginstance = $DB->get_record('tag_instance', array('itemtype' => 'course', 'itemid' => $course->id), '*', MUST_EXIST); + $this->assertEquals('core', $taginstance->component); + $this->assertEquals(context_course::instance($course->id)->id, $taginstance->contextid); + + // Remove the tag we just created. + $tag = $DB->get_record('tag', array('rawname' => 'A random tag')); + tag_delete($tag->id); + + // Now call the tag_set_add function without specifying the component or + // contextid and ensure the function debugging is called. + tag_set_add('course', $course->id, 'Another tag'); + $this->assertDebuggingCalled(); + } + + /** + * Test the tag_set_delete function. + */ + public function test_tag_set_delete() { + global $DB; + + // Create a course to tag. + $course = $this->getDataGenerator()->create_course(); + + // Create the tag and tag instance we are going to delete. + tag_set_add('course', $course->id, 'A random tag', 'core', context_course::instance($course->id)->id); + + // Call the tag_set_delete function. + tag_set_delete('course', $course->id, 'a random tag', 'core', context_course::instance($course->id)->id); + + // Now check that there are no tags or tag instances. + $this->assertEquals(0, $DB->count_records('tag')); + $this->assertEquals(0, $DB->count_records('tag_instance')); + + // Recreate the tag and tag instance. + tag_set_add('course', $course->id, 'A random tag', 'core', context_course::instance($course->id)->id); + + // Now call the tag_set_delete function without specifying the component or + // contextid and ensure the function debugging is called. + tag_set_delete('course', $course->id, 'A random tag'); + $this->assertDebuggingCalled(); + } + + /** + * Test the tag_assign function. + */ + public function test_tag_assign() { + global $DB; + + // Create a course to tag. + $course = $this->getDataGenerator()->create_course(); + + // Create the tag. + $tag = $this->getDataGenerator()->create_tag(); + + // Tag the course with the tag we created. + tag_assign('course', $course->id, $tag->id, 0, 0, 'core', context_course::instance($course->id)->id); + + // Get the tag instance that should have been created. + $taginstance = $DB->get_record('tag_instance', array('itemtype' => 'course', 'itemid' => $course->id), '*', MUST_EXIST); + $this->assertEquals('core', $taginstance->component); + $this->assertEquals(context_course::instance($course->id)->id, $taginstance->contextid); + + // Now call the tag_assign function without specifying the component or + // contextid and ensure the function debugging is called. + tag_assign('course', $course->id, $tag->id, 0, 0); + $this->assertDebuggingCalled(); + } +} diff --git a/tag/upgrade.txt b/tag/upgrade.txt index e8366c8b86e..4340ca4f767 100644 --- a/tag/upgrade.txt +++ b/tag/upgrade.txt @@ -1,6 +1,11 @@ This files describes API changes in tagging, information provided here is intended especially for developers. +=== 2.7 === + +* The functions tag_set, tag_set_add, tag_set_delete and tag_assign now expect the component +and contextid of the item being tagged. + === 2.6 === More cleanup was done to tag cloud sorting which involved some API changes, see MDL_39800 diff --git a/tag/user.php b/tag/user.php index 82b456ac615..f6840f701f9 100644 --- a/tag/user.php +++ b/tag/user.php @@ -21,6 +21,7 @@ if (!confirm_sesskey()) { print_error('sesskey'); } +$usercontext = context_user::instance($USER->id); switch ($action) { case 'addinterest': @@ -28,7 +29,7 @@ switch ($action) { $tag = tag_get_name($id); } - tag_set_add('user', $USER->id, $tag); + tag_set_add('user', $USER->id, $tag, 'core', $usercontext->id); redirect($CFG->wwwroot.'/tag/index.php?tag='. rawurlencode($tag)); break; @@ -38,7 +39,7 @@ switch ($action) { $tag = tag_get_name($id); } - tag_set_delete('user', $USER->id, $tag); + tag_set_delete('user', $USER->id, $tag, 'core', $usercontext->id); redirect($CFG->wwwroot.'/tag/index.php?tag='. rawurlencode($tag)); break; diff --git a/user/editlib.php b/user/editlib.php index 5f436c43972..55c8de2ae9a 100644 --- a/user/editlib.php +++ b/user/editlib.php @@ -174,7 +174,7 @@ function useredit_update_trackforums($user, $usernew) { * @param array $interests */ function useredit_update_interests($user, $interests) { - tag_set('user', $user->id, $interests); + tag_set('user', $user->id, $interests, 'core', context_user::instance($user->id)->id); } /**