From 4cc43cd8a940a232dad34457bf1ed05f367a8385 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 16 Nov 2014 00:13:32 +0800 Subject: [PATCH] MDL-48221 mod_forum: Improve discussion subscription on post If the user has posted in the forum before, follow their previous discussion preference. If the user has not posted before, but is subsscribed to the forum, follow their forum preference. Otherwise, follow their user-preference. This has the result that if a user has unsubscribed from a forum and writes their first response; the user preference will be followed. This may override the forum subscription. This commit also changes from a dropdown to a checkbox in both the post form, and the user profile. --- admin/settings/users.php | 8 +- admin/tool/uploaduser/user_form.php | 2 +- lang/en/moodle.php | 5 +- mod/forum/classes/post_form.php | 23 ++-- mod/forum/lib.php | 2 +- mod/forum/post.php | 23 +++- .../behat/forum_subscriptions_default.feature | 135 ++++++++++++++++++++ user/editlib.php | 6 +- user/lib.php | 7 +- 9 files changed, 177 insertions(+), 34 deletions(-) create mode 100644 mod/forum/tests/behat/forum_subscriptions_default.feature diff --git a/admin/settings/users.php b/admin/settings/users.php index f8d5b74..054dd0e 100644 --- a/admin/settings/users.php +++ b/admin/settings/users.php @@ -47,12 +47,8 @@ if ($hassiteconfig $temp->add(new admin_setting_configselect('defaultpreference_maildigest', new lang_string('emaildigest'), new lang_string('emaildigest_help'), 0, $choices)); - - $choices = array(); - $choices['1'] = new lang_string('autosubscribeyes'); - $choices['0'] = new lang_string('autosubscribeno'); - $temp->add(new admin_setting_configselect('defaultpreference_autosubscribe', new lang_string('autosubscribe'), - '', 1, $choices)); + $temp->add(new admin_setting_configcheckbox('defaultpreference_autosubscribe', + new lang_string('autosubscribediscussion'), get_string('autosubscribediscussion_help'), true)); $choices = array(); $choices['0'] = new lang_string('trackforumsno'); diff --git a/admin/tool/uploaduser/user_form.php b/admin/tool/uploaduser/user_form.php index 1ec16e0..19065d1 100644 --- a/admin/tool/uploaduser/user_form.php +++ b/admin/tool/uploaduser/user_form.php @@ -234,7 +234,7 @@ class admin_uploaduser_form2 extends moodleform { $mform->setAdvanced('maildigest'); $choices = array(1 => get_string('autosubscribeyes'), 0 => get_string('autosubscribeno')); - $mform->addElement('select', 'autosubscribe', get_string('autosubscribe'), $choices); + $mform->addElement('select', 'autosubscribe', get_string('autosubscribediscussion'), $choices); $mform->setDefault('autosubscribe', $CFG->defaultpreference_autosubscribe); $mform->addElement('text', 'city', get_string('city'), 'maxlength="120" size="25"'); diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 1974d59..7ff2843 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -159,9 +159,8 @@ $string['authenticateduser'] = 'Authenticated user'; $string['authenticateduserdescription'] = 'All logged in users.'; $string['authentication'] = 'Authentication'; $string['authenticationplugins'] = 'Authentication plugins'; -$string['autosubscribe'] = 'Forum auto-subscribe'; -$string['autosubscribeno'] = 'No: don\'t automatically subscribe me to forum discussions'; -$string['autosubscribeyes'] = 'Yes: when I post, subscribe me to that forum discussion'; +$string['autosubscribediscussion'] = 'Forum discussion subscription'; +$string['autosubscribediscussion_help'] = 'When you make your first post to a forum that you are not subscribed to, do you wish to subscribe to the discussion you are posting in?'; $string['availability'] = 'Availability'; $string['availablecourses'] = 'Available courses'; $string['back'] = 'Back'; diff --git a/mod/forum/classes/post_form.php b/mod/forum/classes/post_form.php index 01a10bf..5c7eab6 100644 --- a/mod/forum/classes/post_form.php +++ b/mod/forum/classes/post_form.php @@ -117,24 +117,19 @@ class mod_forum_post_form extends moodleform { $manageactivities = has_capability('moodle/course:manageactivities', $coursecontext); if (\mod_forum\subscriptions::is_forcesubscribed($forum)) { - - $mform->addElement('static', 'subscribemessage', get_string('subscription', 'forum'), get_string('everyoneissubscribed', 'forum')); - $mform->addElement('hidden', 'subscribe'); - $mform->setType('subscribe', PARAM_INT); - $mform->addHelpButton('subscribemessage', 'forcesubscribed', 'forum'); + $mform->addElement('checkbox', 'discussionsubscribe', get_string('discussionsubscription', 'forum')); + $mform->freeze('discussionsubscribe'); + $mform->setDefaults('discussionsubscribe', 0); + $mform->addHelpButton('discussionsubscribe', 'forcesubscribed', 'forum'); } else if (\mod_forum\subscriptions::subscription_disabled($forum) && !$manageactivities) { - $mform->addElement('static', 'subscribemessage', get_string('subscription', 'forum'), get_string('disallowsubscribe', 'forum')); - $mform->addElement('hidden', 'discussionsubscribe'); - $mform->setType('discussionsubscribe', PARAM_INT); - $mform->addHelpButton('subscribemessage', 'disallowsubscription', 'forum'); + $mform->addElement('checkbox', 'discussionsubscribe', get_string('discussionsubscription', 'forum')); + $mform->freeze('discussionsubscribe'); + $mform->setDefaults('discussionsubscribe', 0); + $mform->addHelpButton('discussionsubscribe', 'disallowsubscription', 'forum'); } else { - $options = array(); - $options[0] = get_string('discussionsubscribestop', 'forum'); - $options[1] = get_string('discussionsubscribestart', 'forum'); - - $mform->addElement('select', 'discussionsubscribe', get_string('discussionsubscription', 'forum'), $options); + $mform->addElement('checkbox', 'discussionsubscribe', get_string('discussionsubscription', 'forum')); $mform->addHelpButton('discussionsubscribe', 'discussionsubscription', 'forum'); } diff --git a/mod/forum/lib.php b/mod/forum/lib.php index fc81e87..bcf5782 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -4719,7 +4719,7 @@ function forum_post_subscription($fromform, $forum, $discussion) { $info->discussion = format_string($discussion->name); $info->forum = format_string($forum->name); - if ($fromform->discussionsubscribe) { + if (isset($fromform->discussionsubscribe) && $fromform->discussionsubscribe) { if ($result = \mod_forum\subscriptions::subscribe_user_to_discussion($USER->id, $discussion)) { return html_writer::tag('p', get_string('discussionnowsubscribed', 'forum', $info)); } diff --git a/mod/forum/post.php b/mod/forum/post.php index 3464d6b..2903fe3 100644 --- a/mod/forum/post.php +++ b/mod/forum/post.php @@ -616,11 +616,26 @@ $postid = empty($post->id) ? null : $post->id; $draftid_editor = file_get_submitted_draft_itemid('message'); $currenttext = file_prepare_draft_area($draftid_editor, $modcontext->id, 'mod_forum', 'post', $postid, mod_forum_post_form::editor_options($modcontext, $postid), $post->message); -// Respect the user's discussion autosubscribe preference unless they have already posted - in which case, use that preference. -$discussionsubscribe = $USER->autosubscribe; -if (isset($discussion) && forum_user_has_posted($forum->id, $discussion->id, $USER->id)) { - $discussionsubscribe = \mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id, $cm); +$manageactivities = has_capability('moodle/course:manageactivities', $coursecontext); +if (\mod_forum\subscriptions::subscription_disabled($forum) && !$manageactivities) { + // User does not have permission to subscribe to this discussion at all. + $discussionsubscribe = false; +} else if (\mod_forum\subscriptions::is_forcesubscribed($forum)) { + // User does not have permission to unsubscribe from this discussion at all. + $discussionsubscribe = true; +} else { + if (isset($discussion) && \mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id, $cm)) { + // User is subscribed to the discussion - continue the subscription. + $discussionsubscribe = true; + } else if (!isset($discussion) && \mod_forum\subscriptions::is_subscribed($USER->id, $forum, null, $cm)) { + // Starting a new discussion, and the user is subscribed to the forum - subscribe to the discussion. + $discussionsubscribe = true; + } else { + // User is not subscribed to either forum or discussion. Follow user preference. + $discussionsubscribe = $USER->autosubscribe; + } } + $mform_post->set_data(array( 'attachments'=>$draftitemid, 'general'=>$heading, 'subject'=>$post->subject, diff --git a/mod/forum/tests/behat/forum_subscriptions_default.feature b/mod/forum/tests/behat/forum_subscriptions_default.feature new file mode 100644 index 0000000..00334ea --- /dev/null +++ b/mod/forum/tests/behat/forum_subscriptions_default.feature @@ -0,0 +1,135 @@ +@mod @mod_forum +Feature: A user can control their default discussion subscription settings + In order to automatically subscribe to discussions + As a user + I can choose my default subscription preference + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | autosubscribe | + | student1 | Student | One | student.one@example.com | 1 | + | student2 | Student | Two | student.one@example.com | 0 | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | student1 | C1 | student | + | student2 | C1 | student | + And I log in as "admin" + And I follow "Course 1" + And I turn editing mode on + + Scenario: Creating a new discussion in an optional forum follows user preferences + Given I add a "Forum" to section "1" and I fill the form with: + | Forum name | Test forum name | + | Forum type | Standard forum for general use | + | Description | Test forum description | + | Subscription mode | Optional subscription | + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test forum name" + When I press "Add a new discussion topic" + Then "input[name=discussionsubscribe][checked=checked]" "css_element" should exist + And I log out + And I log in as "student2" + And I follow "Course 1" + And I follow "Test forum name" + And I press "Add a new discussion topic" + And "input[name=discussionsubscribe]:not([checked=checked])" "css_element" should exist + + Scenario: Replying to an existing discussion in an optional forum follows user preferences + Given I add a "Forum" to section "1" and I fill the form with: + | Forum name | Test forum name | + | Forum type | Standard forum for general use | + | Description | Test forum description | + | Subscription mode | Optional subscription | + And I add a new discussion to "Test forum name" forum with: + | Subject | Test post subject | + | Message | Test post message | + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test forum name" + And I follow "Test post subject" + When I follow "Reply" + Then "input[name=discussionsubscribe][checked=checked]" "css_element" should exist + And I log out + And I log in as "student2" + And I follow "Course 1" + And I follow "Test forum name" + And I follow "Test post subject" + And I follow "Reply" + And "input[name=discussionsubscribe]:not([checked=checked])" "css_element" should exist + + Scenario: Creating a new discussion in an automatic forum follows forum subscription + Given I add a "Forum" to section "1" and I fill the form with: + | Forum name | Test forum name | + | Forum type | Standard forum for general use | + | Description | Test forum description | + | Subscription mode | Auto subscription | + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test forum name" + When I press "Add a new discussion topic" + Then "input[name=discussionsubscribe][checked=checked]" "css_element" should exist + And I log out + And I log in as "student2" + And I follow "Course 1" + And I follow "Test forum name" + And I press "Add a new discussion topic" + And "input[name=discussionsubscribe][checked=checked]" "css_element" should exist + + Scenario: Replying to an existing discussion in an automatic forum follows forum subscription + Given I add a "Forum" to section "1" and I fill the form with: + | Forum name | Test forum name | + | Forum type | Standard forum for general use | + | Description | Test forum description | + | Subscription mode | Optional subscription | + And I add a new discussion to "Test forum name" forum with: + | Subject | Test post subject | + | Message | Test post message | + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test forum name" + And I follow "Test post subject" + When I follow "Reply" + Then "input[name=discussionsubscribe][checked=checked]" "css_element" should exist + And I log out + And I log in as "student2" + And I follow "Course 1" + And I follow "Test forum name" + And I follow "Test post subject" + And I follow "Reply" + And "input[name=discussionsubscribe]:not([checked=checked])" "css_element" should exist + + Scenario: Replying to an existing discussion in an automatic forum which has been unsubscribed from follows user preferences + Given I add a "Forum" to section "1" and I fill the form with: + | Forum name | Test forum name | + | Forum type | Standard forum for general use | + | Description | Test forum description | + | Subscription mode | Auto subscription | + And I add a new discussion to "Test forum name" forum with: + | Subject | Test post subject | + | Message | Test post message | + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test forum name" + And I click on "You are subscribed to this discussion. Click to unsubscribe." "link" in the "Test post subject" "table_row" + And I follow "Continue" + And I follow "Test post subject" + When I follow "Reply" + And "input[name=discussionsubscribe][checked=checked]" "css_element" should exist + And I log out + And I log in as "student2" + And I follow "Course 1" + And I follow "Test forum name" + And I click on "You are subscribed to this discussion. Click to unsubscribe." "link" in the "Test post subject" "table_row" + And I follow "Continue" + And I follow "Test post subject" + And I follow "Reply" + And "input[name=discussionsubscribe]:not([checked=checked])" "css_element" should exist diff --git a/user/editlib.php b/user/editlib.php index 62c114c..9e8db92 100644 --- a/user/editlib.php +++ b/user/editlib.php @@ -387,11 +387,9 @@ function useredit_shared_definition_preferences($user, &$mform, $editoroptions = $mform->setDefault('maildigest', $CFG->defaultpreference_maildigest); $mform->addHelpButton('maildigest', 'emaildigest'); - $choices = array(); - $choices['1'] = get_string('autosubscribeyes'); - $choices['0'] = get_string('autosubscribeno'); - $mform->addElement('select', 'autosubscribe', get_string('autosubscribe'), $choices); + $mform->addElement('checkbox', 'autosubscribe', get_string('autosubscribediscussion')); $mform->setDefault('autosubscribe', $CFG->defaultpreference_autosubscribe); + $mform->addHelpButton('autosubscribe', 'autosubscribediscussion'); if (!empty($CFG->forum_trackreadposts)) { $choices = array(); diff --git a/user/lib.php b/user/lib.php index 5fac196..8bc8e53 100644 --- a/user/lib.php +++ b/user/lib.php @@ -166,6 +166,11 @@ function user_update_user($user, $updatepassword = true, $triggerevent = true) { unset($user->calendartype); } + // Unset the autosubscribe here if not specified. + if (empty($user->autosubscribe)) { + $user->autosubscribe = false; + } + $user->timemodified = time(); $DB->update_record('user', $user); @@ -878,4 +883,4 @@ function user_get_user_navigation_info($user, $page) { } return $returnobject; -} \ No newline at end of file +} -- 1.7.10.4