From 6fb09beb86739b903da2b07838bfb17d7aa33ef4 Mon Sep 17 00:00:00 2001 From: Sara Arjona Date: Thu, 11 Jan 2024 16:23:43 +0100 Subject: [PATCH] MDL-69225 h5pactivity: Review when and which banners display The UX team proposal is to display two different notifications, to avoid confusion: 1. When the user is in preview mode: Message: 'You are in preview mode.' Notification Type: Information (blue), with the information permanently visible and not dismissible, ensuring continuous awareness. 2. When activity tracking is disabled: Message: 'Attempt tracking is not enabled for this activity. You can enable it in Settings.' Notification Type: Warning (yellow) with a close icon for dismissal if tracking is intentionally disabled. These notifications should only be visible to teachers and content creators, not students. If a teacher also has a student role, this information should not appear when logged in as a student. That's why the pre-existing is_tracking_enabled() function has been updated to only check if tracking is on for an activity, without considering user capabilities (that's why the $user parameter has been removed from this function too). Besides, a new function called can_submit() has been created to find out if users are allowed to submit an activity. --- .../get_h5pactivity_access_information.php | 2 +- mod/h5pactivity/classes/local/manager.php | 17 ++- mod/h5pactivity/classes/xapi/handler.php | 6 +- mod/h5pactivity/lang/en/h5pactivity.php | 4 +- .../tests/behat/attempt_options.feature | 117 +++--------------- .../duplicate_delete_h5pactivity.feature | 6 +- .../tests/behat/sending_attempt.feature | 9 +- mod/h5pactivity/tests/local/manager_test.php | 31 +++-- mod/h5pactivity/view.php | 18 ++- 9 files changed, 79 insertions(+), 131 deletions(-) diff --git a/mod/h5pactivity/classes/external/get_h5pactivity_access_information.php b/mod/h5pactivity/classes/external/get_h5pactivity_access_information.php index 0567cfc671c..be6f1afc965 100644 --- a/mod/h5pactivity/classes/external/get_h5pactivity_access_information.php +++ b/mod/h5pactivity/classes/external/get_h5pactivity_access_information.php @@ -75,7 +75,7 @@ class get_h5pactivity_access_information extends external_api { $field = 'can' . str_replace('mod/h5pactivity:', '', $capname); // For mod/h5pactivity:submit we need to check if tracking is enabled in the h5pactivity for the current user. if ($field == 'cansubmit') { - $result[$field] = $manager->is_tracking_enabled(); + $result[$field] = $manager->is_tracking_enabled() && $manager->can_submit(); } else { $result[$field] = has_capability($capname, $context); } diff --git a/mod/h5pactivity/classes/local/manager.php b/mod/h5pactivity/classes/local/manager.php index ee8e50e0cfd..398af2203e2 100644 --- a/mod/h5pactivity/classes/local/manager.php +++ b/mod/h5pactivity/classes/local/manager.php @@ -165,14 +165,21 @@ class manager { /** * Check if tracking is enabled in a particular h5pactivity for a specific user. * - * @param stdClass|null $user user record (default $USER) * @return bool if tracking is enabled in this activity */ - public function is_tracking_enabled(stdClass $user = null): bool { + public function is_tracking_enabled(): bool { + return $this->instance->enabletracking; + } + + /** + * Check if the user has permission to submit a particular h5pactivity for a specific user. + * + * @param stdClass|null $user user record (default $USER) + * @return bool if the user has permission to submit in this activity + */ + public function can_submit(stdClass $user = null): bool { global $USER; - if (!$this->instance->enabletracking) { - return false; - } + if (empty($user)) { $user = $USER; } diff --git a/mod/h5pactivity/classes/xapi/handler.php b/mod/h5pactivity/classes/xapi/handler.php index df9ae61ab20..34cd29adf16 100644 --- a/mod/h5pactivity/classes/xapi/handler.php +++ b/mod/h5pactivity/classes/xapi/handler.php @@ -99,7 +99,7 @@ class handler extends handler_base { $manager = manager::create_from_coursemodule($cm); - if (!$manager->is_tracking_enabled($user)) { + if (!($manager->is_tracking_enabled() && $manager->can_submit($user))) { return null; } @@ -174,10 +174,10 @@ class handler extends handler_base { return false; } - // If tracking is not enabled, the state won't be considered valid. + // If tracking is not enabled or the user can't submit, the state won't be considered valid. $manager = manager::create_from_coursemodule($cm); $user = $state->get_user(); - if (!$manager->is_tracking_enabled($user)) { + if (!($manager->is_tracking_enabled() && $manager->can_submit($user))) { return false; } diff --git a/mod/h5pactivity/lang/en/h5pactivity.php b/mod/h5pactivity/lang/en/h5pactivity.php index bb061069bd6..81d8eb2229e 100644 --- a/mod/h5pactivity/lang/en/h5pactivity.php +++ b/mod/h5pactivity/lang/en/h5pactivity.php @@ -109,7 +109,7 @@ $string['package_help'] = 'The package file is a h5p file containing H5P interac $string['page-mod-h5pactivity-x'] = 'Any H5P module page'; $string['pluginadministration'] = 'H5P administration'; $string['pluginname'] = 'H5P'; -$string['previewmode'] = 'This content is displayed in preview mode. No attempt tracking will be stored.'; +$string['previewmode'] = 'You are in preview mode.'; $string['privacy:metadata:attempt'] = 'The attempt number'; $string['privacy:metadata:rawscore'] = 'The score obtained'; $string['privacy:metadata:timecreated'] = 'The time when the tracked element was created'; @@ -141,6 +141,8 @@ $string['search:activity'] = 'H5P - activity information'; $string['startdate'] = 'Start date'; $string['statement_received'] = 'xAPI statement received'; $string['totalscore'] = 'Total score'; +$string['trackingdisabled'] = 'Attempt tracking is not enabled for this activity.'; +$string['trackingdisabled_enable'] = 'Attempt tracking is not enabled for this activity. You can enable it in Settings.'; $string['tracking_messages'] = 'Some H5P provide attempt tracking data for advanced reporting such as number of attempts, responses and grades. Note: Some H5P don\'t provide attempt tracking data. In such cases, the following settings will have no effect.'; $string['true'] = 'True'; $string['usecontentbank'] = 'Use the content bank (opens in new window) to manage your H5P files'; diff --git a/mod/h5pactivity/tests/behat/attempt_options.feature b/mod/h5pactivity/tests/behat/attempt_options.feature index 5f79b6924d9..573606d3965 100644 --- a/mod/h5pactivity/tests/behat/attempt_options.feature +++ b/mod/h5pactivity/tests/behat/attempt_options.feature @@ -1,4 +1,4 @@ -@mod @mod_h5pactivity @core_h5p @javascript +@mod @mod_h5pactivity @core_h5p Feature: Attempts review settings. In order to let users to review attempts As a teacher @@ -9,6 +9,7 @@ Feature: Attempts review settings. | username | firstname | lastname | email | | student1 | Student | 1 | student1@example.com | | teacher1 | Teacher | 1 | teacher1@example.com | + | teacher2 | Teacher | 2 | teacher2@example.com | And the following "courses" exist: | fullname | shortname | category | | Course 1 | C1 | 0 | @@ -16,16 +17,17 @@ Feature: Attempts review settings. | user | course | role | | student1 | C1 | student | | teacher1 | C1 | editingteacher | + | teacher2 | C1 | teacher | - Scenario: Student accessing an activity with attempt review + Scenario Outline: Attempt review behaviour when accessing an H5P activity Given the following "activity" exists: | activity | h5pactivity | | name | H5P package | | intro | Test H5P description | | course | C1 | | idnumber | h5ppackage | - | enabletracking | 1 | - | reviewmode | 1 | + | enabletracking | | + | reviewmode | | And the following "mod_h5pactivity > attempt" exists: | user | student1 | | h5pactivity | H5P package | @@ -36,97 +38,18 @@ Feature: Attempts review settings. | duration | 4 | | completion | 1 | | success | 1 | - When I am on the "H5P package" "h5pactivity activity" page logged in as student1 - Then "Attempts report" "link" should exist in current page administration - And I should not see "This content is displayed in preview mode" + When I am on the "H5P package" "h5pactivity activity" page logged in as + Then "Attempts report" "link" should in current page administration + And I should "You are in preview mode." + And I should "Attempt tracking is not enabled for this activity." + And I should "You can enable it in Settings." - Scenario: Student accessing an activity without attempt review - Given the following "activity" exists: - | activity | h5pactivity | - | name | H5P package | - | intro | Test H5P description | - | course | C1 | - | idnumber | h5ppackage | - | enabletracking | 1 | - | reviewmode | 0 | - And the following "mod_h5pactivity > attempt" exists: - | user | student1 | - | h5pactivity | H5P package | - | attempt | 1 | - | interactiontype | compound | - | rawscore | 2 | - | maxscore | 2 | - | duration | 4 | - | completion | 1 | - | success | 1 | - When I am on the "H5P package" "h5pactivity activity" page logged in as student1 - Then "Attempts report" "link" should not exist in current page administration - And I should not see "This content is displayed in preview mode" - - Scenario: Student accessing an activity without tracking - Given the following "activity" exists: - | activity | h5pactivity | - | name | H5P package | - | intro | Test H5P description | - | course | C1 | - | idnumber | h5ppackage | - | enabletracking | 0 | - And the following "mod_h5pactivity > attempt" exists: - | user | student1 | - | h5pactivity | H5P package | - | attempt | 1 | - | interactiontype | compound | - | rawscore | 2 | - | maxscore | 2 | - | duration | 4 | - | completion | 1 | - | success | 1 | - When I am on the "H5P package" "h5pactivity activity" page logged in as student1 - Then "Attempts report" "link" should not exist in current page administration - And I should see "This content is displayed in preview mode" - - Scenario: Teacher accessing an activity with attempt review - Given the following "activity" exists: - | activity | h5pactivity | - | name | H5P package | - | intro | Test H5P description | - | course | C1 | - | idnumber | h5ppackage | - | enabletracking | 1 | - | reviewmode | 1 | - And the following "mod_h5pactivity > attempt" exists: - | user | student1 | - | h5pactivity | H5P package | - | attempt | 1 | - | interactiontype | compound | - | rawscore | 2 | - | maxscore | 2 | - | duration | 4 | - | completion | 1 | - | success | 1 | - When I am on the "H5P package" "h5pactivity activity" page logged in as teacher1 - Then "Attempts report" "link" should exist in current page administration - And I should see "This content is displayed in preview mode" - - Scenario: Teacher accessing an activity without attempt review - Given the following "activity" exists: - | activity | h5pactivity | - | name | H5P package | - | intro | Test H5P description | - | course | C1 | - | idnumber | h5ppackage | - | enabletracking | 1 | - | reviewmode | 0 | - And the following "mod_h5pactivity > attempt" exists: - | user | student1 | - | h5pactivity | H5P package | - | attempt | 1 | - | interactiontype | compound | - | rawscore | 2 | - | maxscore | 2 | - | duration | 4 | - | completion | 1 | - | success | 1 | - When I am on the "H5P package" "h5pactivity activity" page logged in as teacher1 - Then "Attempts report" "link" should exist in current page administration - And I should see "This content is displayed in preview mode" + Examples: + | user | enabletracking | reviewmode | attemptsreportlink | previewmode | attempttracking | attempttrackingsettings | + | student1 | 1 | 1 | exist | not see | not see | not see | + | student1 | 1 | 0 | not exist | not see | not see | not see | + | student1 | 0 | 1 | not exist | not see | not see | not see | + | teacher1 | 1 | 1 | exist | see | not see | not see | + | teacher1 | 1 | 0 | exist | see | not see | not see | + | teacher1 | 0 | 1 | not exist | see | see | see | + | teacher2 | 0 | 1 | not exist | see | see | not see | diff --git a/mod/h5pactivity/tests/behat/duplicate_delete_h5pactivity.feature b/mod/h5pactivity/tests/behat/duplicate_delete_h5pactivity.feature index de55da340f8..43f99d6af93 100644 --- a/mod/h5pactivity/tests/behat/duplicate_delete_h5pactivity.feature +++ b/mod/h5pactivity/tests/behat/duplicate_delete_h5pactivity.feature @@ -21,7 +21,7 @@ Feature: Duplicate and delete a h5pactivity | h5pactivity | C1 | H5P Activity 1 | h5p/tests/fixtures/filltheblanks.h5p | And I am on the "H5P Activity 1" "h5pactivity activity" page logged in as teacher1 # Initial confirmation that no error occurs when viewing h5p activity - And I should see "This content is displayed in preview mode. No attempt tracking will be stored." + And I should see "You are in preview mode." And I am on "Course 1" course homepage with editing mode on # Duplicate the h5p activity When I duplicate "H5P Activity 1" activity @@ -29,7 +29,7 @@ Feature: Duplicate and delete a h5pactivity Then I should see "H5P Activity 1 (copy)" And I am on the "H5P Activity 1 (copy)" "h5pactivity activity" page # Confirm there are no errors when viewing duplicate h5p activity - And I should see "This content is displayed in preview mode. No attempt tracking will be stored." + And I should see "You are in preview mode." And I am on the "Course 1" course page # Delete the duplicate h5p activity And I delete "H5P Activity 1 (copy)" activity @@ -37,4 +37,4 @@ Feature: Duplicate and delete a h5pactivity And I should not see "H5P Activity 1 (copy)" And I am on the "H5P Activity 1" "h5pactivity activity" page # Confirm there are no errors on the original h5p activity after deleting the duplicate - And I should see "This content is displayed in preview mode. No attempt tracking will be stored." + And I should see "You are in preview mode." diff --git a/mod/h5pactivity/tests/behat/sending_attempt.feature b/mod/h5pactivity/tests/behat/sending_attempt.feature index fb745091ea4..fa9620a8085 100644 --- a/mod/h5pactivity/tests/behat/sending_attempt.feature +++ b/mod/h5pactivity/tests/behat/sending_attempt.feature @@ -28,14 +28,12 @@ Feature: Do a H5P attempt Scenario: View an H5P as a teacher When I am on the "Awesome H5P package" "h5pactivity activity" page logged in as teacher1 - And I wait until the page is ready - Then I should see "This content is displayed in preview mode" + Then I should see "You are in preview mode." @javascript Scenario: Do an attempt and check on course log report When I am on the "Awesome H5P package" "h5pactivity activity" page logged in as student1 - And I wait until the page is ready - And I should not see "This content is displayed in preview mode" + And I should not see "You are in preview mode." And I switch to "h5p-player" class iframe And I switch to "h5p-iframe" class iframe And I click on "Correct one" "text" in the ".h5p-question-content" "css_element" @@ -50,8 +48,7 @@ Feature: Do a H5P attempt @javascript Scenario: Do various attempts and check them with the attempts and user grades reports Given I am on the "Awesome H5P package" "h5pactivity activity" page logged in as student1 - And I wait until the page is ready - And I should not see "This content is displayed in preview mode" + And I should not see "You are in preview mode." And I switch to "h5p-player" class iframe And I switch to "h5p-iframe" class iframe And I click on "Wrong one" "text" in the ".h5p-question-content" "css_element" diff --git a/mod/h5pactivity/tests/local/manager_test.php b/mod/h5pactivity/tests/local/manager_test.php index ff73339c788..65055c8dd4a 100644 --- a/mod/h5pactivity/tests/local/manager_test.php +++ b/mod/h5pactivity/tests/local/manager_test.php @@ -61,15 +61,19 @@ class manager_test extends \advanced_testcase { } /** - * Test for is_tracking_enabled. + * Test for is_tracking_enabled and can_submit methods. * + * @covers ::is_tracking_enabled + * @covers ::can_submit * @dataProvider is_tracking_enabled_data * @param bool $login if the user is logged in * @param string $role user role in course * @param int $enabletracking if tracking is enabled - * @param bool $expected expected result + * @param bool $expectedtracking expected result for is_tracking_enabled() + * @param bool $expectedsubmit expected result for can_submit() */ - public function test_is_tracking_enabled(bool $login, string $role, int $enabletracking, bool $expected) { + public function test_is_tracking_enabled_and_can_submit(bool $login, string $role, int $enabletracking, bool $expectedtracking, + bool $expectedsubmit): void { $this->resetAfterTest(); $this->setAdminUser(); @@ -87,39 +91,40 @@ class manager_test extends \advanced_testcase { } $manager = manager::create_from_instance($activity); - $this->assertEquals($expected, $manager->is_tracking_enabled($param)); + $this->assertEquals($expectedtracking, $manager->is_tracking_enabled()); + $this->assertEquals($expectedsubmit, $manager->can_submit($param)); } /** - * Data provider for is_tracking_enabled. + * Data provider for test_is_tracking_enabled_and_can_submit. * * @return array */ public function is_tracking_enabled_data(): array { return [ 'Logged student, tracking enabled' => [ - true, 'student', 1, true + true, 'student', 1, true, true, ], 'Logged student, tracking disabled' => [ - true, 'student', 0, false + true, 'student', 0, false, true, ], 'Logged teacher, tracking enabled' => [ - true, 'editingteacher', 1, false + true, 'editingteacher', 1, true, false, ], 'Logged teacher, tracking disabled' => [ - true, 'editingteacher', 0, false + true, 'editingteacher', 0, false, false, ], 'No logged student, tracking enabled' => [ - true, 'student', 1, true + true, 'student', 1, true, true, ], 'No logged student, tracking disabled' => [ - true, 'student', 0, false + true, 'student', 0, false, true, ], 'No logged teacher, tracking enabled' => [ - true, 'editingteacher', 1, false + true, 'editingteacher', 1, true, false, ], 'No logged teacher, tracking disabled' => [ - true, 'editingteacher', 0, false + true, 'editingteacher', 0, false, false, ], ]; } diff --git a/mod/h5pactivity/view.php b/mod/h5pactivity/view.php index 012fedb6616..31041d598b6 100644 --- a/mod/h5pactivity/view.php +++ b/mod/h5pactivity/view.php @@ -71,9 +71,23 @@ echo $OUTPUT->header(); $instance = $manager->get_instance(); -if (!$manager->is_tracking_enabled()) { +// Only users without permission to submit can see the warning messages. +if (!$manager->can_submit()) { + // Show preview mode message. $message = get_string('previewmode', 'mod_h5pactivity'); - echo $OUTPUT->notification($message, \core\output\notification::NOTIFY_WARNING); + echo $OUTPUT->notification($message, \core\output\notification::NOTIFY_INFO, false); + + // If tracking is disabled, show a warning. + if (!$manager->is_tracking_enabled()) { + if (has_capability('moodle/course:manageactivities', $context)) { + $url = new moodle_url('/course/modedit.php', ['update' => $cm->id]); + $message = get_string('trackingdisabled_enable', 'mod_h5pactivity', $url->out()); + } else { + $message = get_string('trackingdisabled', 'mod_h5pactivity'); + } + echo $OUTPUT->notification($message, \core\output\notification::NOTIFY_WARNING); + } + } echo player::display($fileurl, $config, true, 'mod_h5pactivity', true);