Moodle
  1. Moodle
  2. MDL-34272

mod assign displays all assignments in my moodle whether the student can submit them or not

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.1
    • Fix Version/s: 2.3.2
    • Component/s: Assignment
    • Labels:
      None
    • Testing Instructions:
      Hide
      1. Create an assignment with a due date of yesterday and prevent late submissions enabled.
      2. Login as a student enrolled in the course with the assignment.
      3. View the "Home -> My home" page.
      4. Verify that the assignment is not listed.
      5. Login as a teacher and change the settings for the assignment to allow late submissions.
      6. Login as a student enrolled in the course with the assignment.
      7. View the "Home -> My home" page.
      8. Verify that the assignment is listed.
      Show
      Create an assignment with a due date of yesterday and prevent late submissions enabled. Login as a student enrolled in the course with the assignment. View the "Home -> My home" page. Verify that the assignment is not listed. Login as a teacher and change the settings for the assignment to allow late submissions. Login as a student enrolled in the course with the assignment. View the "Home -> My home" page. Verify that the assignment is listed.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      In Moodle 2.2 and earlier using mod assignment, assignments would only be displayed in the course overview block if:

      1. The assignment had a due date
      2. The student could submit a file / response to that specific assignment (It was not outside of the date restriction, and prevent late submissions was not set)

      In the new mod assign all assignments in a course appear as long as they are visible for example:

      If you create an assignment (using mod assign) and give it a due date of yesterday and set it to prevent late submissions it will appear on the my moodle page (course overview) block where in Moodle 2.2 this assignment would not be displayed.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Stephen Bourget added a comment -

            Digging deeper into this issue, it looks like in mod/assign/lib.php around line 212 - 230 there is some logic to sort out if the individual assignment instance should be displayed.

                foreach ($assignments as $key => $assignment) {
                    $time = time();
                    if ($assignment->duedate) {
                        if ($assignment->preventlatesubmissions) {
                            $isopen = ($assignment->allowsubmissionsfromdate <= $time && $time <= $assignment->duedate);
                        } else {
                            $isopen = ($assignment->allowsubmissionsfromdate <= $time);
                        }
                    }
            

            It looks however that the results of this logic are discarded a few lines lower:

                    if (empty($isopen) || empty($assignment->duedate)) {
                         $assignmentids[] = $assignment->id;
                    } else {
                         $assignmentids[] = $assignment->id;
                    }
            

            Show
            Stephen Bourget added a comment - Digging deeper into this issue, it looks like in mod/assign/lib.php around line 212 - 230 there is some logic to sort out if the individual assignment instance should be displayed. foreach ($assignments as $key => $assignment) { $time = time(); if ($assignment->duedate) { if ($assignment->preventlatesubmissions) { $isopen = ($assignment->allowsubmissionsfromdate <= $time && $time <= $assignment->duedate); } else { $isopen = ($assignment->allowsubmissionsfromdate <= $time); } } It looks however that the results of this logic are discarded a few lines lower: if (empty($isopen) || empty($assignment->duedate)) { $assignmentids[] = $assignment->id; } else { $assignmentids[] = $assignment->id; }
            Hide
            Damyon Wiese added a comment -

            Hmm.

            if (X) {
                Y;
            } else {
                Y;
            }
            

            Not so clever!

            Thanks for reporting this and finding the fix - I'll post a patch now.

            Show
            Damyon Wiese added a comment - Hmm. if (X) { Y; } else { Y; } Not so clever! Thanks for reporting this and finding the fix - I'll post a patch now.
            Hide
            Stephen Bourget added a comment - - edited

            Your patch looks to be an improvement, but it still behaves differently from the old assignment module.
            For example, if you have a course with 50 instances of mod assign, the only way to prevent them from being listed on the mymoodle page would be to either hide them or set a due date in the past and then set them to prevent late submissions.

            In Moodle 2.2 you could just remove the due date to prevent "unassigned" assignments from showing up on the list.

            I know that several of my teachers in the K12 environment (Roughly ages 13 - 18) use this functionality (since our courses cover a full year, but we have four grading periods. It does not make sense to have assignments that will not be assigned for months to be listed on the mymoodle page)

            To get the same behavior with both mod assign in moodle 2.3 and mod assignment in Moodle 2.2, I think the code would need to look like this: (These changes are on top of your current patch)

            Line 213 of lib.php:

                foreach ($assignments as $key => $assignment) {
                    $time = time();
                    $isopen = true;
                    if ($assignment->duedate) {
                        if ($assignment->preventlatesubmissions) {
                            $isopen = ($assignment->allowsubmissionsfromdate <= $time && $time <= $assignment->duedate);
                        } else {
                            $isopen = ($assignment->allowsubmissionsfromdate <= $time);
                        }
            // Add these lines to the proposed patch
                    } else {
                        // Don't list assignments that don't have a due date
                        $isopen = false;
            // End changes
                    }
                    if ($isopen) {  
                         $assignmentids[] = $assignment->id;
                    }
                    
                }
            

            If you feel strongly about having all of the assignments displayed, then could we have it as a module level config setting?

            Show
            Stephen Bourget added a comment - - edited Your patch looks to be an improvement, but it still behaves differently from the old assignment module. For example, if you have a course with 50 instances of mod assign, the only way to prevent them from being listed on the mymoodle page would be to either hide them or set a due date in the past and then set them to prevent late submissions. In Moodle 2.2 you could just remove the due date to prevent "unassigned" assignments from showing up on the list. I know that several of my teachers in the K12 environment (Roughly ages 13 - 18) use this functionality (since our courses cover a full year, but we have four grading periods. It does not make sense to have assignments that will not be assigned for months to be listed on the mymoodle page) To get the same behavior with both mod assign in moodle 2.3 and mod assignment in Moodle 2.2, I think the code would need to look like this: (These changes are on top of your current patch) Line 213 of lib.php: foreach ($assignments as $key => $assignment) { $time = time(); $isopen = true; if ($assignment->duedate) { if ($assignment->preventlatesubmissions) { $isopen = ($assignment->allowsubmissionsfromdate <= $time && $time <= $assignment->duedate); } else { $isopen = ($assignment->allowsubmissionsfromdate <= $time); } // Add these lines to the proposed patch } else { // Don't list assignments that don't have a due date $isopen = false; // End changes } if ($isopen) { $assignmentids[] = $assignment->id; } } If you feel strongly about having all of the assignments displayed, then could we have it as a module level config setting?
            Hide
            Damyon Wiese added a comment -

            I don't like the idea of abusing the due date to hide things from my moodle - I would rather see a "Hide from My Moodle" checkbox if this is required - that would be a separate tracker issue.

            The way this is now is that My Moodle shows all the things you can submit to (which sounds correct to me).

            Show
            Damyon Wiese added a comment - I don't like the idea of abusing the due date to hide things from my moodle - I would rather see a "Hide from My Moodle" checkbox if this is required - that would be a separate tracker issue. The way this is now is that My Moodle shows all the things you can submit to (which sounds correct to me).
            Hide
            Adrian Greeve added a comment -

            Hi Damyon,

            I've had a look through the code and did a bit of testing myself. Just one thing:

            • If you set an assignment to have a start date in the future, and you don't set a due date, that this assignment will turn up in the my moodle page even though the student can't make a submission yet.

            Everything else looks fine and works well.

            Show
            Adrian Greeve added a comment - Hi Damyon, I've had a look through the code and did a bit of testing myself. Just one thing: If you set an assignment to have a start date in the future, and you don't set a due date, that this assignment will turn up in the my moodle page even though the student can't make a submission yet. Everything else looks fine and works well.
            Hide
            Damyon Wiese added a comment -

            Thanks Adrian,

            I've updated the branch to fix that issue.

            Regards, Damyon

            Show
            Damyon Wiese added a comment - Thanks Adrian, I've updated the branch to fix that issue. Regards, Damyon
            Hide
            Adrian Greeve added a comment -

            Thanks Damyon,

            That looks great. Submitting for integration review.

            Show
            Adrian Greeve added a comment - Thanks Damyon, That looks great. Submitting for integration review.
            Hide
            Stephen Bourget added a comment -

            I created MDL-34376 to address the hiding of assignments from the my moodle page

            Show
            Stephen Bourget added a comment - I created MDL-34376 to address the hiding of assignments from the my moodle page
            Hide
            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
            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
            Sam Hemelryk added a comment -

            Thanks guys, this has been integrated now.

            Show
            Sam Hemelryk added a comment - Thanks guys, this has been integrated now.
            Hide
            Tim Barker added a comment -

            Congrats the test has passed.

            Show
            Tim Barker added a comment - Congrats the test has passed.
            Hide
            Aparup Banerjee added a comment -

            yay, it works!

            This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week.

            Thank you all for taking the time to get us here.

            cheers!

            Show
            Aparup Banerjee added a comment - yay, it works! This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week. Thank you all for taking the time to get us here. cheers!
            Hide
            Rajesh Taneja added a comment -

            Hello Guys,

            I am getting warning because of this fix

            Notice: Undefined variable: isopen in /var/www/im/mod/assign/lib.php on line 222 Call Stack: 0.0331 694240 1. {main}() /var/www/im/my/index.php:0 0.6850 62387488 2. core_renderer->blocks_for_region() /var/www/im/my/index.php:148 0.6850 62387488 3. block_manager->get_content_for_region() /var/www/im/lib/outputrenderers.php:1041 0.6850 62387488 4. block_manager->ensure_content_created() /var/www/im/lib/blocklib.php:314 0.6850 62387800 5. block_manager->create_block_contents() /var/www/im/lib/blocklib.php:981 0.6850 62388704 6. block_base->get_content_for_output() /var/www/im/lib/blocklib.php:929 0.6850 62391560 7. block_base->formatted_contents() /var/www/im/blocks/moodleblock.class.php:232 0.6851 62391560 8. block_course_overview->get_content() /var/www/im/blocks/moodleblock.class.php:281 0.7871 62701456 9. block_course_overview_get_overviews() /var/www/im/blocks/course_overview/block_course_overview.php:68 0.9433 77458264 10. assign_print_overview() /var/www/im/blocks/course_overview/locallib.php:35 
            

            Seems like isopen should be initialised outside if statement.

            Show
            Rajesh Taneja added a comment - Hello Guys, I am getting warning because of this fix Notice: Undefined variable: isopen in /var/www/im/mod/assign/lib.php on line 222 Call Stack: 0.0331 694240 1. {main}() /var/www/im/my/index.php:0 0.6850 62387488 2. core_renderer->blocks_for_region() /var/www/im/my/index.php:148 0.6850 62387488 3. block_manager->get_content_for_region() /var/www/im/lib/outputrenderers.php:1041 0.6850 62387488 4. block_manager->ensure_content_created() /var/www/im/lib/blocklib.php:314 0.6850 62387800 5. block_manager->create_block_contents() /var/www/im/lib/blocklib.php:981 0.6850 62388704 6. block_base->get_content_for_output() /var/www/im/lib/blocklib.php:929 0.6850 62391560 7. block_base->formatted_contents() /var/www/im/blocks/moodleblock.class.php:232 0.6851 62391560 8. block_course_overview->get_content() /var/www/im/blocks/moodleblock.class.php:281 0.7871 62701456 9. block_course_overview_get_overviews() /var/www/im/blocks/course_overview/block_course_overview.php:68 0.9433 77458264 10. assign_print_overview() /var/www/im/blocks/course_overview/locallib.php:35 Seems like isopen should be initialised outside if statement.
            Hide
            Eric Merrill added a comment -

            I can't reproduce the error, and $isopen is defined on line 215, so I'm not sure how it's happening.

            Show
            Eric Merrill added a comment - I can't reproduce the error, and $isopen is defined on line 215, so I'm not sure how it's happening.
            Hide
            Bente Olsen added a comment -

            How come that this issue is fixed for ver. 2.3.2 and not 2.3.1?

            Show
            Bente Olsen added a comment - How come that this issue is fixed for ver. 2.3.2 and not 2.3.1?
            Hide
            Eric Merrill added a comment -

            That is just because 2.3.2 is the first whole version number that contains the fix. It is fixed in 2.3.1+ from July 26 2012 on.

            Show
            Eric Merrill added a comment - That is just because 2.3.2 is the first whole version number that contains the fix. It is fixed in 2.3.1+ from July 26 2012 on.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: