Moodle
  1. Moodle
  2. MDL-31545

Make it so that the "single file upload" and "Advanced upload" types do not add an entry to the assignment_submissions table until the user has actually selected to submit the assignment

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Cannot Reproduce
    • Affects Version/s: 2.3
    • Fix Version/s: None
    • Component/s: Assignment (2.2)
    • Labels:
    • Rank:
      38100

      Description

      Currently preliminary rows are created in the assignment_submissions table when a file upload is performed but before the student has actually submitted the assignement.

      Existing code as well as changes made in MDL-30957, MDL-30724 and MDL-29400 make it so that we check the values of certain fields to determine if the assignments have been submitted, however it would be better if the rows were not actually added until a submission is complete.

      Jean-Philippe Gaudreau has already been working on some code to achieve this. This Improvement is for continuation of that work.

        Issue Links

          Activity

          Hide
          Gerard Caulfield added a comment - - edited

          Hi Jean-Philippe

          Your changes so far are fantastic, however I encountered a bug with Advanced file uploads. When grading a submission if you click the "Revert to Draft" button it will no longer work after applying this patch.

          You also mentioned that you felt that a student re-submitting an assignment with no files should essentially delete the submission. This may have ramifications which would need discussion with developers with more Moodle experience that myself. If you do think this is an issue worth addressing, please update the ticket description though so that it also includes such details.

          Thanks for helping make Moodle better

          Show
          Gerard Caulfield added a comment - - edited Hi Jean-Philippe Your changes so far are fantastic, however I encountered a bug with Advanced file uploads. When grading a submission if you click the "Revert to Draft" button it will no longer work after applying this patch. You also mentioned that you felt that a student re-submitting an assignment with no files should essentially delete the submission. This may have ramifications which would need discussion with developers with more Moodle experience that myself. If you do think this is an issue worth addressing, please update the ticket description though so that it also includes such details. Thanks for helping make Moodle better
          Hide
          Michael de Raadt added a comment -

          Hi, Jean-Philippe.

          I'm assigning this issue to you so that you can edit the issue and potentially add Git diffs.

          Show
          Michael de Raadt added a comment - Hi, Jean-Philippe. I'm assigning this issue to you so that you can edit the issue and potentially add Git diffs.
          Hide
          Jean-Philippe Gaudreau added a comment -

          @Michael
          Thx! I've updated Git diffs for master branch (2.3dev). I'll wait for Gerard's confirmation that everything is ok before adding compare diffs for the stable branches.

          @Gerard
          Thanks for creating the issue and giving me some feedbacks! I wasn't able to reproduce the bug with "Advanced file uploads" and "Revert to draft" button. Everything seems to work fine when I just tested it. Can you give me the exact testing instructions you did?

          About "student re-submitting an assignment with no files should essentially delete the submission" : Like you said I think it would need discussion with other Moodle developers cause it's surely not an easy task to do it right without breaking something else. Unfortunately, I don't have that much time to put on this issue for the current sprint so I would say this task do not cover deletion of submissions. Maybe after this issue is closed we could create another improvement task to integrate it.

          Show
          Jean-Philippe Gaudreau added a comment - @Michael Thx! I've updated Git diffs for master branch (2.3dev). I'll wait for Gerard's confirmation that everything is ok before adding compare diffs for the stable branches. @Gerard Thanks for creating the issue and giving me some feedbacks! I wasn't able to reproduce the bug with "Advanced file uploads" and "Revert to draft" button. Everything seems to work fine when I just tested it. Can you give me the exact testing instructions you did? About "student re-submitting an assignment with no files should essentially delete the submission" : Like you said I think it would need discussion with other Moodle developers cause it's surely not an easy task to do it right without breaking something else. Unfortunately, I don't have that much time to put on this issue for the current sprint so I would say this task do not cover deletion of submissions. Maybe after this issue is closed we could create another improvement task to integrate it.
          Hide
          Michael de Raadt added a comment -

          I've assigned this to you, Jean-Philippe.

          Dan has agreed to peer review this issue now that Gerry is no longer with us.

          Show
          Michael de Raadt added a comment - I've assigned this to you, Jean-Philippe. Dan has agreed to peer review this issue now that Gerry is no longer with us.
          Hide
          Jean-Philippe Gaudreau added a comment -

          Hi Michael,

          Is this task still important since there will be a new assignment module released in 2.3?

          Show
          Jean-Philippe Gaudreau added a comment - Hi Michael, Is this task still important since there will be a new assignment module released in 2.3?
          Hide
          Dan Poltawski added a comment -

          Jean-Philippe - sure it is because we are still supporting the stable releases. We will also be keeping the original assignment module in 2.3 in any case

          Show
          Dan Poltawski added a comment - Jean-Philippe - sure it is because we are still supporting the stable releases. We will also be keeping the original assignment module in 2.3 in any case
          Hide
          Jean-Philippe Gaudreau added a comment -

          Ok no problem then! Thx for the clarification!

          Show
          Jean-Philippe Gaudreau added a comment - Ok no problem then! Thx for the clarification!
          Hide
          Dan Poltawski added a comment -

          Ah, this wasn't in the waiting for peer review stage, so escaped my radar.

          Show
          Dan Poltawski added a comment - Ah, this wasn't in the waiting for peer review stage, so escaped my radar.
          Hide
          Dan Poltawski added a comment -

          Hi Jean-Philippe,

          Sorry for my delay in reviewing this. Are you able to provide some testing instructions for how to test this fix? On initial investigation it seems sensible to me.

          Show
          Dan Poltawski added a comment - Hi Jean-Philippe, Sorry for my delay in reviewing this. Are you able to provide some testing instructions for how to test this fix? On initial investigation it seems sensible to me.
          Hide
          Jean-Philippe Gaudreau added a comment -

          Hi Dan,

          I cannot reproduce the behaviour anymore with the current master (2.3beta). It seems that putting the file manager in a required state fixed the problem.

          I suppose we can close this issue!

          Show
          Jean-Philippe Gaudreau added a comment - Hi Dan, I cannot reproduce the behaviour anymore with the current master (2.3beta). It seems that putting the file manager in a required state fixed the problem. I suppose we can close this issue!
          Hide
          Dan Poltawski added a comment -

          Great, Thanks - i'm closing this then. For info, the field was marked required in MDL-29400

          Show
          Dan Poltawski added a comment - Great, Thanks - i'm closing this then. For info, the field was marked required in MDL-29400

            People

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

              Dates

              • Created:
                Updated:
                Resolved: