Moodle
  1. Moodle
  2. MDL-39760

mod_assign: Add missing submission plugins callbacks.

    Details

    • Rank:
      50500

      Description

      Currently there are only two callback methods related to assignment submission (mod_assign) student/teacher actions - "save" and "submit_for_grading". It would be good to add some more and improve existing:

      • submit_for_grading could be more useful with $submission parameter (in pre-2.5 versions),
      • Add lock, unlock, release_to_draft callbacks.

      This could be useful for submission plugins that are interacting with external systems to reflect the submission status on the remote side (e.g. lock the submitted work when it is locked by teacher in Moodle)

        Issue Links

          Activity

          Hide
          Ruslan Kabalin added a comment -

          In case you are curious, the change needed for Mahara assignment plugin I am upgrading to new assignment submission type to make it work with Moodle 2.3 onwards.

          Show
          Ruslan Kabalin added a comment - In case you are curious, the change needed for Mahara assignment plugin I am upgrading to new assignment submission type to make it work with Moodle 2.3 onwards.
          Hide
          Damyon Wiese added a comment -

          Hi Ruslan,

          The code looks good - I have a question about the design though.

          Is it better to have lots of different hooks for the different stages, or is it better to have more generic, 'submission updated', 'grade updated', 'flags updated' hooks and pass in the submission, grades and flags with the updated state? That way we won't have to keep adding more and more hooks as we add new features.

          Let me know what you think of that approach.

          Cheers, Damyon

          Show
          Damyon Wiese added a comment - Hi Ruslan, The code looks good - I have a question about the design though. Is it better to have lots of different hooks for the different stages, or is it better to have more generic, 'submission updated', 'grade updated', 'flags updated' hooks and pass in the submission, grades and flags with the updated state? That way we won't have to keep adding more and more hooks as we add new features. Let me know what you think of that approach. Cheers, Damyon
          Hide
          Ruslan Kabalin added a comment -

          Hello Damyon, I was also thinking about that. In my opinion having many hooks is better, manly because it is easier to follow the code and for backward compatibility. In case of the single hook "submission updated", you will need to pass an action as parameter (e.g. save, lock, unlock, release_to_draft), thus developer will need to check action using the "case" structure and likely end up with many functions for each action anyway if something more than 2-3 lines of code needs to be done for particular action. There is nothing wrong with adding new hooks when new features are added I think.

          Thanks for revision by the way.

          Show
          Ruslan Kabalin added a comment - Hello Damyon, I was also thinking about that. In my opinion having many hooks is better, manly because it is easier to follow the code and for backward compatibility. In case of the single hook "submission updated", you will need to pass an action as parameter (e.g. save, lock, unlock, release_to_draft), thus developer will need to check action using the "case" structure and likely end up with many functions for each action anyway if something more than 2-3 lines of code needs to be done for particular action. There is nothing wrong with adding new hooks when new features are added I think. Thanks for revision by the way.
          Hide
          Ruslan Kabalin added a comment -

          Damyon, have you reviewed it (I saw status change)? If so, shall I submit it for integration then?

          Show
          Ruslan Kabalin added a comment - Damyon, have you reviewed it (I saw status change)? If so, shall I submit it for integration then?
          Hide
          Damyon Wiese added a comment -

          Hi Ruslan,

          No problem - your reasoning about separate functions makes sense and I am happy with that.

          One new issue for 25 and master is that, I have split the "locked" functionality out of the submissions table and into a "flags" table (Because a student may have multiple submissions attempts for a single assignment). I think for those branches you should pass the "flags" to the plugin and not the submission. The submission will just be the latest submission attempt by the student, but the "lock" action applies to all submissions from that user.

          Show
          Damyon Wiese added a comment - Hi Ruslan, No problem - your reasoning about separate functions makes sense and I am happy with that. One new issue for 25 and master is that, I have split the "locked" functionality out of the submissions table and into a "flags" table (Because a student may have multiple submissions attempts for a single assignment). I think for those branches you should pass the "flags" to the plugin and not the submission. The submission will just be the latest submission attempt by the student, but the "lock" action applies to all submissions from that user.
          Hide
          Ruslan Kabalin added a comment -

          I think I am interested in the last one anyway, because in the plugin I need to get submission details to determine which remote item I need to lock (I do not need to lock items in the earlier submission attempts, as they will not be evaluated).

          Show
          Ruslan Kabalin added a comment - I think I am interested in the last one anyway, because in the plugin I need to get submission details to determine which remote item I need to lock (I do not need to lock items in the earlier submission attempts, as they will not be evaluated).
          Hide
          Damyon Wiese added a comment -

          OK - the flags doesn't tell you more than the fact that "lock" has been called anyway and the submission can be useful. Also your current version has the same API for all version which is easier for plugin authors.

          I am convinced - sending for integration (thanks!).

          Note for integrators - backporting: This is 100% safe (new functions added - default empty implementation for existing plugins). IMO this is missing functionality and is required for the Mahara plugin. Feel free to ping me about it.

          Show
          Damyon Wiese added a comment - OK - the flags doesn't tell you more than the fact that "lock" has been called anyway and the submission can be useful. Also your current version has the same API for all version which is easier for plugin authors. I am convinced - sending for integration (thanks!). Note for integrators - backporting: This is 100% safe (new functions added - default empty implementation for existing plugins). IMO this is missing functionality and is required for the Mahara plugin. Feel free to ping me about it.
          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 Ruslan, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Ruslan, this has been integrated now
          Hide
          Rajesh Taneja added a comment -

          Thanks for improving this Ruslan,

          Assignment works fine with different settings.

          Show
          Rajesh Taneja added a comment - Thanks for improving this Ruslan, Assignment works fine with different settings.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Thanks for giving me joys and smiles
          Thanks for sharing my trouble's pile

          Thanks for wipeing the tears of my eye
          Thanks for showing me the glad view of sky

          Thanks for lending me your shoulders to lean
          Thanks for giving my words a proper mean

          Thanks for telling me the value of life
          Thanks for showing me the rules to survive

          Thanks for lending me the sympathetic ears
          Thanks for showing how much you care

          From all this what I mean in the end
          Is thanks for being my special friend.

          – Seema Chowdhury

          Sent upstream so... closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Thanks for giving me joys and smiles Thanks for sharing my trouble's pile Thanks for wipeing the tears of my eye Thanks for showing me the glad view of sky Thanks for lending me your shoulders to lean Thanks for giving my words a proper mean Thanks for telling me the value of life Thanks for showing me the rules to survive Thanks for lending me the sympathetic ears Thanks for showing how much you care From all this what I mean in the end Is thanks for being my special friend. – Seema Chowdhury Sent upstream so... closing, thanks!
          Hide
          Davo Smith added a comment -

          Just a note that changing APIs in stable versions isn't 100% safe. This introduced a strict standards warning which broke my pdf annotation plugin for Moodle 2.4 users with debugging on.

          This is probably because I was the only person using the 'submit_for_grading' function (as I suggested it in the first place and fixed it to add the submission param in 2.5, but without specifying 'stdClass' in front of the parameters).

          This note's just here as a reminder to be careful in the future ...

          Show
          Davo Smith added a comment - Just a note that changing APIs in stable versions isn't 100% safe. This introduced a strict standards warning which broke my pdf annotation plugin for Moodle 2.4 users with debugging on. This is probably because I was the only person using the 'submit_for_grading' function (as I suggested it in the first place and fixed it to add the submission param in 2.5, but without specifying 'stdClass' in front of the parameters). This note's just here as a reminder to be careful in the future ...

            People

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

              Dates

              • Created:
                Updated:
                Resolved: