Moodle
  1. Moodle
  2. MDL-36263

the count of an assignment's number of submissions includes unenrolled users.

    Details

    • Testing Instructions:
      Hide
      1. Create an assignment in a course with at least 2 students manually enrolled (Set "Require students click submit" to No)
      2. Login as each student of the 2 students and make a submission to the assignment
      3. Login as a teacher and view the assignment.
      4. Verify the grading summary shows 2 next to "Needs grading"
      5. Delete the enrolment for one of the students
      6. Verify the grading summary shows 1 next to "Needs grading"
      7. Grade the remaining student
      8. Verify the grading summary shows 1 next to "Submitted"

      2.3 Only test

      1. From the assignment grading page choose "Download all submissions" from the course settings menu.
      2. Verify that the downloaded submissions do not include submissions for the unenrolled user

      2.4 Only test

      1. Create a new assignment in course with no groups and "Students submit in teams" enabled and "Require students click submit" disabled
      2. View the front page of the assignment.
      3. Verify that there is no row for "Needs grading" (This does not make sense for group assignments).
      4. Login as a student and make a submission to the assignment
      5. Login as a teacher and verify that the grading summary for the assignment has 1 in the "Submissions" row.
      Show
      Create an assignment in a course with at least 2 students manually enrolled (Set "Require students click submit" to No) Login as each student of the 2 students and make a submission to the assignment Login as a teacher and view the assignment. Verify the grading summary shows 2 next to "Needs grading" Delete the enrolment for one of the students Verify the grading summary shows 1 next to "Needs grading" Grade the remaining student Verify the grading summary shows 1 next to "Submitted" 2.3 Only test From the assignment grading page choose "Download all submissions" from the course settings menu. Verify that the downloaded submissions do not include submissions for the unenrolled user 2.4 Only test Create a new assignment in course with no groups and "Students submit in teams" enabled and "Require students click submit" disabled View the front page of the assignment. Verify that there is no row for "Needs grading" (This does not make sense for group assignments). Login as a student and make a submission to the assignment Login as a teacher and verify that the grading summary for the assignment has 1 in the "Submissions" row.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-36263-master
    • Rank:
      45047

      Description

      The grading summary page for an assignment shows the number of submissions but include submission of users who are now unenrolled from the course. I suggest a similar fix to http://tracker.moodle.org/browse/MDL-35533

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that and for providing a patch.

          Show
          Michael de Raadt added a comment - Thanks for reporting that and for providing a patch.
          Hide
          Matthew G. Switlik added a comment -

          I revised my patch to take into account a course with no enrolled users.

          Show
          Matthew G. Switlik added a comment - I revised my patch to take into account a course with no enrolled users.
          Hide
          Paul Nijbakker added a comment -

          We tested the combined fixes proposed in MDL-34884, MDL-36263 and MDL-35533 and they appear to work as intended.

          Show
          Paul Nijbakker added a comment - We tested the combined fixes proposed in MDL-34884 , MDL-36263 and MDL-35533 and they appear to work as intended.
          Hide
          Damyon Wiese added a comment - - edited

          Hi Matthew - thanks for the suggested fix - I thought it was better not to load the list of users if we didn't need them and added the enrolled sql to the query directly. (I'll post a 2.4 branch in a minute).

          Show
          Damyon Wiese added a comment - - edited Hi Matthew - thanks for the suggested fix - I thought it was better not to load the list of users if we didn't need them and added the enrolled sql to the query directly. (I'll post a 2.4 branch in a minute).
          Hide
          Damyon Wiese added a comment -

          Also Matthew - just to give you some feedback on your original patch:

          1. You had a few white space errors (both trailing white space and incorrect spacing for your if/else statements)

          2. Your git commit messages were not in the correct format
          The correct format is:

          ISSUE COMPONENT: SHORT MESSAGE
          
          OPTIONAL LONGER MESSAGE
          

          (The blank line is important and required)

          3. You had a merge commit in your history - you should always rebase on top of a clean branch when submitting patches

          4. You had multiple commits that should have been squashed into a single commit when submitting to the tracker

          I hope this feedback is useful.

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Also Matthew - just to give you some feedback on your original patch: 1. You had a few white space errors (both trailing white space and incorrect spacing for your if/else statements) 2. Your git commit messages were not in the correct format The correct format is: ISSUE COMPONENT: SHORT MESSAGE OPTIONAL LONGER MESSAGE (The blank line is important and required) 3. You had a merge commit in your history - you should always rebase on top of a clean branch when submitting patches 4. You had multiple commits that should have been squashed into a single commit when submitting to the tracker I hope this feedback is useful. Regards, Damyon
          Hide
          Damyon Wiese added a comment -

          Note for integrators - I based this ontop of the linked issue branch to avoid a conflict. MDL-36908 should be integrated first.

          Show
          Damyon Wiese added a comment - Note for integrators - I based this ontop of the linked issue branch to avoid a conflict. MDL-36908 should be integrated first.
          Hide
          Damyon Wiese added a comment -

          This patch is ready for integration review (I tested on 23 and 24dev).

          Show
          Damyon Wiese added a comment - This patch is ready for integration review (I tested on 23 and 24dev).
          Hide
          Damyon Wiese added a comment -

          I am re-opening because it it better and easier to combine MDL-36263, MDL-34884 and MDL-35533.

          Show
          Damyon Wiese added a comment - I am re-opening because it it better and easier to combine MDL-36263 , MDL-34884 and MDL-35533 .
          Hide
          Damyon Wiese added a comment -

          Note for integrators - the code for 2.3 and 2.4 is quite different due to support for group assignments in 2.4.

          Show
          Damyon Wiese added a comment - Note for integrators - the code for 2.3 and 2.4 is quite different due to support for group assignments in 2.4.
          Hide
          Damyon Wiese added a comment -

          This patch is ready for integration review.

          Show
          Damyon Wiese added a comment - This patch is ready for integration review.
          Hide
          Damyon Wiese added a comment -

          Just rebased the master branch.

          Show
          Damyon Wiese added a comment - Just rebased the master branch.
          Hide
          Sam Hemelryk added a comment -

          Thanks Damyon, this has been integrated now.
          MDL-36908 was integrated at the same time btw

          Show
          Sam Hemelryk added a comment - Thanks Damyon, this has been integrated now. MDL-36908 was integrated at the same time btw
          Hide
          Mark Nelson added a comment -

          Works as expected. Passing.

          Show
          Mark Nelson added a comment - Works as expected. Passing.
          Hide
          Paul Nijbakker added a comment -

          I am afraid that these fixes do NOT in fact cover all shortfalls described in for example MDL-34884. When we apply these fixes to locallib.php in assign in Moodle 2.3.3, the Grading summary does display the number of users correctly and, provided that the assignment settings force students to click the submit button, a draft shows up correctly once handed in and turns into a submission when submitted and becomes an item that needs grading. BUT when the teacher grades the submission, the submission disappears from the grading summary, i.e. the view is no different from when no submissions at all had been made.
          In addition, when in the assignment settings the students are not forced to click the submit button (This is our default setting because it avoids misunderstandings when students click the Submit button too soon or not at all), nothing at all shows up in the grading summary, whether students hand in anything or not. We require that the files handed in by students in this case are treated as (editable) submissions, so they show up as such and also as items that need grading. And, same as in the above case, once they are graded, they should remain listed as submissions.
          We will try and fix the fixes to arrive at the situation achieved earlier when we tested a combination of fixes suggested in MDL-34884, MDL-36263 and MDL-35533.

          Show
          Paul Nijbakker added a comment - I am afraid that these fixes do NOT in fact cover all shortfalls described in for example MDL-34884 . When we apply these fixes to locallib.php in assign in Moodle 2.3.3, the Grading summary does display the number of users correctly and, provided that the assignment settings force students to click the submit button, a draft shows up correctly once handed in and turns into a submission when submitted and becomes an item that needs grading. BUT when the teacher grades the submission, the submission disappears from the grading summary, i.e. the view is no different from when no submissions at all had been made. In addition, when in the assignment settings the students are not forced to click the submit button (This is our default setting because it avoids misunderstandings when students click the Submit button too soon or not at all), nothing at all shows up in the grading summary, whether students hand in anything or not. We require that the files handed in by students in this case are treated as (editable) submissions, so they show up as such and also as items that need grading. And, same as in the above case, once they are graded, they should remain listed as submissions. We will try and fix the fixes to arrive at the situation achieved earlier when we tested a combination of fixes suggested in MDL-34884 , MDL-36263 and MDL-35533 .
          Hide
          Paul Nijbakker added a comment -

          The file in this link makes the grading summary behave as we want it to http://moodle.tokem.fi/mod/resource/view.php?id=323720 I know it is not the greatest code, but it does the job.

          Show
          Paul Nijbakker added a comment - The file in this link makes the grading summary behave as we want it to http://moodle.tokem.fi/mod/resource/view.php?id=323720 I know it is not the greatest code, but it does the job.
          Hide
          Damyon Wiese added a comment -

          Thanks Paul,

          I had a look and there is a problem with the backport for 2.3 with this patch (it is always using the "needs grading" filter in count_submissions_with_status).

          Show
          Damyon Wiese added a comment - Thanks Paul, I had a look and there is a problem with the backport for 2.3 with this patch (it is always using the "needs grading" filter in count_submissions_with_status).
          Hide
          Dan Poltawski added a comment -

          The fix for 2.3 is on Damyons branch.

          Show
          Dan Poltawski added a comment - The fix for 2.3 is on Damyons branch.
          Hide
          Dan Poltawski added a comment -

          I've pulled in the fix to 2.3 for that.

          Show
          Dan Poltawski added a comment - I've pulled in the fix to 2.3 for that.
          Hide
          Mark Nelson added a comment -

          Test passes with new testing instructions.

          Show
          Mark Nelson added a comment - Test passes with new testing instructions.
          Hide
          Paul Nijbakker added a comment -

          I can confirm that the patch now works as it should. Kudos!

          Show
          Paul Nijbakker added a comment - I can confirm that the patch now works as it should. Kudos!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: