From d2ba493cfea58768d5d47630af53fecdf5100c82 Mon Sep 17 00:00:00 2001 From: Andrew Hancox Date: Fri, 24 Mar 2017 15:41:07 +0000 Subject: [PATCH 1/5] MDL-46929 mod_forum: Implement tagging --- lib/searchlib.php | 38 +++++- .../backup/moodle2/backup_forum_stepslib.php | 16 +++ .../backup/moodle2/restore_forum_stepslib.php | 15 +++ mod/forum/classes/output/big_search_form.php | 32 +++++ mod/forum/classes/post_form.php | 7 + mod/forum/db/tag.php | 35 +++++ mod/forum/lang/en/forum.php | 5 + mod/forum/lib.php | 32 ++++- mod/forum/locallib.php | 125 ++++++++++++++++++ mod/forum/post.php | 15 +++ mod/forum/search.php | 41 ++++-- mod/forum/templates/big_search_form.mustache | 15 +++ mod/forum/tests/behat/advanced_search.feature | 17 +++ mod/forum/tests/behat/behat_mod_forum.php | 1 + mod/forum/tests/behat/edit_tags.feature | 69 ++++++++++ mod/forum/tests/generator/lib.php | 21 ++- mod/forum/tests/generator_test.php | 19 ++- mod/forum/tests/lib_test.php | 89 +++++++++++++ mod/forum/version.php | 2 +- .../mod_forum/big_search_form.mustache | 15 +++ 20 files changed, 583 insertions(+), 26 deletions(-) create mode 100644 mod/forum/db/tag.php create mode 100644 mod/forum/tests/behat/edit_tags.feature diff --git a/lib/searchlib.php b/lib/searchlib.php index 1393cd4dd3266..79217836de010 100644 --- a/lib/searchlib.php +++ b/lib/searchlib.php @@ -38,6 +38,7 @@ define("TOKEN_DATEFROM","6"); define("TOKEN_DATETO","7"); define("TOKEN_INSTANCE","8"); +define("TOKEN_TAGS","9"); /** * Class to hold token/value pairs after they're parsed. @@ -110,6 +111,14 @@ public function __construct(&$parser){ $this->addExitPattern("\s","indatefrom"); + // If we see the string tags: while in the base accept state, start + // parsing tags and go to the intags state. + $this->addEntryPattern("tags:\S+","accept","intags"); + + // Snarf everything into the tags until we see whitespace, then exit + // back to the base accept state. + $this->addExitPattern("\s","intags"); + // Patterns to handle strings of the form dateto:foo // If we see the string dateto: while in the base accept state, start @@ -268,6 +277,17 @@ function indateto($content){ return true; } + // State for handling tags:tagname,tagname constructs. Potentially emits a token. + function intags($content){ + if (strlen($content) < 5) { // State exit or missing parameter. + return true; + } + // Strip off the tags: part and add the reminder to the parsed token array + $param = trim(substr($content,5)); + $this->tokens[] = new search_token(TOKEN_TAGS,$param); + return true; + } + // State for handling instance:foo constructs. Potentially emits a token. function ininstance($content){ if (strlen($content) < 10) { // State exit or missing parameter. @@ -390,7 +410,8 @@ function search_generate_text_SQL($parsetree, $datafield, $metafield, $mainidfie * @global object */ function search_generate_SQL($parsetree, $datafield, $metafield, $mainidfield, $useridfield, - $userfirstnamefield, $userlastnamefield, $timefield, $instancefield) { + $userfirstnamefield, $userlastnamefield, $timefield, $instancefield, + $tagfields = []) { global $CFG, $DB; static $p = 0; @@ -407,7 +428,7 @@ function search_generate_SQL($parsetree, $datafield, $metafield, $mainidfield, $ } $SQLString = ''; - + $nexttagfield = 0; for ($i=0; $i<$ntokens; $i++){ if ($i > 0) {// We have more than one clause, need to tack on AND $SQLString .= ' AND '; @@ -465,6 +486,19 @@ function search_generate_SQL($parsetree, $datafield, $metafield, $mainidfield, $ $SQLString .= "($timefield >= :$name1)"; $params[$name1] = $value; break; + case TOKEN_TAGS: + $sqlstrings = []; + foreach (explode(',', $value) as $tag) { + $paramname = $name1 . '_' . $nexttagfield; + if (!isset($tagfields[$nexttagfield])) { + throw new coding_exception('Not enough tag fields supplied for search.'); + } + $sqlstrings[] = "($tagfields[$nexttagfield] = :$paramname)"; + $params[$paramname] = $tag; + $nexttagfield++; + } + $SQLString .= implode(' AND ', $sqlstrings); + break; case TOKEN_NEGATE: $SQLString .= "(NOT ((".$DB->sql_like($datafield, ":$name1", false).") OR (".$DB->sql_like($metafield, ":$name2", false).")))"; $params[$name1] = "%$value%"; diff --git a/mod/forum/backup/moodle2/backup_forum_stepslib.php b/mod/forum/backup/moodle2/backup_forum_stepslib.php index c01e33a22a31d..50a6b077157d1 100644 --- a/mod/forum/backup/moodle2/backup_forum_stepslib.php +++ b/mod/forum/backup/moodle2/backup_forum_stepslib.php @@ -60,6 +60,9 @@ protected function define_structure() { 'mailed', 'subject', 'message', 'messageformat', 'messagetrust', 'attachment', 'totalscore', 'mailnow')); + $tags = new backup_nested_element('tags'); + $tag = new backup_nested_element('tag', array('id'), array('name', 'rawname')); + $ratings = new backup_nested_element('ratings'); $rating = new backup_nested_element('rating', array('id'), array( @@ -113,6 +116,9 @@ protected function define_structure() { $discussion->add_child($posts); $posts->add_child($post); + $posts->add_child($tags); + $tags->add_child($tag); + $post->add_child($ratings); $ratings->add_child($rating); @@ -147,6 +153,16 @@ protected function define_structure() { 'ratingarea' => backup_helper::is_sqlparam('post'), 'itemid' => backup::VAR_PARENTID)); $rating->set_source_alias('rating', 'value'); + + $tag->set_source_sql('SELECT t.id, t.name, t.rawname + FROM {tag} t + JOIN {tag_instance} ti ON ti.tagid = t.id + WHERE ti.itemtype = ? + AND ti.component = ? + AND ti.itemid = ?', array( + backup_helper::is_sqlparam('forum_posts'), + backup_helper::is_sqlparam('mod_forum'), + backup::VAR_PARENTID)); } // Define id annotations diff --git a/mod/forum/backup/moodle2/restore_forum_stepslib.php b/mod/forum/backup/moodle2/restore_forum_stepslib.php index de8b038fdb6df..26595d60cdbf2 100644 --- a/mod/forum/backup/moodle2/restore_forum_stepslib.php +++ b/mod/forum/backup/moodle2/restore_forum_stepslib.php @@ -40,6 +40,7 @@ protected function define_structure() { if ($userinfo) { $paths[] = new restore_path_element('forum_discussion', '/activity/forum/discussions/discussion'); $paths[] = new restore_path_element('forum_post', '/activity/forum/discussions/discussion/posts/post'); + $paths[] = new restore_path_element('forum_tag', '/activity/forum/discussions/discussion/posts/post/tags/tag'); $paths[] = new restore_path_element('forum_discussion_sub', '/activity/forum/discussions/discussion/discussion_subs/discussion_sub'); $paths[] = new restore_path_element('forum_rating', '/activity/forum/discussions/discussion/posts/post/ratings/rating'); $paths[] = new restore_path_element('forum_subscription', '/activity/forum/subscriptions/subscription'); @@ -117,6 +118,20 @@ protected function process_forum_post($data) { } } + protected function process_forum_tag($data) { + $data = (object)$data; + + if (!core_tag_tag::is_enabled('mod_forum', 'forum_posts')) { // Tags disabled in server, nothing to process. + return; + } + + $tag = $data->rawname; + $itemid = $this->get_new_parentid('forum_post'); + + $context = context_module::instance($this->task->get_moduleid()); + core_tag_tag::add_item_tag('mod_forum', 'forum_posts', $itemid, $context, $tag); + } + protected function process_forum_rating($data) { global $DB; diff --git a/mod/forum/classes/output/big_search_form.php b/mod/forum/classes/output/big_search_form.php index 814510369bb27..b4ae556b0651c 100644 --- a/mod/forum/classes/output/big_search_form.php +++ b/mod/forum/classes/output/big_search_form.php @@ -52,6 +52,7 @@ class big_search_form implements renderable, templatable { public $subject; public $user; public $words; + public $tags; /** @var string The URL of the search form. */ public $actionurl; @@ -64,6 +65,7 @@ class big_search_form implements renderable, templatable { public function __construct($course) { global $DB; $this->course = $course; + $this->tags = []; $this->showfullwords = $DB->get_dbfamily() == 'mysql' || $DB->get_dbfamily() == 'postgres'; $this->actionurl = new moodle_url('/mod/forum/search.php'); @@ -148,6 +150,15 @@ public function set_words($value) { $this->words = $value; } + /** + * Set tags. + * + * @param mixed $value Tags. + */ + public function set_tags($value) { + $this->tags = $value; + } + /** * Forum ID setter search criteria. * @@ -158,6 +169,7 @@ public function set_forumid($forumid) { } public function export_for_template(renderer_base $output) { + global $DB, $CFG, $PAGE; $data = new stdClass(); $data->courseid = $this->course->id; @@ -172,6 +184,26 @@ public function export_for_template(renderer_base $output) { $data->showfullwords = $this->showfullwords; $data->actionurl = $this->actionurl->out(false); + $tagtypestoshow = \core_tag_area::get_showstandard('mod_forum', 'forum_posts'); + $showstandard = ($tagtypestoshow != \core_tag_tag::HIDE_STANDARD); + $typenewtags = ($tagtypestoshow != \core_tag_tag::STANDARD_ONLY); + + $PAGE->requires->js_call_amd('core/form-autocomplete', 'enhance', $params = array('#tags', $typenewtags, '', + get_string('entertags', 'tag'), false, $showstandard, get_string('noselection', 'form'))); + + $data->tagsenabled = \core_tag_tag::is_enabled('mod_forum', 'forum_posts'); + $namefield = empty($CFG->keeptagnamecase) ? 'name' : 'rawname'; + $tags = $DB->get_records('tag', + array('isstandard' => 1, 'tagcollid' => \core_tag_area::get_collection('mod_forum', 'forum_posts')), + $namefield, 'rawname,' . $namefield . ' as fieldname'); + $data->tags = []; + foreach ($tags as $tag) { + $data->tagoptions[] = ['value' => $tag->rawname, + 'text' => $tag->fieldname, + 'selected' => in_array($tag->rawname, $this->tags) + ]; + } + $datefrom = $this->datefrom; if (empty($datefrom)) { $datefrom = make_timestamp(2000, 1, 1, 0, 0, 0); diff --git a/mod/forum/classes/post_form.php b/mod/forum/classes/post_form.php index a9e8e82e7eb82..0ade6fb92307e 100644 --- a/mod/forum/classes/post_form.php +++ b/mod/forum/classes/post_form.php @@ -238,6 +238,13 @@ function definition() { $mform->setConstants(array('timestart' => 0, 'timeend' => 0)); } + if (core_tag_tag::is_enabled('mod_forum', 'forum_posts')) { + $mform->addElement('header', 'tagshdr', get_string('tags', 'tag')); + + $mform->addElement('tags', 'tags', get_string('tags'), + array('itemtype' => 'forum_posts', 'component' => 'mod_forum')); + } + //------------------------------------------------------------------------------- // buttons if (isset($post->edit)) { // hack alert diff --git a/mod/forum/db/tag.php b/mod/forum/db/tag.php new file mode 100644 index 0000000000000..849640849d317 --- /dev/null +++ b/mod/forum/db/tag.php @@ -0,0 +1,35 @@ +. + +/** + * Tag areas in component mod_forum + * + * @package mod_forum + * @copyright 2017 Andrew Hancox + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + + +$tagareas = array( + array( + 'itemtype' => 'forum_posts', + 'component' => 'mod_forum', + 'callback' => 'mod_forum_get_tagged_posts', + 'callbackfile' => '/mod/forum/locallib.php', + ), +); diff --git a/mod/forum/lang/en/forum.php b/mod/forum/lang/en/forum.php index 415c599418500..4ee96940cfd27 100644 --- a/mod/forum/lang/en/forum.php +++ b/mod/forum/lang/en/forum.php @@ -430,6 +430,7 @@ $string['qandanotify'] = 'This is a question and answer forum. In order to see other responses to these questions, you must first post your answer'; $string['re'] = 'Re:'; $string['readtherest'] = 'Read the rest of this topic'; +$string['removeallforumtags'] = 'Remove all forum tags'; $string['replies'] = 'Replies'; $string['repliesmany'] = '{$a} replies so far'; $string['repliesone'] = '{$a} reply so far'; @@ -464,6 +465,7 @@ $string['searchphrase'] = 'This exact phrase must appear in the post'; $string['searchresults'] = 'Search results'; $string['searchsubject'] = 'These words should be in the subject'; +$string['searchtags'] = 'Is tagged with'; $string['searchuser'] = 'This name should match the author'; $string['searchuserid'] = 'The Moodle ID of the author'; $string['searchwhichforums'] = 'Choose which forums to search'; @@ -503,6 +505,9 @@ $string['subscriptionauto'] = 'Auto subscription'; $string['subscriptiondisabled'] = 'Subscription disabled'; $string['subscriptions'] = 'Subscriptions'; +$string['tagarea_forum_posts'] = 'Forum posts'; +$string['tagsdeleted'] = 'Forum tags have been deleted'; +$string['tagtitle'] = 'See the "{$a}" tag'; $string['thisforumisthrottled'] = 'This forum has a limit to the number of forum postings you can make in a given time period - this is currently set at {$a->blockafter} posting(s) in {$a->blockperiod}'; $string['timedhidden'] = 'Timed status: Hidden from students'; $string['timedposts'] = 'Timed posts'; diff --git a/mod/forum/lib.php b/mod/forum/lib.php index b66923e37cac8..3302aecf1f0a2 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -2095,15 +2095,32 @@ function forum_search_posts($searchterms, $courseid=0, $limitfrom=0, $limitnum=5 if ($lexer->parse($searchstring)) { $parsearray = $parser->get_parsed_array(); + + $tagjoins = ''; + $tagfields = []; + foreach ($parsearray as $token) { + if ($token->getType() == TOKEN_TAGS) { + for ($i = substr_count($token->getValue(), ','); $i >= 0; $i--) { + $tagjoins .= "\n LEFT JOIN {tag_instance} ti_$i + ON p.id = ti_$i.itemid + AND ti_$i.component='mod_forum' + AND ti_$i.itemtype = 'forum_posts'"; + $tagjoins .= "\n LEFT JOIN {tag} t_$i ON t_$i.id = ti_$i.tagid"; + $tagfields[] = "t_$i.rawname"; + + } + } + } list($messagesearch, $msparams) = search_generate_SQL($parsearray, 'p.message', 'p.subject', 'p.userid', 'u.id', 'u.firstname', - 'u.lastname', 'p.modified', 'd.forum'); + 'u.lastname', 'p.modified', 'd.forum', + $tagfields); $params = array_merge($params, $msparams); } - $fromsql = "{forum_posts} p, - {forum_discussions} d, - {user} u"; + $fromsql = "{forum_posts} p + INNER JOIN {forum_discussions} d ON d.id = p.discussion + INNER JOIN {user} u ON u.id = p.userid $tagjoins"; $selectsql = " $messagesearch AND p.discussion = d.id @@ -3446,6 +3463,10 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa $postcontent .= html_writer::tag('div', $attachedimages, array('class'=>'attachedimages')); } + if (\core_tag_tag::is_enabled('mod_forum', 'forum_posts')) { + $postcontent .= $OUTPUT->tag_list(core_tag_tag::get_item_tags('mod_forum', 'forum_posts', $post->id), null, 'forum-tags'); + } + // Output the post content $output .= html_writer::tag('div', $postcontent, array('class'=>'posting '.$postclass)); $output .= html_writer::end_tag('div'); // Content @@ -5262,7 +5283,8 @@ function forum_user_can_see_post($forum, $discussion, $post, $user=NULL, $cm=NUL $user = $USER; } - $canviewdiscussion = !empty($cm->cache->caps['mod/forum:viewdiscussion']) || has_capability('mod/forum:viewdiscussion', $modcontext, $user->id); + $canviewdiscussion = (isset($cm->cache) && !empty($cm->cache->caps['mod/forum:viewdiscussion'])) + || has_capability('mod/forum:viewdiscussion', $modcontext, $user->id); if (!$canviewdiscussion && !has_all_capabilities(array('moodle/user:viewdetails', 'moodle/user:readuserposts'), context_user::instance($post->userid))) { return false; } diff --git a/mod/forum/locallib.php b/mod/forum/locallib.php index 057339a97bd3a..527cba67fdb95 100644 --- a/mod/forum/locallib.php +++ b/mod/forum/locallib.php @@ -566,3 +566,128 @@ public function get_parent() { return $this->browser->get_file_info($this->context); } } + +/** + * Returns forum posts tagged with a specified tag. + * + * This is a callback used by the tag area mod_forum/forum_posts to search for forum posts + * tagged with a specific tag. + * + * @param core_tag_tag $tag + * @param bool $exclusivemode if set to true it means that no other entities tagged with this tag + * are displayed on the page and the per-page limit may be bigger + * @param int $fromctx context id where the link was displayed, may be used by callbacks + * to display items in the same context first + * @param int $ctx context id where to search for records + * @param bool $rec search in subcontexts as well + * @param int $page 0-based number of page being displayed + * @return \core_tag\output\tagindex + */ +function mod_forum_get_tagged_posts($tag, $exclusivemode = false, $fromctx = 0, $ctx = 0, $rec = 1, $page = 0) { + global $OUTPUT; + $perpage = $exclusivemode ? 20 : 5; + + // Build the SQL query. + $ctxselect = context_helper::get_preload_record_columns_sql('ctx'); + $query = "SELECT fp.id, fp.subject, fd.forum, fp.discussion, f.type, fd.timestart, fd.timeend, fd.groupid, fp.parent, fp.userid, + cm.id AS cmid, c.id AS courseid, c.shortname, c.fullname, $ctxselect + FROM {forum_posts} fp + JOIN {forum_discussions} fd ON fp.discussion = fd.id + JOIN {forum} f ON f.id = fd.forum + JOIN {modules} m ON m.name='forum' + JOIN {course_modules} cm ON cm.module = m.id AND cm.instance = f.id + JOIN {tag_instance} tt ON fp.id = tt.itemid + JOIN {course} c ON cm.course = c.id + JOIN {context} ctx ON ctx.instanceid = cm.id AND ctx.contextlevel = :coursemodulecontextlevel + WHERE tt.itemtype = :itemtype AND tt.tagid = :tagid AND tt.component = :component + AND cm.deletioninprogress = 0 + AND fp.id %ITEMFILTER% AND c.id %COURSEFILTER%"; + + $params = array('itemtype' => 'forum_posts', 'tagid' => $tag->id, 'component' => 'mod_forum', + 'coursemodulecontextlevel' => CONTEXT_MODULE); + + if ($ctx) { + $context = $ctx ? context::instance_by_id($ctx) : context_system::instance(); + $query .= $rec ? ' AND (ctx.id = :contextid OR ctx.path LIKE :path)' : ' AND ctx.id = :contextid'; + $params['contextid'] = $context->id; + $params['path'] = $context->path.'/%'; + } + + $query .= " ORDER BY "; + if ($fromctx) { + // In order-clause specify that modules from inside "fromctx" context should be returned first. + $fromcontext = context::instance_by_id($fromctx); + $query .= ' (CASE WHEN ctx.id = :fromcontextid OR ctx.path LIKE :frompath THEN 0 ELSE 1 END),'; + $params['fromcontextid'] = $fromcontext->id; + $params['frompath'] = $fromcontext->path.'/%'; + } + $query .= ' c.sortorder, cm.id, fp.id'; + + $totalpages = $page + 1; + + // Use core_tag_index_builder to build and filter the list of items. + $builder = new core_tag_index_builder('mod_forum', 'forum_posts', $query, $params, $page * $perpage, $perpage + 1); + while ($item = $builder->has_item_that_needs_access_check()) { + context_helper::preload_from_record($item); + $courseid = $item->courseid; + if (!$builder->can_access_course($courseid)) { + $builder->set_accessible($item, false); + continue; + } + $modinfo = get_fast_modinfo($builder->get_course($courseid)); + // Set accessibility of this item and all other items in the same course. + $builder->walk(function ($taggeditem) use ($courseid, $modinfo, $builder) { + if ($taggeditem->courseid == $courseid) { + $cm = $modinfo->get_cm($taggeditem->cmid); + $forum = (object)['id' => $taggeditem->forum, + 'course' => $taggeditem->courseid, + 'type' => $taggeditem->type + ]; + $discussion = (object)['id' => $taggeditem->discussion, + 'timestart' => $taggeditem->timestart, + 'timeend' => $taggeditem->timeend, + 'groupid' => $taggeditem->groupid + ]; + $post = (object)['id' => $taggeditem->id, + 'parent' => $taggeditem->parent, + 'userid' => $taggeditem->userid, + 'groupid' => $taggeditem->groupid + ]; + + $accessible = forum_user_can_see_post($forum, $discussion, $post, null, $cm); + $builder->set_accessible($taggeditem, $accessible); + } + }); + } + + $items = $builder->get_items(); + if (count($items) > $perpage) { + $totalpages = $page + 2; // We don't need exact page count, just indicate that the next page exists. + array_pop($items); + } + + // Build the display contents. + if ($items) { + $tagfeed = new core_tag\output\tagfeed(); + foreach ($items as $item) { + context_helper::preload_from_record($item); + $modinfo = get_fast_modinfo($item->courseid); + $cm = $modinfo->get_cm($item->cmid); + $pageurl = new moodle_url('/mod/forum/discuss.php', array('d' => $item->discussion), 'p' . $item->id); + $pagename = format_string($item->subject, true, array('context' => context_module::instance($item->cmid))); + $pagename = html_writer::link($pageurl, $pagename); + $courseurl = course_get_url($item->courseid, $cm->sectionnum); + $cmname = html_writer::link($cm->url, $cm->get_formatted_name()); + $coursename = format_string($item->fullname, true, array('context' => context_course::instance($item->courseid))); + $coursename = html_writer::link($courseurl, $coursename); + $icon = html_writer::link($pageurl, html_writer::empty_tag('img', array('src' => $cm->get_icon_url()))); + $tagfeed->add($icon, $pagename, $cmname.'
'.$coursename); + } + + $content = $OUTPUT->render_from_template('core_tag/tagfeed', + $tagfeed->export_for_template($OUTPUT)); + + return new core_tag\output\tagindex($tag, 'mod_forum', 'forum_posts', $content, + $exclusivemode, $fromctx, $ctx, $rec, $page, $totalpages); + } +} diff --git a/mod/forum/post.php b/mod/forum/post.php index 1933800e6fe74..c87780efdbd48 100644 --- a/mod/forum/post.php +++ b/mod/forum/post.php @@ -757,6 +757,8 @@ $discussionurl = new moodle_url("/mod/forum/discuss.php", array('d' => $discussion->id), 'p' . $fromform->id); } + core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $fromform->id, $modcontext, $fromform->tags); + $params = array( 'context' => $modcontext, 'objectid' => $fromform->id, @@ -809,6 +811,10 @@ $discussionurl = new moodle_url("/mod/forum/discuss.php", array('d' => $discussion->id), 'p'.$fromform->id); } + if (core_tag_tag::is_enabled('mod_forum', 'forum_posts') && isset($data->tags)) { + core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $fromform->id, $modcontext, $fromform->tags); + } + $params = array( 'context' => $modcontext, 'objectid' => $fromform->id, @@ -936,6 +942,8 @@ $completion->update_state($cm, COMPLETION_COMPLETE); } + core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $fromform->firstpost, $modcontext, $fromform->tags); + // Redirect back to the discussion. redirect( forum_go_back_to($redirectto->out()), @@ -1052,6 +1060,13 @@ if (!empty($formheading)) { echo $OUTPUT->heading($formheading, 2, array('class' => 'accesshide')); } + +$data = new StdClass(); +if (isset($postid)) { + $data->tags = core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $postid); + $mform_post->set_data($data); +} + $mform_post->display(); echo $OUTPUT->footer(); diff --git a/mod/forum/search.php b/mod/forum/search.php index ec399f94d7736..9cfed3b66c3be 100644 --- a/mod/forum/search.php +++ b/mod/forum/search.php @@ -38,6 +38,7 @@ $words = trim(optional_param('words', '', PARAM_NOTAGS)); // Words $fullwords = trim(optional_param('fullwords', '', PARAM_NOTAGS)); // Whole words $notwords = trim(optional_param('notwords', '', PARAM_NOTAGS)); // Words we don't want +$tags = optional_param_array('tags', [], PARAM_TEXT); $timefromrestrict = optional_param('timefromrestrict', 0, PARAM_INT); // Use starting date $fromday = optional_param('fromday', 0, PARAM_INT); // Starting date @@ -101,6 +102,9 @@ if (!empty($dateto)) { $search .= ' dateto:'.$dateto; } + if (!empty($tags)) { + $search .= ' tags:' . implode(',', $tags); + } $individualparams = true; } else { $individualparams = false; @@ -186,19 +190,27 @@ $PAGE->set_button($searchform); echo $OUTPUT->header(); echo '
'; -echo ''.get_string('advancedsearch','forum').'...'; + +$params = [ + 'id' => $course->id, + 'user' => $user, + 'userid' => $userid, + 'forumid' => $forumid, + 'subject' => $subject, + 'phrase' => $phrase, + 'words' => $words, + 'fullwords' => $fullwords, + 'notwords' => $notwords, + 'dateto' => $dateto, + 'datefrom' => $datefrom, + 'showform' => 1 +]; +$url = new moodle_url("/mod/forum/search.php", $params); +foreach ($tags as $tag) { + $url .= "&tags[]=$tag"; +} +echo html_writer::link($url, get_string('advancedsearch', 'forum').'...'); + echo '
'; echo $OUTPUT->heading($strforums, 2); @@ -318,7 +330,7 @@ * @return void The function prints the form. */ function forum_print_big_search_form($course) { - global $PAGE, $words, $subject, $phrase, $user, $fullwords, $notwords, $datefrom, $dateto, $forumid; + global $PAGE, $words, $subject, $phrase, $user, $fullwords, $notwords, $datefrom, $dateto, $forumid, $tags; $renderable = new \mod_forum\output\big_search_form($course, $user); $renderable->set_words($words); @@ -330,6 +342,7 @@ function forum_print_big_search_form($course) { $renderable->set_subject($subject); $renderable->set_user($user); $renderable->set_forumid($forumid); + $renderable->set_tags($tags); $output = $PAGE->get_renderer('mod_forum'); echo $output->render($renderable); diff --git a/mod/forum/templates/big_search_form.mustache b/mod/forum/templates/big_search_form.mustache index f9371f2c3e03b..4d26e5c24cb22 100644 --- a/mod/forum/templates/big_search_form.mustache +++ b/mod/forum/templates/big_search_form.mustache @@ -144,6 +144,21 @@ + {{#tagsenabled}} + + + + + + + + + {{/tagsenabled}}
diff --git a/mod/forum/tests/behat/advanced_search.feature b/mod/forum/tests/behat/advanced_search.feature index c9134a85b67ff..0f8a2b4b25766 100644 --- a/mod/forum/tests/behat/advanced_search.feature +++ b/mod/forum/tests/behat/advanced_search.feature @@ -18,6 +18,9 @@ Feature: The forum search allows users to perform advanced searches for forum po | teacher1 | C1 | editingteacher | | teacher2 | C1 | editingteacher | | student1 | C1 | student | + And the following "tags" exist: + | name | isstandard | + | SearchedTag | 1 | And I log in as "teacher1" And I am on "Course 1" course homepage with editing mode on And I add the "Latest announcements" block @@ -27,6 +30,7 @@ Feature: The forum search allows users to perform advanced searches for forum po And I add a new topic to "Announcements" forum with: | Subject | My subject | | Message | My message | + | Tags | SearchedTag | And I am on "Course 1" course homepage And I add a new topic to "Announcements" forum with: | Subject | My subjective| @@ -108,3 +112,16 @@ Feature: The forum search allows users to perform advanced searches for forum po When I press "Search forums" Then I should not see "My message" And I should see "My subjective" + + @javascript + Scenario: Perform an advanced search using tags + Given I log in as "student1" + And I follow "Course 1" + And I follow "Announcements" + And I press "Search forums" + And I should see "Advanced search" + And I set the field "Is tagged with" to "SearchedTag" + And I click on "[data-value='SearchedTag']" "css_element" + When I press "Search forums" + Then I should see "My subject" + And I should not see "My subjective" diff --git a/mod/forum/tests/behat/behat_mod_forum.php b/mod/forum/tests/behat/behat_mod_forum.php index 3a8cd701f23ad..b546457a798d4 100644 --- a/mod/forum/tests/behat/behat_mod_forum.php +++ b/mod/forum/tests/behat/behat_mod_forum.php @@ -77,6 +77,7 @@ public function i_reply_post_from_forum_with($postsubject, $forumname, TableNode // Fill form and post. $this->execute('behat_forms::i_set_the_following_fields_to_these_values', $table); + $this->execute("behat_general::wait_until_the_page_is_ready"); $this->execute('behat_forms::press_button', get_string('posttoforum', 'forum')); $this->execute('behat_general::i_wait_to_be_redirected'); diff --git a/mod/forum/tests/behat/edit_tags.feature b/mod/forum/tests/behat/edit_tags.feature new file mode 100644 index 0000000000000..0ab3fa84b9a63 --- /dev/null +++ b/mod/forum/tests/behat/edit_tags.feature @@ -0,0 +1,69 @@ +@mod @mod_forum @core_tag @javascript +Feature: Edited forum posts handle tags correctly + In order to get forum posts properly labelled + As a user + I need to introduce the tags while editing + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + And the following "courses" exist: + | fullname | shortname | format | + | Course 1 | C1 | topics | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + And I log in as "teacher1" + And I follow "Course 1" + And I turn editing mode on + And I add a "Forum" to section "1" and I fill the form with: + | Forum name | Test forum name | + | Description | Test forum description | + And I add a new discussion to "Test forum name" forum with: + | Subject | Teacher post subject | + | Message | Teacher post message | + And I log out + + Scenario: Forum post edition of custom tags works as expected + Given I log in as "student1" + And I follow "Course 1" + And I reply "Teacher post subject" post from "Test forum name" forum with: + | Subject | Student post subject | + | Message | Student post message | + | Tags | Tag1 | + Then I should see "Tag1" in the ".forum-tags" "css_element" + And I click on "Edit" "link" in the "//div[@aria-label='Student post subject by Student 1']" "xpath_element" + Then I should see "Tag1" in the ".form-autocomplete-selection" "css_element" + + @javascript + Scenario: Forum post edition of standard tags works as expected + Given I log in as "admin" + And I navigate to "Appearance > Manage tags" in site administration + And I follow "Default collection" + And I follow "Add standard tags" + And I set the field "Enter comma-separated list of new tags" to "OT1, OT2, OT3" + And I press "Continue" + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Test forum" + And I click on "Add a new discussion topic" "button" + And I expand all fieldsets + And I click on ".form-autocomplete-downarrow" "css_element" + And I should see "OT1" in the ".form-autocomplete-suggestions" "css_element" + And I should see "OT2" in the ".form-autocomplete-suggestions" "css_element" + And I should see "OT3" in the ".form-autocomplete-suggestions" "css_element" + And I reply "Teacher post subject" post from "Test forum name" forum with: + | Subject | Student post subject | + | Message | Student post message | + | Tags | OT1, OT3 | + Then I should see "OT1" in the ".forum-tags" "css_element" + And I should see "OT3" in the ".forum-tags" "css_element" + And I should not see "OT2" in the ".forum-tags" "css_element" + And I click on "Edit" "link" in the "//div[@aria-label='Student post subject by Teacher 1']" "xpath_element" + And I should see "OT1" in the ".form-autocomplete-selection" "css_element" + And I should see "OT3" in the ".form-autocomplete-selection" "css_element" + And I should not see "OT2" in the ".form-autocomplete-selection" "css_element" diff --git a/mod/forum/tests/generator/lib.php b/mod/forum/tests/generator/lib.php index 818b8cee93b1c..151a38cde6b3f 100644 --- a/mod/forum/tests/generator/lib.php +++ b/mod/forum/tests/generator/lib.php @@ -202,9 +202,9 @@ public function create_discussion($record = null) { // Add the discussion. $record->id = forum_add_discussion($record, null, null, $record->userid); - if (isset($timemodified) || isset($mailed)) { - $post = $DB->get_record('forum_posts', array('discussion' => $record->id)); + $post = $DB->get_record('forum_posts', array('discussion' => $record->id)); + if (isset($timemodified) || isset($mailed)) { if (isset($mailed)) { $post->mailed = $mailed; } @@ -220,6 +220,14 @@ public function create_discussion($record = null) { $DB->update_record('forum_posts', $post); } + if (property_exists($record, 'tags')) { + $cm = get_coursemodule_from_instance('forum', $record->forum); + $tags = is_array($record->tags) ? $record->tags : preg_split('/,/', $record->tags); + + core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $post->id, + context_module::instance($cm->id), $tags); + } + return $record; } @@ -297,6 +305,15 @@ public function create_post($record = null) { // Add the post. $record->id = $DB->insert_record('forum_posts', $record); + if (property_exists($record, 'tags')) { + $discussion = $DB->get_record('forum_discussions', ['id' => $record->discussion]); + $cm = get_coursemodule_from_instance('forum', $discussion->forum); + $tags = is_array($record->tags) ? $record->tags : preg_split('/,/', $record->tags); + + core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $record->id, + context_module::instance($cm->id), $tags); + } + // Update the last post. forum_discussion_update_last_post($record->discussion); diff --git a/mod/forum/tests/generator_test.php b/mod/forum/tests/generator_test.php index 0be1d4ac96ab1..8a4e20fc4038b 100644 --- a/mod/forum/tests/generator_test.php +++ b/mod/forum/tests/generator_test.php @@ -117,6 +117,11 @@ public function test_create_discussion() { // Check the discussions were correctly created. $this->assertEquals(3, $DB->count_records_select('forum_discussions', 'forum = :forum', array('forum' => $forum->id))); + + $record['tags'] = array('Cats', 'mice'); + $record = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); + $this->assertEquals(array('Cats', 'mice'), + array_values(core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $record->firstpost))); } /** @@ -160,6 +165,11 @@ public function test_create_post() { // is generated as well, so we should have 4 posts, not 3. $this->assertEquals(4, $DB->count_records_select('forum_posts', 'discussion = :discussion', array('discussion' => $discussion->id))); + + $record->tags = array('Cats', 'mice'); + $record = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); + $this->assertEquals(array('Cats', 'mice'), + array_values(core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $record->id))); } public function test_create_content() { @@ -187,16 +197,21 @@ public function test_create_content() { $post3 = $generator->create_content($forum, array('discussion' => $post1->discussion)); // This should create posts answering another post. $post4 = $generator->create_content($forum, array('parent' => $post2->id)); + // This should create post with tags. + $post5 = $generator->create_content($forum, array('parent' => $post2->id, 'tags' => array('Cats', 'mice'))); $discussionrecords = $DB->get_records('forum_discussions', array('forum' => $forum->id)); $postrecords = $DB->get_records('forum_posts'); $postrecords2 = $DB->get_records('forum_posts', array('discussion' => $post1->discussion)); $this->assertEquals(1, count($discussionrecords)); - $this->assertEquals(4, count($postrecords)); - $this->assertEquals(4, count($postrecords2)); + $this->assertEquals(5, count($postrecords)); + $this->assertEquals(5, count($postrecords2)); $this->assertEquals($post1->id, $discussionrecords[$post1->discussion]->firstpost); $this->assertEquals($post1->id, $postrecords[$post2->id]->parent); $this->assertEquals($post1->id, $postrecords[$post3->id]->parent); $this->assertEquals($post2->id, $postrecords[$post4->id]->parent); + + $this->assertEquals(array('Cats', 'mice'), + array_values(core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $post5->id))); } } diff --git a/mod/forum/tests/lib_test.php b/mod/forum/tests/lib_test.php index 0af8f26159876..59ed1cb6fea8b 100644 --- a/mod/forum/tests/lib_test.php +++ b/mod/forum/tests/lib_test.php @@ -26,6 +26,7 @@ global $CFG; require_once($CFG->dirroot . '/mod/forum/lib.php'); +require_once($CFG->dirroot . '/mod/forum/locallib.php'); require_once($CFG->dirroot . '/rating/lib.php'); class mod_forum_lib_testcase extends advanced_testcase { @@ -3410,6 +3411,94 @@ public function test_forum_core_calendar_provide_event_action_already_completed( $this->assertNull($actionevent); } + public function test_mod_forum_get_tagged_posts() { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + // Setup test data. + $forumgenerator = $this->getDataGenerator()->get_plugin_generator('mod_forum'); + $course3 = $this->getDataGenerator()->create_course(); + $course2 = $this->getDataGenerator()->create_course(); + $course1 = $this->getDataGenerator()->create_course(); + $forum1 = $this->getDataGenerator()->create_module('forum', array('course' => $course1->id)); + $forum2 = $this->getDataGenerator()->create_module('forum', array('course' => $course2->id)); + $forum3 = $this->getDataGenerator()->create_module('forum', array('course' => $course3->id)); + $post11 = $forumgenerator->create_content($forum1, array('tags' => array('Cats', 'Dogs'))); + $post12 = $forumgenerator->create_content($forum1, array('tags' => array('Cats', 'mice'))); + $post13 = $forumgenerator->create_content($forum1, array('tags' => array('Cats'))); + $post14 = $forumgenerator->create_content($forum1); + $post15 = $forumgenerator->create_content($forum1, array('tags' => array('Cats'))); + $post16 = $forumgenerator->create_content($forum1, array('tags' => array('Cats'), 'hidden' => true)); + $post21 = $forumgenerator->create_content($forum2, array('tags' => array('Cats'))); + $post22 = $forumgenerator->create_content($forum2, array('tags' => array('Cats', 'Dogs'))); + $post23 = $forumgenerator->create_content($forum2, array('tags' => array('mice', 'Cats'))); + $post31 = $forumgenerator->create_content($forum3, array('tags' => array('mice', 'Cats'))); + + $tag = core_tag_tag::get_by_name(0, 'Cats'); + + // Admin can see everything. + $res = mod_forum_get_tagged_posts($tag, /*$exclusivemode = */false, + /*$fromctx = */0, /*$ctx = */0, /*$rec = */1, /*$post = */0); + $this->assertRegExp('/'.$post11->subject.'content); + $this->assertRegExp('/'.$post12->subject.'content); + $this->assertRegExp('/'.$post13->subject.'content); + $this->assertNotRegExp('/'.$post14->subject.'content); + $this->assertRegExp('/'.$post15->subject.'content); + $this->assertRegExp('/'.$post16->subject.'content); + $this->assertNotRegExp('/'.$post21->subject.'content); + $this->assertNotRegExp('/'.$post22->subject.'content); + $this->assertNotRegExp('/'.$post23->subject.'content); + $this->assertNotRegExp('/'.$post31->subject.'content); + $this->assertEmpty($res->prevpageurl); + $this->assertNotEmpty($res->nextpageurl); + $res = mod_forum_get_tagged_posts($tag, /*$exclusivemode = */false, + /*$fromctx = */0, /*$ctx = */0, /*$rec = */1, /*$post = */1); + $this->assertNotRegExp('/'.$post11->subject.'content); + $this->assertNotRegExp('/'.$post12->subject.'content); + $this->assertNotRegExp('/'.$post13->subject.'content); + $this->assertNotRegExp('/'.$post14->subject.'content); + $this->assertNotRegExp('/'.$post15->subject.'content); + $this->assertNotRegExp('/'.$post16->subject.'content); + $this->assertRegExp('/'.$post21->subject.'content); + $this->assertRegExp('/'.$post22->subject.'content); + $this->assertRegExp('/'.$post23->subject.'content); + $this->assertRegExp('/'.$post31->subject.'content); + $this->assertNotEmpty($res->prevpageurl); + $this->assertEmpty($res->nextpageurl); + + // Create and enrol a user. + $student = self::getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $this->getDataGenerator()->enrol_user($student->id, $course1->id, $studentrole->id, 'manual'); + $this->getDataGenerator()->enrol_user($student->id, $course2->id, $studentrole->id, 'manual'); + $this->setUser($student); + core_tag_index_builder::reset_caches(); + + // User can not see posts in course 3 because he is not enrolled. + $res = mod_forum_get_tagged_posts($tag, /*$exclusivemode = */false, + /*$fromctx = */0, /*$ctx = */0, /*$rec = */1, /*$post = */1); + $this->assertRegExp('/'.$post22->subject.'/', $res->content); + $this->assertRegExp('/'.$post23->subject.'/', $res->content); + $this->assertNotRegExp('/'.$post31->subject.'/', $res->content); + + // User can search forum posts inside a course. + $coursecontext = context_course::instance($course1->id); + $res = mod_forum_get_tagged_posts($tag, /*$exclusivemode = */false, + /*$fromctx = */0, /*$ctx = */$coursecontext->id, /*$rec = */1, /*$post = */0); + $this->assertRegExp('/'.$post11->subject.'/', $res->content); + $this->assertRegExp('/'.$post12->subject.'/', $res->content); + $this->assertRegExp('/'.$post13->subject.'/', $res->content); + $this->assertNotRegExp('/'.$post14->subject.'/', $res->content); + $this->assertRegExp('/'.$post15->subject.'/', $res->content); + $this->assertRegExp('/'.$post16->subject.'/', $res->content); + $this->assertNotRegExp('/'.$post21->subject.'/', $res->content); + $this->assertNotRegExp('/'.$post22->subject.'/', $res->content); + $this->assertNotRegExp('/'.$post23->subject.'/', $res->content); + $this->assertEmpty($res->nextpageurl); + } + /** * Creates an action event. * diff --git a/mod/forum/version.php b/mod/forum/version.php index 4246a2fe83ec3..da77f51def73b 100644 --- a/mod/forum/version.php +++ b/mod/forum/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2016120500; // The current module version (Date: YYYYMMDDXX) +$plugin->version = 2016120501; // The current module version (Date: YYYYMMDDXX) $plugin->requires = 2016112900; // Requires this Moodle version $plugin->component = 'mod_forum'; // Full name of the plugin (used for diagnostics) diff --git a/theme/boost/templates/mod_forum/big_search_form.mustache b/theme/boost/templates/mod_forum/big_search_form.mustache index 241d842692f52..5c05e5c7edc7a 100644 --- a/theme/boost/templates/mod_forum/big_search_form.mustache +++ b/theme/boost/templates/mod_forum/big_search_form.mustache @@ -144,6 +144,21 @@ + {{#tagsenabled}} + + + + + + + + + {{/tagsenabled}} From 34c8585d83df68f7ac11e197ca3f94aff9e6c54b Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Mon, 3 Apr 2017 09:58:48 +0800 Subject: [PATCH 2/5] MDL-46929 mod_forum: small fixes for post tagging --- mod/forum/lib.php | 24 +++++++++++++++++++---- mod/forum/locallib.php | 11 +++++++---- mod/forum/post.php | 8 -------- mod/forum/tests/behat/behat_mod_forum.php | 1 - 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 3302aecf1f0a2..606e4799e2298 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -4434,6 +4434,10 @@ function forum_add_new_post($post, $mform, $unused = null) { forum_tp_mark_post_read($post->userid, $post); } + if (isset($post->tags)) { + core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $post->id, $context, $post->tags); + } + // Let Moodle know that assessable content is uploaded (eg for plagiarism detection) forum_trigger_content_uploaded_event($post, $cm, 'forum_add_new_post'); @@ -4495,6 +4499,10 @@ function forum_update_post($newpost, $mform, $unused = null) { forum_add_attachment($post, $forum, $cm, $mform); + if (isset($newpost->tags)) { + core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $post->id, $context, $newpost->tags); + } + if (forum_tp_can_track_forums($forum) && forum_tp_is_tracked($forum)) { forum_tp_mark_post_read($USER->id, $post); } @@ -4573,6 +4581,10 @@ function forum_add_discussion($discussion, $mform=null, $unused=null, $userid=nu forum_add_attachment($post, $forum, $cm, $mform, $unused); } + if (isset($discussion->tags)) { + core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $post->id, context_module::instance($cm->id), $discussion->tags); + } + if (forum_tp_can_track_forums($forum) && forum_tp_is_tracked($forum)) { forum_tp_mark_post_read($post->userid, $post); } @@ -5308,12 +5320,16 @@ function forum_user_can_see_post($forum, $discussion, $post, $user=NULL, $cm=NUL } if ($forum->type == 'qanda') { + if (has_capability('mod/forum:viewqandawithoutposting', $modcontext, $user->id) || $post->userid == $user->id + || (isset($discussion->firstpost) && $discussion->firstpost == $post->id)) { + return true; + } $firstpost = forum_get_firstpost_from_discussion($discussion->id); + if ($firstpost->userid == $user->id) { + return true; + } $userfirstpost = forum_get_user_posted_time($discussion->id, $user->id); - - return (($userfirstpost !== false && (time() - $userfirstpost >= $CFG->maxeditingtime)) || - $firstpost->id == $post->id || $post->userid == $user->id || $firstpost->userid == $user->id || - has_capability('mod/forum:viewqandawithoutposting', $modcontext, $user->id)); + return (($userfirstpost !== false && (time() - $userfirstpost >= $CFG->maxeditingtime))); } return true; } diff --git a/mod/forum/locallib.php b/mod/forum/locallib.php index 527cba67fdb95..b446d27339cbc 100644 --- a/mod/forum/locallib.php +++ b/mod/forum/locallib.php @@ -589,7 +589,8 @@ function mod_forum_get_tagged_posts($tag, $exclusivemode = false, $fromctx = 0, // Build the SQL query. $ctxselect = context_helper::get_preload_record_columns_sql('ctx'); - $query = "SELECT fp.id, fp.subject, fd.forum, fp.discussion, f.type, fd.timestart, fd.timeend, fd.groupid, fp.parent, fp.userid, + $query = "SELECT fp.id, fp.subject, fd.forum, fp.discussion, f.type, fd.timestart, fd.timeend, fd.groupid, fd.firstpost, + fp.parent, fp.userid, cm.id AS cmid, c.id AS courseid, c.shortname, c.fullname, $ctxselect FROM {forum_posts} fp JOIN {forum_discussions} fd ON fp.discussion = fd.id @@ -636,8 +637,9 @@ function mod_forum_get_tagged_posts($tag, $exclusivemode = false, $fromctx = 0, } $modinfo = get_fast_modinfo($builder->get_course($courseid)); // Set accessibility of this item and all other items in the same course. - $builder->walk(function ($taggeditem) use ($courseid, $modinfo, $builder) { - if ($taggeditem->courseid == $courseid) { + $builder->walk(function ($taggeditem) use ($courseid, $modinfo, $builder, $item) { + // Checking permission for Q&A forums performs additional DB queries, do not do them in bulk. + if ($taggeditem->courseid == $courseid && ($taggeditem->type != 'qanda' || $taggeditem->id == $item->id)) { $cm = $modinfo->get_cm($taggeditem->cmid); $forum = (object)['id' => $taggeditem->forum, 'course' => $taggeditem->courseid, @@ -646,7 +648,8 @@ function mod_forum_get_tagged_posts($tag, $exclusivemode = false, $fromctx = 0, $discussion = (object)['id' => $taggeditem->discussion, 'timestart' => $taggeditem->timestart, 'timeend' => $taggeditem->timeend, - 'groupid' => $taggeditem->groupid + 'groupid' => $taggeditem->groupid, + 'firstpost' => $taggeditem->firstpost ]; $post = (object)['id' => $taggeditem->id, 'parent' => $taggeditem->parent, diff --git a/mod/forum/post.php b/mod/forum/post.php index c87780efdbd48..88acf308fd526 100644 --- a/mod/forum/post.php +++ b/mod/forum/post.php @@ -757,8 +757,6 @@ $discussionurl = new moodle_url("/mod/forum/discuss.php", array('d' => $discussion->id), 'p' . $fromform->id); } - core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $fromform->id, $modcontext, $fromform->tags); - $params = array( 'context' => $modcontext, 'objectid' => $fromform->id, @@ -811,10 +809,6 @@ $discussionurl = new moodle_url("/mod/forum/discuss.php", array('d' => $discussion->id), 'p'.$fromform->id); } - if (core_tag_tag::is_enabled('mod_forum', 'forum_posts') && isset($data->tags)) { - core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $fromform->id, $modcontext, $fromform->tags); - } - $params = array( 'context' => $modcontext, 'objectid' => $fromform->id, @@ -942,8 +936,6 @@ $completion->update_state($cm, COMPLETION_COMPLETE); } - core_tag_tag::set_item_tags('mod_forum', 'forum_posts', $fromform->firstpost, $modcontext, $fromform->tags); - // Redirect back to the discussion. redirect( forum_go_back_to($redirectto->out()), diff --git a/mod/forum/tests/behat/behat_mod_forum.php b/mod/forum/tests/behat/behat_mod_forum.php index b546457a798d4..3a8cd701f23ad 100644 --- a/mod/forum/tests/behat/behat_mod_forum.php +++ b/mod/forum/tests/behat/behat_mod_forum.php @@ -77,7 +77,6 @@ public function i_reply_post_from_forum_with($postsubject, $forumname, TableNode // Fill form and post. $this->execute('behat_forms::i_set_the_following_fields_to_these_values', $table); - $this->execute("behat_general::wait_until_the_page_is_ready"); $this->execute('behat_forms::press_button', get_string('posttoforum', 'forum')); $this->execute('behat_general::i_wait_to_be_redirected'); From 151113f93acfa508b5071ac3449ef56629be680f Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Tue, 4 Apr 2017 13:06:39 +0800 Subject: [PATCH 3/5] MDL-46929 mod_forum: performance improvement for tags backup/restore --- .../backup/moodle2/backup_forum_stepslib.php | 31 ++++++++++--------- .../backup/moodle2/restore_forum_stepslib.php | 7 +++-- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/mod/forum/backup/moodle2/backup_forum_stepslib.php b/mod/forum/backup/moodle2/backup_forum_stepslib.php index 50a6b077157d1..63f45d9c6eb45 100644 --- a/mod/forum/backup/moodle2/backup_forum_stepslib.php +++ b/mod/forum/backup/moodle2/backup_forum_stepslib.php @@ -60,8 +60,8 @@ protected function define_structure() { 'mailed', 'subject', 'message', 'messageformat', 'messagetrust', 'attachment', 'totalscore', 'mailnow')); - $tags = new backup_nested_element('tags'); - $tag = new backup_nested_element('tag', array('id'), array('name', 'rawname')); + $tags = new backup_nested_element('poststags'); + $tag = new backup_nested_element('tag', array('id'), array('itemid', 'rawname')); $ratings = new backup_nested_element('ratings'); @@ -113,12 +113,12 @@ protected function define_structure() { $forum->add_child($trackedprefs); $trackedprefs->add_child($track); + $forum->add_child($tags); + $tags->add_child($tag); + $discussion->add_child($posts); $posts->add_child($post); - $posts->add_child($tags); - $tags->add_child($tag); - $post->add_child($ratings); $ratings->add_child($rating); @@ -154,15 +154,18 @@ protected function define_structure() { 'itemid' => backup::VAR_PARENTID)); $rating->set_source_alias('rating', 'value'); - $tag->set_source_sql('SELECT t.id, t.name, t.rawname - FROM {tag} t - JOIN {tag_instance} ti ON ti.tagid = t.id - WHERE ti.itemtype = ? - AND ti.component = ? - AND ti.itemid = ?', array( - backup_helper::is_sqlparam('forum_posts'), - backup_helper::is_sqlparam('mod_forum'), - backup::VAR_PARENTID)); + if (core_tag_tag::is_enabled('mod_forum', 'forum_posts')) { + // Backup all tags for all forum posts in this forum. + $tag->set_source_sql('SELECT t.id, ti.itemid, t.rawname + FROM {tag} t + JOIN {tag_instance} ti ON ti.tagid = t.id + WHERE ti.itemtype = ? + AND ti.component = ? + AND ti.contextid = ?', array( + backup_helper::is_sqlparam('forum_posts'), + backup_helper::is_sqlparam('mod_forum'), + backup::VAR_CONTEXTID)); + } } // Define id annotations diff --git a/mod/forum/backup/moodle2/restore_forum_stepslib.php b/mod/forum/backup/moodle2/restore_forum_stepslib.php index 26595d60cdbf2..c026123247458 100644 --- a/mod/forum/backup/moodle2/restore_forum_stepslib.php +++ b/mod/forum/backup/moodle2/restore_forum_stepslib.php @@ -40,7 +40,7 @@ protected function define_structure() { if ($userinfo) { $paths[] = new restore_path_element('forum_discussion', '/activity/forum/discussions/discussion'); $paths[] = new restore_path_element('forum_post', '/activity/forum/discussions/discussion/posts/post'); - $paths[] = new restore_path_element('forum_tag', '/activity/forum/discussions/discussion/posts/post/tags/tag'); + $paths[] = new restore_path_element('forum_tag', '/activity/forum/poststags/tag'); $paths[] = new restore_path_element('forum_discussion_sub', '/activity/forum/discussions/discussion/discussion_subs/discussion_sub'); $paths[] = new restore_path_element('forum_rating', '/activity/forum/discussions/discussion/posts/post/ratings/rating'); $paths[] = new restore_path_element('forum_subscription', '/activity/forum/subscriptions/subscription'); @@ -126,7 +126,10 @@ protected function process_forum_tag($data) { } $tag = $data->rawname; - $itemid = $this->get_new_parentid('forum_post'); + if (!$itemid = $this->get_mappingid('forum_post', $data->itemid)) { + // Some orphaned tag, we could not find the restored post for it - ignore. + return; + } $context = context_module::instance($this->task->get_moduleid()); core_tag_tag::add_item_tag('mod_forum', 'forum_posts', $itemid, $context, $tag); From e9c46768b5731174e928f3ff7d4b7bb99e8d04a7 Mon Sep 17 00:00:00 2001 From: Andrew Hancox Date: Tue, 4 Apr 2017 17:48:20 +0100 Subject: [PATCH 4/5] MDL-46929 mod_forum: Fix for edge cases when searching --- lang/en/moodle.php | 1 + lib/searchlib.php | 11 +++++++---- mod/forum/lib.php | 21 +++++++++++++-------- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 2b0ab861d01a5..c377d403efc92 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -1890,6 +1890,7 @@ $string['todaylogs'] = 'Today\'s logs'; $string['toeveryone'] = 'to everyone'; $string['toomanybounces'] = 'That email address has had too many bounces. You must change it to continue.'; +$string['toomanytags'] = 'This search included too many tags, some will have been ignored.'; $string['toomanytoshow'] = 'There are too many users to show.'; $string['toomanyusersmatchsearch'] = 'Too many users ({$a->count}) match \'{$a->search}\''; $string['toomanyuserstoshow'] = 'Too many users ({$a}) to show'; diff --git a/lib/searchlib.php b/lib/searchlib.php index 79217836de010..440eb4b9f29a9 100644 --- a/lib/searchlib.php +++ b/lib/searchlib.php @@ -490,11 +490,14 @@ function search_generate_SQL($parsetree, $datafield, $metafield, $mainidfield, $ $sqlstrings = []; foreach (explode(',', $value) as $tag) { $paramname = $name1 . '_' . $nexttagfield; - if (!isset($tagfields[$nexttagfield])) { - throw new coding_exception('Not enough tag fields supplied for search.'); + if (isset($tagfields[$nexttagfield])) { + $sqlstrings[] = "($tagfields[$nexttagfield] = :$paramname)"; + $params[$paramname] = $tag; + } else if (!isset($tagfields[$nexttagfield]) && !isset($stoppedprocessingtags)) { + // Show a debugging message the first time we hit this. + $stoppedprocessingtags = true; + \core\notification::add(get_string('toomanytags'), \core\notification::WARNING); } - $sqlstrings[] = "($tagfields[$nexttagfield] = :$paramname)"; - $params[$paramname] = $tag; $nexttagfield++; } $SQLString .= implode(' AND ', $sqlstrings); diff --git a/mod/forum/lib.php b/mod/forum/lib.php index 606e4799e2298..02ce582e734da 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -2098,16 +2098,21 @@ function forum_search_posts($searchterms, $courseid=0, $limitfrom=0, $limitnum=5 $tagjoins = ''; $tagfields = []; + $tagfieldcount = 0; foreach ($parsearray as $token) { if ($token->getType() == TOKEN_TAGS) { - for ($i = substr_count($token->getValue(), ','); $i >= 0; $i--) { - $tagjoins .= "\n LEFT JOIN {tag_instance} ti_$i - ON p.id = ti_$i.itemid - AND ti_$i.component='mod_forum' - AND ti_$i.itemtype = 'forum_posts'"; - $tagjoins .= "\n LEFT JOIN {tag} t_$i ON t_$i.id = ti_$i.tagid"; - $tagfields[] = "t_$i.rawname"; - + for ($i = 0; $i <= substr_count($token->getValue(), ','); $i++) { + // Queries can only have a limited number of joins so set a limit sensible users won't exceed. + if ($tagfieldcount > 10) { + continue; + } + $tagjoins .= " LEFT JOIN {tag_instance} ti_$tagfieldcount + ON p.id = ti_$tagfieldcount.itemid + AND ti_$tagfieldcount.component = 'mod_forum' + AND ti_$tagfieldcount.itemtype = 'forum_posts'"; + $tagjoins .= " LEFT JOIN {tag} t_$tagfieldcount ON t_$tagfieldcount.id = ti_$tagfieldcount.tagid"; + $tagfields[] = "t_$tagfieldcount.rawname"; + $tagfieldcount++; } } } From 32a96916a6732c6664c7f3aae626b6e9535f3871 Mon Sep 17 00:00:00 2001 From: Andrew Hancox Date: Tue, 11 Apr 2017 13:34:22 +0100 Subject: [PATCH 5/5] MDL-46929 mod_forum: Improve behat tests --- mod/forum/tests/behat/advanced_search.feature | 3 ++- mod/forum/tests/behat/edit_tags.feature | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/mod/forum/tests/behat/advanced_search.feature b/mod/forum/tests/behat/advanced_search.feature index 0f8a2b4b25766..f80c565668bb9 100644 --- a/mod/forum/tests/behat/advanced_search.feature +++ b/mod/forum/tests/behat/advanced_search.feature @@ -25,6 +25,7 @@ Feature: The forum search allows users to perform advanced searches for forum po And I am on "Course 1" course homepage with editing mode on And I add the "Latest announcements" block And I navigate to "Edit settings" node in "Course administration" + And I expand all fieldsets And I set the field "id_newsitems" to "1" And I press "Save and display" And I add a new topic to "Announcements" forum with: @@ -116,7 +117,7 @@ Feature: The forum search allows users to perform advanced searches for forum po @javascript Scenario: Perform an advanced search using tags Given I log in as "student1" - And I follow "Course 1" + And I am on "Course 1" course homepage And I follow "Announcements" And I press "Search forums" And I should see "Advanced search" diff --git a/mod/forum/tests/behat/edit_tags.feature b/mod/forum/tests/behat/edit_tags.feature index 0ab3fa84b9a63..d7c70588bc00d 100644 --- a/mod/forum/tests/behat/edit_tags.feature +++ b/mod/forum/tests/behat/edit_tags.feature @@ -1,4 +1,4 @@ -@mod @mod_forum @core_tag @javascript +@mod @mod_forum @core_tag Feature: Edited forum posts handle tags correctly In order to get forum posts properly labelled As a user @@ -17,8 +17,7 @@ Feature: Edited forum posts handle tags correctly | teacher1 | C1 | editingteacher | | student1 | C1 | student | And I log in as "teacher1" - And I follow "Course 1" - And I turn editing mode on + And I am on "Course 1" course homepage with editing mode on And I add a "Forum" to section "1" and I fill the form with: | Forum name | Test forum name | | Description | Test forum description | @@ -27,9 +26,10 @@ Feature: Edited forum posts handle tags correctly | Message | Teacher post message | And I log out + @javascript Scenario: Forum post edition of custom tags works as expected Given I log in as "student1" - And I follow "Course 1" + And I am on "Course 1" course homepage And I reply "Teacher post subject" post from "Test forum name" forum with: | Subject | Student post subject | | Message | Student post message | @@ -48,7 +48,7 @@ Feature: Edited forum posts handle tags correctly And I press "Continue" And I log out And I log in as "teacher1" - And I follow "Course 1" + And I am on "Course 1" course homepage And I follow "Test forum" And I click on "Add a new discussion topic" "button" And I expand all fieldsets