Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

          Attachments

            Issue Links

              Activity

              Hide
              timhunt Tim Hunt added a comment -

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

              Show
              timhunt Tim Hunt added a comment - I think this logic is correct. Any chance of a peer review please?
              Hide
              timhunt 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
              timhunt 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
              timhunt 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
              timhunt 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
              timhunt Tim Hunt added a comment -

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

              Show
              timhunt Tim Hunt added a comment - I have now added Andy's unit tests to my branch.
              Hide
              timhunt 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
              timhunt 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
              timhunt 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
              timhunt 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
              andyjdavis Andrew Davis added a comment -

              Adding testing instructions.

              Show
              andyjdavis Andrew Davis added a comment - Adding testing instructions.
              Hide
              andyjdavis 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
              andyjdavis 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
              timhunt Tim Hunt added a comment -

              Submitting for integration.

              Show
              timhunt Tim Hunt added a comment - Submitting for integration.
              Hide
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski Dan Poltawski added a comment - - edited

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

              Show
              poltawski Dan Poltawski added a comment - - edited Hmm, also, now I think more deeply. How is this query going to perform?
              Hide
              poltawski 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
              poltawski 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
              poltawski 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
              poltawski 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
              timhunt 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
              timhunt 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
              timhunt Tim Hunt added a comment -

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

              Show
              timhunt Tim Hunt added a comment - I guess that last error from Dan is because Oracle cannot do UNION DISTINCT.
              Hide
              timhunt 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
              timhunt 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
              timhunt 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
              timhunt 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
              poltawski Dan Poltawski added a comment -

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

              Show
              poltawski Dan Poltawski added a comment - I've pushed a change removing the DISTINCT. It seems to please oracle.
              Hide
              timhunt 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
              timhunt 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
              salvetore Michael de Raadt added a comment -

              Testing instructions suggest two browsers but list three.

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

              Test result: Success!

              Tested in 2.3 and master.

              Well done, Tim.

              Show
              salvetore Michael de Raadt added a comment - Test result: Success! Tested in 2.3 and master. Well done, Tim.
              Hide
              stronk7 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
              stronk7 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:
                    Fix Release Date:
                    14/Jan/13