Moodle
  1. Moodle
  2. MDL-32772

Assignment grading table Firstname/Surname filter breaks SQL

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Assignment
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      Test 1:
      Go to an instance of mod_assign. Click "View/grade all submissions". Click any of the letters in the Firstname / Surname filters above the submissions table.

      Success: See the submissions table correctly filtered
      Error: See error reported ERROR: Mixed types of sql query parameters!!

      Test 2:
      Create a new instance of mod_assign in a course with no enrolled users. Click "View/grade all submissions".

      Success: See a message "Nothing to display"
      Error: See error reported ERROR: missing param "user" in query

      Show
      Test 1: Go to an instance of mod_assign. Click "View/grade all submissions". Click any of the letters in the Firstname / Surname filters above the submissions table. Success: See the submissions table correctly filtered Error: See error reported ERROR: Mixed types of sql query parameters!! Test 2: Create a new instance of mod_assign in a course with no enrolled users. Click "View/grade all submissions". Success: See a message "Nothing to display" Error: See error reported ERROR: missing param "user" in query
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-32772-CLEAN
    • Rank:
      39769

      Description

      Go to an instance of mod_assign. Click "View/grade all submissions". Click any of the letters in the Firstname / Surname filters above the submissions table. See broken SQL error: ERROR: Mixed types of sql query parameters!!

      This bug was introduced yesterday when I changed the SQL to use query parameters.

        Issue Links

          Activity

          Hide
          Damyon Wiese added a comment - - edited

          I found an additional bug with the grading table I am investigating. It also throws an error when there are no enrolled students in the course.

          Show
          Damyon Wiese added a comment - - edited I found an additional bug with the grading table I am investigating. It also throws an error when there are no enrolled students in the course.
          Hide
          Andrew Nicols added a comment -

          Just having a quick look on my way through, but should the foreach ($users as $userid) and associated $where = ... line not sure the $DB->get_in_or_equal() function instead? This would simplify the code a fair amount.

          list($userwhere, $userparams) = $DB->get_in_or_equal($users,, SQL_PARAMS_NAMED, 'user');
          $where = 'u.id ' . $userwhere;
          $params = array_merge($params, $userparams);
          
          Show
          Andrew Nicols added a comment - Just having a quick look on my way through, but should the foreach ($users as $userid) and associated $where = ... line not sure the $DB->get_in_or_equal() function instead? This would simplify the code a fair amount. list($userwhere, $userparams) = $DB->get_in_or_equal($users,, SQL_PARAMS_NAMED, 'user'); $where = 'u.id ' . $userwhere; $params = array_merge($params, $userparams);
          Hide
          Dan Poltawski added a comment -

          I think Andrew is right - that surely will make it easier?

          Show
          Dan Poltawski added a comment - I think Andrew is right - that surely will make it easier?
          Hide
          Damyon Wiese added a comment -

          Thanks Andrew - I have made this change and tested it. I also changed this to be on the master branch and updated the diff url.

          Show
          Damyon Wiese added a comment - Thanks Andrew - I have made this change and tested it. I also changed this to be on the master branch and updated the diff url.
          Hide
          Dan Poltawski added a comment -

          Hi Damyon,

          Could you provide a branch with just this fix on it (based on moodle master) rather than giving your master branch?

          Ideally it'd be squashed down to only the one change (rather than showing the changes before and after the review from Andrew).

          Show
          Dan Poltawski added a comment - Hi Damyon, Could you provide a branch with just this fix on it (based on moodle master) rather than giving your master branch? Ideally it'd be squashed down to only the one change (rather than showing the changes before and after the review from Andrew).
          Hide
          Damyon Wiese added a comment -

          Moved to new branch based on moodle master.

          Show
          Damyon Wiese added a comment - Moved to new branch based on moodle master.
          Hide
          Sam Hemelryk added a comment -

          Changes look good thanks Damyon, I've put this up for integration now.

          Show
          Sam Hemelryk added a comment - Changes look good thanks Damyon, I've put this up for integration now.
          Hide
          Dan Poltawski added a comment -

          Integrated, thanks!

          Show
          Dan Poltawski added a comment - Integrated, thanks!
          Hide
          Ankit Agarwal added a comment -

          This works as expected. Just to note that the filter to select firstname/lastname is only shown when there are large number of users, I had a course with 3 users and the navbar was not displayed. From memory that is normal behavior, hence passing the issue.
          I found another issue related to filters, created MDL-32841 to deal with that.
          Thanks

          Show
          Ankit Agarwal added a comment - This works as expected. Just to note that the filter to select firstname/lastname is only shown when there are large number of users, I had a course with 3 users and the navbar was not displayed. From memory that is normal behavior, hence passing the issue. I found another issue related to filters, created MDL-32841 to deal with that. Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This is now part of Moodle and a few millions people around the globe will be using it soon. Isn't that awesome?

          Many, many thanks and don't forget http://youtu.be/4N7dPaP5Z8U

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - This is now part of Moodle and a few millions people around the globe will be using it soon. Isn't that awesome? Many, many thanks and don't forget http://youtu.be/4N7dPaP5Z8U Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: