Moodle
  1. Moodle
  2. MDL-30121

Draft files included in zip file from "advanced uploading of files" activity

    Details

    • Testing Instructions:
      Hide
      1. As a teacher create "advanced uploading of files" activity
      2. As a student1 upload file to assignment but don't send it for marking.
      3. As a student2 upload file to assignment and send it for marking.
      4. As teacher view submission and click "Download all assignments as a zip" (mod/assignment/submissions.php?id= {activity_id}&download=zip)
        # Make sure files uploaded by student 2 are only included in zip
        # As student 1 send activity for marking
        # As teacher view submission again and click "Download all assignments as a zip" (mod/assignment/submissions.php?id={activity_id}

        &download=zip)

      5. Make sure files from student 1 and student 2 are included in zip.
      Show
      As a teacher create "advanced uploading of files" activity As a student1 upload file to assignment but don't send it for marking. As a student2 upload file to assignment and send it for marking. As teacher view submission and click "Download all assignments as a zip" (mod/assignment/submissions.php?id= {activity_id}&download=zip) # Make sure files uploaded by student 2 are only included in zip # As student 1 send activity for marking # As teacher view submission again and click "Download all assignments as a zip" (mod/assignment/submissions.php?id={activity_id} &download=zip) Make sure files from student 1 and student 2 are included in zip.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      wip-mdl-30121
    • Rank:
      24769

      Description

      When you download zip file on "advanced uploading of files" activity, students drafts are in zip file too.

      We think that teachers don't need to evaluate draft files...

        Issue Links

          Activity

          Hide
          Julien Boulen added a comment -

          I attach our patch to improve this issue.

          We also have a script to validate all draft files. I can attach it too, if needed.

          Show
          Julien Boulen added a comment - I attach our patch to improve this issue. We also have a script to validate all draft files. I can attach it too, if needed.
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that and providing a solution.

          I'm going to put this in as a bug because I think we probably should not be including drafts in that download. It's also more likely to get more attention as a bug.

          Show
          Michael de Raadt added a comment - Thanks for reporting that and providing a solution. I'm going to put this in as a bug because I think we probably should not be including drafts in that download. It's also more likely to get more attention as a bug.
          Hide
          Rajesh Taneja added a comment -

          Thanks for the patch Julien,

          I have modified the check to see if submission is finalised, rather then hard-coding the check.

          Pushing it for peer-review.

          FYI:
          This is not the case in new assignment type, as there is no draft status, probably something to consider.

          Show
          Rajesh Taneja added a comment - Thanks for the patch Julien, I have modified the check to see if submission is finalised, rather then hard-coding the check. Pushing it for peer-review. FYI: This is not the case in new assignment type, as there is no draft status, probably something to consider.
          Hide
          Frédéric Massart added a comment -

          Hi Raj, Julien,

          the patch looks good, but here are my thoughts:

          1. Are we sure that the teachers will never want to download drafts? Some might be interested to see where the students are at.
          2. The link to download as zip is shown as soon as there are submissions. When there are only draft submissions, clicking on the link will result in an error telling that there are no submissions. This could be confusing for a teacher which sees submissions and don't think of a difference between drafts and submitted.
          3. In relation with the previous point, the teacher not finding some submissions in the zip (because they're drafts) could be confused as well.

          IMO, there should be a more explicit way of excluding drafts. Maybe having two links ('Download all', 'Download final submission only').

          I'll leave it up to your thoughts Raj. And if you find this irrelevant, please feel free to push if forward.

          Cheers!

          Show
          Frédéric Massart added a comment - Hi Raj, Julien, the patch looks good, but here are my thoughts: Are we sure that the teachers will never want to download drafts? Some might be interested to see where the students are at. The link to download as zip is shown as soon as there are submissions. When there are only draft submissions, clicking on the link will result in an error telling that there are no submissions. This could be confusing for a teacher which sees submissions and don't think of a difference between drafts and submitted. In relation with the previous point, the teacher not finding some submissions in the zip (because they're drafts) could be confused as well. IMO, there should be a more explicit way of excluding drafts. Maybe having two links ('Download all', 'Download final submission only'). I'll leave it up to your thoughts Raj. And if you find this irrelevant, please feel free to push if forward. Cheers!
          Hide
          Rajesh Taneja added a comment -

          I agree with you Fred, but Julien's point is valid as well.

          IMO Fred has a valid point, because this being a legacy behaviour and files visible on page (draft and submitted files), user can expect them to be in zip.

          Pushing it forward to get final words from Integrators.

          Show
          Rajesh Taneja added a comment - I agree with you Fred, but Julien's point is valid as well. IMO Fred has a valid point, because this being a legacy behaviour and files visible on page (draft and submitted files), user can expect them to be in zip. Pushing it forward to get final words from Integrators.
          Hide
          Julien Boulen added a comment -

          Hi,

          And thanks for patching. =))

          About Frédéric's comment I'm not agree with his part 1 (it's little bit strange to download/to evaluate draft files, it isn't ?), but you are probably right when you propose 2 links (maybe 3 links: draft, final and both ?).

          Here, we have also a link to validate all submissions (draft to final), especially when teachers set a time limit. Then, they can download all files.

          Show
          Julien Boulen added a comment - Hi, And thanks for patching. =)) About Frédéric's comment I'm not agree with his part 1 (it's little bit strange to download/to evaluate draft files, it isn't ?), but you are probably right when you propose 2 links (maybe 3 links: draft, final and both ?). Here, we have also a link to validate all submissions (draft to final), especially when teachers set a time limit. Then, they can download all files.
          Hide
          Rajesh Taneja added a comment -

          Thanks for the inputs Julien,

          I am not sure about two links, as this might create confusion for teachers.
          Also, this being a change in behaviour, should land in master only.

          Show
          Rajesh Taneja added a comment - Thanks for the inputs Julien, I am not sure about two links, as this might create confusion for teachers. Also, this being a change in behaviour, should land in master only.
          Hide
          Michael de Raadt added a comment -

          Thanks for looking at this, Fred.

          I agree with your second and third point. We should not offer the download link until there is something to download.

          As for whether teachers will want to download draft files, I think not. Teachers are downloading students submissions as a zip for offline marking. If we offer the all files in the zip, we are removing the distinction between fully submitted files and drafts. The only other alternative would be to somehow make it obvious which files are draft submissions within the zip folder structure, but even that may be bad as we would be forcing teachers to download files they won't want to mark.

          Teachers will still be able to access draft files from the Web interface if they need to.

          I think a single, simple link is appropriate. Let's keep it simple..

          Show
          Michael de Raadt added a comment - Thanks for looking at this, Fred. I agree with your second and third point. We should not offer the download link until there is something to download. As for whether teachers will want to download draft files, I think not. Teachers are downloading students submissions as a zip for offline marking. If we offer the all files in the zip, we are removing the distinction between fully submitted files and drafts. The only other alternative would be to somehow make it obvious which files are draft submissions within the zip folder structure, but even that may be bad as we would be forcing teachers to download files they won't want to mark. Teachers will still be able to access draft files from the Web interface if they need to. I think a single, simple link is appropriate. Let's keep it simple..
          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
          Rajesh Taneja added a comment -

          Branches re-based.

          Show
          Rajesh Taneja added a comment - Branches re-based.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi, just noted that, following the code, that draft zipping is only prevented if the assignment is "open". That directly implies that, once the assignment is not open anymore... both final and draft files will be included in the zip.

          Just to be in the safe side...is that behavior the expected one?

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi, just noted that, following the code, that draft zipping is only prevented if the assignment is "open". That directly implies that, once the assignment is not open anymore... both final and draft files will be included in the zip. Just to be in the safe side...is that behavior the expected one? Ciao
          Hide
          Rajesh Taneja added a comment -

          Yes Eloy,
          If Assignment is open then only user can edit draft files, hence they should not be downloaded for marking. Once assignment is closed, all assignment files should be downloaded.

          Show
          Rajesh Taneja added a comment - Yes Eloy, If Assignment is open then only user can edit draft files, hence they should not be downloaded for marking. Once assignment is closed, all assignment files should be downloaded.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (22, 23 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 & master), thanks!
          Hide
          Adrian Greeve added a comment -

          Tested in 2.2, 2.3 and master.
          Draft files are no longer included in the zip files.
          No problems found.
          Test passed.

          Show
          Adrian Greeve added a comment - Tested in 2.2, 2.3 and master. Draft files are no longer included in the zip files. No problems found. Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm so proud...of you, many thanks!

          http://youtu.be/n64CdfDRnZY

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I'm so proud...of you, many thanks! http://youtu.be/n64CdfDRnZY Closing as fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: