Moodle
  1. Moodle
  2. MDL-31220

Deleting Assignment submissions isn't adequately logged

    Details

    • Testing Instructions:
      Hide

      Create assignment
      Set Maximum number of uploaded files = 2
      (NB Listed logs created excludes 'view' type logs)

      Login as student
      Add file test1.txt
      Logs created:
      assign submit Submission status: Submitted for grading. The number of file(s) : 1 file(s).
      assign file add Add: /test1.txt (dba7673010f19a94af4345453005933fd511bea9)

      Add file test2.txt
      Logs created:
      assign submit Submission status: Submitted for grading. The number of file(s) : 2 file(s).
      assign add file Add: /test2.txt (9054fbe0b622c638224d50d20824d2ff6782e308)

      Delete file test1.txt
      Edit file test1.txt
      Add file test1.txt
      Logs created:
      assign add file Add: /test1.txt (aa6de490c7f3168cceebf533a08ea0f0a8a5c7fe)
      assign delete file Delete: /test1.txt (dba7673010f19a94af4345453005933fd511bea9)
      assign submit Submission status: Submitted for grading. The number of file(s) : 2 file(s).

      Update license for file test1.txt
      Delete file test2.txt
      Add file test3.txt
      Logs created:
      assign add file Add: /test3.txt (41c5985fc771b6ecfe8feaa99f8fa9b77ac7d6ce)
      assign update file Update: /test1.txt (aa6de490c7f3168cceebf533a08ea0f0a8a5c7fe)
      assign delete file Delete: /test2.txt (9054fbe0b622c638224d50d20824d2ff6782e308)
      assign submit Submission status: Submitted for grading. The number of file(s) : 2 file(s).

      Delete all the files
      Logs created:
      assign submit update (empty) Submission status: Submitted for grading. The number of file(s) : 0 file(s).
      assign delete file Delete: /test3.txt (41c5985fc771b6ecfe8feaa99f8fa9b77ac7d6ce)
      assign delete file Delete: /test1.txt (aa6de490c7f3168cceebf533a08ea0f0a8a5c7fe)

      Show
      Create assignment Set Maximum number of uploaded files = 2 (NB Listed logs created excludes 'view' type logs) Login as student Add file test1.txt Logs created: assign submit Submission status: Submitted for grading. The number of file(s) : 1 file(s). assign file add Add: /test1.txt (dba7673010f19a94af4345453005933fd511bea9) Add file test2.txt Logs created: assign submit Submission status: Submitted for grading. The number of file(s) : 2 file(s). assign add file Add: /test2.txt (9054fbe0b622c638224d50d20824d2ff6782e308) Delete file test1.txt Edit file test1.txt Add file test1.txt Logs created: assign add file Add: /test1.txt (aa6de490c7f3168cceebf533a08ea0f0a8a5c7fe) assign delete file Delete: /test1.txt (dba7673010f19a94af4345453005933fd511bea9) assign submit Submission status: Submitted for grading. The number of file(s) : 2 file(s). Update license for file test1.txt Delete file test2.txt Add file test3.txt Logs created: assign add file Add: /test3.txt (41c5985fc771b6ecfe8feaa99f8fa9b77ac7d6ce) assign update file Update: /test1.txt (aa6de490c7f3168cceebf533a08ea0f0a8a5c7fe) assign delete file Delete: /test2.txt (9054fbe0b622c638224d50d20824d2ff6782e308) assign submit Submission status: Submitted for grading. The number of file(s) : 2 file(s). Delete all the files Logs created: assign submit update (empty) Submission status: Submitted for grading. The number of file(s) : 0 file(s). assign delete file Delete: /test3.txt (41c5985fc771b6ecfe8feaa99f8fa9b77ac7d6ce) assign delete file Delete: /test1.txt (aa6de490c7f3168cceebf533a08ea0f0a8a5c7fe)
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_27_STABLE
    • Pull Master Branch:
      MDL-31220-logging-assignment-submissions
    • Rank (Obsolete):
      37713

      Description

      Whilst investigating an issue where a user had deleted their submission, we discovered that this is inadequately logged.
      If a user deletes a file attachment, then saves changes, it is logged as 'assignment upload'.

      To Reproduce

      • Create a new upload assignment
      • Log in as a user on the course
      • Upload a submission
      • Save Changes
      • View the Assignment again
      • Choose 'Edit these files'
      • From the dropdown menu choose 'Delete' and confirm
      • Choose 'Save Changes'
      • Log in as administrator
      • View the logs for the user
      • Notice that the user has two log entries for 'assignment upload' but in reality the assignment files were removed.

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for reporting this.

        Show
        Michael de Raadt added a comment - Thanks for reporting this.
        Hide
        Lewis Carr added a comment -

        Agreed.
        This is a huge issue at UK organisations.
        Students can cheat the system by uploading a file before deadline so that it registers on the system, then then delete it. The logs don't show this has happened and then they can say Moodle "lost2 their submission.

        A log entry needs adding when a user deletes a file.

        Show
        Lewis Carr added a comment - Agreed. This is a huge issue at UK organisations. Students can cheat the system by uploading a file before deadline so that it registers on the system, then then delete it. The logs don't show this has happened and then they can say Moodle "lost2 their submission. A log entry needs adding when a user deletes a file.
        Hide
        Dan Poltawski added a comment -

        Is it still an issue in the new Assignment module? If not, I suggest closing this wont fix.

        Show
        Dan Poltawski added a comment - Is it still an issue in the new Assignment module? If not, I suggest closing this wont fix.
        Hide
        Andrew Nicols added a comment -

        Just confirmed with mod_assign on master - still an issue and a major concern for us.

        More of a concern however, is that the original files that were uploaded are subsequently deleted, and no reference remains. This means that it's very difficult to view and retrieve past copies of files. We have to do this periodically for one reason or another. Even with the $CFG->fileslastcleanup = time(); workaround in place, the file references are removed from the files table.

        Show
        Andrew Nicols added a comment - Just confirmed with mod_assign on master - still an issue and a major concern for us. More of a concern however, is that the original files that were uploaded are subsequently deleted, and no reference remains. This means that it's very difficult to view and retrieve past copies of files. We have to do this periodically for one reason or another. Even with the $CFG->fileslastcleanup = time(); workaround in place, the file references are removed from the files table.
        Hide
        Andrew Nicols added a comment -

        Sorry, just to confirm - my screenshot showed the original file being replaced with another.
        Deleting a file is still unlogged too though.

        Show
        Andrew Nicols added a comment - Sorry, just to confirm - my screenshot showed the original file being replaced with another. Deleting a file is still unlogged too though.
        Hide
        Lewis Carr added a comment -

        Even on the new assignment types (2.4+), there is no log of an assignment deletion.
        The log acton for a submission reads:
        "assign submit"

        But if a user deletes their submitted file and saves the page then the log reads
        "assign view submit assignment form"

        So it appears as though they simply viewed their assignment and no record of a deletion.
        Even if the log showed an edit that may help.
        I have noticed that "update" option is not reflected in the logs too.

        Show
        Lewis Carr added a comment - Even on the new assignment types (2.4+), there is no log of an assignment deletion. The log acton for a submission reads: "assign submit" But if a user deletes their submitted file and saves the page then the log reads "assign view submit assignment form" So it appears as though they simply viewed their assignment and no record of a deletion. Even if the log showed an edit that may help. I have noticed that "update" option is not reflected in the logs too.
        Hide
        Lewis Carr added a comment -

        Also, it still shows as a submission in the assignment reports with the timestamp when the user uploaded a file. And it also counts as a submission.

        For example, let's say 1 student uploads a file submission. The reports reads "No of submissions: 1".
        If then the student deletes the file the number of assignment submissions still shows 1.
        When you go to grade the assignment the timestamp is there indicating a successful submission but no file.

        Show
        Lewis Carr added a comment - Also, it still shows as a submission in the assignment reports with the timestamp when the user uploaded a file. And it also counts as a submission. For example, let's say 1 student uploads a file submission. The reports reads "No of submissions: 1". If then the student deletes the file the number of assignment submissions still shows 1. When you go to grade the assignment the timestamp is there indicating a successful submission but no file.
        Hide
        Amanda Doughty added a comment -

        I have been looking at options for additional logging. The function file_save_draft_area_files in lib/filelib.php is the function that differentiates between new, deleted and updated files. My current thinking is to add event triggers in file_save_draft_area_files for each of these, and then to catch them in mod/assign where they will be used to create log records.

        Does that sound sensible to you Andrew or have you already looked at a solution?

        Show
        Amanda Doughty added a comment - I have been looking at options for additional logging. The function file_save_draft_area_files in lib/filelib.php is the function that differentiates between new, deleted and updated files. My current thinking is to add event triggers in file_save_draft_area_files for each of these, and then to catch them in mod/assign where they will be used to create log records. Does that sound sensible to you Andrew or have you already looked at a solution?
        Hide
        Lewis Carr added a comment -

        Amanda that sounds plausible.
        Are you thinking MySQL triggers or PHP function triggers?
        I think a MySQL trigger would work best taking the filename, datemodified, userid and assignment ID.
        We could therefore create an assignment log table.

        I have some time next week to explore this, I'll post back my findings.

        Show
        Lewis Carr added a comment - Amanda that sounds plausible. Are you thinking MySQL triggers or PHP function triggers? I think a MySQL trigger would work best taking the filename, datemodified, userid and assignment ID. We could therefore create an assignment log table. I have some time next week to explore this, I'll post back my findings.
        Hide
        Andrew Nicols added a comment -

        Hi Amanda Doughty, that sounds pretty plausible.
        I wonder how realistic it would be to include the contenthash in the log message so that it can be found again from trashdir if required. The downside is that it would be displayed to users, but it may be worth it for the extra information that could be used for verification. Have you read the new events system API - may be worth pinging devchat to see what people think of the events suggestion. Damyon Wiese may have some useful thoughts here too.

        Lewis Carr Amanda is talking about the Moodle Events system. I don't believe that we support DB triggers. Remember that MySQL is not the only DB we support.

        Show
        Andrew Nicols added a comment - Hi Amanda Doughty , that sounds pretty plausible. I wonder how realistic it would be to include the contenthash in the log message so that it can be found again from trashdir if required. The downside is that it would be displayed to users, but it may be worth it for the extra information that could be used for verification. Have you read the new events system API - may be worth pinging devchat to see what people think of the events suggestion. Damyon Wiese may have some useful thoughts here too. Lewis Carr Amanda is talking about the Moodle Events system. I don't believe that we support DB triggers. Remember that MySQL is not the only DB we support.
        Hide
        Amanda Doughty added a comment -

        Hi Andrew Nicols, I have added comments to the Events API forum discussion (https://moodle.org/mod/forum/discuss.php?d=229425#p1000651). I do not have access to devchat and I can find no documentation on how to request it?

        Show
        Amanda Doughty added a comment - Hi Andrew Nicols, I have added comments to the Events API forum discussion ( https://moodle.org/mod/forum/discuss.php?d=229425#p1000651 ). I do not have access to devchat and I can find no documentation on how to request it?
        Hide
        Amanda Doughty added a comment -

        Andrew, could you look this over before I submit for peer review?

        Show
        Amanda Doughty added a comment - Andrew, could you look this over before I submit for peer review?
        Hide
        Andrew Nicols added a comment -

        Hi Amanda,

        I really like the way that this works on the whole. It demonstrates reasonably well how we could add logging with the new Events API

        You're accessing $COURSE->id in file_save_draft_area_files() - how would this perform in a non-course context (e.g. private files)? Same goes for a non-module context (same example) with $modulecontext->instanceid.

        Trailing whitespace on line 837 of lib/filelib.php

        There are a few bits of coding guidelines not quite right - nothing major, just missing punctuation at the end of a line of comment. Also, generally the use of the short if form is discouraged as it's a pain to read

        Show
        Andrew Nicols added a comment - Hi Amanda, I really like the way that this works on the whole. It demonstrates reasonably well how we could add logging with the new Events API You're accessing $COURSE->id in file_save_draft_area_files() - how would this perform in a non-course context (e.g. private files)? Same goes for a non-module context (same example) with $modulecontext->instanceid. Trailing whitespace on line 837 of lib/filelib.php There are a few bits of coding guidelines not quite right - nothing major, just missing punctuation at the end of a line of comment. Also, generally the use of the short if form is discouraged as it's a pain to read
        Hide
        Amanda Doughty added a comment -

        Thanks, I'll look at those issues. I have also missed out some logic in the event handler for checking it is an assignment submission.

        Show
        Amanda Doughty added a comment - Thanks, I'll look at those issues. I have also missed out some logic in the event handler for checking it is an assignment submission.
        Hide
        Amanda Doughty added a comment -

        I've made most of the changes suggested. I just need to test the $COURSE issue and add some punctuation. Most of the existing comments have no punctuation. Should I edit all of them?

        Show
        Amanda Doughty added a comment - I've made most of the changes suggested. I just need to test the $COURSE issue and add some punctuation. Most of the existing comments have no punctuation. Should I edit all of them?
        Hide
        Andrew Nicols added a comment -

        Nope - generally we try not to make that kind of change unless the line is already going to be modified. Doing so makes it harder to track back the history.

        Show
        Andrew Nicols added a comment - Nope - generally we try not to make that kind of change unless the line is already going to be modified. Doing so makes it harder to track back the history.
        Hide
        Damyon Wiese added a comment -

        Hi - my opinion on this is that the assignment update event just records the list of files attached to this submission update and does not try and workout if which files have been added or deleted.

        For people on 2.5 - the attempt history feature helps here because students can't change their old submission attempts - so you can actually go and look at the files for each attempt.

        For something simpler - just change the "format_for_log" function in "mod/assign/submission/file/locallib.php" to return the names/hashes of all the files in the current submission.

        Show
        Damyon Wiese added a comment - Hi - my opinion on this is that the assignment update event just records the list of files attached to this submission update and does not try and workout if which files have been added or deleted. For people on 2.5 - the attempt history feature helps here because students can't change their old submission attempts - so you can actually go and look at the files for each attempt. For something simpler - just change the "format_for_log" function in "mod/assign/submission/file/locallib.php" to return the names/hashes of all the files in the current submission.
        Hide
        Amanda Doughty added a comment -

        Thank you Damyon. That's really constructive. I'll take a look.

        Show
        Amanda Doughty added a comment - Thank you Damyon. That's really constructive. I'll take a look.
        Hide
        Damyon Wiese added a comment -

        This is what is logged in master:

        The user with id '2' updated a file submission and uploaded '0' file/s in the assignment with the course module id '281'.

        I think this covers this issue - so I will close it as fixed in 27

        Show
        Damyon Wiese added a comment - This is what is logged in master: The user with id '2' updated a file submission and uploaded '0' file/s in the assignment with the course module id '281'. I think this covers this issue - so I will close it as fixed in 27

          People

          • Votes:
            5 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: