Uploaded image for project: '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

      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

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting that and for providing a patch.

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

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

            Show
            swit Matthew G. Switlik added a comment - I revised my patch to take into account a course with no enrolled users.
            Hide
            paaskynen 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
            paaskynen 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 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 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 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 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 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 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 Damyon Wiese added a comment -

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

            Show
            damyon Damyon Wiese added a comment - This patch is ready for integration review (I tested on 23 and 24dev).
            Hide
            damyon 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 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 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 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 Damyon Wiese added a comment -

            This patch is ready for integration review.

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

            Just rebased the master branch.

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

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

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

            Works as expected. Passing.

            Show
            markn Mark Nelson added a comment - Works as expected. Passing.
            Hide
            paaskynen 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
            paaskynen 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
            paaskynen 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
            paaskynen 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 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 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
            poltawski Dan Poltawski added a comment -

            The fix for 2.3 is on Damyons branch.

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

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

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

            Test passes with new testing instructions.

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

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

            Show
            paaskynen Paul Nijbakker added a comment - I can confirm that the patch now works as it should. Kudos!
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  14/Jan/13