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:
    • Rank:
      42626

      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.

        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: