Moodle
  1. Moodle
  2. MDL-31515

Updating a file in a database activity only works once

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide

      Setup

      • Create a new database instance
      • Add a file field
      • Set the default template

      Testing

      • Upload a file and save
        • Confirm that the file has the correct name and is the same file uploaded
      • Edit the record, and upload a new (different file)
        • Confirm that the file has the correct name and is the same file uploaded
      • Edit the record, and upload a new (different file)
        • Bug: The second file is shown
        • With Fix: The correct file is shown
      • Edit the record, and upload the first file again
        • Bug: The second file is shown
        • With Fix: The correct file is shown
      Show
      Setup Create a new database instance Add a file field Set the default template Testing Upload a file and save Confirm that the file has the correct name and is the same file uploaded Edit the record, and upload a new (different file) Confirm that the file has the correct name and is the same file uploaded Edit the record, and upload a new (different file) Bug: The second file is shown With Fix: The correct file is shown Edit the record, and upload the first file again Bug: The second file is shown With Fix: The correct file is shown
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-31515-master-1
    • Rank:
      38065

      Description

      With a database activity offering a 'file' field, you can update the field only once. If you add subsequent files, they are ignored by the interface and are never attached.

      It seems that when the filepicker was introduced, the loop over the updated records counts:

      $count = 0;
      foreach ($files as $draftfile) {
        // do some stuff
        $DB->update_record(...);
      
        if ($count > 0) {
          break;
        } else {
          $count++;
        }
      }
      

      This has the result that the first non-directory gets a count of 0, which is then increased to 1
      On the second file, the db is updated, but the count is now > 0, so we break.

      If a user has uploaded several files recently (or draft deletion is disabled), and then uploads a new file for the context, it will be at the end of the list, and so is ignored.

      I'm not sure as to the intention of this, so it's difficult to determine the best way of patching. My current thought is to change the $fs->get_area_files() to sort by timecreated DESC and break after the first update. I've uploaded a patch to this affect, and also adjusted some of the logic to make it only deal with files it's actually doing work with.

      If there's an alternative/preferred approach, I'm all ears!

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for working on that, Andrew.

          Show
          Michael de Raadt added a comment - Thanks for working on that, Andrew.
          Hide
          Rajesh Taneja added a comment -

          Thanks for spot-on patch Andrew
          Please feel free to push it for integration review.

          @integrator: This can be cherry-picked on 22 and 21.

          Show
          Rajesh Taneja added a comment - Thanks for spot-on patch Andrew Please feel free to push it for integration review. @integrator: This can be cherry-picked on 22 and 21.
          Hide
          Sam Hemelryk added a comment -

          Thanks Andrew, this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Andrew, this has been integrated now.
          Hide
          Andrew Davis added a comment -

          Works as expected.

          Show
          Andrew Davis added a comment - Works as expected.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          It is late here and I'm very tired but I didn't want to go to sleep before expressing my admiration for your amazing collaboration. Thanks!

          Closing as fixed, heading to zzzZZZzzz, niao

          Show
          Eloy Lafuente (stronk7) added a comment - It is late here and I'm very tired but I didn't want to go to sleep before expressing my admiration for your amazing collaboration. Thanks! Closing as fixed, heading to zzzZZZzzz, niao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: