Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-41907

Overwriting a file should maintain the original sortorder

    Details

    • Testing Instructions:
      Hide
      1. Create a file resource with two files, A and B.
      2. Set file A as the Main File - it should bold.
      3. Save and verify that file A is served.
      4. Edit the resource.
      5. Re-upload file A, when prompted, choose overwrite.
      6. Verify that A is still bolded.
      7. Save and verify that file A is still served.
      Show
      Create a file resource with two files, A and B. Set file A as the Main File - it should bold. Save and verify that file A is served. Edit the resource. Re-upload file A, when prompted, choose overwrite. Verify that A is still bolded. Save and verify that file A is still served.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:

      Description

      When overwriting a file, the sortorder field is lost. In particular, this causes the "Main File" setting for File Resources to be lost.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              emerrill Eric Merrill added a comment -

              I've added the patch, but I'm really not sure how to write unit test for it, and the existing tests covering this area seem to be light to nonexistent. I'm open to suggestions.

              I'm also not sure who should be pulled in to review or comment on this...

              Show
              emerrill Eric Merrill added a comment - I've added the patch, but I'm really not sure how to write unit test for it, and the existing tests covering this area seem to be light to nonexistent. I'm open to suggestions. I'm also not sure who should be pulled in to review or comment on this...
              Hide
              marina Marina Glancy added a comment -

              Hi Eric, patch looks very good.
              BTW we don't use the field 'sortorder' for anything else except main file in resource

              Show
              marina Marina Glancy added a comment - Hi Eric, patch looks very good. BTW we don't use the field 'sortorder' for anything else except main file in resource
              Hide
              marina Marina Glancy added a comment -

              There are no unittests in this area I'm afraid. It'd be too difficult to create new just for this small change, so I'm submitting this for integration for you.
              Thanks a lot for working on it

              Show
              marina Marina Glancy added a comment - There are no unittests in this area I'm afraid. It'd be too difficult to create new just for this small change, so I'm submitting this for integration for you. Thanks a lot for working on it
              Hide
              emerrill Eric Merrill added a comment -

              Thanks for the feedback and submission Marina!

              Show
              emerrill Eric Merrill added a comment - Thanks for the feedback and submission Marina!
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks Eric this has been integrated now.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Eric this has been integrated now.
              Hide
              dmonllao David Monllaó added a comment -

              It passes, tested in 24, 25 and master

              Show
              dmonllao David Monllaó added a comment - It passes, tested in 24, 25 and master
              Hide
              marina Marina Glancy added a comment -

              And THANK YOU again for making Moodle better every day!

              Another weekly release has been released.

              Show
              marina Marina Glancy added a comment - And THANK YOU again for making Moodle better every day! Another weekly release has been released.

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    11/Nov/13