Moodle
  1. Moodle
  2. MDL-42703

Getting course sort order fails under MSSQL

    Details

    • Testing Instructions:
      Hide

      Requires testing across multiple DBs including MSSQL.

      1. Run course-related unit tests (phpunit core_course_courselib_testcase course\tests\courselib_test.php)
      2. Watch your web server error logs in the console while performing these steps.
      3. Log in as an admin
      4. Browse to the course management interface.
      5. Select a category with at least 3 courses in it.
      6. Drag the bottom course up so that it is above the top course.
      7. Refresh the page and make sure it worked.
      8. Drag the top course back to the bottom.
      9. Refresh the page and make sure it worked.
      10. Try resorting courses.
      Show
      Requires testing across multiple DBs including MSSQL. Run course-related unit tests (phpunit core_course_courselib_testcase course\tests\courselib_test.php) Watch your web server error logs in the console while performing these steps. Log in as an admin Browse to the course management interface. Select a category with at least 3 courses in it. Drag the bottom course up so that it is above the top course. Refresh the page and make sure it worked. Drag the top course back to the bottom. Refresh the page and make sure it worked. Try resorting courses.
    • Affected Branches:
      MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-mdl-42703-master
    • Rank:
      54547

      Description

      The following error is reported when running unit tests under MSSQL...

      2) core_course_courselib_testcase::test_course_change_sortorder_after_course
      dml_write_exception: Error writing to database (SQLState: 42000<br>
      Error Code: 102<br>
      Message: [Microsoft][SQL Server Native Client 11.0][SQL Server]Incorrect syntax
      near 'c'.<br>
      
      UPDATE phpu_course c
                             SET sortorder = sortorder + 1
                           WHERE c.category = '2'
                             AND sortorder > '20003'
      [array (
        0 => '2',
        1 => '20003',
      )])
      
      D:\xampp\htdocs\master_integration\lib\dml\moodle_database.php:444
      D:\xampp\htdocs\master_integration\lib\dml\sqlsrv_native_moodle_database.php:245
      
      D:\xampp\htdocs\master_integration\lib\dml\sqlsrv_native_moodle_database.php:352
      
      D:\xampp\htdocs\master_integration\lib\dml\sqlsrv_native_moodle_database.php:743
      
      D:\xampp\htdocs\master_integration\course\lib.php:3469
      D:\xampp\htdocs\master_integration\course\tests\courselib_test.php:2241
      D:\xampp\htdocs\master_integration\lib\phpunit\classes\advanced_testcase.php:80
      
      To re-run:
       \xampp\php\phpunit core_course_courselib_testcase course\tests\courselib_test.php
      

      In the function course_change_sortorder_after_course() there are a number of raw DB calls (I'm not sure why this was allowed). In each of the queries an alias is used for the table, however MSSQL seems to dislike this alias syntactically. As there is only a single table involved in each query the alias is not needed and can be removed. When it is removed, the queries work in MSSQL.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          I've removed the alias from all queries in the course_change_sortorder_after_course() function; they were not needed. The alias could remain in the select query, but I changed it so it would be consistent with the other queries in the function.

          The testing instructions could probably be improved. It would be good to know what other areas use this function.

          Show
          Michael de Raadt added a comment - I've removed the alias from all queries in the course_change_sortorder_after_course() function; they were not needed. The alias could remain in the select query, but I changed it so it would be consistent with the other queries in the function. The testing instructions could probably be improved. It would be good to know what other areas use this function.
          Hide
          Sam Hemelryk added a comment -

          Thanks Michael - submitting for integration review and tweaking the testing instructions

          Show
          Sam Hemelryk added a comment - Thanks Michael - submitting for integration review and tweaking the testing instructions
          Hide
          Damyon Wiese added a comment -

          Thanks Michael,

          I ran the unit tests for course/tests/courselib_test.php in all databases with this patch and got no errors.

          Integrated to master. (tester to do the remaining tests).

          Show
          Damyon Wiese added a comment - Thanks Michael, I ran the unit tests for course/tests/courselib_test.php in all databases with this patch and got no errors. Integrated to master. (tester to do the remaining tests).
          Hide
          Dan Poltawski added a comment -

          Looks good on mssql.

          Show
          Dan Poltawski added a comment - Looks good on mssql.
          Hide
          Dan Poltawski added a comment -

          Ok on oracle

          Show
          Dan Poltawski added a comment - Ok on oracle
          Hide
          Dan Poltawski added a comment -

          Passing, thanks

          Show
          Dan Poltawski added a comment - Passing, thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          It's Friday, I'm tired so I won't be very imaginative today.

          No matter of that, yes, you did it! Thanks for your collaboration!

          Closing this as fixed!

          Show
          Eloy Lafuente (stronk7) added a comment - It's Friday, I'm tired so I won't be very imaginative today. No matter of that, yes, you did it! Thanks for your collaboration! Closing this as fixed!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: