Moodle
  1. Moodle
  2. MDL-27990

Students do not receive notifications when their assignment submission is graded

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.1
    • Component/s: Assignment (2.2)
    • Labels:
    • Rank:
      17628

      Description

      This bug relates to a QA test...

      This test requires an assignment with a student submission. The student should have popup notification and email checked for assignment submissions in their messaging settings.

      1. Login as a teacher and access the assignment.
      2. Follow the 'View x submitted assignments' link and click a Grade link.
      3. Add a grade and comment.
      4. Login as the student and check that they receive a popup notification that their assignment submission has been graded.
      5. Check that the student also receives email notification.

      Grade feedback was saved and was available via 'view my submission' but no pop up notification was viewed by the student and no email was sent to the student.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Promoting this issue as it is related to a QA test.

          Show
          Michael de Raadt added a comment - Promoting this issue as it is related to a QA test.
          Hide
          Andrew Davis added a comment -

          This is actually due to the test not being quite complete. You need to run cron.php before you will receive your notifications. They are not instant notifications. Ill add a comment to the test detailing what needs to change.

          While verifying this I did however find little problems that I cleaned up. A user preference wasnt being observed and some strings were inaccurate.

          Show
          Andrew Davis added a comment - This is actually due to the test not being quite complete. You need to run cron.php before you will receive your notifications. They are not instant notifications. Ill add a comment to the test detailing what needs to change. While verifying this I did however find little problems that I cleaned up. A user preference wasnt being observed and some strings were inaccurate.
          Hide
          Rajesh Taneja added a comment -

          Looks Great to me

          Show
          Rajesh Taneja added a comment - Looks Great to me
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I think the changes from "email notif" to "notif" are ok. So this can be integrated... just for curiosity:

          • Which is the reason for the $lastmailinfo thingy? Isn't the checkbox automatically checked without need for that?

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I think the changes from "email notif" to "notif" are ok. So this can be integrated... just for curiosity: Which is the reason for the $lastmailinfo thingy? Isn't the checkbox automatically checked without need for that? Ciao
          Hide
          Andrew Davis added a comment -

          re $lastmailinfo afaik its just so that Moodle remembers whether the user had that checkbox ticked or unticked last time.

          Show
          Andrew Davis added a comment - re $lastmailinfo afaik its just so that Moodle remembers whether the user had that checkbox ticked or unticked last time.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          yesyes, but I mean, with your change, you are already setting:

          $mformdata->mailinfo = get_user_preferences('assignment_mailinfo', 0);
          

          so it seems that there is no need to look for the same preference again and set the checked=checked attribute manually, isn't it? The form element, based on the value will show the box checked / unchecked as necessary without needing $lastmailinfo. I think, hehe.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - yesyes, but I mean, with your change, you are already setting: $mformdata->mailinfo = get_user_preferences('assignment_mailinfo', 0); so it seems that there is no need to look for the same preference again and set the checked=checked attribute manually, isn't it? The form element, based on the value will show the box checked / unchecked as necessary without needing $lastmailinfo. I think, hehe. Ciao
          Hide
          Andrew Davis added a comment -

          oh yeah. youre right. seems a bit pointless. Ill fix that shortly.

          Show
          Andrew Davis added a comment - oh yeah. youre right. seems a bit pointless. Ill fix that shortly.
          Hide
          Andrew Davis added a comment -

          Actually, looking at the full code I think git may be playing tricks on us. Those various bits of code are in different functions and operating on different mform instances I think.

          Show
          Andrew Davis added a comment - Actually, looking at the full code I think git may be playing tricks on us. Those various bits of code are in different functions and operating on different mform instances I think.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          ah, crap, different forms! I didn't go to the whole file so assumed they were the same. Re-looking...

          Show
          Eloy Lafuente (stronk7) added a comment - ah, crap, different forms! I didn't go to the whole file so assumed they were the same. Re-looking...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Uhm, no, they are the SAME form (mod_assignment_grading_form). The first change (line 1024, setting $mformdata->mailinfo) goes straight to the 2nd one by instantiating one new mod_assignment_grading_form() in line 1031.

          Also, looking at the nature of the lang string changes... does that require some sort of AMOS-MOVE command in order to get those strings moved in all langs?

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Uhm, no, they are the SAME form (mod_assignment_grading_form). The first change (line 1024, setting $mformdata->mailinfo) goes straight to the 2nd one by instantiating one new mod_assignment_grading_form() in line 1031. Also, looking at the nature of the lang string changes... does that require some sort of AMOS-MOVE command in order to get those strings moved in all langs? Ciao
          Hide
          Andrew Davis added a comment -

          re-looking at the forms stuff now.

          there are amos moves in the commit message. https://github.com/andyjdavis/moodle/commit/be34802d23c2e0779a8ca0d5a1cad71fd0d212f4

          Show
          Andrew Davis added a comment - re-looking at the forms stuff now. there are amos moves in the commit message. https://github.com/andyjdavis/moodle/commit/be34802d23c2e0779a8ca0d5a1cad71fd0d212f4
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Wow, about AMOS I'm sure i went to gitx to get the complete commit message looking for AMOS and didn't see it. Sure I looked the wrong commit, apologizes.

          Show
          Eloy Lafuente (stronk7) added a comment - Wow, about AMOS I'm sure i went to gitx to get the complete commit message looking for AMOS and didn't see it. Sure I looked the wrong commit, apologizes.
          Hide
          Andrew Davis added a comment -

          Ok, I think Ive fixed the problem with the extra retrieval of the user preference. For some reason I find mform code tough to follow.

          Show
          Andrew Davis added a comment - Ok, I think Ive fixed the problem with the extra retrieval of the user preference. For some reason I find mform code tough to follow.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This has been integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - This has been integrated, thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Passing. Seems to observe maxeditingtime properly and messages were sent over popup and jabber as expected.

          Note1: As commented in sister issue, the message received is not friendly at all.

          Note2: The most I think on it, the less I can understand what is doing that user preference there. IMO it should be one assignment setting + user messaging configuration. The preference seems really strange, because teacher enables/disables it GLOBALLY (for all the assignments in the course). So really, cannot find any reason for it to behave in that way.

          Show
          Eloy Lafuente (stronk7) added a comment - Passing. Seems to observe maxeditingtime properly and messages were sent over popup and jabber as expected. Note1: As commented in sister issue, the message received is not friendly at all. Note2: The most I think on it, the less I can understand what is doing that user preference there. IMO it should be one assignment setting + user messaging configuration. The preference seems really strange, because teacher enables/disables it GLOBALLY (for all the assignments in the course). So really, cannot find any reason for it to behave in that way.
          Hide
          Andrew Davis added a comment -

          Multiple issues opened to deal with the unfriendly notification

          I think the user preference kind of makes some sense. Maybe. If I, one of multiple teachers in the course, want to quietly add or update assignment feedback across several students and several assignments, the current setup allows me to uncheck that box and modify my feedback without having to uncheck that box each time or having my change affect other teachers.

          For example, the students hand in 2 assignments each Friday. Its Thursday night. I'm going to do my marking tomorrow however I can't sleep so I'm going to start recording some feedback tonight. I'll review tonight's feedback tomorrow morning then notify the student(s) after the official assignment deadline has passed. This ad hoc process of getting a head start on my marking isn't something that should really affect other teachers. We don't want other teachers starting their marking on Friday and not noticing that you've turned off notifications and doing some or all of their marking without notifying the students.

          Show
          Andrew Davis added a comment - Multiple issues opened to deal with the unfriendly notification I think the user preference kind of makes some sense. Maybe. If I, one of multiple teachers in the course, want to quietly add or update assignment feedback across several students and several assignments, the current setup allows me to uncheck that box and modify my feedback without having to uncheck that box each time or having my change affect other teachers. For example, the students hand in 2 assignments each Friday. Its Thursday night. I'm going to do my marking tomorrow however I can't sleep so I'm going to start recording some feedback tonight. I'll review tonight's feedback tomorrow morning then notify the student(s) after the official assignment deadline has passed. This ad hoc process of getting a head start on my marking isn't something that should really affect other teachers. We don't want other teachers starting their marking on Friday and not noticing that you've turned off notifications and doing some or all of their marking without notifying the students.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This is now spread around the globe. Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - This is now spread around the globe. Thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: