Moodle
  1. Moodle
  2. MDL-34376

mod assign displays assignments on my moodle inconsistant with other core modules.

    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:
    • Testing Instructions:
      Hide

      You will need a course with a student.

      1. Add an assignment with a due date in the future, and prevent late submission set to Yes - name "Future Prevent".
      2. Add an assignment with a due date in the future, and prevent late submission set to No - name "Future".
      3. Add an assignment with a due date in the past, and prevent late submission set to Yes - name "Past Prevent".
      4. Add an assignment with a due date in the past, and prevent late submission set to No - name "Past".
      5. Add an assignment with no due date - name "No Date".
      6. Login as student and go to the My Moodle page.
      7. You should see 'You have assignments that need attention'. Click to expand it.
      8. You should see the assignments "Future Prevent", "Future", and "Past". You should not see "Past Prevent" and "No Date".
      Show
      You will need a course with a student. Add an assignment with a due date in the future, and prevent late submission set to Yes - name "Future Prevent". Add an assignment with a due date in the future, and prevent late submission set to No - name "Future". Add an assignment with a due date in the past, and prevent late submission set to Yes - name "Past Prevent". Add an assignment with a due date in the past, and prevent late submission set to No - name "Past". Add an assignment with no due date - name "No Date". Login as student and go to the My Moodle page. You should see 'You have assignments that need attention'. Click to expand it. You should see the assignments "Future Prevent", "Future", and "Past". You should not see "Past Prevent" and "No Date".
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
    • Rank:
      42767

      Description

      This is a followup to MDL-34272:

      The current implementation of assign_print_overview() is unusable to hybrid classes (classes that meet both in person and online) with large numbers of assignments. If you have a course with 16 topics and 5 assignments per topic, the course overview block will automatically display all 80 assignments. Currently the only way to not have them display in the course overview block is to change the visibility of each of the individual instances, or make them not able to be submitted (set a available date in the future, or a due date in the past with and not allow late submissions).

      This behavior seems inconstant with the other modules in Moodle core.

      For example, 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 AND
      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)

      Assignments without a due date would not be displayed on the my moodle page. This allowed the course overview block on the my moodle page to work as an agenda of sorts showing just the items that had been assigned.

      In the new mod assign all assignments in a course appear as long as they are able to be submitted (whether there is a due date or not) This seems inconsistent with the other core modules. (It effectively displays the same information as the assignment overview page (http://localhost:8888/moodle23/mod/assign/index.php?id=5)

      Looking through the moodle codebase. mod_assignment, mod_chat, mod_lesson, and mod_quiz all behave where the activity needs to have both a due date and be able to submit / participate in order to have the item displayed.

      mod_forum and mod_assign are the only two that do not behave this way.

      Is it possible that mod_assign could either:

      A. Change the logic to be similar to the rest of the core modules.

      or

      B. implement a global configuration variable for mod_assign (something like $CFG->showallassignmentsonmymoodle ) that would allow us to change the behavior of mod_assign allowing it only to display assignments that are both submittable and have a due date.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks again, Stephen.

          Show
          Michael de Raadt added a comment - Thanks again, Stephen.
          Hide
          Eric Merrill added a comment -

          Daymon, I believe this item is ready for review and integration, if you could take a look.

          I did double check against 2.2, and this seems to be consistent with the behavior there, including for graders.

          Show
          Eric Merrill added a comment - Daymon, I believe this item is ready for review and integration, if you could take a look. I did double check against 2.2, and this seems to be consistent with the behavior there, including for graders.
          Hide
          Damyon Wiese added a comment -

          The patch looks fine and is consistent with the rest of Moodle so I think it should be integrated.

          I will create another Jira issue though as I think that people are setting or not setting a due date purely to hide assignments from My Moodle which is not intuitive. I would like to see a checkbox in the assignment settings "Hide from My Moodle".

          Show
          Damyon Wiese added a comment - The patch looks fine and is consistent with the rest of Moodle so I think it should be integrated. I will create another Jira issue though as I think that people are setting or not setting a due date purely to hide assignments from My Moodle which is not intuitive. I would like to see a checkbox in the assignment settings "Hide from My Moodle".
          Hide
          Eloy Lafuente (stronk7) 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
          Eloy Lafuente (stronk7) 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
          Dan Poltawski added a comment -

          Hi Guys,

          Agreed that this is not very intuitive but also that its good to have this consistent across modules. I've integrated this to 23 and master.

          Thanks!

          Show
          Dan Poltawski added a comment - Hi Guys, Agreed that this is not very intuitive but also that its good to have this consistent across modules. I've integrated this to 23 and master. Thanks!
          Hide
          Tim Barker added a comment -

          Could see the assignments Future prevent, Future and Past but could also see No Date. I think that this is correct though and the test is wrong so I'm passing the test.

          Show
          Tim Barker added a comment - Could see the assignments Future prevent, Future and Past but could also see No Date. I think that this is correct though and the test is wrong so I'm passing the test.
          Hide
          Koen Roggemans added a comment -

          Hi Tim,
          When there is no due date, the assignment should not be visible on the My Moodle page. I think the test instructions are correct and the test should fail.

          See description of the bug: "Looking through the moodle codebase. mod_assignment, mod_chat, mod_lesson, and mod_quiz all behave where the activity needs to have both a due date and be able to submit / participate in order to have the item displayed."

          Show
          Koen Roggemans added a comment - Hi Tim, When there is no due date, the assignment should not be visible on the My Moodle page. I think the test instructions are correct and the test should fail. See description of the bug: "Looking through the moodle codebase. mod_assignment, mod_chat, mod_lesson, and mod_quiz all behave where the activity needs to have both a due date and be able to submit / participate in order to have the item displayed."
          Hide
          Tim Barker added a comment -

          It's minor, but if it's wrong, it's wrong, let's see what the integrators say about it. I'll ask Dan P in the morning, unless Eloy is watching.

          Show
          Tim Barker added a comment - It's minor, but if it's wrong, it's wrong, let's see what the integrators say about it. I'll ask Dan P in the morning, unless Eloy is watching.
          Hide
          Eric Merrill added a comment -

          Correct, it should fail if that is the case - as that is largely what this is to fix.

          So far I can't reproduce it though. We have been using this patch (cherry-picked) in production, and has been working fine, and in my tests on master and 2.3, I can't get No Date to show.

          Can you screen shot how your assignment is setup?

          Show
          Eric Merrill added a comment - Correct, it should fail if that is the case - as that is largely what this is to fix. So far I can't reproduce it though. We have been using this patch (cherry-picked) in production, and has been working fine, and in my tests on master and 2.3, I can't get No Date to show. Can you screen shot how your assignment is setup?
          Hide
          Helen Foster added a comment -

          Tim, why do you think the test is wrong? From reading the issue description and comments it seems to me that if the assignment with no due date, named "No Date" is visible on the My Moodle page it means that the problem is not yet fixed.

          Show
          Helen Foster added a comment - Tim, why do you think the test is wrong? From reading the issue description and comments it seems to me that if the assignment with no due date, named "No Date" is visible on the My Moodle page it means that the problem is not yet fixed.
          Hide
          Eric Merrill added a comment -

          Can someone else also run the test if possible? I've been trying to reproduce the flaw with the patch, but have not been able to.

          Show
          Eric Merrill added a comment - Can someone else also run the test if possible? I've been trying to reproduce the flaw with the patch, but have not been able to.
          Hide
          Eric Merrill added a comment -

          Ok, I can reproduce it on integration, but not on the latest weekly master with the patch applied, so there is some conflict with another patch, or it was not applied correctly. Looking into it.

          Show
          Eric Merrill added a comment - Ok, I can reproduce it on integration, but not on the latest weekly master with the patch applied, so there is some conflict with another patch, or it was not applied correctly. Looking into it.
          Hide
          Eric Merrill added a comment -

          Found it.

          Just after mod/assign/lib.php, line 215, this need to be added:

          $isopen = false;

          so it becomes:

          ...
                  $time = time();
                  $isopen = false;
                  if ($assignment->duedate) {
                      $isopen = $assignment->allowsubmissionsfromdate <= $time;
                      if ($assignment->preventlatesubmissions) {
                          $isopen = ($isopen && $time <= $assignment->duedate);
                      }
                  }
                  if ($isopen) {
                      $assignmentids[] = $assignment->id;
                  }
          ...

          Not sure how you all want to fix this. Note that this causes a E_STRICT notice in some cases without this fix. So either I can make a patch to apply over integration, or you pull it and Ill resubmit it.

          Sorry

          Show
          Eric Merrill added a comment - Found it. Just after mod/assign/lib.php, line 215, this need to be added: $isopen = false ; so it becomes: ... $time = time(); $isopen = false ; if ($assignment->duedate) { $isopen = $assignment->allowsubmissionsfromdate <= $time; if ($assignment->preventlatesubmissions) { $isopen = ($isopen && $time <= $assignment->duedate); } } if ($isopen) { $assignmentids[] = $assignment->id; } ... Not sure how you all want to fix this. Note that this causes a E_STRICT notice in some cases without this fix. So either I can make a patch to apply over integration, or you pull it and Ill resubmit it. Sorry
          Hide
          Eric Merrill added a comment - - edited

          Ok, I've made a patch branch that can be applied over integration to fix this - just to give that option if that is the route that is decided.

          git://github.com/merrill-oakland/moodle.git

          MDL-34376_int_master
          https://github.com/merrill-oakland/moodle/commit/710f1a344f634af23c35f3906e75ddba4409bfa2

          MDL-34376_int_m_23
          https://github.com/merrill-oakland/moodle/commit/5c57df4c9dd16b0f523a890523d4f1e6f6b0636a

          I also updated the testing instructions to make it clear that No Date should not appear.

          Show
          Eric Merrill added a comment - - edited Ok, I've made a patch branch that can be applied over integration to fix this - just to give that option if that is the route that is decided. git://github.com/merrill-oakland/moodle.git MDL-34376 _int_master https://github.com/merrill-oakland/moodle/commit/710f1a344f634af23c35f3906e75ddba4409bfa2 MDL-34376 _int_m_23 https://github.com/merrill-oakland/moodle/commit/5c57df4c9dd16b0f523a890523d4f1e6f6b0636a I also updated the testing instructions to make it clear that No Date should not appear.
          Hide
          Dan Poltawski added a comment -

          Thanks Eric, reopening but can't apply this right now.

          Show
          Dan Poltawski added a comment - Thanks Eric, reopening but can't apply this right now.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Extra commits added.

          Show
          Eloy Lafuente (stronk7) added a comment - Extra commits added.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Passing, now assigns without duedate are not shown by xxxx_print_overview(), making it consistent with behavior in other modules.

          Thanks Eric and Helen for supporting me.

          Show
          Eloy Lafuente (stronk7) added a comment - Passing, now assigns without duedate are not shown by xxxx_print_overview(), making it consistent with behavior in other modules. Thanks Eric and Helen for supporting me.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          For the good and the bad... this is now part of Moodle and people around the world will start using it immediately, what a responsibility!

          Many thanks for your collaboration, yay!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - For the good and the bad... this is now part of Moodle and people around the world will start using it immediately, what a responsibility! Many thanks for your collaboration, yay! Closing, ciao
          Hide
          Koen Roggemans added a comment -

          Thanks a lot for fixing this so quickly!

          On one particular My Moodle page, the number of shown assignments went from 80 to 0 and the loading time of the My Moodle page from 12 secs to 2 secs!

          Show
          Koen Roggemans added a comment - Thanks a lot for fixing this so quickly! On one particular My Moodle page, the number of shown assignments went from 80 to 0 and the loading time of the My Moodle page from 12 secs to 2 secs!
          Hide
          Dan Poltawski added a comment -

          Hi Koen,

          I think the performance improvement may have been from a different issue: MDL-34783

          Show
          Dan Poltawski added a comment - Hi Koen, I think the performance improvement may have been from a different issue: MDL-34783

            People

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

              Dates

              • Created:
                Updated:
                Resolved: