diff --git a/classes/post_form.php b/classes/post_form.php index 5d60710001..206a2109a7 100644 --- a/classes/post_form.php +++ b/classes/post_form.php @@ -100,7 +100,6 @@ public function definition() { // Is it a reply? $modform->addElement('hidden', 'reply'); $modform->setType('reply', PARAM_INT); - } /** @@ -142,7 +141,6 @@ public static function attachment_options($moodleoverflow) { 'return_types' => FILE_INTERNAL | FILE_CONTROLLED_LINK ); } - } diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index b65afab450..139ec513c4 100644 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -143,9 +143,15 @@ public static function get_contexts_for_userid(int $userid) : contextlist { INNER JOIN {modules} m ON m.id = cm.module AND m.name = :modname INNER JOIN {moodleoverflow} mof ON mof.id = cm.instance WHERE EXISTS ( - SELECT 1 FROM {moodleoverflow_discussions} d WHERE d.moodleoverflow = mof.id AND (d.userid = :duserid OR d.usermodified = :dmuserid) + SELECT 1 + FROM {moodleoverflow_discussions} d + WHERE d.moodleoverflow = mof.id AND (d.userid = :duserid OR d.usermodified = :dmuserid) ) OR EXISTS ( - SELECT 1 FROM {moodleoverflow_posts} p WHERE p.discussion IN (SELECT id FROM {moodleoverflow_discussions} WHERE moodleoverflow = mof.id) AND p.userid = :puserid + SELECT 1 + FROM {moodleoverflow_posts} p + WHERE p.discussion IN (SELECT id + FROM {moodleoverflow_discussions} + WHERE moodleoverflow = mof.id) AND p.userid = :puserid ) OR EXISTS ( SELECT 1 FROM {moodleoverflow_read} r WHERE r.moodleoverflowid = mof.id AND r.userid = :ruserid ) OR EXISTS ( diff --git a/lib.php b/lib.php index 5ee67061d7..7651957cd6 100644 --- a/lib.php +++ b/lib.php @@ -409,47 +409,49 @@ function moodleoverflow_get_file_info($browser, $areas, $course, $cm, $context, * @param bool $forcedownload whether or not force download * @param array $options additional options affecting the file serving */ -function moodleoverflow_pluginfile($course, $cm, $context, $filearea, array $args, $forcedownload, array $options = array()) { - global $DB; - +function moodleoverflow_pluginfile($course, $cm, $context, $filearea, $args, $forcedownload, array $options = array()) { + global $DB, $CFG; if ($context->contextlevel != CONTEXT_MODULE) { return false; } - require_course_login($course, true, $cm); $areas = moodleoverflow_get_file_areas($course, $cm, $context); - // Filearea must contain a real area. if (!isset($areas[$filearea])) { return false; } - $postid = (int) array_shift($args); + $filename = array_pop($args); + $itemid = array_pop($args); - if (!$post = $DB->get_record('moodleoverflow_posts', array('id' => $postid))) { + // Check if post, discussion or moodleoverflow still exists. + if (!$post = $DB->get_record('moodleoverflow_posts', array('id' => $itemid))) { return false; } - if (!$discussion = $DB->get_record('moodleoverflow_discussions', array('id' => $post->discussion))) { return false; } - if (!$moodleoverflow = $DB->get_record('moodleoverflow', array('id' => $cm->instance))) { return false; } - $fs = get_file_storage(); - $relativepath = implode('/', $args); - $fullpath = "/$context->id/mod_moodleoverflow/$filearea/$postid/$relativepath"; - if (!$file = $fs->get_file_by_hash(sha1($fullpath))||$file->is_directory()) { - return false; + if (!$args) { + // Empty path, use root. + $filepath = '/'; + } else { + // Assemble filepath. + $filepath = '/' . implode('/', $args) . '/'; } + $fs = get_file_storage(); + + $file = $fs->get_file($context->id, 'mod_moodleoverflow', $filearea, $itemid, $filepath, $filename); // Make sure groups allow this user to see this file. if ($discussion->groupid > 0) { $groupmode = groups_get_activity_groupmode($cm, $course); if ($groupmode == SEPARATEGROUPS) { + if (!groups_is_member($discussion->groupid) && !has_capability('moodle/site:accessallgroups', $context)) { return false; } @@ -462,7 +464,7 @@ function moodleoverflow_pluginfile($course, $cm, $context, $filearea, array $arg } // Finally send the file. - send_stored_file($file, 0, 0, true, $options); // Download MUST be forced - security! + send_stored_file($file, 86400, 0, true, $options); // Download MUST be forced - security! } /* Navigation API */ diff --git a/locallib.php b/locallib.php index d16acc068a..64b993ff93 100644 --- a/locallib.php +++ b/locallib.php @@ -27,6 +27,8 @@ use mod_moodleoverflow\anonymous; use mod_moodleoverflow\capabilities; +use mod_moodleoverflow\event\post_deleted; +use mod_moodleoverflow\readtracking; use mod_moodleoverflow\review; defined('MOODLE_INTERNAL') || die(); @@ -37,8 +39,8 @@ * Get all discussions in a moodleoverflow instance. * * @param object $cm - * @param int $page - * @param int $perpage + * @param int $page + * @param int $perpage * * @return array */ @@ -178,8 +180,8 @@ function moodleoverflow_print_latest_discussions($moodleoverflow, $cm, $page = - $replies = moodleoverflow_count_discussion_replies($cm); // Check whether the moodleoverflow instance can be tracked and is tracked. - if ($cantrack = \mod_moodleoverflow\readtracking::moodleoverflow_can_track_moodleoverflows($moodleoverflow)) { - $istracked = \mod_moodleoverflow\readtracking::moodleoverflow_is_tracked($moodleoverflow); + if ($cantrack = readtracking::moodleoverflow_can_track_moodleoverflows($moodleoverflow)) { + $istracked = readtracking::moodleoverflow_is_tracked($moodleoverflow); } else { $istracked = false; } @@ -288,7 +290,7 @@ function moodleoverflow_print_latest_discussions($moodleoverflow, $cm, $page = - // Check if a single post was marked by the question owner and a teacher. $statusboth = false; - if ($markedhelpful && $markedsolution) { + if ($markedhelpful && $markedsolution) { if ($markedhelpful->postid == $markedsolution->postid) { $statusboth = true; } @@ -849,10 +851,10 @@ function moodleoverflow_add_discussion($discussion, $modulecontext, $userid = nu moodleoverflow_add_attachment($post, $moodleoverflow, $cm); // Mark the created post as read. - $cantrack = \mod_moodleoverflow\readtracking::moodleoverflow_can_track_moodleoverflows($moodleoverflow); - $istracked = \mod_moodleoverflow\readtracking::moodleoverflow_is_tracked($moodleoverflow); + $cantrack = readtracking::moodleoverflow_can_track_moodleoverflows($moodleoverflow); + $istracked = readtracking::moodleoverflow_is_tracked($moodleoverflow); if ($cantrack && $istracked) { - \mod_moodleoverflow\readtracking::moodleoverflow_mark_post_read($post->userid, $post); + readtracking::moodleoverflow_mark_post_read($post->userid, $post); } // Trigger event. @@ -932,7 +934,7 @@ function moodleoverflow_print_discussion($course, $cm, $moodleoverflow, $discuss $modulecontext = context_module::instance($cm->id); // Is the forum tracked? - $istracked = \mod_moodleoverflow\readtracking::moodleoverflow_is_tracked($moodleoverflow); + $istracked = readtracking::moodleoverflow_is_tracked($moodleoverflow); // Retrieve all posts of the discussion. $posts = moodleoverflow_get_all_discussion_posts($discussion->id, $istracked, $modulecontext); @@ -1072,7 +1074,7 @@ function moodleoverflow_get_all_discussion_posts($discussionid, $tracking, $modc // Is it an old post? if ($tracking) { - if (\mod_moodleoverflow\readtracking::moodleoverflow_is_old_post($post)) { + if (readtracking::moodleoverflow_is_old_post($post)) { $posts[$postid]->postread = true; } } @@ -1470,8 +1472,8 @@ function moodleoverflow_print_post($post, $discussion, $moodleoverflow, $cm, $co $mustachedata->footer = $footer; // Mark the forum post as read. - if ($istracked && !$postisread) { - \mod_moodleoverflow\readtracking::moodleoverflow_mark_post_read($USER->id, $post); + if ($istracked && !$postisread) { + readtracking::moodleoverflow_mark_post_read($USER->id, $post); } $mustachedata->iscomment = $level == 2; @@ -1595,8 +1597,8 @@ function get_attachments($post, $cm) { $iconimage = $OUTPUT->pix_icon(file_file_icon($file), get_mimetype_description($file), 'moodle', array('class' => 'icon')); - $path = file_encode_url($CFG->wwwroot . '/pluginfile.php', '/' . - $context->id . '/mod_moodleoverflow/attachment/' . $post->id . '/' . $attachments[$i]['filename']); + $path = moodle_url::make_pluginfile_url($file->get_contextid(), $file->get_component(), $file->get_filearea(), + $file->get_itemid(), $file->get_filepath(), $file->get_filename()); $attachments[$i]['icon'] = $iconimage; $attachments[$i]['filepath'] = $path; @@ -1610,7 +1612,6 @@ function get_attachments($post, $cm) { $i += 1; } } - return $attachments; } @@ -1685,10 +1686,10 @@ function moodleoverflow_add_new_post($post) { } // Mark the created post as read if the user is tracking the discussion. - $cantrack = \mod_moodleoverflow\readtracking::moodleoverflow_can_track_moodleoverflows($moodleoverflow); - $istracked = \mod_moodleoverflow\readtracking::moodleoverflow_is_tracked($moodleoverflow); - if ($cantrack && $istracked) { - \mod_moodleoverflow\readtracking::moodleoverflow_mark_post_read($post->userid, $post); + $cantrack = readtracking::moodleoverflow_can_track_moodleoverflows($moodleoverflow); + $istracked = readtracking::moodleoverflow_is_tracked($moodleoverflow); + if ($cantrack && $istracked) { + readtracking::moodleoverflow_mark_post_read($post->userid, $post); } // Return the id of the created post. @@ -1745,10 +1746,10 @@ function moodleoverflow_update_post($newpost) { moodleoverflow_add_attachment($newpost, $moodleoverflow, $cm); // Mark the edited post as read. - $cantrack = \mod_moodleoverflow\readtracking::moodleoverflow_can_track_moodleoverflows($moodleoverflow); - $istracked = \mod_moodleoverflow\readtracking::moodleoverflow_is_tracked($moodleoverflow); - if ($cantrack && $istracked) { - \mod_moodleoverflow\readtracking::moodleoverflow_mark_post_read($USER->id, $post); + $cantrack = readtracking::moodleoverflow_can_track_moodleoverflows($moodleoverflow); + $istracked = readtracking::moodleoverflow_is_tracked($moodleoverflow); + if ($cantrack && $istracked) { + readtracking::moodleoverflow_mark_post_read($USER->id, $post); } // The post has been edited successfully. @@ -1808,7 +1809,7 @@ function moodleoverflow_delete_discussion($discussion, $course, $cm, $moodleover } // Delete the read-records for the discussion. - \mod_moodleoverflow\readtracking::moodleoverflow_delete_read_records(-1, -1, $discussion->id); + readtracking::moodleoverflow_delete_read_records(-1, -1, $discussion->id); // Remove the subscriptions for this discussion. $DB->delete_records('moodleoverflow_discuss_subs', array('discussion' => $discussion->id)); @@ -1823,10 +1824,10 @@ function moodleoverflow_delete_discussion($discussion, $course, $cm, $moodleover /** * Deletes a single moodleoverflow post. * - * @param object $post The post ID - * @param bool $deletechildren The child posts - * @param object $cm The course module - * @param object $moodleoverflow The moodleoverflow ID + * @param object $post The post + * @param bool $deletechildren The child posts + * @param object $cm The course module + * @param object $moodleoverflow The moodleoverflow * * @return bool Whether the deletion was successful */ @@ -1834,21 +1835,41 @@ function moodleoverflow_delete_post($post, $deletechildren, $cm, $moodleoverflow global $DB, $USER; // Iterate through all children and delete them. - $childposts = $DB->get_records('moodleoverflow_posts', array('parent' => $post->id)); - if ($deletechildren && $childposts) { - foreach ($childposts as $childpost) { - moodleoverflow_delete_post($childpost, true, $cm, $moodleoverflow); + // In case something does not work we throw the error as it should be known that something went ... terribly wrong. + // All DB transactions are rolled back. + try { + $transaction = $DB->start_delegated_transaction(); + + $childposts = $DB->get_records('moodleoverflow_posts', array('parent' => $post->id)); + if ($deletechildren && $childposts) { + foreach ($childposts as $childpost) { + moodleoverflow_delete_post($childpost, true, $cm, $moodleoverflow); + } } - } - // Delete the ratings. - if ($DB->delete_records('moodleoverflow_ratings', array('postid' => $post->id))) { + // Delete the ratings. + $DB->delete_records('moodleoverflow_ratings', array('postid' => $post->id)); // Delete the post. if ($DB->delete_records('moodleoverflow_posts', array('id' => $post->id))) { - // Delete the read records. - \mod_moodleoverflow\readtracking::moodleoverflow_delete_read_records(-1, $post->id); + readtracking::moodleoverflow_delete_read_records(-1, $post->id); + + // Delete the attachments. + // First delete the actual files on the disk. + $fs = get_file_storage(); + $context = context_module::instance($cm->id); + $attachments = $fs->get_area_files($context->id, 'mod_moodleoverflow', 'attachment', + $post->id, "filename", true); + foreach ($attachments as $attachment) { + // Get file + $file = $fs->get_file($context->id, 'mod_moodleoverflow', 'attachment', $post->id, + $attachment->get_filepath(), $attachment->get_filename()); + // Delete it if it exists + if ($file) { + $file->delete(); + } + } // Just in case, check for the new last post of the discussion. moodleoverflow_discussion_update_last_post($post->discussion); @@ -1868,12 +1889,15 @@ function moodleoverflow_delete_post($post, $deletechildren, $cm, $moodleoverflow if ($post->userid !== $USER->id) { $params['relateduserid'] = $post->userid; } - $event = \mod_moodleoverflow\event\post_deleted::create($params); + $event = post_deleted::create($params); $event->trigger(); // The post has been deleted. + $transaction->allow_commit(); return true; } + } catch (Exception $e) { + $transaction->rollback($e); } // Deleting the post failed. @@ -2043,7 +2067,7 @@ function moodleoverflow_update_user_grade_on_db($moodleoverflow, $postuserrating if ($DB->record_exists('moodleoverflow_grades', array('userid' => $userid, 'moodleoverflowid' => $moodleoverflow->id))) { $DB->set_field('moodleoverflow_grades', 'grade', $grade, array('userid' => $userid, - 'moodleoverflowid' => $moodleoverflow->id )); + 'moodleoverflowid' => $moodleoverflow->id)); } else { diff --git a/tests/behat/delete_file.feature b/tests/behat/delete_file.feature new file mode 100644 index 0000000000..0d1e7dd4eb --- /dev/null +++ b/tests/behat/delete_file.feature @@ -0,0 +1,57 @@ +@mod @mod_moodleoverflow @javascript @mod_moodleoverflow_delete @javascript @_file_upload +Feature: Add moodleoverflow activities and discussions + In order to delete discussions also files need to be deleted + + Background: Add a moodleoverflow and a discussion + 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 | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I turn editing mode on + And I add a "Moodleoverflow" to section "1" and I fill the form with: + | Moodleoverflow name | Test moodleoverflow name | + | Description | Test forum description | + And I add a new discussion to "Test moodleoverflow name" moodleoverflow with: + | Subject | Forum post 1 | + | Message | This is the body | + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And I follow "Test moodleoverflow name" + And I follow "Forum post 1" + And I click on "Answer" "link" + And I set the following fields to these values: + | Subject | A reply post | + | Message | This is the message of the answer post | + And I upload "mod/moodleoverflow/tests/fixtures/NH.jpg" file to "Attachment" filemanager + And I press "Post to forum" + + Scenario Outline: + Given I log in as "" + And I am on "Course 1" course homepage + And I follow "Test moodleoverflow name" + And I follow "Forum post 1" + And I should see "This is the message of the answer post" + And "//div[contains(@class, 'moodleoverflowpost')]//div[contains(@class, 'attachments')]//img[contains(@src, 'NH.jpg')]" "xpath_element" should exist + And I click on "Delete" "link" + And I click on "Continue" "button" + Then I should see "Test moodleoverflow name" + And I should see "This is the body" + And I should not see "This is the message of the answer post" + And "//div[contains(@class, 'moodleoverflowpost')]//div[contains(@class, 'attachments')]//img[contains(@src, 'NH.jpg')]" "xpath_element" should not exist + + Examples: + | role | + | student1 | + + + diff --git a/tests/fixtures/NH.jpg b/tests/fixtures/NH.jpg new file mode 100644 index 0000000000..d4085965ce Binary files /dev/null and b/tests/fixtures/NH.jpg differ diff --git a/tests/locallib_test.php b/tests/locallib_test.php index 33d5ae9c49..6d03e704c2 100644 --- a/tests/locallib_test.php +++ b/tests/locallib_test.php @@ -173,5 +173,4 @@ public function test_moodleoverflow_disallow_subscribe_on_create() { } } - } diff --git a/tests/post_test.php b/tests/post_test.php new file mode 100644 index 0000000000..e5c1d0e55d --- /dev/null +++ b/tests/post_test.php @@ -0,0 +1,156 @@ +. + +/** + * PHP Unit test for post related functions in the locallib. + * + * @package mod_moodleoverflow + * @copyright 2023 Tamaro Walter + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace mod_moodleoverflow; + +defined('MOODLE_INTERNAL') || die(); +require_once(__DIR__ . '/../locallib.php'); + +/** + * PHP Unit test for post related functions in the locallib. + * + * @package mod_moodleoverflow + * @copyright 2023 Tamaro Walter + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class post_test extends \advanced_testcase { + + /** @var \stdClass test course */ + private $course; + + /** @var \stdClass coursemodule */ + private $coursemodule; + + /** @var \stdClass test moodleoverflow */ + private $moodleoverflow; + + /** @var \stdClass test teacher */ + private $teacher; + + /** @var \stdClass a discussion */ + private $discussion; + + /** @var \stdClass a post */ + private $post; + + /** @var \stdClass an attachment */ + private $attachment; + + /** @var \mod_moodleoverflow_generator $generator */ + private $generator; + + + public function setUp(): void { + $this->resetAfterTest(); + $this->helper_course_set_up(); + } + + public function tearDown(): void { + // Clear all caches. + \mod_moodleoverflow\subscriptions::reset_moodleoverflow_cache(); + \mod_moodleoverflow\subscriptions::reset_discussion_cache(); + } + + /** + * Test if a post and its attachment are deleted successfully. + * @covers ::moodleoverflow_delete_post + */ + public function test_moodleoverflow_delete_post() { + global $DB; + + // The attachment should exist. + $numberofattachments = count($DB->get_records('files', array('itemid' => $this->post->id))); + $this->assertEquals(2, $numberofattachments); + + // Delete the post from the teacher with its attachment. + moodleoverflow_delete_post($this->post, false, $this->coursemodule, $this->moodleoverflow); + + // Now try to get the attachment. + $numberofattachments = count($DB->get_records('files', array('itemid' => $this->post->id))); + + $this->assertEquals(0, $numberofattachments); + } + + /** + * Test if a post and its attachment are deleted successfully. + * @covers ::moodleoverflow_delete_discussion + */ + public function test_moodleoverflow_delete_discussion() { + global $DB; + + $numberofattachments = count($DB->get_records('files', array('itemid' => $this->post->id, 'filearea' => 'attachment'))); + $this->assertEquals(2, $numberofattachments); + + // Delete the post from the teacher with its attachment. + moodleoverflow_delete_discussion($this->discussion[0], $this->course, $this->coursemodule, $this->moodleoverflow); + + // Now try to get the attachment. + $numberofattachments = count($DB->get_records('files', array('itemid' => $this->post->id))); + $this->assertEquals(0, $numberofattachments); + } + + /** + * This function creates: + * - a course with a moodleoverflow + * - a new discussion with a post. The post has an attachment. + */ + private function helper_course_set_up() { + global $DB; + // Create a new course with a moodleoverflow forum. + $this->course = $this->getDataGenerator()->create_course(); + $location = array('course' => $this->course->id); + $this->moodleoverflow = $this->getDataGenerator()->create_module('moodleoverflow', $location); + $this->coursemodule = get_coursemodule_from_instance('moodleoverflow', $this->moodleoverflow->id); + + // Create a teacher. + $this->teacher = $this->getDataGenerator()->create_user(array('firstname' => 'Tamaro', 'lastname' => 'Walter')); + $this->getDataGenerator()->enrol_user($this->teacher->id, $this->course->id, 'student'); + + // Create a discussion started from the teacher. + $this->generator = $this->getDataGenerator()->get_plugin_generator('mod_moodleoverflow'); + $this->discussion = $this->generator->post_to_forum($this->moodleoverflow, $this->teacher); + $this->post = $DB->get_record('moodleoverflow_posts', array('id' => $this->discussion[0]->firstpost), '*'); + + // Create an attachment by inserting it directly in the database and update the post record. + + $modulecontext = \context_module::instance($this->coursemodule->id); + + $fileinfo = [ + 'contextid' => $modulecontext->id, // ID of the context. + 'component' => 'mod_moodleoverflow', // Your component name. + 'filearea' => 'attachment', // Usually = table name. + 'itemid' => $this->post->id, // Usually = ID of row in table. + 'filepath' => '/', // Any path beginning and ending in /. + 'filename' => 'NH.jpg', // Any filename. + ]; + + $fs = get_file_storage(); + + // Create a new file containing the text 'hello world'. + $fs->create_file_from_string($fileinfo, 'hello world'); + + $this->post->attachment = 1; + $DB->update_record('moodleoverflow_posts', $this->post); + + } +} diff --git a/tests/ratings_test.php b/tests/ratings_test.php index c7fb701f6e..a74de7e581 100644 --- a/tests/ratings_test.php +++ b/tests/ratings_test.php @@ -194,62 +194,29 @@ public function test_answersorting_twogroups() { // Test case 1: helpful and solved post, only solved posts. $group1 = 'sh'; $group2 = 's'; - $this->create_twogroups($group1, $group2); - $posts = array($this->post, $this->answer1, $this->answer2, $this->answer3, $this->answer4, $this->answer5, $this->answer6); - $sortedposts = ratings::moodleoverflow_sort_answers_by_ratings($posts); - $rightorderposts = array($this->post, $this->answer2, $this->answer1, $this->answer3, - $this->answer6, $this->answer5, $this->answer4); - $result = $this->postsorderequal($sortedposts, $rightorderposts); - $this->assertEquals(1, $result); + $this->process_groups($group1, $group2); // Test case 2: helpful and solved post, only helpful posts. - $group1 = 'sh'; $group2 = 'h'; - $this->create_twogroups($group1, $group2); - $posts = array($this->post, $this->answer1, $this->answer2, $this->answer3, $this->answer4, $this->answer5, $this->answer6); - $sortedposts = ratings::moodleoverflow_sort_answers_by_ratings($posts); - $rightorderposts = array($this->post, $this->answer2, $this->answer1, $this->answer3, - $this->answer6, $this->answer5, $this->answer4); - $result = $this->postsorderequal($sortedposts, $rightorderposts); - $this->assertEquals(1, $result); + $this->process_groups($group1, $group2); // Test case 3: helpful and solved post, not-marked posts. - $group1 = 'sh'; $group2 = 'o'; - $this->create_twogroups($group1, $group2); - $posts = array($this->post, $this->answer1, $this->answer2, $this->answer3, $this->answer4, $this->answer5, $this->answer6); - $sortedposts = ratings::moodleoverflow_sort_answers_by_ratings($posts); - $rightorderposts = array($this->post, $this->answer2, $this->answer1, $this->answer3, - $this->answer6, $this->answer5, $this->answer4); - $result = $this->postsorderequal($sortedposts, $rightorderposts); - $this->assertEquals(1, $result); + $this->process_groups($group1, $group2); // Test case 4: only solved posts and only helpful posts with ratingpreferences = 0. $group1 = 's'; $group2 = 'h'; $this->set_ratingpreferences(0); - $this->create_twogroups($group1, $group2); - $posts = array($this->post, $this->answer1, $this->answer2, $this->answer3, $this->answer4, $this->answer5, $this->answer6); - $sortedposts = ratings::moodleoverflow_sort_answers_by_ratings($posts); $rightorderposts = array($this->post, $this->answer6, $this->answer5, $this->answer4, - $this->answer2, $this->answer1, $this->answer3); - $result = $this->postsorderequal($sortedposts, $rightorderposts); - $this->assertEquals(1, $result); + $this->answer2, $this->answer1, $this->answer3); + $this->process_groups($group1, $group2, $rightorderposts); // Test case 5: only solved posts and only helpful posts with ratingpreferences = 1. - $group1 = 's'; - $group2 = 'h'; $this->set_ratingpreferences(1); - $this->create_twogroups($group1, $group2); - $posts = array($this->post, $this->answer1, $this->answer2, $this->answer3, $this->answer4, $this->answer5, $this->answer6); - $sortedposts = ratings::moodleoverflow_sort_answers_by_ratings($posts); - $rightorderposts = array($this->post, $this->answer2, $this->answer1, $this->answer3, - $this->answer6, $this->answer5, $this->answer4); - $result = $this->postsorderequal($sortedposts, $rightorderposts); - $this->assertEquals(1, $result); + $this->process_groups($group1, $group2); // Test case 6: only solved posts and not-marked posts. - $group1 = 's'; $group2 = 'o'; $this->create_twogroups($group1, $group2); $posts = array($this->post, $this->answer1, $this->answer2, $this->answer3, $this->answer4, $this->answer5, $this->answer6); @@ -261,14 +228,7 @@ public function test_answersorting_twogroups() { // Test case 6: only helpful posts and not-marked posts. $group1 = 'h'; - $group2 = 'o'; - $this->create_twogroups($group1, $group2); - $posts = array($this->post, $this->answer1, $this->answer2, $this->answer3, $this->answer4, $this->answer5, $this->answer6); - $sortedposts = ratings::moodleoverflow_sort_answers_by_ratings($posts); - $rightorderposts = array($this->post, $this->answer2, $this->answer1, $this->answer3, - $this->answer6, $this->answer5, $this->answer4); - $result = $this->postsorderequal($sortedposts, $rightorderposts); - $this->assertEquals(1, $result); + $this->process_groups($group1, $group2); } /** @@ -651,4 +611,24 @@ private function create_twogroups($group1, $group2) { // Rightorder (h,o) = answer2, answer1, answer3, answer6, answer5, answer4. } + + /** + * Executing the sort function and comparing the sorted post to the expected order. + * @param String $group1 + * @param string $group2 + * @param array|null $orderposts + * @return void + */ + private function process_groups(String $group1, string $group2, array $orderposts = null) { + $this->create_twogroups($group1, $group2); + $posts = array($this->post, $this->answer1, $this->answer2, $this->answer3, $this->answer4, $this->answer5, $this->answer6); + $sortedposts = ratings::moodleoverflow_sort_answers_by_ratings($posts); + $rightorderposts = array($this->post, $this->answer2, $this->answer1, $this->answer3, + $this->answer6, $this->answer5, $this->answer4); + if ($orderposts) { + $rightorderposts = $orderposts; + } + $result = $this->postsorderequal($sortedposts, $rightorderposts); + $this->assertEquals(1, $result); + } } diff --git a/tests/review_test.php b/tests/review_test.php index 29aae0090a..82a6823a16 100644 --- a/tests/review_test.php +++ b/tests/review_test.php @@ -114,23 +114,9 @@ public function test_forum_review_everything() { $options = array('course' => $this->course->id, 'needsreview' => review::EVERYTHING, 'forcesubscribe' => MOODLEOVERFLOW_FORCESUBSCRIBE); - $moodleoverflow = $this->getDataGenerator()->create_module('moodleoverflow', $options); - - list(, $teacherpost) = $this->generator->post_to_forum($moodleoverflow, $this->teacher); - list(, $studentpost) = $this->generator->post_to_forum($moodleoverflow, $this->student); - - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $teacherpost->id])); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 0, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id])); - - $this->run_send_mails(); - $this->run_send_mails(); // Execute twice to ensure no duplicate mails. - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $teacherpost->id])); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_REVIEW_SUCCESS, 'reviewed' => 0, - 'timereviewed' => null], $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id])); + $posts = $this->create_post($options); + $this->check_mail_records($posts['teacherpost'], $posts['studentpost'], 1, 0, MOODLEOVERFLOW_MAILED_REVIEW_SUCCESS); $this->assertEquals(1, $this->mailsink->count()); // Teacher has to approve student message. $this->assertEquals(2, $this->messagesink->count()); // Student and teacher get notification for student message. @@ -138,12 +124,12 @@ public function test_forum_review_everything() { $this->mailsink->clear(); $this->messagesink->clear(); - $this->assertNull(\mod_moodleoverflow_external::review_approve_post($studentpost->id)); + $this->assertNull(\mod_moodleoverflow_external::review_approve_post($posts['studentpost']->id)); $this->run_send_mails(); $this->run_send_mails(); // Execute twice to ensure no duplicate mails. - $post = $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id]); + $post = $DB->get_record('moodleoverflow_posts', ['id' => $posts['studentpost']->id]); $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1], $post); $this->assertNotNull($post->timereviewed ?? null); @@ -152,8 +138,8 @@ public function test_forum_review_everything() { $this->messagesink->clear(); - $studentanswer1 = $this->generator->reply_to_post($teacherpost, $this->student, false); - $studentanswer2 = $this->generator->reply_to_post($teacherpost, $this->student, false); + $studentanswer1 = $this->generator->reply_to_post($posts['teacherpost'], $this->student, false); + $studentanswer2 = $this->generator->reply_to_post($posts['teacherpost'], $this->student, false); $this->run_send_mails(); $this->run_send_mails(); // Execute twice to ensure no duplicate mails. @@ -193,23 +179,8 @@ public function test_forum_review_only_questions() { $options = array('course' => $this->course->id, 'needsreview' => review::QUESTIONS, 'forcesubscribe' => MOODLEOVERFLOW_FORCESUBSCRIBE); - $moodleoverflow = $this->getDataGenerator()->create_module('moodleoverflow', $options); - - list(, $teacherpost) = $this->generator->post_to_forum($moodleoverflow, $this->teacher); - list(, $studentpost) = $this->generator->post_to_forum($moodleoverflow, $this->student); - - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $teacherpost->id])); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 0, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id])); - - $this->run_send_mails(); - $this->run_send_mails(); // Execute twice to ensure no duplicate mails. - - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $teacherpost->id])); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_REVIEW_SUCCESS, 'reviewed' => 0, - 'timereviewed' => null], $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id])); + $posts = $this->create_post($options); + $this->check_mail_records($posts['teacherpost'], $posts['studentpost'], 1, 0, MOODLEOVERFLOW_MAILED_REVIEW_SUCCESS); $this->assertEquals(1, $this->mailsink->count()); // Teacher has to approve student message. $this->assertEquals(2, $this->messagesink->count()); // Student and teacher get notification for student message. @@ -217,12 +188,12 @@ public function test_forum_review_only_questions() { $this->mailsink->clear(); $this->messagesink->clear(); - $this->assertNull(\mod_moodleoverflow_external::review_approve_post($studentpost->id)); + $this->assertNull(\mod_moodleoverflow_external::review_approve_post($posts['studentpost']->id)); $this->run_send_mails(); $this->run_send_mails(); // Execute twice to ensure no duplicate mails. - $post = $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id]); + $post = $DB->get_record('moodleoverflow_posts', ['id' => $posts['studentpost']->id]); $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1], $post); $this->assertNotNull($post->timereviewed ?? null); @@ -231,53 +202,26 @@ public function test_forum_review_only_questions() { $this->messagesink->clear(); - $studentanswer1 = $this->generator->reply_to_post($teacherpost, $this->student, false); - $studentanswer2 = $this->generator->reply_to_post($teacherpost, $this->student, false); + $studentanswer1 = $this->generator->reply_to_post($posts['teacherpost'], $this->student, false); + $studentanswer2 = $this->generator->reply_to_post($posts['teacherpost'], $this->student, false); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentanswer1->id])); - - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentanswer2->id])); - - $this->run_send_mails(); - $this->run_send_mails(); // Execute twice to ensure no duplicate mails. + $this->check_mail_records($studentanswer1, $studentanswer2, 1, 1, MOODLEOVERFLOW_MAILED_SUCCESS); $this->assertEquals(0, $this->mailsink->count()); $this->assertEquals(4, $this->messagesink->count()); - - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentanswer1->id])); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentanswer2->id])); } /** * Test reviews functionality when reviewing is allowed in admin settings. */ public function test_forum_review_disallowed() { - global $DB; $options = array('course' => $this->course->id, 'needsreview' => review::EVERYTHING, 'forcesubscribe' => MOODLEOVERFLOW_FORCESUBSCRIBE); - $moodleoverflow = $this->getDataGenerator()->create_module('moodleoverflow', $options); set_config('allowreview', 0, 'moodleoverflow'); - list(, $teacherpost) = $this->generator->post_to_forum($moodleoverflow, $this->teacher); - list(, $studentpost) = $this->generator->post_to_forum($moodleoverflow, $this->student); - - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $teacherpost->id])); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id])); - - $this->run_send_mails(); - $this->run_send_mails(); // Execute twice to ensure no duplicate mails. - - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $teacherpost->id])); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id])); + $posts = $this->create_post($options); + $this->check_mail_records($posts['teacherpost'], $posts['studentpost'], 1, 1, MOODLEOVERFLOW_MAILED_SUCCESS); $this->assertEquals(0, $this->mailsink->count()); // Teacher has to approve student message. $this->assertEquals(4, $this->messagesink->count()); // Student and teacher get notification for student message. @@ -285,25 +229,13 @@ public function test_forum_review_disallowed() { $this->mailsink->clear(); $this->messagesink->clear(); - $studentanswer1 = $this->generator->reply_to_post($teacherpost, $this->student, false); - $studentanswer2 = $this->generator->reply_to_post($teacherpost, $this->student, false); - - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentanswer1->id])); + $studentanswer1 = $this->generator->reply_to_post($posts['teacherpost'], $this->student, false); + $studentanswer2 = $this->generator->reply_to_post($posts['teacherpost'], $this->student, false); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentanswer2->id])); - - $this->run_send_mails(); - $this->run_send_mails(); // Execute twice to ensure no duplicate mails. + $this->check_mail_records($studentanswer1, $studentanswer2, 1, 1, MOODLEOVERFLOW_MAILED_SUCCESS); $this->assertEquals(0, $this->mailsink->count()); $this->assertEquals(4, $this->messagesink->count()); - - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentanswer1->id])); - $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => 1, 'timereviewed' => null], - $DB->get_record('moodleoverflow_posts', ['id' => $studentanswer2->id])); } /** @@ -325,11 +257,52 @@ private function run_send_mails() { * @param object|array $actual */ private function assert_matches_properties($expected, $actual) { - $expected = (array) $expected; - $actual = (object) $actual; + $expected = (array)$expected; + $actual = (object)$actual; foreach ($expected as $key => $value) { $this->assertObjectHasAttribute($key, $actual, "Failed asserting that attribute '$key' exists."); $this->assertEquals($value, $actual->$key, "Failed asserting that \$obj->$key '" . $actual->$key . "' equals '$value'"); } } + + /** + * Create two posts. + * @param array $options + * @return array the teacher and the studentpost. + */ + private function create_post($options) { + $moodleoverflow = $this->getDataGenerator()->create_module('moodleoverflow', $options); + + list(, $teacherpost) = $this->generator->post_to_forum($moodleoverflow, $this->teacher); + list(, $studentpost) = $this->generator->post_to_forum($moodleoverflow, $this->student); + + return array('teacherpost' => $teacherpost, 'studentpost' => $studentpost); + } + + /** + * Check Mail object before and after sending. + * @param \stdClass $teacherpost + * @param \stdClass $studentpost + * @param int $review1 + * @param int $review2 + * @param int $mailed + * @return void + * @throws \dml_exception + */ + private function check_mail_records($teacherpost, $studentpost, $review1, $review2, $mailed) { + global $DB; + + $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => $review1, 'timereviewed' => null], + $DB->get_record('moodleoverflow_posts', ['id' => $teacherpost->id])); + $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_PENDING, 'reviewed' => $review2, 'timereviewed' => null], + $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id])); + + $this->run_send_mails(); + $this->run_send_mails(); // Execute twice to ensure no duplicate mails. + + $this->assert_matches_properties(['mailed' => MOODLEOVERFLOW_MAILED_SUCCESS, 'reviewed' => $review1, 'timereviewed' => null], + $DB->get_record('moodleoverflow_posts', ['id' => $teacherpost->id])); + $this->assert_matches_properties(['mailed' => $mailed, 'reviewed' => $review2, 'timereviewed' => null], + $DB->get_record('moodleoverflow_posts', ['id' => $studentpost->id])); + } }