From e1fb8e4aec22ac1dd9ff5d8154a6b64396d237cb Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Fri, 27 Sep 2019 13:22:52 +0800 Subject: [PATCH 1/3] MDL-64745 block_online_users: Option to disable users to set visibility --- blocks/online_users/block_online_users.php | 16 +++++----- blocks/online_users/classes/fetcher.php | 31 ++++++++++--------- .../lang/en/block_online_users.php | 2 ++ blocks/online_users/settings.php | 4 +++ blocks/online_users/version.php | 2 +- 5 files changed, 32 insertions(+), 23 deletions(-) diff --git a/blocks/online_users/block_online_users.php b/blocks/online_users/block_online_users.php index 01ada957dad..9d1fc90b7f9 100644 --- a/blocks/online_users/block_online_users.php +++ b/blocks/online_users/block_online_users.php @@ -129,14 +129,16 @@ class block_online_users extends block_base { $this->content->text .= $OUTPUT->user_picture($user, array('size'=>16, 'alttext'=>false, 'link'=>false)) .$user->fullname.''; if ($USER->id == $user->id) { - $action = ($user->uservisibility != null && $user->uservisibility == 0) ? 'show' : 'hide'; - $anchortagcontents = $OUTPUT->pix_icon('t/' . $action, - get_string('online_status:' . $action, 'block_online_users')); - $anchortag = html_writer::link("", $anchortagcontents, - array('title' => get_string('online_status:' . $action, 'block_online_users'), - 'data-action' => $action, 'data-userid' => $user->id, 'id' => 'change-user-visibility')); + if ($CFG->block_online_users_onlinestatushiding) { + $action = ($user->uservisibility != null && $user->uservisibility == 0) ? 'show' : 'hide'; + $anchortagcontents = $OUTPUT->pix_icon('t/' . $action, + get_string('online_status:' . $action, 'block_online_users')); + $anchortag = html_writer::link("", $anchortagcontents, + array('title' => get_string('online_status:' . $action, 'block_online_users'), + 'data-action' => $action, 'data-userid' => $user->id, 'id' => 'change-user-visibility')); - $this->content->text .= '
' . $anchortag . '
'; + $this->content->text .= '
' . $anchortag . '
'; + } } else { if ($canshowicon) { // Only when logged in and messaging active etc. $anchortagcontents = $OUTPUT->pix_icon('t/message', get_string('messageselectadd')); diff --git a/blocks/online_users/classes/fetcher.php b/blocks/online_users/classes/fetcher.php index 20f706f97e2..a18e625caaf 100644 --- a/blocks/online_users/classes/fetcher.php +++ b/blocks/online_users/classes/fetcher.php @@ -67,7 +67,7 @@ class fetcher { * @param int $courseid The course id to check */ protected function set_sql($currentgroup, $now, $timetoshowusers, $context, $sitelevel, $courseid) { - global $USER, $DB; + global $USER, $DB, $CFG; $timefrom = 100 * floor(($now - $timetoshowusers) / 100); // Round to nearest 100 seconds for better query cache. @@ -76,7 +76,14 @@ class fetcher { $groupby = ""; $lastaccess = ", lastaccess"; $timeaccess = ", ul.timeaccess AS lastaccess"; - $uservisibility = ", up.value AS uservisibility"; + $uservisibility = ""; + $uservisibilityselect = ""; + if ($CFG->block_online_users_onlinestatushiding) { + $uservisibility = ", up.value AS uservisibility"; + $uservisibilityselect = "AND (" . $DB->sql_cast_char2int('up.value') . " = 1 + OR up.value IS NULL + OR u.id = :userid)"; + } $params = array(); $userfields = \user_picture::fields('u', array('username')); @@ -88,7 +95,9 @@ class fetcher { $groupby = "GROUP BY $userfields"; $lastaccess = ", MAX(u.lastaccess) AS lastaccess"; $timeaccess = ", MAX(ul.timeaccess) AS lastaccess"; - $uservisibility = ", MAX(up.value) AS uservisibility"; + if ($CFG->block_online_users_onlinestatushiding) { + $uservisibility = ", MAX(up.value) AS uservisibility"; + } $params['currentgroup'] = $currentgroup; } @@ -105,9 +114,7 @@ class fetcher { WHERE u.lastaccess > :timefrom AND u.lastaccess <= :now AND u.deleted = 0 - AND (" . $DB->sql_cast_char2int('up.value') . " = 1 - OR up.value IS NULL - OR u.id = :userid) + $uservisibilityselect $groupselect $groupby ORDER BY lastaccess DESC "; @@ -118,9 +125,7 @@ class fetcher { WHERE u.lastaccess > :timefrom AND u.lastaccess <= :now AND u.deleted = 0 - AND (" . $DB->sql_cast_char2int('up.value') . " = 1 - OR up.value IS NULL - OR u.id = :userid) + $uservisibilityselect $groupselect"; } else { // Course level - show only enrolled users for now. @@ -138,9 +143,7 @@ class fetcher { AND ul.courseid = :courseid AND ul.timeaccess <= :now AND u.deleted = 0 - AND (" . $DB->sql_cast_char2int('up.value') . " = 1 - OR up.value IS NULL - OR u.id = :userid) + $uservisibilityselect $groupselect $groupby ORDER BY lastaccess DESC"; @@ -154,9 +157,7 @@ class fetcher { AND ul.courseid = :courseid AND ul.timeaccess <= :now AND u.deleted = 0 - AND (" . $DB->sql_cast_char2int('up.value') . " = 1 - OR up.value IS NULL - OR u.id = :userid) + $uservisibilityselect $groupselect"; $params['courseid'] = $courseid; diff --git a/blocks/online_users/lang/en/block_online_users.php b/blocks/online_users/lang/en/block_online_users.php index c6bc0223f1a..f34f5d68583 100644 --- a/blocks/online_users/lang/en/block_online_users.php +++ b/blocks/online_users/lang/en/block_online_users.php @@ -23,7 +23,9 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +$string['onlinestatushiding_desc'] = 'If enabled, users have the option to hide their online status from other users.'; $string['configtimetosee'] = 'Number of minutes determining the period of inactivity after which a user is no longer considered to be online.'; +$string['onlinestatushiding'] = 'Online status hiding'; $string['nouser'] = 'No online users'; $string['numuser'] = '{$a} online user'; $string['numusers'] = '{$a} online users'; diff --git a/blocks/online_users/settings.php b/blocks/online_users/settings.php index b9d7d81524d..39d3bf0dee7 100644 --- a/blocks/online_users/settings.php +++ b/blocks/online_users/settings.php @@ -27,5 +27,9 @@ defined('MOODLE_INTERNAL') || die; if ($ADMIN->fulltree) { $settings->add(new admin_setting_configtext('block_online_users_timetosee', get_string('timetosee', 'block_online_users'), get_string('configtimetosee', 'block_online_users'), 5, PARAM_INT)); + + $settings->add(new admin_setting_configcheckbox('block_online_users_onlinestatushiding', + get_string('onlinestatushiding', 'block_online_users'), + get_string('onlinestatushiding_desc', 'block_online_users'), 1)); } diff --git a/blocks/online_users/version.php b/blocks/online_users/version.php index eaf8a55d39b..c5806d31e88 100644 --- a/blocks/online_users/version.php +++ b/blocks/online_users/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2019052000; // The current plugin version (Date: YYYYMMDDXX) +$plugin->version = 2019052001; // The current plugin version (Date: YYYYMMDDXX) $plugin->requires = 2019051100; // Requires this Moodle version $plugin->component = 'block_online_users'; // Full name of the plugin (used for diagnostics) From e94de2d6d3e9dde34eeabab4b1a6494618680db3 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Fri, 27 Sep 2019 13:23:28 +0800 Subject: [PATCH 2/3] MDL-64745 block_online_users: Add unit tests --- .../online_users/tests/online_users_test.php | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/blocks/online_users/tests/online_users_test.php b/blocks/online_users/tests/online_users_test.php index 946e456fbed..da436ac5a4a 100644 --- a/blocks/online_users/tests/online_users_test.php +++ b/blocks/online_users/tests/online_users_test.php @@ -155,6 +155,8 @@ class block_online_users_testcase extends advanced_testcase { public function test_user_visibility_course1_group1_members() { global $CFG; + // Enable users to set their visibility to others in the online users block. + $CFG->block_online_users_onlinestatushiding = true; $groupid = $this->data['group1']->id; $now = time(); $timetoshowusers = $CFG->block_online_users_timetosee * 60; @@ -190,6 +192,18 @@ class block_online_users_testcase extends advanced_testcase { // User1 should not be displayed in the online users block. $this->assertEquals(2, $usercount); $this->assertFalse(array_key_exists($user1->id, $users)); + + // Disable users to set their visibility to others in the online users block. + // All users should be displayed now and the visibility status of a users should be ignored, + // as the capability of setting the visibility to other user has been disabled. + $CFG->block_online_users_onlinestatushiding = false; + // Test if the fetcher gets all the users including user1. + $onlineusers = new fetcher($groupid, $now, $timetoshowusers, $context, false, $courseid); + $users = $onlineusers->get_users(); + $usercount = $onlineusers->count_users(); + // User1 should be displayed in the online users block. + $this->assertEquals(3, $usercount); + $this->assertTrue(array_key_exists($user1->id, $users)); } /** @@ -234,6 +248,18 @@ class block_online_users_testcase extends advanced_testcase { // User1 should not be displayed in the online users block. $this->assertEquals(8, $usercount); $this->assertFalse(array_key_exists($user1->id, $users)); + + // Disable users to set their visibility to others in the online users block. + // All users should be displayed now and the visibility status of a users should be ignored, + // as the capability of setting the visibility to other user has been disabled. + $CFG->block_online_users_onlinestatushiding = false; + // Test if the fetcher gets all the users including user1. + $onlineusers = new fetcher($currentgroup, $now, $timetoshowusers, $context, false, $courseid); + $users = $onlineusers->get_users(); + $usercount = $onlineusers->count_users(); + // User1 should be displayed in the online users block. + $this->assertEquals(9, $usercount); + $this->assertTrue(array_key_exists($user1->id, $users)); } /** @@ -277,5 +303,17 @@ class block_online_users_testcase extends advanced_testcase { // User1 should not be displayed in the online users block. $this->assertEquals(11, $usercount); $this->assertFalse(array_key_exists($user1->id, $users)); + + // Disable users to set their visibility to others in the online users block. + // All users should be displayed now and the visibility status of a users should be ignored, + // as the capability of setting the visibility to other user has been disabled. + $CFG->block_online_users_onlinestatushiding = false; + // Test if the fetcher gets all the users including user1. + $onlineusers = new fetcher($currentgroup, $now, $timetoshowusers, $context, true); + $users = $onlineusers->get_users(); + $usercount = $onlineusers->count_users(); + // User1 should be displayed in the online users block. + $this->assertEquals(12, $usercount); + $this->assertTrue(array_key_exists($user1->id, $users)); } } From 0a9150c009fc1de609dd37e1460856b08474075c Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Fri, 27 Sep 2019 13:23:52 +0800 Subject: [PATCH 3/3] MDL-64745 block_online_users: Add behat tests --- .../behat/block_online_users_course.feature | 46 ++++++++++++++- .../block_online_users_dashboard.feature | 45 ++++++++++++++- .../block_online_users_frontpage.feature | 56 ++++++++++++++++++- 3 files changed, 144 insertions(+), 3 deletions(-) diff --git a/blocks/online_users/tests/behat/block_online_users_course.feature b/blocks/online_users/tests/behat/block_online_users_course.feature index 20b0d950f1e..097cee10c43 100644 --- a/blocks/online_users/tests/behat/block_online_users_course.feature +++ b/blocks/online_users/tests/behat/block_online_users_course.feature @@ -42,7 +42,9 @@ Feature: The online users block allow you to see who is currently online @javascript Scenario: Hide/show user's online status from/to other users in the online users block on course page - Given I log in as "teacher1" + Given the following config values are set as admin: + | block_online_users_onlinestatushiding | 1 | + And I log in as "teacher1" And I am on "Course 1" course homepage with editing mode on And I add the "Online users" block And I log out @@ -71,3 +73,45 @@ Feature: The online users block allow you to see who is currently online Then I should see "2 online users" in the "Online users" "block" And I should see "Teacher 1" in the "Online users" "block" And I should see "Student 1" in the "Online users" "block" + + @javascript + Scenario: Hide/show icon is not visible in the online users block on course page when the setting is disabled + Given the following config values are set as admin: + | block_online_users_onlinestatushiding | 1 | + And I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I add the "Online users" block + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And "Hide" "icon" should exist in the ".block.block_online_users" "css_element" + And I log out + And the following config values are set as admin: + | block_online_users_onlinestatushiding | 0 | + When I log in as "student1" + Then I should see "Student 1" in the "Online users" "block" + And "Hide" "icon" should not exist in the ".block.block_online_users" "css_element" + + @javascript + Scenario: User is displayed in the online users block on course page when visibility setting is disabled, + ignoring the previously set visibility state + Given the following config values are set as admin: + | block_online_users_onlinestatushiding | 1 | + And I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I add the "Online users" block + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And "Hide" "icon" should exist in the "#change-user-visibility" "css_element" + And I click on "#change-user-visibility" "css_element" + And I wait "1" seconds + And "Show" "icon" should exist in the "#change-user-visibility" "css_element" + And I log out + And the following config values are set as admin: + | block_online_users_onlinestatushiding | 0 | + And I log in as "teacher1" + When I am on "Course 1" course homepage + Then I should see "2 online users" in the "Online users" "block" + And I should see "Teacher 1" in the "Online users" "block" + And I should see "Student 1" in the "Online users" "block" diff --git a/blocks/online_users/tests/behat/block_online_users_dashboard.feature b/blocks/online_users/tests/behat/block_online_users_dashboard.feature index 0d01207a605..2d32fc61caf 100644 --- a/blocks/online_users/tests/behat/block_online_users_dashboard.feature +++ b/blocks/online_users/tests/behat/block_online_users_dashboard.feature @@ -29,7 +29,9 @@ Feature: The online users block allow you to see who is currently online on dash @javascript Scenario: Hide/show user's online status from/to other users in the online users block on dashboard - Given I log in as "student1" + Given the following config values are set as admin: + | block_online_users_onlinestatushiding | 1 | + And I log in as "student1" And I should see "1 online user" in the "Online users" "block" And I should see "Student 1" in the "Online users" "block" And "Hide" "icon" should exist in the "#change-user-visibility" "css_element" @@ -52,3 +54,44 @@ Feature: The online users block allow you to see who is currently online on dash Then I should see "2 online users" in the "Online users" "block" And I should see "Student 2" in the "Online users" "block" And I should see "Student 1" in the "Online users" "block" + + @javascript + Scenario: Hide/show icon is not visible in the online users block when the setting is disabled + Given the following config values are set as admin: + | block_online_users_onlinestatushiding | 1 | + And I log in as "student1" + And I should see "1 online user" in the "Online users" "block" + And I should see "Student 1" in the "Online users" "block" + And "Hide" "icon" should exist in the ".block.block_online_users" "css_element" + And I log out + And the following config values are set as admin: + | block_online_users_onlinestatushiding | 0 | + When I log in as "student1" + Then I should see "1 online user" in the "Online users" "block" + And I should see "Student 1" in the "Online users" "block" + And "Hide" "icon" should not exist in the ".block.block_online_users" "css_element" + + @javascript + Scenario: User is displayed in the online users block when visibility setting is disabled, + ignoring the previously set visibility state + Given the following config values are set as admin: + | block_online_users_onlinestatushiding | 1 | + And I log in as "student1" + And I should see "1 online user" in the "Online users" "block" + And I should see "Student 1" in the "Online users" "block" + And "Hide" "icon" should exist in the "#change-user-visibility" "css_element" + And I click on "#change-user-visibility" "css_element" + And I wait "1" seconds + And "Show" "icon" should exist in the "#change-user-visibility" "css_element" + And I log out + And I log in as "student2" + And I should see "1 online user" in the "Online users" "block" + And I should see "Student 2" in the "Online users" "block" + And I should not see "Student 1" in the "Online users" "block" + And I log out + And the following config values are set as admin: + | block_online_users_onlinestatushiding | 0 | + When I log in as "student2" + Then I should see "2 online users" in the "Online users" "block" + And I should see "Student 2" in the "Online users" "block" + And I should see "Student 1" in the "Online users" "block" diff --git a/blocks/online_users/tests/behat/block_online_users_frontpage.feature b/blocks/online_users/tests/behat/block_online_users_frontpage.feature index 097c9e7f49a..ca817e8142d 100644 --- a/blocks/online_users/tests/behat/block_online_users_frontpage.feature +++ b/blocks/online_users/tests/behat/block_online_users_frontpage.feature @@ -52,7 +52,9 @@ Feature: The online users block allow you to see who is currently online on fron @javascript Scenario: Hide/show user's online status from/to other users in the online users block on front page - Given I log in as "admin" + Given the following config values are set as admin: + | block_online_users_onlinestatushiding | 1 | + And I log in as "admin" And I am on site homepage And I navigate to "Turn editing on" in current page administration And I add the "Online users" block @@ -84,3 +86,55 @@ Feature: The online users block allow you to see who is currently online on fron And I should see "Admin" in the "Online users" "block" And I should see "Student 2" in the "Online users" "block" And I should see "Student 1" in the "Online users" "block" + + @javascript + Scenario: Hide/show icon is not visible in the online users block on front page when the setting is disabled + Given the following config values are set as admin: + | block_online_users_onlinestatushiding | 1 | + And I log in as "admin" + And I am on site homepage + And I navigate to "Turn editing on" in current page administration + And I add the "Online users" block + And I log out + And I log in as "student1" + And I am on site homepage + And "Hide" "icon" should exist in the ".block.block_online_users" "css_element" + And I log out + And the following config values are set as admin: + | block_online_users_onlinestatushiding | 0 | + When I log in as "student1" + Then I should see "Student 1" in the "Online users" "block" + And "Hide" "icon" should not exist in the ".block.block_online_users" "css_element" + + @javascript + Scenario: User is displayed in the online users block on front page when visibility setting is disabled, + ignoring the previously set visibility state + Given the following config values are set as admin: + | block_online_users_onlinestatushiding | 1 | + And I log in as "admin" + And I am on site homepage + And I navigate to "Turn editing on" in current page administration + And I add the "Online users" block + And I log out + And I log in as "student1" + And I am on site homepage + And "Hide" "icon" should exist in the "#change-user-visibility" "css_element" + And I click on "#change-user-visibility" "css_element" + And I wait "1" seconds + And "Show" "icon" should exist in the "#change-user-visibility" "css_element" + And I log out + And I log in as "student2" + And I am on site homepage + And I should see "2 online user" in the "Online users" "block" + And I should see "Admin" in the "Online users" "block" + And I should see "Student 2" in the "Online users" "block" + And I should not see "Student 1" in the "Online users" "block" + And I log out + And the following config values are set as admin: + | block_online_users_onlinestatushiding | 0 | + And I log in as "student2" + When I am on site homepage + Then I should see "3 online users" in the "Online users" "block" + And I should see "Admin" in the "Online users" "block" + And I should see "Student 2" in the "Online users" "block" + And I should see "Student 1" in the "Online users" "block"