Merge branch 'MDL-47922-master' of git://github.com/ankitagarwal/moodle
authorDan Poltawski <dan@moodle.com>
Thu, 6 Nov 2014 08:53:55 +0000 (08:53 +0000)
committerDan Poltawski <dan@moodle.com>
Thu, 6 Nov 2014 08:53:55 +0000 (08:53 +0000)
16 files changed:
enrol/ldap/lib.php
grade/report/singleview/classes/local/screen/screen.php
grade/report/singleview/classes/local/screen/select.php
grade/report/singleview/classes/local/ui/bulk_insert.php
grade/report/singleview/lang/en/gradereport_singleview.php
grade/report/singleview/styles.css
grade/report/singleview/tests/behat/singleview.feature
lib/dml/moodle_database.php
lib/ldaplib.php
lib/tests/event_test.php
lib/tests/ldaplib_test.php [new file with mode: 0644]
lib/tests/messagelib_test.php
mod/forum/discuss.php
mod/forum/lang/en/forum.php
mod/forum/lib.php
mod/forum/subscribe_ajax.php

index ba2e4c8..ae1a9e4 100644 (file)
@@ -724,6 +724,7 @@ class enrol_ldap_plugin extends enrol_plugin {
             $usergroups = $this->ldap_find_user_groups($extmemberuid);
             if(count($usergroups) > 0) {
                 foreach ($usergroups as $group) {
+                    $group = ldap_filter_addslashes($group);
                     $ldap_search_pattern .= '('.$this->get_config('memberattribute_role'.$role->id).'='.$group.')';
                 }
             }
index c8b46e0..0550af3 100644 (file)
@@ -157,12 +157,25 @@ abstract class screen {
     public function make_toggle($key) {
         $attrs = array('href' => '#');
 
+        // Do proper lang strings for title attributes exist for the given key?
+        $strmanager = \get_string_manager();
+        $titleall = get_string('all');
+        $titlenone = get_string('none');
+        if ($strmanager->string_exists(strtolower($key) . 'all', 'gradereport_singleview')) {
+            $titleall = get_string(strtolower($key) . 'all', 'gradereport_singleview');
+        }
+        if ($strmanager->string_exists(strtolower($key) . 'none', 'gradereport_singleview')) {
+            $titlenone = get_string(strtolower($key) . 'none', 'gradereport_singleview');
+        }
+
         $all = html_writer::tag('a', get_string('all'), $attrs + array(
-            'class' => 'include all ' . $key
+            'class' => 'include all ' . $key,
+            'title' => $titleall
         ));
 
         $none = html_writer::tag('a', get_string('none'), $attrs + array(
-            'class' => 'include none ' . $key
+            'class' => 'include none ' . $key,
+            'title' => $titlenone
         ));
 
         return html_writer::tag('span', "$all / $none", array(
index 4554043..0719fd0 100644 (file)
@@ -99,9 +99,10 @@ class select extends screen {
             );
 
             $url = new moodle_url('/grade/report/singleview/index.php', $params);
-            $html .= $OUTPUT->heading($screen->description());
 
-            $html .= $OUTPUT->single_select($url, 'itemid', $options);
+            $select = new \single_select($url, 'itemid', $options);
+            $select->set_label($screen->description());
+            $html .= $OUTPUT->render($select);
         }
 
         if (empty($html)) {
index 57d45c4..250f654 100644 (file)
@@ -85,22 +85,56 @@ class bulk_insert extends element {
      * @return string HTML
      */
     public function html() {
-        $insertgrade = get_string('bulkinsertgrade', 'gradereport_singleview');
+        $insertvalue = get_string('bulkinsertvalue', 'gradereport_singleview');
         $insertappliesto = get_string('bulkappliesto', 'gradereport_singleview');
 
-        $apply = html_writer::checkbox($this->applyname, 1, false, $insertgrade);
         $insertoptions = array(
             'all' => get_string('all_grades', 'gradereport_singleview'),
             'blanks' => get_string('blanks', 'gradereport_singleview')
         );
 
+        $selectlabel = html_writer::label(
+            $insertappliesto,
+            $this->selectname
+        );
         $select = html_writer::select(
             $insertoptions, $this->selectname, 'blanks', false
         );
 
-        $label = html_writer::tag('label', $insertappliesto);
+        $textlabel = html_writer::label(
+            $insertvalue,
+            $this->insertname
+        );
         $text = new text_attribute($this->insertname, "0", 'bulk');
-        return implode(' ', array($apply, $text->html(), $label, $select));
+
+        $inner = implode(' ', array(
+            $selectlabel,
+            $select,
+            $textlabel,
+            $text->html()
+        ));
+
+        $fieldset = html_writer::tag(
+            'fieldset',
+            html_writer::tag(
+                'legend',
+                get_string('bulklegend', 'gradereport_singleview'),
+                array(
+                    'class' => 'accesshide'
+                )
+            ) .
+            $inner
+        );
+
+        $apply = html_writer::checkbox(
+            $this->applyname,
+            1,
+            false,
+            get_string('bulkperform', 'gradereport_singleview')
+        );
+        $applydiv = html_writer::div($apply, 'enable');
+
+        return $applydiv . $fieldset;
     }
 
     /**
index d8a1844..704268c 100644 (file)
 $string['all_grades'] = 'All grades';
 $string['assessmentname'] = 'Assessment Name';
 $string['blanks'] = 'Empty grades';
-$string['bulkappliesto'] = 'for';
-$string['bulkinsertgrade'] = 'Bulk insert';
+$string['bulkappliesto'] = 'For';
+$string['bulkinsertvalue'] = 'Insert value';
+$string['bulklegend'] = 'Bulk insert';
+$string['bulkperform'] = 'Perform bulk insert';
 $string['bulkfor'] = 'Grades for {$a}';
 $string['exclude'] = 'Exclude';
 $string['excludeall'] = 'Exclude all grades';
index a2a7470..888b1ce 100644 (file)
@@ -1,5 +1,6 @@
 .path-grade-report-singleview div.generalbox {
   margin: 0 20px 15px 20px;
+  text-align: center;
 }
 
 .path-grade-report-singleview div.generalbox div.singleselect form div {
 .dir-rtl.path-grade-report-singleview .generaltable th {
   text-align: right;
 }
+
+.path-grade-report-singleview div.generalbox form div.singleview_bulk {
+  display: inline-block;
+  text-align: left;
+  margin-bottom: 1em;
+}
+.dir-rtl.path-grade-report-singleview div.generalbox form div.singleview_bulk {
+  text-align: right;
+}
+
+.path-grade-report-singleview .singleview_bulk div > *,
+.path-grade-report-singleview .singleview_bulk fieldset > * {
+  display: inline-block;
+  vertical-align: middle;
+  margin: 0;
+}
+
+.path-grade-report-singleview .singleview_bulk > fieldset {
+  display: block;
+}
+
+.path-grade-report-singleview div.generalbox form .singleview_bulk > div.enable {
+  text-align: left;
+}
+.dir-rtl.path-grade-report-singleview div.generalbox form .singleview_bulk > div.enable {
+  text-align: right;
+}
\ No newline at end of file
index ef4aba7..d33a7e0 100644 (file)
@@ -95,6 +95,16 @@ Feature: We can use Single view
     And I follow "Single view for Student 1"
     Then I should see "Student 1"
 
+  @javascript
+  Scenario: I can bulk update grades.
+    Given I follow "Single view for Student 1"
+    Then I should see "Student 1"
+    When I click on "All grades" "option"
+    And I set the field "Insert value" to "1.0"
+    And I click on "Perform bulk insert" "checkbox"
+    And I press "Update"
+    Then I should see "Grades were set for 9 items"
+
   Scenario: Navigation works in the Single view.
     Given I follow "Single view for Student 1"
     Then I should see "Student 1"
index 80c44b4..00e3dbb 100644 (file)
@@ -2490,6 +2490,9 @@ abstract class moodle_database {
         // now enable transactions again
         $this->transactions = array();
         $this->force_rollback = false;
+
+        \core\event\manager::database_transaction_rolledback();
+        \core\message\manager::database_transaction_rolledback();
     }
 
     /**
index 0a941a0..92afdc9 100644 (file)
@@ -327,6 +327,9 @@ if(!defined('LDAP_DN_SPECIAL_CHARS_QUOTED_NUM')) {
 if(!defined('LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA')) {
     define('LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA', 2);
 }
+if(!defined('LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA_REGEX')) {
+    define('LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA_REGEX', 3);
+}
 
 /**
  * The order of the special characters in these arrays _IS IMPORTANT_.
@@ -334,22 +337,36 @@ if(!defined('LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA')) {
  * Otherwise we'll double replace '\' with '\5C' which is Bad(tm)
  */
 function ldap_get_dn_special_chars() {
-    return array (
+    static $specialchars = null;
+
+    if ($specialchars !== null) {
+        return $specialchars;
+    }
+
+    $specialchars = array (
         LDAP_DN_SPECIAL_CHARS              => array('\\',  ' ',   '"',   '#',   '+',   ',',   ';',   '<',   '=',   '>',   "\0"),
         LDAP_DN_SPECIAL_CHARS_QUOTED_NUM   => array('\\5c','\\20','\\22','\\23','\\2b','\\2c','\\3b','\\3c','\\3d','\\3e','\\00'),
-        LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA => array('\\\\','\\ ', '\\"', '\\#', '\\+', '\\,', '\\;', '\\<', '\\>', '\\=', '\\00'),
+        LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA => array('\\\\','\\ ', '\\"', '\\#', '\\+', '\\,', '\\;', '\\<', '\\=', '\\>', '\\00'),
         );
+    $alpharegex = implode('|', array_map (function ($expr) { return preg_quote($expr); },
+                                          $specialchars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA]));
+    $specialchars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA_REGEX] = $alpharegex;
+
+    return $specialchars;
 }
 
 /**
- * Quote control characters in distinguished names used in LDAP - See RFC 4514/2253
+ * Quote control characters in AttributeValue parts of a RelativeDistinguishedName
+ * used in LDAP distinguished names - See RFC 4514/2253
  *
- * @param string The text to quote
- * @return string The text quoted
+ * @param string the AttributeValue to quote
+ * @return string the AttributeValue quoted
  */
 function ldap_addslashes($text) {
     $special_dn_chars = ldap_get_dn_special_chars();
 
+    // Use the preferred/universal quotation method: ESC HEX HEX
+    // (i.e., the 'numerically' quoted characters)
     $text = str_replace ($special_dn_chars[LDAP_DN_SPECIAL_CHARS],
                          $special_dn_chars[LDAP_DN_SPECIAL_CHARS_QUOTED_NUM],
                          $text);
@@ -357,25 +374,34 @@ function ldap_addslashes($text) {
 }
 
 /**
- * Unquote control characters in distinguished names used in LDAP - See RFC 4514/2253
+ * Unquote control characters in AttributeValue parts of a RelativeDistinguishedName
+ * used in LDAP distinguished names - See RFC 4514/2253
  *
- * @param string The text quoted
- * @return string The text unquoted
+ * @param string the AttributeValue quoted
+ * @return string the AttributeValue unquoted
  */
 function ldap_stripslashes($text) {
-    $special_dn_chars = ldap_get_dn_special_chars();
-
-    // First unquote the simply backslashed special characters. If we
-    // do it the other way, we remove too many slashes.
-    $text = str_replace($special_dn_chars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA],
-                        $special_dn_chars[LDAP_DN_SPECIAL_CHARS],
-                        $text);
+    $specialchars = ldap_get_dn_special_chars();
 
-    // Next unquote the 'numerically' quoted characters. We don't use
-    // LDAP_DN_SPECIAL_CHARS_QUOTED_NUM because the standard allows us
-    // to quote any character with this encoding, not just the special
+    // We can't unquote in two steps, as we end up unquoting too much in certain cases. So
+    // we need to build a regexp containing both the 'numerically' and 'alphabetically'
+    // quoted characters. We don't use LDAP_DN_SPECIAL_CHARS_QUOTED_NUM because the
+    // standard allows us to quote any character with this encoding, not just the special
     // ones.
-    $text = preg_replace('/\\\([0-9A-Fa-f]{2})/e', "chr(hexdec('\\1'))", $text);
+    // @TODO: This still misses some special (and rarely used) cases, but we need
+    // a full state machine to handle them.
+    $quoted = '/(\\\\[0-9A-Fa-f]{2}|' . $specialchars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA_REGEX] . ')/';
+    $text = preg_replace_callback($quoted,
+                                  function ($match) use ($specialchars) {
+                                      if (ctype_xdigit(ltrim($match[1], '\\'))) {
+                                          return chr(hexdec($match[1]));
+                                      } else {
+                                          return str_replace($specialchars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA],
+                                                             $specialchars[LDAP_DN_SPECIAL_CHARS],
+                                                             $match[1]);
+                                      }
+                                  },
+                                  $text);
 
     return $text;
 }
index ad84112..8d578c8 100644 (file)
@@ -458,6 +458,101 @@ class core_event_testcase extends advanced_testcase {
             \core_tests\event\unittest_observer::$info);
     }
 
+    public function test_rollback() {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->preventResetByRollback();
+
+        $observers = array(
+            array(
+                'eventname'   => '\core_tests\event\unittest_executed',
+                'callback'    => '\core_tests\event\unittest_observer::external_observer',
+                'internal'    => 0,
+            ),
+        );
+
+        \core\event\manager::phpunit_replace_observers($observers);
+        \core_tests\event\unittest_observer::reset();
+
+        $this->assertCount(0, \core_tests\event\unittest_observer::$event);
+
+        \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10)))->trigger();
+        $this->assertCount(1, \core_tests\event\unittest_observer::$event);
+        \core_tests\event\unittest_observer::reset();
+
+        $transaction1 = $DB->start_delegated_transaction();
+
+        \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10)))->trigger();
+        $this->assertCount(0, \core_tests\event\unittest_observer::$event);
+
+        $transaction2 = $DB->start_delegated_transaction();
+
+        \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10)))->trigger();
+        $this->assertCount(0, \core_tests\event\unittest_observer::$event);
+
+        try {
+            $transaction2->rollback(new Exception('x'));
+            $this->fail('Expecting exception');
+        } catch (Exception $e) {}
+        $this->assertCount(0, \core_tests\event\unittest_observer::$event);
+
+        $this->assertTrue($DB->is_transaction_started());
+
+        try {
+            $transaction1->rollback(new Exception('x'));
+            $this->fail('Expecting exception');
+        } catch (Exception $e) {}
+        $this->assertCount(0, \core_tests\event\unittest_observer::$event);
+
+        $this->assertFalse($DB->is_transaction_started());
+
+        \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10)))->trigger();
+        $this->assertCount(1, \core_tests\event\unittest_observer::$event);
+    }
+
+    public function test_forced_rollback() {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->preventResetByRollback();
+
+        $observers = array(
+            array(
+                'eventname'   => '\core_tests\event\unittest_executed',
+                'callback'    => '\core_tests\event\unittest_observer::external_observer',
+                'internal'    => 0,
+            ),
+        );
+
+        \core\event\manager::phpunit_replace_observers($observers);
+        \core_tests\event\unittest_observer::reset();
+
+        $this->assertCount(0, \core_tests\event\unittest_observer::$event);
+
+        \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10)))->trigger();
+        $this->assertCount(1, \core_tests\event\unittest_observer::$event);
+        \core_tests\event\unittest_observer::reset();
+
+        $transaction1 = $DB->start_delegated_transaction();
+
+        \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10)))->trigger();
+        $this->assertCount(0, \core_tests\event\unittest_observer::$event);
+
+        $transaction2 = $DB->start_delegated_transaction();
+
+        \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10)))->trigger();
+        $this->assertCount(0, \core_tests\event\unittest_observer::$event);
+
+        $DB->force_transaction_rollback();
+        $this->assertCount(0, \core_tests\event\unittest_observer::$event);
+
+        $this->assertFalse($DB->is_transaction_started());
+
+        \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10)))->trigger();
+        $this->assertCount(1, \core_tests\event\unittest_observer::$event);
+    }
+
     public function test_deprecated() {
         global $DB;
 
diff --git a/lib/tests/ldaplib_test.php b/lib/tests/ldaplib_test.php
new file mode 100644 (file)
index 0000000..21ab254
--- /dev/null
@@ -0,0 +1,169 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * ldap tests.
+ *
+ * @package    core
+ * @category   phpunit
+ * @copyright  Damyon Wiese, Iñaki Arenaza 2014
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU Public License
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->libdir . '/ldaplib.php');
+
+class core_ldaplib_testcase extends advanced_testcase {
+
+    public function test_ldap_addslashes() {
+        // See http://tools.ietf.org/html/rfc4514#section-5.2 if you want
+        // to add additional tests.
+
+        $tests = array(
+            array (
+                'test' => 'Simplest',
+                'expected' => 'Simplest',
+            ),
+            array (
+                'test' => 'Simple case',
+                'expected' => 'Simple\\20case',
+            ),
+            array (
+                'test' => 'Medium ‒ case',
+                'expected' => 'Medium\\20‒\\20case',
+            ),
+            array (
+                'test' => '#Harder+case#',
+                'expected' => '\\23Harder\\2bcase\\23',
+            ),
+            array (
+                'test' => ' Harder (and); harder case ',
+                'expected' => '\\20Harder\\20(and)\\3b\\20harder\\20case\\20',
+            ),
+            array (
+                'test' => 'Really \\0 (hard) case!\\',
+                'expected' => 'Really\\20\\5c0\\20(hard)\\20case!\\5c',
+            ),
+            array (
+                'test' => 'James "Jim" = Smith, III',
+                'expected' => 'James\\20\\22Jim\22\\20\\3d\\20Smith\\2c\\20III',
+            ),
+            array (
+                'test' => '  <jsmith@test.local> ',
+                'expected' => '\\20\\20\\3cjsmith@test.local\\3e\\20',
+            ),
+        );
+
+
+        foreach ($tests as $test) {
+            $this->assertSame($test['expected'], ldap_addslashes($test['test']));
+        }
+    }
+
+    public function test_ldap_stripslashes() {
+        // See http://tools.ietf.org/html/rfc4514#section-5.2 if you want
+        // to add additional tests.
+
+        // IMPORTANT NOTICE: While ldap_addslashes() only produces one
+        // of the two defined ways of escaping/quoting (the ESC HEX
+        // HEX way defined in the grammar in Section 3 of RFC-4514)
+        // ldap_stripslashes() has to deal with both of them. So in
+        // addition to testing the same strings we test in
+        // test_ldap_stripslashes(), we need to also test strings
+        // using the second method.
+
+        $tests = array(
+            array (
+                'test' => 'Simplest',
+                'expected' => 'Simplest',
+            ),
+            array (
+                'test' => 'Simple\\20case',
+                'expected' => 'Simple case',
+            ),
+            array (
+                'test' => 'Simple\\ case',
+                'expected' => 'Simple case',
+            ),
+            array (
+                'test' => 'Simple\\ \\63\\61\\73\\65',
+                'expected' => 'Simple case',
+            ),
+            array (
+                'test' => 'Medium\\ ‒\\ case',
+                'expected' => 'Medium ‒ case',
+            ),
+            array (
+                'test' => 'Medium\\20‒\\20case',
+                'expected' => 'Medium ‒ case',
+            ),
+            array (
+                'test' => 'Medium\\20\\E2\\80\\92\\20case',
+                'expected' => 'Medium ‒ case',
+            ),
+            array (
+                'test' => '\\23Harder\\2bcase\\23',
+                'expected' => '#Harder+case#',
+            ),
+            array (
+                'test' => '\\#Harder\\+case\\#',
+                'expected' => '#Harder+case#',
+            ),
+            array (
+                'test' => '\\20Harder\\20(and)\\3b\\20harder\\20case\\20',
+                'expected' => ' Harder (and); harder case ',
+            ),
+            array (
+                'test' => '\\ Harder\\ (and)\\;\\ harder\\ case\\ ',
+                'expected' => ' Harder (and); harder case ',
+            ),
+            array (
+                'test' => 'Really\\20\\5c0\\20(hard)\\20case!\\5c',
+                'expected' => 'Really \\0 (hard) case!\\',
+            ),
+            array (
+                'test' => 'Really\\ \\\\0\\ (hard)\\ case!\\\\',
+                'expected' => 'Really \\0 (hard) case!\\',
+            ),
+            array (
+                'test' => 'James\\20\\22Jim\\22\\20\\3d\\20Smith\\2c\\20III',
+                'expected' => 'James "Jim" = Smith, III',
+            ),
+            array (
+                'test' => 'James\\ \\"Jim\\" \\= Smith\\, III',
+                'expected' => 'James "Jim" = Smith, III',
+            ),
+            array (
+                'test' => '\\20\\20\\3cjsmith@test.local\\3e\\20',
+                'expected' => '  <jsmith@test.local> ',
+            ),
+            array (
+                'test' => '\\ \\<jsmith@test.local\\>\\ ',
+                'expected' => ' <jsmith@test.local> ',
+            ),
+            array (
+                'test' => 'Lu\\C4\\8Di\\C4\\87',
+                'expected' => 'Lučić',
+            ),
+        );
+
+        foreach ($tests as $test) {
+            $this->assertSame($test['expected'], ldap_stripslashes($test['test']));
+        }
+    }
+}
index 61d9717..2d076be 100644 (file)
@@ -712,6 +712,104 @@ class core_messagelib_testcase extends advanced_testcase {
         $DB->delete_records('message_read', array());
     }
 
+    public function test_rollback() {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->preventResetByRollback();
+        set_config('noemailever', 1);
+
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+
+        $message = new stdClass();
+        $message->component         = 'moodle';
+        $message->name              = 'instantmessage';
+        $message->userfrom          = $user1;
+        $message->userto            = $user2;
+        $message->subject           = 'message subject 1';
+        $message->fullmessage       = 'message body';
+        $message->fullmessageformat = FORMAT_MARKDOWN;
+        $message->fullmessagehtml   = '<p>message body</p>';
+        $message->smallmessage      = 'small message';
+        $message->notification      = '0';
+
+        message_send($message);
+        $this->assertDebuggingCalled('Not sending email due to $CFG->noemailever config setting');
+
+        $transaction1 = $DB->start_delegated_transaction();
+
+        message_send($message);
+        $this->assertDebuggingNotCalled();
+
+        $transaction2 = $DB->start_delegated_transaction();
+
+        message_send($message);
+        $this->assertDebuggingNotCalled();
+
+        try {
+            $transaction2->rollback(new Exception('x'));
+            $this->fail('Expecting exception');
+        } catch (Exception $e) {}
+        $this->assertDebuggingNotCalled();
+
+        $this->assertTrue($DB->is_transaction_started());
+
+        try {
+            $transaction1->rollback(new Exception('x'));
+            $this->fail('Expecting exception');
+        } catch (Exception $e) {}
+        $this->assertDebuggingNotCalled();
+
+        $this->assertFalse($DB->is_transaction_started());
+
+        message_send($message);
+        $this->assertDebuggingCalled('Not sending email due to $CFG->noemailever config setting');
+    }
+
+    public function test_forced_rollback() {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->preventResetByRollback();
+        set_config('noemailever', 1);
+
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+
+        $message = new stdClass();
+        $message->component         = 'moodle';
+        $message->name              = 'instantmessage';
+        $message->userfrom          = $user1;
+        $message->userto            = $user2;
+        $message->subject           = 'message subject 1';
+        $message->fullmessage       = 'message body';
+        $message->fullmessageformat = FORMAT_MARKDOWN;
+        $message->fullmessagehtml   = '<p>message body</p>';
+        $message->smallmessage      = 'small message';
+        $message->notification      = '0';
+
+        message_send($message);
+        $this->assertDebuggingCalled('Not sending email due to $CFG->noemailever config setting');
+
+        $transaction1 = $DB->start_delegated_transaction();
+
+        message_send($message);
+        $this->assertDebuggingNotCalled();
+
+        $transaction2 = $DB->start_delegated_transaction();
+
+        message_send($message);
+        $this->assertDebuggingNotCalled();
+
+        $DB->force_transaction_rollback();
+        $this->assertFalse($DB->is_transaction_started());
+        $this->assertDebuggingNotCalled();
+
+        message_send($message);
+        $this->assertDebuggingCalled('Not sending email due to $CFG->noemailever config setting');
+    }
+
     public function test_message_attachment_send() {
         global $CFG;
         $this->preventResetByRollback();
index f28e0fa..47faf50 100644 (file)
     echo $OUTPUT->heading(format_string($forum->name), 2);
     echo $OUTPUT->heading(format_string($discussion->name), 3, 'discussionname');
 
-    if ((!isguestuser() && isloggedin()) && has_capability('mod/forum:viewdiscussion', $modcontext)) {
+    // is_guest should be used here as this also checks whether the user is a guest in the current course.
+    // Guests and visitors cannot subscribe - only enrolled users.
+    if ((!is_guest($modcontext, $USER) && isloggedin()) && has_capability('mod/forum:viewdiscussion', $modcontext)) {
         // Discussion subscription.
         if (\mod_forum\subscriptions::is_subscribable($forum)) {
             echo html_writer::div(
index 7ed7e41..38770ae 100644 (file)
@@ -332,6 +332,7 @@ $string['nodiscussions'] = 'There are no discussion topics yet in this forum';
 $string['nodiscussionsstartedby'] = '{$a} has not started any discussions';
 $string['nodiscussionsstartedbyyou'] = 'You haven\'t started any discussions yet';
 $string['noguestpost'] = 'Sorry, guests are not allowed to post.';
+$string['noguestsubscribe'] = 'Sorry, guests are not allowed to subscribe.';
 $string['noguesttracking'] = 'Sorry, guests are not allowed to set tracking options.';
 $string['nomorepostscontaining'] = 'No more posts containing \'{$a}\' were found';
 $string['nonews'] = 'No news has been posted yet';
index 2265ebe..a7e49c2 100644 (file)
@@ -1192,7 +1192,7 @@ function forum_make_mail_text($course, $cm, $forum, $discussion, $post, $userfro
 
     if (!$bare) {
         $shortname = format_string($course->shortname, true, array('context' => context_course::instance($course->id)));
-        $posttext  = "$shortname -> $strforums -> ".format_string($forum->name,true);
+        $posttext  .= "$shortname -> $strforums -> ".format_string($forum->name,true);
 
         if ($discussion->name != $forum->name) {
             $posttext  .= " -> ".format_string($discussion->name,true);
@@ -1287,7 +1287,7 @@ function forum_make_mail_html($course, $cm, $forum, $discussion, $post, $userfro
     $posthtml .= "\n<body id=\"email\">\n\n";
 
     if ($replyaddress) {
-        $posttext .= "<p><em>--" . get_string('deleteoriginalonreply', 'mod_forum') . "--</em></p>";
+        $posthtml .= "<p><em>--" . get_string('deleteoriginalonreply', 'mod_forum') . "--</em></p>";
     }
 
     $posthtml .= '<div class="navbar">'.
@@ -3781,7 +3781,9 @@ function forum_print_discussion_header(&$post, $forum, $group=-1, $datestring=""
           userdate($usedate, $datestring).'</a>';
     echo "</td>\n";
 
-    if ((!isguestuser() && isloggedin()) && has_capability('mod/forum:viewdiscussion', $modcontext)) {
+    // is_guest should be used here as this also checks whether the user is a guest in the current course.
+    // Guests and visitors cannot subscribe - only enrolled users.
+    if ((!is_guest($modcontext, $USER) && isloggedin()) && has_capability('mod/forum:viewdiscussion', $modcontext)) {
         // Discussion subscription.
         if (\mod_forum\subscriptions::is_subscribable($forum)) {
             echo '<td class="discussionsubscription">';
@@ -5425,7 +5427,7 @@ function forum_print_latest_discussions($course, $forum, $maxdiscussions = -1, $
             }
         }
         echo '<th class="header lastpost" scope="col">'.get_string('lastpost', 'forum').'</th>';
-        if (has_capability('mod/forum:viewdiscussion', $context)) {
+        if ((!is_guest($context, $USER) && isloggedin()) && has_capability('mod/forum:viewdiscussion', $context)) {
             if (\mod_forum\subscriptions::is_subscribable($forum)) {
                 echo '<th class="header discussionsubscription" scope="col">';
                 echo forum_get_discussion_subscription_icon_preloaders();
index 666abb1..fe7b43e 100644 (file)
@@ -42,6 +42,12 @@ require_capability('mod/forum:viewdiscussion', $context);
 
 $return = new stdClass();
 
+if (is_guest($context, $USER)) {
+    // is_guest should be used here as this also checks whether the user is a guest in the current course.
+    // Guests and visitors cannot subscribe - only enrolled users.
+    throw new moodle_exception('noguestsubscribe', 'mod_forum');
+}
+
 if (!\mod_forum\subscriptions::is_subscribable($forum)) {
     // Nothing to do. We won't actually output any content here though.
     echo json_encode($return);