Moodle
  1. Moodle
  2. MDL-30545

someone enrolled as a teacher in a course doesnt seem to be able to see quiz related message notifications in their messaging preferences

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.5, 2.2
    • Fix Version/s: 2.3.4
    • Component/s: Messages, Quiz
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      For this test you'll need two browsers ie firefox and chrome.
      Log in in one as admin. Log in in the second one as student.

      As admin go to site administration > users > permissions > check system permissions.
      Check that your student doesn't have "mod/quiz:emailconfirmsubmission" at the system level. If they do, remove it.

      As admin go into a course. Go to course administration > users > permissions.
      Check that the student role doesnt have "mod/quiz:emailconfirmsubmission". If they do, remove it.
      As the student go to your messaging preferences (my profile settings > messaging) and check that quiz "Confirmation of your own quiz submissions" is not shown.

      As admin (still within the course) give the student role the "mod/quiz:emailconfirmsubmission" capability.
      As the student refresh the messaging preferences and check that quiz attempt confirmations are now shown.

      As admin (still within the course) remove the "mod/quiz:emailconfirmsubmission" capability from the student role.
      As the student refresh the messaging preferences and check that the quiz attempt confirmations are not shown.

      As admin go to site administration > users > permissions > define roles
      Edit the student role and set "mod/quiz:emailconfirmsubmission" to "allow"
      As the student refresh the messaging preferences and check that quiz attempt confirmations are now shown.

      As admin edit the student role and set "mod/quiz:emailconfirmsubmission" to "not set" to return it to its original state.

      Show
      For this test you'll need two browsers ie firefox and chrome. Log in in one as admin. Log in in the second one as student. As admin go to site administration > users > permissions > check system permissions. Check that your student doesn't have "mod/quiz:emailconfirmsubmission" at the system level. If they do, remove it. As admin go into a course. Go to course administration > users > permissions. Check that the student role doesnt have "mod/quiz:emailconfirmsubmission". If they do, remove it. As the student go to your messaging preferences (my profile settings > messaging) and check that quiz "Confirmation of your own quiz submissions" is not shown. As admin (still within the course) give the student role the "mod/quiz:emailconfirmsubmission" capability. As the student refresh the messaging preferences and check that quiz attempt confirmations are now shown. As admin (still within the course) remove the "mod/quiz:emailconfirmsubmission" capability from the student role. As the student refresh the messaging preferences and check that the quiz attempt confirmations are not shown. As admin go to site administration > users > permissions > define roles Edit the student role and set "mod/quiz:emailconfirmsubmission" to "allow" As the student refresh the messaging preferences and check that quiz attempt confirmations are now shown. As admin edit the student role and set "mod/quiz:emailconfirmsubmission" to "not set" to return it to its original state.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Create a user, enrol them as a teacher in a course. Log in as them and go to their messaging preferences (my profile settings > messaging). Nothing quiz related appears. Quiz notifications however appear for an admin.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Tim Hunt added a comment -

            I think this logic is correct. Any chance of a peer review please?

            Show
            Tim Hunt added a comment - I think this logic is correct. Any chance of a peer review please?
            Hide
            Tim Hunt added a comment -

            Now with unit tests, although embarrassingly, I cannot run the unit tests on my laptop, so they probably need to be debugged.

            Show
            Tim Hunt added a comment - Now with unit tests, although embarrassingly, I cannot run the unit tests on my laptop, so they probably need to be debugged.
            Hide
            Tim Hunt added a comment -

            More unit tests at https://github.com/andyjdavis/moodle/compare/master...message_caps, thanks to Andrew. They need to be merged in to the tests here.

            Show
            Tim Hunt added a comment - More unit tests at https://github.com/andyjdavis/moodle/compare/master...message_caps , thanks to Andrew. They need to be merged in to the tests here.
            Hide
            Tim Hunt added a comment -

            I have now added Andy's unit tests to my branch.

            Show
            Tim Hunt added a comment - I have now added Andy's unit tests to my branch.
            Hide
            Tim Hunt added a comment -

            Right, stupid errors fixed, and branch rebased. There is now a fighting chance that the unit tests will actually pass!

            Show
            Tim Hunt added a comment - Right, stupid errors fixed, and branch rebased. There is now a fighting chance that the unit tests will actually pass!
            Hide
            Tim Hunt added a comment -

            With more help from Andy, the unit tests now actually pass. Finally, we are ready for a proper peer review.

            Show
            Tim Hunt added a comment - With more help from Andy, the unit tests now actually pass. Finally, we are ready for a proper peer review.
            Hide
            Andrew Davis added a comment -

            Adding testing instructions.

            Show
            Andrew Davis added a comment - Adding testing instructions.
            Hide
            Andrew Davis added a comment - - edited

            [Y] Syntax
            [NA] Output
            [Y] Whitespace
            [NA] Language
            [Y] Databases
            [Y] Testing
            [Y] Security
            [NA] Documentation
            [Y] Git
            [Y] Sanity check

            Looks good. I've bashed on it a bit and it seems to all be working well. It's good to see a generator for quiz. I've previously made a brief attempt at writing one. Looking at $defaultquizsettings I can see why I had so much trouble.

            This should be a big improvement. Once this is in I'll write a bunch more unit tests for messagelib.php. Submit for integration whenever you're ready.

            Show
            Andrew Davis added a comment - - edited [Y] Syntax [NA] Output [Y] Whitespace [NA] Language [Y] Databases [Y] Testing [Y] Security [NA] Documentation [Y] Git [Y] Sanity check Looks good. I've bashed on it a bit and it seems to all be working well. It's good to see a generator for quiz. I've previously made a brief attempt at writing one. Looking at $defaultquizsettings I can see why I had so much trouble. This should be a big improvement. Once this is in I'll write a bunch more unit tests for messagelib.php. Submit for integration whenever you're ready.
            Hide
            Tim Hunt added a comment -

            Submitting for integration.

            Show
            Tim Hunt added a comment - Submitting for integration.
            Hide
            Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            Dan Poltawski added a comment -

            For some reason the 2.2 branch isn't present here. I am quite keen to get this integrated and tested even for the 23/master branches, so I have integrated it without clarifying more about 2.2

            We can create a new issue for 2.2 or try and resolve before tomorrow.

            Show
            Dan Poltawski added a comment - For some reason the 2.2 branch isn't present here. I am quite keen to get this integrated and tested even for the 23/master branches, so I have integrated it without clarifying more about 2.2 We can create a new issue for 2.2 or try and resolve before tomorrow.
            Hide
            Dan Poltawski added a comment -

            Currently going boom because of MDL-36199 not being backported. GRR.

            Configuration read from /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/phpunit.xml
             
            .............................................................   61 / 1383 (  4%)
            .............................................................  122 / 1383 (  8%)
            .............................................................  183 / 1383 ( 13%)
            .............................................................  244 / 1383 ( 17%)
            .............................................................  305 / 1383 ( 22%)
            ........................................................E....  366 / 1383 ( 26%)
            .............................................................  427 / 1383 ( 30%)
            .............................................................  488 / 1383 ( 35%)
            .................FFEEPHP Fatal error:  Call to undefined method available_update_checker::fake_current_environment() in /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/tests/pluginlib_test.php on line 77
            PHP Stack trace:
            PHP   1. {main}() /usr/bin/phpunit:0
            PHP   2. PHPUnit_TextUI_Command::main() /usr/bin/phpunit:46
            PHP   3. PHPUnit_TextUI_Command->run() /usr/share/php/PHPUnit/TextUI/Command.php:130
            PHP   4. PHPUnit_TextUI_TestRunner->doRun() /usr/share/php/PHPUnit/TextUI/Command.php:192
            PHP   5. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/TextUI/TestRunner.php:325
            PHP   6. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:705
            PHP   7. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:705
            PHP   8. PHPUnit_Framework_TestSuite->runTest() /usr/share/php/PHPUnit/Framework/TestSuite.php:745
            PHP   9. PHPUnit_Framework_TestCase->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:772
            PHP  10. PHPUnit_Framework_TestResult->run() /usr/share/php/PHPUnit/Framework/TestCase.php:751
            PHP  11. advanced_testcase->runBare() /usr/share/php/PHPUnit/Framework/TestResult.php:649
            PHP  12. PHPUnit_Framework_TestCase->runBare() /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/phpunit/classes/advanced_testcase.php:76
            PHP  13. PHPUnit_Framework_TestCase->runTest() /usr/share/php/PHPUnit/Framework/TestCase.php:804
            PHP  14. ReflectionMethod->invokeArgs() /usr/share/php/PHPUnit/Framework/TestCase.php:942
            PHP  15. available_update_checker_test->test_core_available_update() /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/tests/pluginlib_test.php:0
             
            Fatal error: Call to undefined method available_update_checker::fake_current_environment() in /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/tests/pluginlib_test.php on line 77
             
            Call Stack:
                0.0003     637304   1. {main}() /usr/bin/phpunit:0
                0.0068    1282600   2. PHPUnit_TextUI_Command::main() /usr/bin/phpunit:46
                0.0068    1283616   3. PHPUnit_TextUI_Command->run() /usr/share/php/PHPUnit/TextUI/Command.php:130
                3.9150  234482056   4. PHPUnit_TextUI_TestRunner->doRun() /usr/share/php/PHPUnit/TextUI/Command.php:192
                3.9192  234939200   5. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/TextUI/TestRunner.php:325
               19.2490  245145272   6. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:705
              137.3687  269720792   7. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:705
              137.3690  269723104   8. PHPUnit_Framework_TestSuite->runTest() /usr/share/php/PHPUnit/Framework/TestSuite.php:745
              137.3690  269723104   9. PHPUnit_Framework_TestCase->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:772
              137.3691  269723104  10. PHPUnit_Framework_TestResult->run() /usr/share/php/PHPUnit/Framework/TestCase.php:751
              137.3693  269725056  11. advanced_testcase->runBare() /usr/share/php/PHPUnit/Framework/TestResult.php:649
              137.3694  269725056  12. PHPUnit_Framework_TestCase->runBare() /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/phpunit/classes/advanced_testcase.php:76
              137.3695  269766304  13. PHPUnit_Framework_TestCase->runTest() /usr/share/php/PHPUnit/Framework/TestCase.php:804
              137.3695  269768560  14. ReflectionMethod->invokeArgs() /usr/share/php/PHPUnit/Framework/TestCase.php:942
              137.3695  269768616  15. available_update_checker_test->test_core_available_update() /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/tests/pluginlib_test.php:0
             
            Build step 'Execute shell' marked build as failure
            

            Show
            Dan Poltawski added a comment - Currently going boom because of MDL-36199 not being backported. GRR. Configuration read from /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/phpunit.xml   ............................................................. 61 / 1383 ( 4%) ............................................................. 122 / 1383 ( 8%) ............................................................. 183 / 1383 ( 13%) ............................................................. 244 / 1383 ( 17%) ............................................................. 305 / 1383 ( 22%) ........................................................E.... 366 / 1383 ( 26%) ............................................................. 427 / 1383 ( 30%) ............................................................. 488 / 1383 ( 35%) .................FFEEPHP Fatal error: Call to undefined method available_update_checker::fake_current_environment() in /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/tests/pluginlib_test.php on line 77 PHP Stack trace: PHP 1. {main}() /usr/bin/phpunit:0 PHP 2. PHPUnit_TextUI_Command::main() /usr/bin/phpunit:46 PHP 3. PHPUnit_TextUI_Command->run() /usr/share/php/PHPUnit/TextUI/Command.php:130 PHP 4. PHPUnit_TextUI_TestRunner->doRun() /usr/share/php/PHPUnit/TextUI/Command.php:192 PHP 5. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/TextUI/TestRunner.php:325 PHP 6. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:705 PHP 7. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:705 PHP 8. PHPUnit_Framework_TestSuite->runTest() /usr/share/php/PHPUnit/Framework/TestSuite.php:745 PHP 9. PHPUnit_Framework_TestCase->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:772 PHP 10. PHPUnit_Framework_TestResult->run() /usr/share/php/PHPUnit/Framework/TestCase.php:751 PHP 11. advanced_testcase->runBare() /usr/share/php/PHPUnit/Framework/TestResult.php:649 PHP 12. PHPUnit_Framework_TestCase->runBare() /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/phpunit/classes/advanced_testcase.php:76 PHP 13. PHPUnit_Framework_TestCase->runTest() /usr/share/php/PHPUnit/Framework/TestCase.php:804 PHP 14. ReflectionMethod->invokeArgs() /usr/share/php/PHPUnit/Framework/TestCase.php:942 PHP 15. available_update_checker_test->test_core_available_update() /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/tests/pluginlib_test.php:0   Fatal error: Call to undefined method available_update_checker::fake_current_environment() in /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/tests/pluginlib_test.php on line 77   Call Stack: 0.0003 637304 1. {main}() /usr/bin/phpunit:0 0.0068 1282600 2. PHPUnit_TextUI_Command::main() /usr/bin/phpunit:46 0.0068 1283616 3. PHPUnit_TextUI_Command->run() /usr/share/php/PHPUnit/TextUI/Command.php:130 3.9150 234482056 4. PHPUnit_TextUI_TestRunner->doRun() /usr/share/php/PHPUnit/TextUI/Command.php:192 3.9192 234939200 5. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/TextUI/TestRunner.php:325 19.2490 245145272 6. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:705 137.3687 269720792 7. PHPUnit_Framework_TestSuite->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:705 137.3690 269723104 8. PHPUnit_Framework_TestSuite->runTest() /usr/share/php/PHPUnit/Framework/TestSuite.php:745 137.3690 269723104 9. PHPUnit_Framework_TestCase->run() /usr/share/php/PHPUnit/Framework/TestSuite.php:772 137.3691 269723104 10. PHPUnit_Framework_TestResult->run() /usr/share/php/PHPUnit/Framework/TestCase.php:751 137.3693 269725056 11. advanced_testcase->runBare() /usr/share/php/PHPUnit/Framework/TestResult.php:649 137.3694 269725056 12. PHPUnit_Framework_TestCase->runBare() /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/phpunit/classes/advanced_testcase.php:76 137.3695 269766304 13. PHPUnit_Framework_TestCase->runTest() /usr/share/php/PHPUnit/Framework/TestCase.php:804 137.3695 269768560 14. ReflectionMethod->invokeArgs() /usr/share/php/PHPUnit/Framework/TestCase.php:942 137.3695 269768616 15. available_update_checker_test->test_core_available_update() /var/lib/jenkins/git_repositories/MOODLE_23_STABLE/lib/tests/pluginlib_test.php:0   Build step 'Execute shell' marked build as failure
            Hide
            Dan Poltawski added a comment -

            And with that backported

            There was 1 error:
             
            1) messagelib_testcase::test_message_get_providers_for_user_more
            include_once(/Users/danp/git/integration/mod/assign/tests/generator/lib.php): failed to open stream: No such file or directory
             
            /Users/danp/git/integration/lib/phpunit/classes/data_generator.php:107
            /Users/danp/git/integration/lib/phpunit/classes/data_generator.php:107
            /Users/danp/git/integration/lib/phpunit/classes/data_generator.php:406
            /Users/danp/git/integration/lib/tests/messagelib_test.php:94
            /Users/danp/git/integration/lib/phpunit/classes/advanced_testcase.php:76
             
            To re-run:
             phpunit messagelib_testcase lib/tests/messagelib_test.php
            

            Show
            Dan Poltawski added a comment - And with that backported There was 1 error:   1) messagelib_testcase::test_message_get_providers_for_user_more include_once(/Users/danp/git/integration/mod/assign/tests/generator/lib.php): failed to open stream: No such file or directory   /Users/danp/git/integration/lib/phpunit/classes/data_generator.php:107 /Users/danp/git/integration/lib/phpunit/classes/data_generator.php:107 /Users/danp/git/integration/lib/phpunit/classes/data_generator.php:406 /Users/danp/git/integration/lib/tests/messagelib_test.php:94 /Users/danp/git/integration/lib/phpunit/classes/advanced_testcase.php:76   To re-run: phpunit messagelib_testcase lib/tests/messagelib_test.php
            Hide
            Dan Poltawski added a comment -

            Ah the irony..

            // It would probably be better to use a quiz instance as it has capability controlled messages
                    // however mod_quiz doesn't have a data generator
            

            Show
            Dan Poltawski added a comment - Ah the irony.. // It would probably be better to use a quiz instance as it has capability controlled messages // however mod_quiz doesn't have a data generator
            Hide
            Dan Poltawski added a comment -

            OK, pushed a fix to both the pluginlib problem ( MDL-36199) and the mod_assign problem. Lets hope this isn't completely broken on 2.3 stable..

            Show
            Dan Poltawski added a comment - OK, pushed a fix to both the pluginlib problem ( MDL-36199 ) and the mod_assign problem. Lets hope this isn't completely broken on 2.3 stable..
            Hide
            Dan Poltawski added a comment - - edited

            What a disaster, missing $DB->sql_concat()

            messagelib_testcase::test_message_get_providers_for_user_more
            dml_read_exception: Error reading from database (ERROR:  function concat(character varying, unknown) does not exist
            LINE 11:                AND (CONCAT(actx.path, '/') LIKE CONCAT(cctx....
                                         ^
            HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
            SELECT DISTINCT rc.capability, 1
             
                          FROM p_role_assignments ra
                          JOIN p_context actx ON actx.id = ra.contextid
                          JOIN p_role_capabilities rc ON rc.roleid = ra.roleid
                          JOIN p_context cctx ON cctx.id = rc.contextid
             
                         WHERE ra.userid = $1
                           AND rc.capability IN ($2,$3,$4,$5,$6)
                           AND rc.permission > 0
                           AND (CONCAT(actx.path, '/') LIKE CONCAT(cctx.path, '/%') OR CONCAT(cctx.path, '/') LIKE CONCAT(actx.path, '/%'))
                         UNION DISTINCT
             
                        SELECT DISTINCT rc.capability, 1
             
                          FROM p_role_capabilities rc
                          JOIN p_context cctx ON cctx.id = rc.contextid
             
                         WHERE rc.roleid = $7
                           AND rc.capability IN ($8,$9,$10,$11,$12)
                           AND rc.permission > 0
                           AND CONCAT(cctx.path, '/') LIKE $13
            [array (
              0 => '3',
              1 => 'mod/quiz:emailwarnoverdue',
              2 => 'moodle/site:config',
              3 => 'mod/quiz:emailconfirmsubmission',
              4 => 'moodle/site:approvecourse',
              5 => 'mod/quiz:emailnotifysubmission',
              6 => '8',
              7 => 'mod/quiz:emailwarnoverdue',
              8 => 'moodle/site:config',
              9 => 'mod/quiz:emailconfirmsubmission',
              10 => 'moodle/site:approvecourse',
              11 => 'mod/quiz:emailnotifysubmission',
              12 => '/1/2/',
            )])
             
            /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/dml/moodle_database.php:407
            /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/dml/pgsql_native_moodle_database.php:239
            /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/dml/pgsql_native_moodle_database.php:708
            /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/dml/moodle_database.php:1284
            /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/messagelib.php:475
            /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/tests/messagelib_test.php:116
            /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/phpunit/classes/advanced_testcase.php:76
            

            (Ironically, my postgres server working fine, I guess they must've added CONCACT to postgres). EDIT: seems like it was introduced in 9.1, must look out for this!

            Show
            Dan Poltawski added a comment - - edited What a disaster, missing $DB->sql_concat() messagelib_testcase::test_message_get_providers_for_user_more dml_read_exception: Error reading from database (ERROR: function concat(character varying, unknown) does not exist LINE 11: AND (CONCAT(actx.path, '/') LIKE CONCAT(cctx.... ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. SELECT DISTINCT rc.capability, 1   FROM p_role_assignments ra JOIN p_context actx ON actx.id = ra.contextid JOIN p_role_capabilities rc ON rc.roleid = ra.roleid JOIN p_context cctx ON cctx.id = rc.contextid   WHERE ra.userid = $1 AND rc.capability IN ($2,$3,$4,$5,$6) AND rc.permission > 0 AND (CONCAT(actx.path, '/') LIKE CONCAT(cctx.path, '/%') OR CONCAT(cctx.path, '/') LIKE CONCAT(actx.path, '/%')) UNION DISTINCT   SELECT DISTINCT rc.capability, 1   FROM p_role_capabilities rc JOIN p_context cctx ON cctx.id = rc.contextid   WHERE rc.roleid = $7 AND rc.capability IN ($8,$9,$10,$11,$12) AND rc.permission > 0 AND CONCAT(cctx.path, '/') LIKE $13 [array ( 0 => '3', 1 => 'mod/quiz:emailwarnoverdue', 2 => 'moodle/site:config', 3 => 'mod/quiz:emailconfirmsubmission', 4 => 'moodle/site:approvecourse', 5 => 'mod/quiz:emailnotifysubmission', 6 => '8', 7 => 'mod/quiz:emailwarnoverdue', 8 => 'moodle/site:config', 9 => 'mod/quiz:emailconfirmsubmission', 10 => 'moodle/site:approvecourse', 11 => 'mod/quiz:emailnotifysubmission', 12 => '/1/2/', )])   /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/dml/moodle_database.php:407 /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/dml/pgsql_native_moodle_database.php:239 /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/dml/pgsql_native_moodle_database.php:708 /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/dml/moodle_database.php:1284 /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/messagelib.php:475 /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/tests/messagelib_test.php:116 /Users/Shared/Jenkins/Home/git_repositories/MOODLE_23_STABLE/lib/phpunit/classes/advanced_testcase.php:76 (Ironically, my postgres server working fine, I guess they must've added CONCACT to postgres). EDIT: seems like it was introduced in 9.1, must look out for this!
            Hide
            Dan Poltawski added a comment - - edited

            Hmm, also, now I think more deeply. How is this query going to perform?

            Show
            Dan Poltawski added a comment - - edited Hmm, also, now I think more deeply. How is this query going to perform?
            Hide
            Dan Poltawski added a comment - - edited

            OK, i've put in the $DB->sql_concat()'s now. Heres hoping thats the last of that problems..

            (Please could you review my changes to make sure I haven't screwed up)

            Show
            Dan Poltawski added a comment - - edited OK, i've put in the $DB->sql_concat()'s now. Heres hoping thats the last of that problems.. (Please could you review my changes to make sure I haven't screwed up)
            Hide
            Dan Poltawski added a comment -

            YAY Oracle!

            3) messagelib_testcase::test_message_get_providers_for_user_more
            dml_read_exception: Error reading from database (ORA-00928: missing SELECT keyword
            SELECT DISTINCT rc.capability, 1
             
                          FROM u_role_assignments ra
                          JOIN u_context actx ON actx.id = ra.contextid
                          JOIN u_role_capabilities rc ON rc.roleid = ra.roleid
                          JOIN u_context cctx ON cctx.id = rc.contextid
             
                         WHERE ra.userid = :o_userid
                           AND rc.capability IN (:o_param617,:o_param618,:o_param619,:o_param620,:o_param621)
                           AND rc.permission > 0
                           AND ( actx.path || '/'  LIKE  cctx.path || '/%'  OR  cctx.path || '/'  LIKE  actx.path || '/%' )
                         UNION DISTINCT
             
                        SELECT DISTINCT rc.capability, 1
             
                          FROM u_role_capabilities rc
                          JOIN u_context cctx ON cctx.id = rc.contextid
             
                         WHERE rc.roleid = :o_frontpageroleid
                           AND rc.capability IN (:o_param622,:o_param623,:o_param624,:o_param625,:o_param626)
                           AND rc.permission > 0
                           AND  cctx.path || '/'  LIKE :o_frontpagepathpattern
            [array (
              'o_userid' => '3',
              'o_param617' => 'mod/quiz:emailwarnoverdue',
              'o_param618' => 'moodle/site:config',
              'o_param619' => 'mod/quiz:emailconfirmsubmission',
              'o_param620' => 'moodle/site:approvecourse',
              'o_param621' => 'mod/quiz:emailnotifysubmission',
              'o_frontpageroleid' => '8',
              'o_param622' => 'mod/quiz:emailwarnoverdue',
              'o_param623' => 'moodle/site:config',
              'o_param624' => 'mod/quiz:emailconfirmsubmission',
              'o_param625' => 'moodle/site:approvecourse',
              'o_param626' => 'mod/quiz:emailnotifysubmission',
              'o_frontpagepathpattern' => '/1/2/',
            )])
             
            /server/workspace/moodle/lib/dml/moodle_database.php:424
            /server/workspace/moodle/lib/dml/oci_native_moodle_database.php:274
            /server/workspace/moodle/lib/dml/oci_native_moodle_database.php:1101
            /server/workspace/moodle/lib/dml/moodle_database.php:1304
            /server/workspace/moodle/lib/messagelib.php:503
            /server/workspace/moodle/lib/tests/messagelib_test.php:116
            /server/workspace/moodle/lib/phpunit/classes/advanced_testcase.php:76
             
            To re-run:
             phpunit messagelib_testcase lib/tests/messagelib_test.php
            

            Show
            Dan Poltawski added a comment - YAY Oracle! 3) messagelib_testcase::test_message_get_providers_for_user_more dml_read_exception: Error reading from database (ORA-00928: missing SELECT keyword SELECT DISTINCT rc.capability, 1   FROM u_role_assignments ra JOIN u_context actx ON actx.id = ra.contextid JOIN u_role_capabilities rc ON rc.roleid = ra.roleid JOIN u_context cctx ON cctx.id = rc.contextid   WHERE ra.userid = :o_userid AND rc.capability IN (:o_param617,:o_param618,:o_param619,:o_param620,:o_param621) AND rc.permission > 0 AND ( actx.path || '/' LIKE cctx.path || '/%' OR cctx.path || '/' LIKE actx.path || '/%' ) UNION DISTINCT   SELECT DISTINCT rc.capability, 1   FROM u_role_capabilities rc JOIN u_context cctx ON cctx.id = rc.contextid   WHERE rc.roleid = :o_frontpageroleid AND rc.capability IN (:o_param622,:o_param623,:o_param624,:o_param625,:o_param626) AND rc.permission > 0 AND cctx.path || '/' LIKE :o_frontpagepathpattern [array ( 'o_userid' => '3', 'o_param617' => 'mod/quiz:emailwarnoverdue', 'o_param618' => 'moodle/site:config', 'o_param619' => 'mod/quiz:emailconfirmsubmission', 'o_param620' => 'moodle/site:approvecourse', 'o_param621' => 'mod/quiz:emailnotifysubmission', 'o_frontpageroleid' => '8', 'o_param622' => 'mod/quiz:emailwarnoverdue', 'o_param623' => 'moodle/site:config', 'o_param624' => 'mod/quiz:emailconfirmsubmission', 'o_param625' => 'moodle/site:approvecourse', 'o_param626' => 'mod/quiz:emailnotifysubmission', 'o_frontpagepathpattern' => '/1/2/', )])   /server/workspace/moodle/lib/dml/moodle_database.php:424 /server/workspace/moodle/lib/dml/oci_native_moodle_database.php:274 /server/workspace/moodle/lib/dml/oci_native_moodle_database.php:1101 /server/workspace/moodle/lib/dml/moodle_database.php:1304 /server/workspace/moodle/lib/messagelib.php:503 /server/workspace/moodle/lib/tests/messagelib_test.php:116 /server/workspace/moodle/lib/phpunit/classes/advanced_testcase.php:76   To re-run: phpunit messagelib_testcase lib/tests/messagelib_test.php
            Hide
            Tim Hunt added a comment -

            Dan, if you can wait a few hours, I can probably try that DB query against a copy of the OU's database, which will give us an idea of performance on one real, large, site, which does override the key capabilities.

            Show
            Tim Hunt added a comment - Dan, if you can wait a few hours, I can probably try that DB query against a copy of the OU's database, which will give us an idea of performance on one real, large, site, which does override the key capabilities.
            Hide
            Tim Hunt added a comment -

            I guess that last error from Dan is because Oracle cannot do UNION DISTINCT.

            Show
            Tim Hunt added a comment - I guess that last error from Dan is because Oracle cannot do UNION DISTINCT.
            Hide
            Tim Hunt added a comment -

            Ah! http://www.orafaq.com/usenet/comp.databases.theory/2006/04/05/0064.htm

            Looks like the word UNION DISTINCT is wrong. That is the default behaviour, and if you want something different, you have to say UNION ALL. That is a nice easy fix! I still have not reviewed the other changes.

            Show
            Tim Hunt added a comment - Ah! http://www.orafaq.com/usenet/comp.databases.theory/2006/04/05/0064.htm Looks like the word UNION DISTINCT is wrong. That is the default behaviour, and if you want something different, you have to say UNION ALL. That is a nice easy fix! I still have not reviewed the other changes.
            Hide
            Tim Hunt added a comment -

            Our 3 other DBs also do the right thing if UNION DISTINCT -> UNION

            That was a detail of SQL that I did not previously know!

            Show
            Tim Hunt added a comment - Our 3 other DBs also do the right thing if UNION DISTINCT -> UNION http://dev.mysql.com/doc/refman/5.0/en/union.html http://www.postgresql.org/docs/8.1/static/sql-select.html http://msdn.microsoft.com/en-us/library/ms180026.aspx That was a detail of SQL that I did not previously know!
            Hide
            Dan Poltawski added a comment -

            I've pushed a change removing the DISTINCT. It seems to please oracle.

            Show
            Dan Poltawski added a comment - I've pushed a change removing the DISTINCT. It seems to please oracle.
            Hide
            Tim Hunt added a comment -

            Testing the query performance against a copy of the OU live database:

            For a student enroled in 9 courses, where they have mod/quiz:emailconfirmsubmission due to an override in one of those courses, the query returns 1 row in 45ms

            For a user who is manager in the system context, the query returns 2 rows in 15ms.

            For the user with the most role assignments (361), the query returns 2 rows in 15ms.

            So, I think that is OK.

            Show
            Tim Hunt added a comment - Testing the query performance against a copy of the OU live database: For a student enroled in 9 courses, where they have mod/quiz:emailconfirmsubmission due to an override in one of those courses, the query returns 1 row in 45ms For a user who is manager in the system context, the query returns 2 rows in 15ms. For the user with the most role assignments (361), the query returns 2 rows in 15ms. So, I think that is OK.
            Hide
            Michael de Raadt added a comment -

            Testing instructions suggest two browsers but list three.

            Show
            Michael de Raadt added a comment - Testing instructions suggest two browsers but list three.
            Hide
            Michael de Raadt added a comment -

            Test result: Success!

            Tested in 2.3 and master.

            Well done, Tim.

            Show
            Michael de Raadt added a comment - Test result: Success! Tested in 2.3 and master. Well done, Tim.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Many, many thanks for your effort!

            Millions of people will enjoy the results of your work, yay!

            Closing as fixed. Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Many, many thanks for your effort! Millions of people will enjoy the results of your work, yay! Closing as fixed. Ciao

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: