Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Assignment
    • Database:
      Any
    • Testing Instructions:
      Hide

      It would be great to have this tested on all DBs.

      1. Student submission receipts:
      Enable submissionreceipts on the admin settings page for mod_assign.
      Make a submission to an instance of mod_assign.
      Check My Profile -> Messages -> Recent Notifications
      There should be a notification about the assignment submission.

      Disable submissionreceipts on the admin settings page for mod_assign.
      Make a submission to an instance of mod_assign.
      Check My Profile -> Messages -> Recent Notifications
      There should NOT be a notification about the assignment submission.

      2. Grader notifications
      Setup a course with at least one Teacher and one Student user.
      Create an instance of mod_assign in the course. Enable the sendnotifications setting on the module settings page. Set the Due date to yesterday. Login as the student and make a submission. Login as the Teacher and check the recent notifications to see that they were notified about the submission. Disable the sendnotifications setting and enable the sendlatenotifications setting on the module settings page. Login as the student and update the submission. Login as the teacher and check recent notifications to see that they were notified about the submission. Change the due date to tomorrow. Login as a student and update the submission. Login as the teacher and check recent notifications to see that they were NOT notified about the submission. Disable the sendlatenotifications setting on the module settings page. Login as a student and update the submission. Login as the teacher and check recent notifications to see that they were NOT notified about the submission.

      3. Student feedback notifications
      Login as a Teacher and grade a student submission in mod_assign (not the Teachers submissions). Run cron. Run cron again. Login as one of the students and check the recentnotifications to see that only one notification about the grade update was received.

      Show
      It would be great to have this tested on all DBs. 1. Student submission receipts: Enable submissionreceipts on the admin settings page for mod_assign. Make a submission to an instance of mod_assign. Check My Profile -> Messages -> Recent Notifications There should be a notification about the assignment submission. Disable submissionreceipts on the admin settings page for mod_assign. Make a submission to an instance of mod_assign. Check My Profile -> Messages -> Recent Notifications There should NOT be a notification about the assignment submission. 2. Grader notifications Setup a course with at least one Teacher and one Student user. Create an instance of mod_assign in the course. Enable the sendnotifications setting on the module settings page. Set the Due date to yesterday. Login as the student and make a submission. Login as the Teacher and check the recent notifications to see that they were notified about the submission. Disable the sendnotifications setting and enable the sendlatenotifications setting on the module settings page. Login as the student and update the submission. Login as the teacher and check recent notifications to see that they were notified about the submission. Change the due date to tomorrow. Login as a student and update the submission. Login as the teacher and check recent notifications to see that they were NOT notified about the submission. Disable the sendlatenotifications setting on the module settings page. Login as a student and update the submission. Login as the teacher and check recent notifications to see that they were NOT notified about the submission. 3. Student feedback notifications Login as a Teacher and grade a student submission in mod_assign (not the Teachers submissions). Run cron. Run cron again. Login as one of the students and check the recentnotifications to see that only one notification about the grade update was received.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
    • Rank:
      37937

      Description

      Implement the notification feature for mod_assign.

      Notifications are sent to graders when assignment submissions are updated. There are 2 settings for the assign instance that control this - sendnotifications and sendlatenotifications. If sendnotifications is enabled the notification will always be sent. If sendlatenotifications is enabled the notification will be send if the assignment submission is past the due date.

      Notifications are sent to students when their assignment submission has been updated. There is one setting on the admin setting page for mod_assign - submissionreceipts. If submissionreceipts is enabled, students will receive a confirmation notification whenever they update their own assignment.

      Notifications are sent to students when there is feedback available for their assignment. Notifications are sent on the next cron after an assignment has been graded or feedback provided.

      All notifications can be controlled through the 2 message types in the users messaging preferences - Assignment student notifications and Assignment grader notifications.

        Issue Links

          Activity

          Hide
          Amanda Doughty added a comment -

          Hi Damyon,
          Could you consider including this request: MDL-31423 ?

          Thankyou

          Show
          Amanda Doughty added a comment - Hi Damyon, Could you consider including this request: MDL-31423 ? Thankyou
          Hide
          Sam Hemelryk added a comment -

          Hi Damyon,

          This issue needs a little more clarification and care sorry.
          First up could you please separate your changes here out into a separate branch that is based upon moodle.git master.
          At the moment it is based upon mod_assign.git master which means we can't merge it in. I've tried cherry-picking the changes however that resulted in a conflict so chances are these changes have been built upon other changes in another issue.

          A few things I spotted looking at the github diff:

          1. A bit more information in the description as well as testing instructions would be nice.
          2. Would be better if the SQL statements used bound params (:yesterday, :today, rather than ?, ?)
          3. Within cron you should be using mtrace rather than echo (better handling output that may still result in echo)
          4. I'm not a cron expert so take the following as such (we should ask someone who knows cron best practices to look this over).
            • Many of the checks made for each submission could be accounted for by joining in the SQL and making use of enrolment API functions for generating SQL that can be used to return only records associated with an enrolled used. Perhaps worth looking at.
            • Caching some of that information should be considered. Especially the collection of $mod. Likely there are going to be many calls for the same module when mailing on an assignment due date.
          5. Is there any need for the new global in mod/assign/mod_form.php?

          I'll have a better look when there is a clear branch I can review.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Damyon, This issue needs a little more clarification and care sorry. First up could you please separate your changes here out into a separate branch that is based upon moodle.git master. At the moment it is based upon mod_assign.git master which means we can't merge it in. I've tried cherry-picking the changes however that resulted in a conflict so chances are these changes have been built upon other changes in another issue. A few things I spotted looking at the github diff: A bit more information in the description as well as testing instructions would be nice. Would be better if the SQL statements used bound params (:yesterday, :today, rather than ?, ?) Within cron you should be using mtrace rather than echo (better handling output that may still result in echo) I'm not a cron expert so take the following as such (we should ask someone who knows cron best practices to look this over). Many of the checks made for each submission could be accounted for by joining in the SQL and making use of enrolment API functions for generating SQL that can be used to return only records associated with an enrolled used. Perhaps worth looking at. Caching some of that information should be considered. Especially the collection of $mod. Likely there are going to be many calls for the same module when mailing on an assignment due date. Is there any need for the new global in mod/assign/mod_form.php? I'll have a better look when there is a clear branch I can review. Cheers Sam
          Hide
          Damyon Wiese added a comment -

          Sorry - I submitted this a couple of weeks ago before all the recent changes to the master branch - I'll rebase this of the current moodle master and clean it up.

          I do think this is a critical set of functionality that should be supported in 2.3 otherwise this will be a regression with the old assignment module.

          Show
          Damyon Wiese added a comment - Sorry - I submitted this a couple of weeks ago before all the recent changes to the master branch - I'll rebase this of the current moodle master and clean it up. I do think this is a critical set of functionality that should be supported in 2.3 otherwise this will be a regression with the old assignment module.
          Hide
          Damyon Wiese added a comment -

          Hi Sam,

          I have cleaned this up. I put it on a new separate branch based off of master + MDL-32813. This is because MDL-32813 changes the settings code and this change adds a new setting (which would conflict).

          To get this change - checkout master then cherry-pick cfc81f03c564628068bf809688d12b53dac20b78, then cherry-pick 3e9d2f6639f5cfe89f3b92fa44d2ba926995cd30. The first cherry-pick will not be required if MDL-32813 has been integrated already.

          I'll update the issue to require MDL-32813.

          Re your comments above:

          1. A bit more information in the description as well as testing instructions would be nice.
          Done.

          2. Would be better if the SQL statements used bound params (:yesterday, :today, rather than ?, ?)
          Done.

          3. Within cron you should be using mtrace rather than echo (better handling output that may still result in echo)
          Done.

          4. I'm not a cron expert so take the following as such (we should ask someone who knows cron best practices to look this over).
          Many of the checks made for each submission could be accounted for by joining in the SQL and making use of enrolment API functions for generating SQL that can be used to return only records associated with an enrolled used. Perhaps worth looking at.
          Caching some of that information should be considered. Especially the collection of $mod. Likely there are going to be many calls for the same module when mailing on an assignment due date.

          I can't use the get_enrolled_sql functions because they require a context - where as this code is loading this for all assignments in one hit.
          I have added caching to course and mod. I'm assuming the context's have their own caching so I haven't touched them and there will just be too many users to cache so I didn't touch them.

          5. Is there any need for the new global in mod/assign/mod_form.php?
          Removed.

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Hi Sam, I have cleaned this up. I put it on a new separate branch based off of master + MDL-32813 . This is because MDL-32813 changes the settings code and this change adds a new setting (which would conflict). To get this change - checkout master then cherry-pick cfc81f03c564628068bf809688d12b53dac20b78, then cherry-pick 3e9d2f6639f5cfe89f3b92fa44d2ba926995cd30. The first cherry-pick will not be required if MDL-32813 has been integrated already. I'll update the issue to require MDL-32813 . Re your comments above: 1. A bit more information in the description as well as testing instructions would be nice. Done. 2. Would be better if the SQL statements used bound params (:yesterday, :today, rather than ?, ?) Done. 3. Within cron you should be using mtrace rather than echo (better handling output that may still result in echo) Done. 4. I'm not a cron expert so take the following as such (we should ask someone who knows cron best practices to look this over). Many of the checks made for each submission could be accounted for by joining in the SQL and making use of enrolment API functions for generating SQL that can be used to return only records associated with an enrolled used. Perhaps worth looking at. Caching some of that information should be considered. Especially the collection of $mod. Likely there are going to be many calls for the same module when mailing on an assignment due date. I can't use the get_enrolled_sql functions because they require a context - where as this code is loading this for all assignments in one hit. I have added caching to course and mod. I'm assuming the context's have their own caching so I haven't touched them and there will just be too many users to cache so I didn't touch them. 5. Is there any need for the new global in mod/assign/mod_form.php? Removed. Regards, Damyon
          Hide
          Damyon Wiese added a comment -

          Sorry Sam,

          I missed backup/restore - I'll add that now and squash the commit into one.

          • Damyon
          Show
          Damyon Wiese added a comment - Sorry Sam, I missed backup/restore - I'll add that now and squash the commit into one. Damyon
          Hide
          Damyon Wiese added a comment -

          Backup/restore has been added.

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Backup/restore has been added. Regards, Damyon
          Hide
          Sam Hemelryk added a comment -

          Hi Damyon,

          Looking over this again now, there are yet more conflicts in both locallib.php and settings.php when merging this to master.
          They seemed pretty easy resolutions so I have continued with the review and made the following notes:

          1. lang/en/assign.php
            1. strings need to be checked carefully, assign:submit and assign:view have already been defined in that file.
            2. gradersubmissionupdatedtext and gradersubmissionupdatedhtml need AMOS syntax to inform the system that they have been renamed. Use the MOV command for that I think.
            3. newline on ln 197, is that meant to be there?
          2. locallib.php
            1. Error line `mtrace("Could not find user $user->id");` references the variable $user which could not be found.
            2. Why have those two new methods being created (cron_cache_get/cron_cache_put)? Given its just an array within the scope of the function wouldn't it be muh simplier just to work with the array.
            3. unused global in email_student_submission_receipt & email_graders

          Rest looks Ok I think you can put this up for integration once those have been cleaned up

          Also one more thing, I noted a couple of wierd changed in locallib.php that looked like they may have been because of a messy merge. It may pay to cherry-pick this commit to a branch based of git.moodle.org/moodle.git and just check things.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Damyon, Looking over this again now, there are yet more conflicts in both locallib.php and settings.php when merging this to master. They seemed pretty easy resolutions so I have continued with the review and made the following notes: lang/en/assign.php strings need to be checked carefully, assign:submit and assign:view have already been defined in that file. gradersubmissionupdatedtext and gradersubmissionupdatedhtml need AMOS syntax to inform the system that they have been renamed. Use the MOV command for that I think. newline on ln 197, is that meant to be there? locallib.php Error line `mtrace("Could not find user $user->id");` references the variable $user which could not be found. Why have those two new methods being created (cron_cache_get/cron_cache_put)? Given its just an array within the scope of the function wouldn't it be muh simplier just to work with the array. unused global in email_student_submission_receipt & email_graders Rest looks Ok I think you can put this up for integration once those have been cleaned up Also one more thing, I noted a couple of wierd changed in locallib.php that looked like they may have been because of a messy merge. It may pay to cherry-pick this commit to a branch based of git.moodle.org/moodle.git and just check things. Cheers Sam
          Hide
          Damyon Wiese added a comment -

          Hi Sam,

          I have made all these changes and pushed them as a new branch rebased on top of latest weekly dev.

          I added one more change to update the message providers on upgrade (and bumped the version number to accomodate). Not sure if we should be doing this for dev versions or not but it will help people pulling the weekly dev releases.

          This is ready to look at again.

          Cheers, Damyon

          Show
          Damyon Wiese added a comment - Hi Sam, I have made all these changes and pushed them as a new branch rebased on top of latest weekly dev. I added one more change to update the message providers on upgrade (and bumped the version number to accomodate). Not sure if we should be doing this for dev versions or not but it will help people pulling the weekly dev releases. This is ready to look at again. Cheers, Damyon
          Hide
          Dan Poltawski added a comment -

          Ping

          Show
          Dan Poltawski added a comment - Ping
          Hide
          Sam Hemelryk added a comment -

          Hi Damyon, sorry about the delay (thanks Dan for the ping)

          I've been looking over this now. Its so darn close (both spot on and to release) that rather than place feedback I've made the changes on a local branch myself.

          I've pushed the changes to the following location for integration.
          Repo: git://github.com/samhemelryk/moodle.git
          Branch: wip-MDL-31414-m23
          Diff: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-31414-m23

          I've made a single commit that makes the following changes:

          • No need to manually call update of providers that happens automatically.
          • Added upgrade code for the new sendlatenotifications field.
          • Added AMOS syntax for the two renamed strings
          • Refactored assign::cron to use half as many DB queries
          • Cleaned up unused vars and globals, coding style and phpdocs.
          • Removed string notifications added previously but never used.

          Putting this up for integration now, Dan would you mind looking to this.

          The only thing I am further not sure about is the time handling in the submission query within the cron method.
          But then I'm no expert and I'm sure if it is an issue we can deal with it as a separate bug.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Damyon, sorry about the delay (thanks Dan for the ping) I've been looking over this now. Its so darn close (both spot on and to release) that rather than place feedback I've made the changes on a local branch myself. I've pushed the changes to the following location for integration. Repo: git://github.com/samhemelryk/moodle.git Branch: wip- MDL-31414 -m23 Diff: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-31414-m23 I've made a single commit that makes the following changes: No need to manually call update of providers that happens automatically. Added upgrade code for the new sendlatenotifications field. Added AMOS syntax for the two renamed strings Refactored assign::cron to use half as many DB queries Cleaned up unused vars and globals, coding style and phpdocs. Removed string notifications added previously but never used. Putting this up for integration now, Dan would you mind looking to this. The only thing I am further not sure about is the time handling in the submission query within the cron method. But then I'm no expert and I'm sure if it is an issue we can deal with it as a separate bug. Cheers Sam
          Hide
          Damyon Wiese added a comment -

          Hi Sam,

          Thanks for doing the code updates - I'm just taking a look through now.

          I thought the call to update the message provider was required but I have just re-tested it and you are right - it gets updated during the upgrade.

          All looks good to me - thanks again.

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Hi Sam, Thanks for doing the code updates - I'm just taking a look through now. I thought the call to update the message provider was required but I have just re-tested it and you are right - it gets updated during the upgrade. All looks good to me - thanks again. Regards, Damyon
          Hide
          Dan Poltawski added a comment -

          Adding the qa_test_required flag. I'm not sure if this is covered by other qa tests on mod_assign.

          Show
          Dan Poltawski added a comment - Adding the qa_test_required flag. I'm not sure if this is covered by other qa tests on mod_assign.
          Hide
          Dan Poltawski added a comment -

          Thank you guys for this. I've integrated this now.

          Some notes from my review about this:

          1. Great that we've made the switch to get_enrolled_users from get_users_by_capability - I think that prevents a regression that had previously been fixed in mod_assignment (big sites where admins geting lots of mails because they have the grade capability in system context)
          2. I'm not so sure about the cutoff time for sending notifcations. However its better than having no notifcations at all, so in it goes.a
          3. There is a strftime which has creeped in. I think it should be using userdate. Although I have not examined that in detail, so I am going to create another issue for this - I seem to remember pointing out another time related issue in the initial mod_assign landing.
          Show
          Dan Poltawski added a comment - Thank you guys for this. I've integrated this now. Some notes from my review about this: Great that we've made the switch to get_enrolled_users from get_users_by_capability - I think that prevents a regression that had previously been fixed in mod_assignment (big sites where admins geting lots of mails because they have the grade capability in system context) I'm not so sure about the cutoff time for sending notifcations. However its better than having no notifcations at all, so in it goes.a There is a strftime which has creeped in. I think it should be using userdate. Although I have not examined that in detail, so I am going to create another issue for this - I seem to remember pointing out another time related issue in the initial mod_assign landing.
          Hide
          Frédéric Massart added a comment -

          Testing in progress with MySQL and PostgreSQL

          Show
          Frédéric Massart added a comment - Testing in progress with MySQL and PostgreSQL
          Hide
          Damyon Wiese added a comment -

          FYI - I mostly tested this with PostgreSQL.

          Show
          Damyon Wiese added a comment - FYI - I mostly tested this with PostgreSQL.
          Hide
          Frédéric Massart added a comment -

          Everything went all fine on MySQL and PostgreSQL, except:

          When running the cron for the second (or more) time, I have this error:

          Server Time: Wed, 23 May 2012 11:59:06 +0800
          
          
          Running clean-up tasks...
           Deleted old backup records
           Deleted old cache_text records
           Executed tag cron
           Cleaned up context instances
           Built context paths
           Cleaned cache flags
           Cleaned up read notifications
          ...finished clean-up tasks
           Created missing context instances
          Cleaned up stale user sessions
          Running auth crons if required...
          Running enrol crons if required...
          Starting activity modules
          Processing module function assign_cron ...Processing 0 assignment submissions ...
          !!! Coding error detected, it must be fixed by a programmer: moodle_database::get_in_or_equal() does not accept empty arrays !!!
          !! 
          Error code: codingerror !!
          !! Stack trace: * line 616 of /lib/dml/moodle_database.php: coding_exception thrown
          * line 1108 of /mod/assign/locallib.php: call to moodle_database->get_in_or_equal()
          * line 677 of /mod/assign/lib.php: call to assign::cron()
          * line 260 of /lib/cronlib.php: call to assign_cron()
          * line 88 of /admin/cron.php: call to cron_run()
           !!
          
          Show
          Frédéric Massart added a comment - Everything went all fine on MySQL and PostgreSQL, except: When running the cron for the second (or more) time, I have this error: Server Time: Wed, 23 May 2012 11:59:06 +0800 Running clean-up tasks... Deleted old backup records Deleted old cache_text records Executed tag cron Cleaned up context instances Built context paths Cleaned cache flags Cleaned up read notifications ...finished clean-up tasks Created missing context instances Cleaned up stale user sessions Running auth crons if required... Running enrol crons if required... Starting activity modules Processing module function assign_cron ...Processing 0 assignment submissions ... !!! Coding error detected, it must be fixed by a programmer: moodle_database::get_in_or_equal() does not accept empty arrays !!! !! Error code: codingerror !! !! Stack trace: * line 616 of /lib/dml/moodle_database.php: coding_exception thrown * line 1108 of /mod/assign/locallib.php: call to moodle_database->get_in_or_equal() * line 677 of /mod/assign/lib.php: call to assign::cron() * line 260 of /lib/cronlib.php: call to assign_cron() * line 88 of /admin/cron.php: call to cron_run() !!
          Hide
          Dan Poltawski added a comment -

          Thank you Fred! Damyon: are you able to look at a fix for this?

          Show
          Dan Poltawski added a comment - Thank you Fred! Damyon: are you able to look at a fix for this?
          Hide
          Sam Hemelryk added a comment - - edited

          My appologies thats a bug in the refactoring I did.
          The following patch resolves it:

          diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php
          index abf9fec..ff37319 100644
          --- a/mod/assign/locallib.php
          +++ b/mod/assign/locallib.php
          @@ -1095,6 +1095,11 @@ class assign {
                   $params = array('yesterday' => $yesterday, 'today' => $timenow);
                   $submissions = $DB->get_records_sql($sql, $params);
           
          +        if (empty($submissions)) {
          +            mtrace('done.');
          +            return true;
          +        }
          +
                   mtrace('Processing ' . count($submissions) . ' assignment submissions ...');
           
                   // Preload courses we are going to need those.
          

          The error occurs because there is nothing to process, so no need to preload courses or continue.
          The patch basically returns true if there are no submissions to process.
          The output from the patch is:

          Processing module function assign_cron ...done.

          hence the `done.` works OK.

          Dan if you like I can commit and push to integration, otherwise all yours again.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - - edited My appologies thats a bug in the refactoring I did. The following patch resolves it: diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index abf9fec..ff37319 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -1095,6 +1095,11 @@ class assign { $params = array('yesterday' => $yesterday, 'today' => $timenow); $submissions = $DB->get_records_sql($sql, $params); + if (empty($submissions)) { + mtrace('done.'); + return true ; + } + mtrace('Processing ' . count($submissions) . ' assignment submissions ...'); // Preload courses we are going to need those. The error occurs because there is nothing to process, so no need to preload courses or continue. The patch basically returns true if there are no submissions to process. The output from the patch is: Processing module function assign_cron ...done. hence the `done.` works OK. Dan if you like I can commit and push to integration, otherwise all yours again. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Ready for testing, the changes have now been pushed. All yours again Fred

          Show
          Sam Hemelryk added a comment - Ready for testing, the changes have now been pushed. All yours again Fred
          Hide
          Frédéric Massart added a comment -

          Success. Works beautifully!

          Show
          Frédéric Massart added a comment - Success. Works beautifully!
          Hide
          Damyon Wiese added a comment -

          Frédéric Massart reported a bug on MDL-32895 that would have been caused by this change:

          A notice appears when updating all assignments.

          Notice: Undefined property: stdClass::$sendlatenotifications in /home/fred/www/repositories/testing_master/moodle/mod/assign/locallib.php on line 422

          Followed by this exception:

          Error writing to database

          Debug info: Column 'sendlatenotifications' cannot be null
          INSERT INTO mdl_assign (name,timemodified,course,intro,introformat,alwaysshowdescription,preventlatesubmissions,submissiondrafts,sendnotifications,sendlatenotifications,duedate,allowsubmissionsfromdate,grade) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?)
          [array (
          0 => 'Assignment 0',
          1 => 1337751381,
          2 => '18',
          3 => 'This assignment has been randomly generated by a very useful script, for the purpose of testing the boundaries of Moodle in various contexts. Moodle should be able to scale to any size without its speed and ease of use being affected dramatically.',
          4 => '0',
          5 => 1,
          6 => '0',
          7 => '0',
          8 => '0',
          9 => NULL,
          10 => '1427238041',
          11 => '0',
          12 => '62',
          )]
          Error code: dmlwriteexception
          Stack trace:

          line 416 of /lib/dml/moodle_database.php: dml_write_exception thrown
          line 952 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end()
          line 994 of /lib/dml/mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->insert_record_raw()
          line 426 of /mod/assign/locallib.php: call to mysqli_native_moodle_database->insert_record()
          line 95 of /mod/assign/upgradelib.php: call to assign->add_instance()
          line 190 of /admin/tool/assignmentupgrade/locallib.php: call to assign_upgrade_manager->upgrade_assignment()
          line 63 of /admin/tool/assignmentupgrade/batchupgrade.php: call to tool_assignmentupgrade_upgrade_assignment()

          Show
          Damyon Wiese added a comment - Frédéric Massart reported a bug on MDL-32895 that would have been caused by this change: A notice appears when updating all assignments. Notice: Undefined property: stdClass::$sendlatenotifications in /home/fred/www/repositories/testing_master/moodle/mod/assign/locallib.php on line 422 Followed by this exception: Error writing to database Debug info: Column 'sendlatenotifications' cannot be null INSERT INTO mdl_assign (name,timemodified,course,intro,introformat,alwaysshowdescription,preventlatesubmissions,submissiondrafts,sendnotifications,sendlatenotifications,duedate,allowsubmissionsfromdate,grade) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?) [array ( 0 => 'Assignment 0', 1 => 1337751381, 2 => '18', 3 => 'This assignment has been randomly generated by a very useful script, for the purpose of testing the boundaries of Moodle in various contexts. Moodle should be able to scale to any size without its speed and ease of use being affected dramatically.', 4 => '0', 5 => 1, 6 => '0', 7 => '0', 8 => '0', 9 => NULL, 10 => '1427238041', 11 => '0', 12 => '62', )] Error code: dmlwriteexception Stack trace: line 416 of /lib/dml/moodle_database.php: dml_write_exception thrown line 952 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end() line 994 of /lib/dml/mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->insert_record_raw() line 426 of /mod/assign/locallib.php: call to mysqli_native_moodle_database->insert_record() line 95 of /mod/assign/upgradelib.php: call to assign->add_instance() line 190 of /admin/tool/assignmentupgrade/locallib.php: call to assign_upgrade_manager->upgrade_assignment() line 63 of /admin/tool/assignmentupgrade/batchupgrade.php: call to tool_assignmentupgrade_upgrade_assignment()
          Hide
          Damyon Wiese added a comment -

          A fix for the upgrade issue is here:

          Repo: git@github.com:netspotau/moodle-mod_assign.git
          Branch: MDL-31414-5
          Diff: https://github.com/netspotau/moodle-mod_assign/commit/56ff1300d97e7c07d3e3e3f46ed01d1a16eec719

          Regards, Damyon

          Show
          Damyon Wiese added a comment - A fix for the upgrade issue is here: Repo: git@github.com:netspotau/moodle-mod_assign.git Branch: MDL-31414 -5 Diff: https://github.com/netspotau/moodle-mod_assign/commit/56ff1300d97e7c07d3e3e3f46ed01d1a16eec719 Regards, Damyon
          Hide
          Dan Poltawski added a comment -

          Thanks Damyon, i've pulled that in now

          Show
          Dan Poltawski added a comment - Thanks Damyon, i've pulled that in now
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

          Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks!

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: