diff --git a/public/mod/h5pactivity/classes/courseformat/overview.php b/public/mod/h5pactivity/classes/courseformat/overview.php index 9cb170d03fc..1cc86f7f6d6 100644 --- a/public/mod/h5pactivity/classes/courseformat/overview.php +++ b/public/mod/h5pactivity/classes/courseformat/overview.php @@ -42,14 +42,11 @@ class overview extends \core_courseformat\activityoverviewbase { * * @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, /** @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); @@ -63,7 +60,7 @@ class overview extends \core_courseformat\activityoverviewbase { return null; } - $viewresults = $this->stringmanager->get_string('viewresults', 'mod_h5pactivity'); + $viewresults = get_string('view'); $content = new action_link( url: new url('/mod/h5pactivity/report.php', ['id' => $this->cm->id]), text: $viewresults, @@ -71,7 +68,7 @@ class overview extends \core_courseformat\activityoverviewbase { ); return new overviewitem( - name: $this->stringmanager->get_string('actions'), + name: get_string('actions'), value: '', content: $content, textalign: text_align::CENTER, @@ -80,48 +77,50 @@ class overview extends \core_courseformat\activityoverviewbase { #[\Override] public function get_extra_overview_items(): array { - global $USER; - - if (!$this->manager->can_view_own_attempts() && $this->manager->can_view_all_attempts()) { - return []; - } - - if (!$this->manager->can_view_all_attempts()) { - return [ - 'myattempts' => $this->get_extra_userattempts_overview($USER->id), - ]; - } - return [ 'h5ptype' => $this->get_extra_h5ptype_overview(), 'attempted' => $this->get_extra_studentsattempted_overview(), 'totalattempts' => $this->get_extra_totalattempts_overview(), + 'myattempts' => $this->get_extra_userattempts_overview(), ]; } /** * Get the attempts of the given user. * - * @param int $userid The user to return the attempts from. - * @return overviewitem The overview item. + * @param int|null $userid The user to return the attempts from (uses current user if null). + * @return overviewitem|null The overview item or null for teachers. */ - private function get_extra_userattempts_overview(int $userid): overviewitem { + private function get_extra_userattempts_overview(?int $userid = null): ?overviewitem { + global $USER; + + if ($this->manager->can_view_all_attempts()) { + return null; + } + + if ($userid === null) { + $userid = $USER->id; + } $attempts = $this->manager->count_attempts($userid); return new overviewitem( - name: $this->stringmanager->get_string('attempts', 'mod_h5pactivity'), - value: $attempts, - content: $attempts ?? '-', - textalign: text_align::CENTER, + name: get_string('attempts', 'mod_h5pactivity'), + value: $attempts, + content: $attempts ?? '-', + textalign: text_align::CENTER, ); } /** * Get the students who attempted. * - * @return overviewitem The overview item. + * @return overviewitem|null The overview item or null for students. */ - private function get_extra_studentsattempted_overview(): overviewitem { + private function get_extra_studentsattempted_overview(): ?overviewitem { + + if (!$this->manager->can_view_all_attempts()) { + return null; + } $attempts = $this->manager->count_users_attempts(); $participants = get_users_by_capability($this->context, 'mod/h5pactivity:submit'); @@ -130,9 +129,9 @@ class overview extends \core_courseformat\activityoverviewbase { 'total' => count($participants), ]; return new overviewitem( - name: $this->stringmanager->get_string('attempted', 'mod_h5pactivity'), + name: get_string('attempted', 'mod_h5pactivity'), value: count($attempts), - content: $this->stringmanager->get_string('count_of_total', 'core', $params), + content: get_string('count_of_total', 'core', $params), textalign: text_align::CENTER, ); } @@ -140,38 +139,48 @@ class overview extends \core_courseformat\activityoverviewbase { /** * Get the "Total attempts" colum data. * - * @return overviewitem The overview item. + * @return overviewitem|null The overview item or null for students. */ - private function get_extra_totalattempts_overview(): overviewitem { + private function get_extra_totalattempts_overview(): ?overviewitem { + + if (!$this->manager->can_view_all_attempts()) { + return null; + } + $totalattempts = $this->manager->count_attempts(); $totalusers = $this->manager->count_users_attempts(); - $content = '-'; + $averageattempts = 0; if ($totalusers && count($totalusers) > 0 && $totalattempts) { $averageattempts = (int) round($totalattempts / count($totalusers)); - $content = new overviewdialog( - buttoncontent: $totalattempts, - title: $this->stringmanager->get_string('totalattempts', 'mod_h5pactivity'), - definition: ['buttonclasses' => button::BODY_OUTLINE->classes()], - ); - $method = $this->manager::get_grading_methods()[$this->manager->get_instance()->grademethod]; - $content->add_item($this->stringmanager->get_string('gradingmethod', 'grading'), $method); - $content->add_item($this->stringmanager->get_string('averageattempts', 'mod_h5pactivity'), $averageattempts); } + $content = new overviewdialog( + buttoncontent: $totalattempts, + title: get_string('totalattempts', 'mod_h5pactivity'), + definition: ['buttonclasses' => button::BODY_OUTLINE->classes()], + ); + $method = $this->manager::get_grading_methods()[$this->manager->get_instance()->grademethod]; + $content->add_item(get_string('gradingmethod', 'grading'), $method); + $content->add_item(get_string('averageattempts', 'mod_h5pactivity'), $averageattempts); return new overviewitem( - name: $this->stringmanager->get_string('totalattempts', 'mod_h5pactivity'), - value: $totalattempts, - content: $content, + name: get_string('totalattempts', 'mod_h5pactivity'), + value: $totalattempts, + content: $content, ); } /** * Get the H5P content type. * - * @return overviewitem The overview item. + * @return overviewitem|null The overview item or null for students. */ - private function get_extra_h5ptype_overview(): overviewitem { + private function get_extra_h5ptype_overview(): ?overviewitem { + + if (!$this->manager->can_view_all_attempts()) { + return null; + } + $fs = get_file_storage(); $files = $fs->get_area_files($this->context->id, 'mod_h5pactivity', 'package', 0, 'id', false); $file = reset($files); @@ -179,9 +188,9 @@ class overview extends \core_courseformat\activityoverviewbase { $h5p = \core_h5p\api::get_content_from_pathnamehash($file->get_pathnamehash()); $unknonwoverview = new overviewitem( - name: $this->stringmanager->get_string('contenttype', 'mod_h5pactivity'), - value: $this->stringmanager->get_string('unknowntype', 'mod_h5pactivity'), - content: $this->stringmanager->get_string('unknowntype', 'mod_h5pactivity'), + name: get_string('contenttype', 'mod_h5pactivity'), + value: get_string('unknowntype', 'mod_h5pactivity'), + content: get_string('unknowntype', 'mod_h5pactivity'), ); if (empty($h5p)) { @@ -196,7 +205,7 @@ class overview extends \core_courseformat\activityoverviewbase { } return new overviewitem( - name: $this->stringmanager->get_string('contenttype', 'mod_h5pactivity'), + name: get_string('contenttype', 'mod_h5pactivity'), value: $h5plib->title, content: $h5plib->title, ); diff --git a/public/mod/h5pactivity/lang/en/h5pactivity.php b/public/mod/h5pactivity/lang/en/h5pactivity.php index fc61b467173..0f2ca16c2f8 100644 --- a/public/mod/h5pactivity/lang/en/h5pactivity.php +++ b/public/mod/h5pactivity/lang/en/h5pactivity.php @@ -153,5 +153,4 @@ $string['unknowntype'] = 'Unknown H5P type'; $string['usecontentbank'] = 'Use the content bank (opens in new window) to manage your H5P files'; $string['view'] = 'View'; $string['viewattempts'] = 'View attempts ({$a})'; -$string['viewresults'] = 'View results'; $string['view_report'] = 'View report'; diff --git a/public/mod/h5pactivity/tests/behat/overview_report.feature b/public/mod/h5pactivity/tests/behat/overview_report.feature index 5a6cdb028af..a6385e24b50 100644 --- a/public/mod/h5pactivity/tests/behat/overview_report.feature +++ b/public/mod/h5pactivity/tests/behat/overview_report.feature @@ -81,16 +81,18 @@ Feature: Testing overview integration in H5P activity And I should see "Total attempts" in the "h5pactivity_overview_collapsible" "region" # Check column values. And the following should exist in the "Table listing all H5P activities" table: - | Name | H5P type | Students who attempted | Total attempts | Actions | - | H5P activity | Find The Words | 2 of 3 | 5 | View results | - | Empty H5P activity | Unknown H5P type | 0 of 3 | - | View results | - # Check the Total attempts value. - And I should see "-" in the "Empty H5P activity" "table_row" + | Name | H5P type | Students who attempted | Total attempts | Actions | + | H5P activity | Find The Words | 2 of 3 | 5 | View | + | Empty H5P activity | Unknown H5P type | 0 of 3 | 0 | View | When I click on "5" "button" in the "H5P activity" "table_row" Then I should see "Grading method: Average grade" And I should see "Average attempts per student: 3" - # Check the View results link. - And I click on "View results" "link" in the "H5P activity" "table_row" + And I press the escape key + And I click on "0" "button" in the "Empty H5P activity" "table_row" + And I should see "Grading method: Highest grade" + And I should see "Average attempts per student: 0" + # Check the View link. + And I click on "View" "link" in the "H5P activity" "table_row" And I should see "Attempts (5)" Scenario: The H5P activity index redirect to the activities overview diff --git a/public/mod/h5pactivity/tests/courseformat/overview_test.php b/public/mod/h5pactivity/tests/courseformat/overview_test.php index c86665dadd7..1617e8dfd12 100644 --- a/public/mod/h5pactivity/tests/courseformat/overview_test.php +++ b/public/mod/h5pactivity/tests/courseformat/overview_test.php @@ -119,7 +119,7 @@ final class overview_test extends \advanced_testcase { $cm = get_fast_modinfo($course)->get_cm($activity->cmid); $items = overviewfactory::create($cm)->get_extra_overview_items(); - $this->assertArrayNotHasKey('h5ptype', $items); + $this->assertNull($items['h5ptype']); } /** @@ -174,10 +174,12 @@ final class overview_test extends \advanced_testcase { $this->setUser($teacher); $items = overviewfactory::create($cm)->get_extra_overview_items(); $this->assertEquals(0, $items['totalattempts']->get_value()); + $this->assertNull($items['myattempts']); $this->setUser($student); $items = overviewfactory::create($cm)->get_extra_overview_items(); $this->assertEquals(0, $items['myattempts']->get_value()); + $this->assertNull($items['totalattempts']); // Attempts done by other student. $params = ['cmid' => $cm->id, 'userid' => $other->id]; @@ -187,10 +189,12 @@ final class overview_test extends \advanced_testcase { $this->setUser($teacher); $items = overviewfactory::create($cm)->get_extra_overview_items(); $this->assertEquals(2, $items['totalattempts']->get_value()); + $this->assertNull($items['myattempts']); $this->setUser($student); $items = overviewfactory::create($cm)->get_extra_overview_items(); $this->assertEquals(0, $items['myattempts']->get_value()); + $this->assertNull($items['totalattempts']); // Attempts done by the student. $params = ['cmid' => $cm->id, 'userid' => $student->id]; @@ -200,10 +204,12 @@ final class overview_test extends \advanced_testcase { $this->setUser($teacher); $items = overviewfactory::create($cm)->get_extra_overview_items(); $this->assertEquals(4, $items['totalattempts']->get_value()); + $this->assertNull($items['myattempts']); $this->setUser($student); $items = overviewfactory::create($cm)->get_extra_overview_items(); $this->assertEquals(2, $items['myattempts']->get_value()); + $this->assertNull($items['totalattempts']); } /** @@ -243,5 +249,10 @@ final class overview_test extends \advanced_testcase { $items = overviewfactory::create($cm)->get_extra_overview_items(); $this->assertEquals(1, $items['attempted']->get_value()); $this->assertEquals('1 of 2', $items['attempted']->get_content()); + + // Student. + $this->setUser($student); + $items = overviewfactory::create($cm)->get_extra_overview_items(); + $this->assertNull($items['attempted']); } }