diff --git a/public/mod/data/classes/courseformat/overview.php b/public/mod/data/classes/courseformat/overview.php index c9a40e44191..c4db045b789 100644 --- a/public/mod/data/classes/courseformat/overview.php +++ b/public/mod/data/classes/courseformat/overview.php @@ -40,18 +40,28 @@ class overview extends \core_courseformat\activityoverviewbase { /** @var bool whether the user can see pendent entries or not. */ private $canviewall; + /** @var array All the entries belonging to groups that the current user can view. */ + private $allentries = []; + /** * Constructor. * * @param cm_info $cm the course module instance. + * @param \core\output\renderer_helper $rendererhelper the renderer helper. + * @param \core_string_manager $stringmanager the string manager. */ public function __construct( - cm_info $cm, + cm_info $cm, + /** @var \core\output\renderer_helper $rendererhelper the renderer helper */ + protected readonly \core\output\renderer_helper $rendererhelper, + /** @var \core_string_manager $stringmanager the string manager */ + protected readonly \core_string_manager $stringmanager, ) { parent::__construct($cm); $this->manager = manager::create_from_coursemodule($cm); $this->canviewall = has_capability('mod/data:approve', $cm->context); + $this->allentries = $this->manager->get_all_entries($this->get_groups_for_filtering()); } #[\Override] @@ -89,7 +99,7 @@ class overview extends \core_courseformat\activityoverviewbase { $toapprove = 0; if ($this->manager->get_approval_requested()) { // Let's calculate how many entries need to be approved. - $entries = $this->manager->filter_entries_by_approval($this->manager->get_all_entries(), 0); + $entries = $this->manager->filter_entries_by_approval($this->allentries, 0); $toapprove = count($entries); if ($toapprove > 0) { $name = get_string('approve', 'data'); @@ -116,19 +126,11 @@ class overview extends \core_courseformat\activityoverviewbase { #[\Override] public function get_extra_overview_items(): array { - $columns = []; - // Add entry columns for each view. - if ($this->canviewall) { - $columns['totalentries'] = $this->get_extra_entries_overview(); - } else { - $columns['totalentries'] = $this->get_extra_totalentries_overview(); - $columns['myentries'] = $this->get_extra_myentries_overview(); - } - - // Add comments column for all views. - $columns['comments'] = $this->get_extra_comments_overview(); - - return $columns; + return [ + 'totalentries' => $this->get_extra_entries_overview(), + 'myentries' => $this->get_extra_myentries_overview(), + 'comments' => $this->get_extra_comments_overview(), + ]; } /** @@ -145,7 +147,7 @@ class overview extends \core_courseformat\activityoverviewbase { // Add total entries. return new overviewitem( - name: get_string('totalentries', 'data'), + name: $this->stringmanager->get_string('totalentries', 'data'), value: $totalentries, content: $totalentries, textalign: text_align::END, @@ -155,12 +157,16 @@ class overview extends \core_courseformat\activityoverviewbase { /** * Get the "My entries" overview item. * - * @return overviewitem The overview item. + * @return ?overviewitem The overview item or null when the user is a student. */ - private function get_extra_myentries_overview(): overviewitem { + private function get_extra_myentries_overview(): ?overviewitem { global $USER; - $myentries = $this->manager->filter_entries_by_user($this->manager->get_all_entries(), $USER->id); + if ($this->canviewall) { + return null; + } + + $myentries = $this->manager->filter_entries_by_user($this->allentries, $USER->id); $totalmyentries = count($myentries); return new overviewitem( @@ -177,12 +183,19 @@ class overview extends \core_courseformat\activityoverviewbase { * @return overviewitem The overview item. */ private function get_extra_entries_overview(): overviewitem { - $allentries = $this->manager->get_all_entries(); - $totalentries = count($allentries); - - // Add total entries. + if ($this->canviewall) { + $name = get_string('entries', 'data'); + $totalentries = count($this->allentries); + } else { + $allentries = $this->allentries; + if ($this->manager->get_approval_requested()) { + $allentries = $this->manager->filter_entries_by_approval($this->allentries, 1); + } + $name = get_string('totalentries', 'data'); + $totalentries = count($allentries); + } return new overviewitem( - name: get_string('entries', 'data'), + name: $name, value: $totalentries, content: $totalentries, textalign: text_align::END, @@ -208,7 +221,7 @@ class overview extends \core_courseformat\activityoverviewbase { } $approved = ($this->canviewall) ? null : 1; - $comments = $this->manager->get_comments(approved: $approved); + $comments = $this->manager->get_comments(approved: $approved, groups: $this->get_groups_for_filtering()); $totalcomments = ($comments) ? count($comments) : 0; return new overviewitem( name: get_string('comments', 'data'), diff --git a/public/mod/data/classes/manager.php b/public/mod/data/classes/manager.php index fe5c05e9de4..c7f7262ea7f 100644 --- a/public/mod/data/classes/manager.php +++ b/public/mod/data/classes/manager.php @@ -69,16 +69,6 @@ class manager { /** @var cm_info course_modules record. */ private $cm; - /** @var array the current data records. - * Do not access this attribute directly, use $this->get_all_entries instead - */ - private $_entries = null; - - /** @var array the current data comments. - * Do not access this attribute directly, use $this->get_all_comments instead - */ - private $_comments = null; - /** @var array the current data_fields records. * Do not access this attribute directly, use $this->get_field_records instead */ @@ -246,15 +236,20 @@ class manager { /** * Return the database entries. * + * @param array $groups to filter by. * @return [] the data records array. */ - public function get_all_entries(): array { + public function get_all_entries(array $groups = []): array { global $DB; - if (empty($this->_entries)) { - $this->_entries = $DB->get_records('data_records', ['dataid' => $this->instance->id]); + if (empty($groups)) { + return $DB->get_records('data_records', ['dataid' => $this->instance->id]); + } else { + [$sql, $params] = $DB->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED); + $sql = 'dataid = :id AND (groupid ' . $sql . ' OR groupid = 0)'; + $params['id'] = $this->instance->id; + return $DB->get_records_select('data_records', $sql, $params); } - return $this->_entries; } /** @@ -293,21 +288,18 @@ class manager { * Return the database comments filtered by approved entries. * * @param ?int $approved Approved value to filter by. Null for not filtering. + * @param ?array $groups to filter by. * * @return [] the filtered data comments array or null if there is no comment. */ - public function get_comments(?int $approved = null): ?array { + public function get_comments(?int $approved = null, array $groups = []): ?array { - if ($this->_comments) { - return $this->_comments; - } - - $entries = $this->get_all_entries(); + $entries = $this->get_all_entries($groups); if (!is_null($approved)) { $entries = $this->filter_entries_by_approval($entries, $approved); } - $this->_comments = []; + $comments = []; // Initilising comment object. $args = new stdClass; @@ -322,11 +314,11 @@ class manager { $comment = new \comment($args); $morecomments = $comment->get_comments(); if ($morecomments) { - $this->_comments = array_merge($this->_comments, $morecomments); + $comments = array_merge($comments, $morecomments); } } - return $this->_comments; + return $comments; } /** diff --git a/public/mod/data/tests/behat/overview_report.feature b/public/mod/data/tests/behat/overview_report.feature index c67b0bf6e72..ef6c1cf7c34 100644 --- a/public/mod/data/tests/behat/overview_report.feature +++ b/public/mod/data/tests/behat/overview_report.feature @@ -13,9 +13,9 @@ Feature: Testing overview integration in database activity | fullname | shortname | category | enablecompletion | | Course 1 | C1 | 0 | 1 | And the following "course enrolments" exist: - | user | course | role | - | teacher1 | C1 | editingteacher | - | student1 | C1 | student | + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | And the following "activity" exists: | course | C1 | | activity | data | diff --git a/public/mod/data/tests/overview_test.php b/public/mod/data/tests/courseformat/overview_test.php similarity index 79% rename from public/mod/data/tests/overview_test.php rename to public/mod/data/tests/courseformat/overview_test.php index 0411319af98..0d72f988a5e 100644 --- a/public/mod/data/tests/overview_test.php +++ b/public/mod/data/tests/courseformat/overview_test.php @@ -14,9 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -namespace mod_data; +namespace mod_data\courseformat; use core_courseformat\local\overview\overviewfactory; +use mod_data\manager; /** * Tests for Database activity overview @@ -135,8 +136,8 @@ final class overview_test extends \advanced_testcase { 'needsapproval' => true, 'entries' => [1, 1], 'expected' => [ - 'link' => get_string('view', 'moodle'), - 'value' => 0, + 'link' => get_string('view', 'moodle'), + 'value' => 0, ], ], ]; @@ -156,11 +157,11 @@ final class overview_test extends \advanced_testcase { * @return void */ public function test_get_extra_entries_overview( - string $role, - bool $needsapproval, - array $entries, - array $myentries, - array $expected + string $role, + bool $needsapproval, + array $entries, + array $myentries, + array $expected ): void { $this->resetAfterTest(); @@ -206,13 +207,13 @@ final class overview_test extends \advanced_testcase { $items = overviewfactory::create($cm)->get_extra_overview_items(); if (is_null($expected['totalentries'])) { - $this->assertArrayNotHasKey('totalentries', $items); + $this->assertNull($items['totalentries']); } else { $this->assertEquals($expected['totalentries'], $items['totalentries']->get_value()); } if (is_null($expected['myentries'])) { - $this->assertArrayNotHasKey('myentries', $items); + $this->assertNull($items['myentries']); } else { $this->assertEquals($expected['myentries'], $items['myentries']->get_value()); } @@ -288,6 +289,96 @@ final class overview_test extends \advanced_testcase { ]; } + /** + * Test get_extra_overview_items with groups. + * + * @covers ::get_extra_overview_items + */ + public function test_get_extra_entries_overview_with_groups(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $g1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $g2 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $this->setAdminUser(); + + $activity = $this->getDataGenerator()->create_module( + manager::MODULE, + ['course' => $course, 'groupmode' => SEPARATEGROUPS], + ); + + // Add a field. + /** @var \mod_data_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('mod_data'); + $fieldrecord = (object)[ + 'name' => 'myfield', + 'type' => 'text', + ]; + $field = $generator->create_field($fieldrecord, $activity); + $generator->create_entry( + $activity, + [$field->field->id => 'Example entry: All participants'], + ); + + // Create entries for each group. + $generator->create_entry( + $activity, + [$field->field->id => 'G1'], + $g1->id, + ); + $generator->create_entry( + $activity, + [$field->field->id => 'G2'], + $g2->id, + ); + + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'editingteacher'); + $noneditingteacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + groups_add_member($g1, $noneditingteacher->id); + $student = $this->getDataGenerator()->create_and_enrol($course, 'student'); + groups_add_member($g1, $student->id); + $otherstudent = $this->getDataGenerator()->create_and_enrol($course, 'student'); + groups_add_member($g2, $otherstudent->id); + + $generator->create_entry( + $activity, + [$field->field->id => 'G1'], + $g1->id, + [], + null, + $student->id + ); + $generator->create_entry( + $activity, + [$field->field->id => 'G2'], + $g2->id, + [], + null, + $otherstudent->id + ); + + // Editing teachers can see everything. + $this->setUser($teacher); + $cm = get_fast_modinfo($course)->get_cm($activity->cmid); + $items = overviewfactory::create($cm)->get_extra_overview_items(); + $this->assertEquals(5, $items['totalentries']->get_value()); + $this->assertNull($items['myentries']); + + // Non-editing teachers can see their groups and all participants. + $this->setUser($noneditingteacher); + $cm = get_fast_modinfo($course)->get_cm($activity->cmid); + $items = overviewfactory::create($cm)->get_extra_overview_items(); + $this->assertEquals(3, $items['totalentries']->get_value()); + $this->assertNull($items['myentries']); + + // Students can see their groups and all participants. + $this->setUser($student); + $cm = get_fast_modinfo($course)->get_cm($activity->cmid); + $items = overviewfactory::create($cm)->get_extra_overview_items(); + $this->assertEquals(3, $items['totalentries']->get_value()); + $this->assertEquals(1, $items['myentries']->get_value()); + } + /** * Test get_extra_comments_overview. * @@ -301,10 +392,10 @@ final class overview_test extends \advanced_testcase { * @return void */ public function test_get_extra_comments_overview( - string $role, - bool $needsapproval, - array $entries, - int $expected + string $role, + bool $needsapproval, + array $entries, + int $expected ): void { global $CFG; diff --git a/public/mod/data/tests/manager_test.php b/public/mod/data/tests/manager_test.php index 713d5b4f583..85c0623e7b7 100644 --- a/public/mod/data/tests/manager_test.php +++ b/public/mod/data/tests/manager_test.php @@ -786,17 +786,12 @@ final class manager_test extends \advanced_testcase { // Empty database should return empty array. $this->assertEmpty($manager->get_all_entries()); - // Create data record. - $datarecords = new \stdClass(); - $datarecords->userid = '2'; - $datarecords->dataid = $data->id; - // Add a field. /** @var \mod_data_generator $generator */ $generator = $this->getDataGenerator()->get_plugin_generator('mod_data'); $fieldrecord = (object)[ - 'name' => 'myfield', - 'type' => 'text', + 'name' => 'myfield', + 'type' => 'text', ]; $field = $generator->create_field($fieldrecord, $data); $generator->create_entry( @@ -806,6 +801,53 @@ final class manager_test extends \advanced_testcase { $this->assertCount(1, $manager->get_all_entries()); } + /** + * Test for get_all_entries() with groups parameter. + * + * @covers ::get_all_entries + */ + public function test_get_all_entries_with_groups(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $g1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $g2 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + + $data = $this->getDataGenerator()->create_module(manager::MODULE, ['course' => $course, 'groupmode' => SEPARATEGROUPS]); + $manager = manager::create_from_instance($data); + + // Add a field. + /** @var \mod_data_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('mod_data'); + $fieldrecord = (object)[ + 'name' => 'myfield', + 'type' => 'text', + ]; + $field = $generator->create_field($fieldrecord, $data); + $generator->create_entry( + $data, + [$field->field->id => 'Example entry'], + ); + + // Create entries for each group. + $generator->create_entry( + $data, + [$field->field->id => 'G1'], + $g1->id, + ); + $generator->create_entry( + $data, + [$field->field->id => 'G2'], + $g2->id, + ); + + $this->assertCount(3, $manager->get_all_entries([])); + $this->assertCount(2, $manager->get_all_entries([$g1->id => $g1])); + $this->assertCount(3, $manager->get_all_entries([$g1->id => $g1, $g2->id => $g2])); + // Non-existing group. It returns the entries with no group. + $this->assertCount(1, $manager->get_all_entries([666 => null])); + } + /** * Test filter_entries_by_user. * @@ -818,9 +860,9 @@ final class manager_test extends \advanced_testcase { * @return void */ public function test_filter_entries_by_user( - array $entries, - array $myentries, - int $expected, + array $entries, + array $myentries, + int $expected, ): void { $this->resetAfterTest(); @@ -828,25 +870,25 @@ final class manager_test extends \advanced_testcase { $this->setAdminUser(); $activity = $this->getDataGenerator()->create_module( - manager::MODULE, - ['course' => $course, 'approval' => 1], + manager::MODULE, + ['course' => $course, 'approval' => 1], ); // Add a field. /** @var \mod_data_generator $generator */ $generator = $this->getDataGenerator()->get_plugin_generator('mod_data'); $fieldrecord = (object)[ - 'name' => 'myfield', - 'type' => 'text', + 'name' => 'myfield', + 'type' => 'text', ]; $field = $generator->create_field($fieldrecord, $activity); foreach ($entries as $entry => $approved) { $generator->create_entry( - $activity, - [$field->field->id => 'Example entry: '.$entry], - 0, - [], - ['approved' => $approved], + $activity, + [$field->field->id => 'Example entry: ' . $entry], + 0, + [], + ['approved' => $approved], ); } @@ -854,11 +896,11 @@ final class manager_test extends \advanced_testcase { $this->setUser($currentuser); foreach ($myentries as $entry => $approved) { $generator->create_entry( - $activity, - [$field->field->id => 'Example entry: '.$entry], - 0, - [], - ['approved' => $approved], + $activity, + [$field->field->id => 'Example entry: ' . $entry], + 0, + [], + ['approved' => $approved], ); } @@ -874,21 +916,21 @@ final class manager_test extends \advanced_testcase { */ public static function provider_test_filter_entries_by_user(): array { return [ - 'Empty database filtered by current user' => [ - 'entries' => [], - 'myentries' => [], - 'expected' => 0, - ], - 'User without own entries filtered by current user' => [ - 'entries' => [1, 0], - 'myentries' => [], - 'expected' => 0, - ], - 'User with own entries filtered by current user' => [ - 'entries' => [1, 0], - 'myentries' => [1, 0], - 'expected' => 2, - ], + 'Empty database filtered by current user' => [ + 'entries' => [], + 'myentries' => [], + 'expected' => 0, + ], + 'User without own entries filtered by current user' => [ + 'entries' => [1, 0], + 'myentries' => [], + 'expected' => 0, + ], + 'User with own entries filtered by current user' => [ + 'entries' => [1, 0], + 'myentries' => [1, 0], + 'expected' => 2, + ], ]; } @@ -904,9 +946,9 @@ final class manager_test extends \advanced_testcase { * @return void */ public function test_filter_entries_by_approval( - array $entries, - int $approvalfilter, - int $expected, + array $entries, + int $approvalfilter, + int $expected, ): void { $this->resetAfterTest(); @@ -914,25 +956,25 @@ final class manager_test extends \advanced_testcase { $this->setAdminUser(); $activity = $this->getDataGenerator()->create_module( - manager::MODULE, - ['course' => $course, 'approval' => 1], + manager::MODULE, + ['course' => $course, 'approval' => 1], ); // Add a field. /** @var \mod_data_generator $generator */ $generator = $this->getDataGenerator()->get_plugin_generator('mod_data'); $fieldrecord = (object)[ - 'name' => 'myfield', - 'type' => 'text', + 'name' => 'myfield', + 'type' => 'text', ]; $field = $generator->create_field($fieldrecord, $activity); foreach ($entries as $entry => $approved) { $generator->create_entry( - $activity, - [$field->field->id => 'Example entry: '.$entry], - 0, - [], - ['approved' => $approved], + $activity, + [$field->field->id => 'Example entry: ' . $entry], + 0, + [], + ['approved' => $approved], ); } @@ -949,21 +991,21 @@ final class manager_test extends \advanced_testcase { */ public static function provider_test_filter_entries_by_approval(): array { return [ - 'Empty database filtered by approved' => [ - 'entries' => [], - 'approvalfilter' => 1, - 'expected' => 0, - ], - 'Database with entries filtered by approved' => [ - 'entries' => [1, 0, 1, 1], - 'approvalfilter' => 1, - 'expected' => 3, - ], - 'Database with entries filtered by non approved' => [ - 'entries' => [1, 0, 1, 1], - 'approvalfilter' => 0, - 'expected' => 1, - ], + 'Empty database filtered by approved' => [ + 'entries' => [], + 'approvalfilter' => 1, + 'expected' => 0, + ], + 'Database with entries filtered by approved' => [ + 'entries' => [1, 0, 1, 1], + 'approvalfilter' => 1, + 'expected' => 3, + ], + 'Database with entries filtered by non approved' => [ + 'entries' => [1, 0, 1, 1], + 'approvalfilter' => 0, + 'expected' => 1, + ], ]; } @@ -1032,7 +1074,7 @@ final class manager_test extends \advanced_testcase { } /** - * Data provider for test comments extras. + * Data provider for test get_comments function. * * @return array */ @@ -1089,6 +1131,80 @@ final class manager_test extends \advanced_testcase { ]; } + /** + * Test get_comments with groups parameter. + * + * @covers ::get_comments + */ + public function test_get_comments_with_groups(): void { + global $CFG; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $CFG->usecomments = true; + + $course = $this->getDataGenerator()->create_course(); + $g1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $g2 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + + $data = $this->getDataGenerator()->create_module( + manager::MODULE, + ['course' => $course, 'approval' => 1, 'comments' => 1, 'groupmode' => SEPARATEGROUPS], + ); + + // Add a field. + /** @var \mod_data_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('mod_data'); + $fieldrecord = (object)[ + 'name' => 'myfield', + 'type' => 'text', + ]; + $field = $generator->create_field($fieldrecord, $data); + + $comments = []; + $comment = [ + 'contextlevel' => 'module', + 'instanceid' => $data->cmid, + 'component' => 'mod_data', + 'content' => 'abc', + 'area' => 'database_entry', + ]; + + $nogroupsentry = $generator->create_entry( + $data, + [$field->field->id => 'Example entry'], + ); + $comment['itemid'] = $nogroupsentry; + $comments[] = $comment; + + // Create entries for each group. + $g1entry = $generator->create_entry( + $data, + [$field->field->id => 'G1'], + $g1->id, + ); + $comment['itemid'] = $g1entry; + $comments[] = $comment; + + $g2entry = $generator->create_entry( + $data, + [$field->field->id => 'G2'], + $g2->id, + ); + $comment['itemid'] = $g2entry; + $comments[] = $comment; + + \core_comment_external::add_comments($comments); + $manager = manager::create_from_instance($data); + + $this->assertCount(3, $manager->get_comments()); + $this->assertCount(2, $manager->get_comments(groups: [$g1->id => $g1])); + $this->assertCount(3, $manager->get_comments(groups: [$g1->id => $g1, $g2->id => $g2])); + // Non-existing group. It returns the entries with no group. + $this->assertCount(1, $manager->get_comments(groups: [666 => null])); + } + /** * Test get_approval_requested. * @@ -1101,15 +1217,15 @@ final class manager_test extends \advanced_testcase { $this->setAdminUser(); $activity = $this->getDataGenerator()->create_module( - manager::MODULE, - ['course' => $course, 'approval' => 1], + manager::MODULE, + ['course' => $course, 'approval' => 1], ); $manager = manager::create_from_instance($activity); $this->assertEquals(1, $manager->get_approval_requested()); $activity = $this->getDataGenerator()->create_module( - manager::MODULE, - ['course' => $course, 'approval' => 0], + manager::MODULE, + ['course' => $course, 'approval' => 0], ); $manager = manager::create_from_instance($activity); $this->assertEquals(0, $manager->get_approval_requested());