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:
    • Rank:
      33259

      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.

        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: