Moodle
  1. Moodle
  2. MDL-34607

Files added to a Folder do not sort in order if filename contains a number

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.3
    • Fix Version/s: 2.2.6, 2.3.3
    • Component/s: Files API, Resource
    • Labels:
    • Testing Instructions:
      Hide

      Test pre-requisites

      • A folder with files (play with names, or use the zip)
      • Contains a new upgrade query, should be tested against all 4 DB engines (for 2.3 and master)
      • Do NOT upgrade yet!

      Test for 2.2

      1. Create a file resource and add a file named '0.txt'
      2. Set the file as 'main file'
      3. Go to your folder, edit and click to add a file
      4. Browse the server files repository to pick the file you added in the file resource
      5. Save
      6. Make sure files are displayed in correct order

      Test for 2.3 and master

      1. Before upgrading your instance, hack your database to set a sortorder of 1 for some files in your folder resource.
      2. Upgrade and make sure all sortorder have been set to 0
      3. Go to your folder and make sure the file order is correct
      Show
      Test pre-requisites A folder with files (play with names, or use the zip) Contains a new upgrade query, should be tested against all 4 DB engines (for 2.3 and master) Do NOT upgrade yet! Test for 2.2 Create a file resource and add a file named '0.txt' Set the file as 'main file' Go to your folder, edit and click to add a file Browse the server files repository to pick the file you added in the file resource Save Make sure files are displayed in correct order Test for 2.3 and master Before upgrading your instance, hack your database to set a sortorder of 1 for some files in your folder resource. Upgrade and make sure all sortorder have been set to 0 Go to your folder and make sure the file order is correct
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34607-master
    • Rank:
      43042

      Description

      Would like to see files sort in order if numbers are added to the beginning of the file name. Currently they only show in order if editing mode is on. Please see screen shots attached.

      Replication steps:

      1. Create a folder in your course.
      2. Choose to add files to the folder. Make sure the file names begin with a number.

      In editing mode the files appear sorted correctly by the number in their name.

      Once editing is off the files no longer are sorted by the number in their name.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this.

          I've put that on the backlog.

          In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. I've put that on the backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.
          Hide
          Marina Glancy added a comment -

          I had a quick look at this issue and this is what I noticed.
          On 'edit mode' screenshot one can see that some files names are in bold, this means somebody once somehow set them as 'main files'. 'Main file' functionality is not applicable to folder resources. There could be only one main file for FILE resource.

          With the evolution of DB and code the 'main file' is stored in our DB as

          {files}

          .sortorder=1. For other files field sortorder=0 and it is actually not used for sorting files.

          BUT in file_storage::get_area_tree() we for some reason sort by sortorder. Which results in mess actually

          So I see following issues here:

          1. files can not be set as 'main file' in folder resource. This is already fixed in 2.3 but we might need an upgrade script that fixes the existing mess in DB before upgrade.
          2. files should not be sorted by 'sortorder' in file_storage. This is tricky because we need to check for regressions.
          3. as a wishlist file_storage::get_area_tree() should sort file names using collatorlib because DB sorting may fail with localized names
          Show
          Marina Glancy added a comment - I had a quick look at this issue and this is what I noticed. On 'edit mode' screenshot one can see that some files names are in bold, this means somebody once somehow set them as 'main files'. 'Main file' functionality is not applicable to folder resources. There could be only one main file for FILE resource. With the evolution of DB and code the 'main file' is stored in our DB as {files} .sortorder=1. For other files field sortorder=0 and it is actually not used for sorting files. BUT in file_storage::get_area_tree() we for some reason sort by sortorder. Which results in mess actually So I see following issues here: files can not be set as 'main file' in folder resource. This is already fixed in 2.3 but we might need an upgrade script that fixes the existing mess in DB before upgrade. files should not be sorted by 'sortorder' in file_storage. This is tricky because we need to check for regressions. as a wishlist file_storage::get_area_tree() should sort file names using collatorlib because DB sorting may fail with localized names
          Hide
          Marina Glancy added a comment -

          please note if teacher creates a folder and picks a file from 'Server files' from a file resource (where this file is marked as 'main'), the file will be bold (=marked as main) in the folder as well, and there is no UI in folder to change it. I remember fixing it in 2.3 but it was not backported.

          Show
          Marina Glancy added a comment - please note if teacher creates a folder and picks a file from 'Server files' from a file resource (where this file is marked as 'main'), the file will be bold (=marked as main) in the folder as well, and there is no UI in folder to change it. I remember fixing it in 2.3 but it was not backported.
          Hide
          Frédéric Massart added a comment -

          The main problem of this issue comes from files which have a sort order when they should not have any. File resources use sort order to set the main file but this is not supported by the folder component.

          Comments to reviewers:

          • All branches are different!
          • The 2.2 branch only contains a minor change, which was discussed with Marina to not care about the sortorder. This should not affect any other components.
          • The 2.3 branch contains an upgrade script which will fix the wrong sort order in folder resources.
          • The master branch adds some logic to sort the records independently of the DB engines, using collatorlib.
          • Adding more logic to 2.2 does not make much sense as everything is fixed in 2.3 onwards and would not be possible with a small patch.
          Show
          Frédéric Massart added a comment - The main problem of this issue comes from files which have a sort order when they should not have any. File resources use sort order to set the main file but this is not supported by the folder component. Comments to reviewers: All branches are different! The 2.2 branch only contains a minor change, which was discussed with Marina to not care about the sortorder. This should not affect any other components. The 2.3 branch contains an upgrade script which will fix the wrong sort order in folder resources. The master branch adds some logic to sort the records independently of the DB engines, using collatorlib. Adding more logic to 2.2 does not make much sense as everything is fixed in 2.3 onwards and would not be possible with a small patch.
          Hide
          Marina Glancy added a comment -

          Hi Fred,
          some DB do not understand != in SQL, you should use <> (in update query)
          I would also rename function order_area_tree into sort_area_tree but it's up to you
          overall looks fine for me
          Marina

          Show
          Marina Glancy added a comment - Hi Fred, some DB do not understand != in SQL, you should use <> (in update query) I would also rename function order_area_tree into sort_area_tree but it's up to you overall looks fine for me Marina
          Hide
          Frédéric Massart added a comment -

          Thanks Marina, I have amended my branches and am pushing for integration. Cheers!

          Show
          Frédéric Massart added a comment - Thanks Marina, I have amended my branches and am pushing for integration. Cheers!
          Hide
          Dan Poltawski 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
          Dan Poltawski 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
          Sam Hemelryk added a comment -

          Hi Fred,

          This has been integrated now.
          Noting that I had to make three changes during integration:

          1. You forgot the version bump in 23
          2. PHPDocs for sort_area_tree in master had a invalid @param
          3. Conflicts in master (to be expected)

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi Fred, This has been integrated now. Noting that I had to make three changes during integration: You forgot the version bump in 23 PHPDocs for sort_area_tree in master had a invalid @param Conflicts in master (to be expected) Many thanks Sam
          Hide
          Frédéric Massart added a comment -

          Oh, sorry about those! Thanks for fixing it Sam!

          Show
          Frédéric Massart added a comment - Oh, sorry about those! Thanks for fixing it Sam!
          Hide
          Rajesh Taneja added a comment -

          Thanks Fred,
          Works Great, sort order is reset to 0 for folder resource and files are visible in alphabetical order.

          Show
          Rajesh Taneja added a comment - Thanks Fred, Works Great, sort order is reset to 0 for folder resource and files are visible in alphabetical order.
          Hide
          Dan Poltawski added a comment -

          Congratulations, you've done it!

          Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc

          Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

          Show
          Dan Poltawski added a comment - Congratulations, you've done it! Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: