Moodle
  1. Moodle
  2. MDL-27236

Filepicker, Server files, Improve tree view (Useability)

    Details

    • Testing Instructions:
      Hide

      1. Login as admin, create Folder resource, access filepicker from both Add Files and TinyMCE insert image and video.
      2. Observe the directory structure in Server Files. Make sure that navigation is easy, folders are not empty and number of subfolders is not too big
      3. Login as teacher and do the same. Make sure you can see only courses where you are a teacher, that you don't see course categories

      Note: if you login as a student, the Server Files repository will be empty. There is a separate issue MDL-32058 to hide repository from students.
      Also repository may appear empty if there are no available files with the accepted filetypes

      Show
      1. Login as admin, create Folder resource, access filepicker from both Add Files and TinyMCE insert image and video. 2. Observe the directory structure in Server Files. Make sure that navigation is easy, folders are not empty and number of subfolders is not too big 3. Login as teacher and do the same. Make sure you can see only courses where you are a teacher, that you don't see course categories Note: if you login as a student, the Server Files repository will be empty. There is a separate issue MDL-32058 to hide repository from students. Also repository may appear empty if there are no available files with the accepted filetypes
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-MDL-27236-master
    • Rank:
      16914

      Description

      In the Filepicker (Server files) the tree view is quite complex.
      There are at least two layers that may be able to be removed.
      See the accompanying image.

      Options could be:
      Course>Files
      Course>sections>files
      Course>Activity/Resource>files

      In the second image I've highlighted a few divisions that really seem superfluous

      1. smurf.xml
        1 kB
        Dan Poltawski
      1. Moodle.org Filepicker Tree view.png
        28 kB
      2. Moodle.org System Folders.png
        19 kB

        Activity

        Hide
        Derek Chirnside added a comment -

        It seems to me that if these files are in a database, they they should be able to be displayed easily in a range of ways: alphabetically, reverse chronological, by section, by file type, by folder they are in.
        Is there any coder who can comment?

        -Derek

        Show
        Derek Chirnside added a comment - It seems to me that if these files are in a database, they they should be able to be displayed easily in a range of ways: alphabetically, reverse chronological, by section, by file type, by folder they are in. Is there any coder who can comment? -Derek
        Hide
        Marina Glancy added a comment -

        Also in this issue I removed Private files from Server files (no need to duplicate another repository).

        Created issue MDL-32058 that will allow to hide 'Server files' repository for students, since they will not see anything there anyway

        Show
        Marina Glancy added a comment - Also in this issue I removed Private files from Server files (no need to duplicate another repository). Created issue MDL-32058 that will allow to hide 'Server files' repository for students, since they will not see anything there anyway
        Hide
        Marina Glancy added a comment -

        MDL-27236: Server files, improve tree view

        • Removed 'Private files' from 'Server files' repository;
        • Show only non-empty directories (taking into account filetype filter);
        • If there is only one non-empty filearea within a module, do not show it and skip the extra subdirectory level;
        • If the user is not admin (does not have 'moodle/course:update' capability in system context), do not show course categories, just list available courses;
        • Also when retrieving the course files capability to managefiles is checked before retrieving the modules list for performance tuning on sites with a lot of courses

        Derek, please note that 'Images' in your example is the directory you created in Folder resource. It will not be skipped from the tree because that may confuse teachers.

        Show
        Marina Glancy added a comment - MDL-27236 : Server files, improve tree view Removed 'Private files' from 'Server files' repository; Show only non-empty directories (taking into account filetype filter); If there is only one non-empty filearea within a module, do not show it and skip the extra subdirectory level; If the user is not admin (does not have 'moodle/course:update' capability in system context), do not show course categories, just list available courses; Also when retrieving the course files capability to managefiles is checked before retrieving the modules list for performance tuning on sites with a lot of courses Derek, please note that 'Images' in your example is the directory you created in Folder resource. It will not be skipped from the tree because that may confuse teachers.
        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
        Dan Poltawski added a comment -

        Hi Marina,

        Unfortunately this is failing some phpdoc checks as seen in the smurf file

        I also think $cached_files shouldn't have an underscore http://docs.moodle.org/dev/Coding_style#Variables

        I have a question on the change - did you look at the memory implications? From the looks of the caching I expect you are reducing the amount of work. But i've previously done a 'blind' bugfix in this area [ MDL-28605 ] as a site I was looking after was running out of memory when using this repository.. So would be interesting to know if you discovered anything while working on it.

        thanks,

        dan

        Show
        Dan Poltawski added a comment - Hi Marina, Unfortunately this is failing some phpdoc checks as seen in the smurf file Variable repository_local_ file::$cached_files is not documented Phpdocs for function repository_local_ file::__construct has incomplete parameters list Phpdocs for function repository_local_ file::retrieve_file_info has incomplete parameters list I also think $cached_files shouldn't have an underscore http://docs.moodle.org/dev/Coding_style#Variables I have a question on the change - did you look at the memory implications? From the looks of the caching I expect you are reducing the amount of work. But i've previously done a 'blind' bugfix in this area [ MDL-28605 ] as a site I was looking after was running out of memory when using this repository.. So would be interesting to know if you discovered anything while working on it. thanks, dan
        Hide
        Marina Glancy added a comment -

        Hi Dan,
        I fixed phpdocs and var name, thanks.
        Re memory implications - I don't expect it to be very big. There is mostly necessary information retrieved and stored in $cachedfiles. And I check if the element is empty, it means I inspect it's children UNTIL I meet non-empty child. Should not be a memory problem as well because in this case I'd only cache empty children (requiring not much memory).

        Show
        Marina Glancy added a comment - Hi Dan, I fixed phpdocs and var name, thanks. Re memory implications - I don't expect it to be very big. There is mostly necessary information retrieved and stored in $cachedfiles. And I check if the element is empty, it means I inspect it's children UNTIL I meet non-empty child. Should not be a memory problem as well because in this case I'd only cache empty children (requiring not much memory).
        Hide
        Dan Poltawski added a comment -

        Thanks for fixing that up Marina.

        This has been integrated now

        Show
        Dan Poltawski added a comment - Thanks for fixing that up Marina. This has been integrated now
        Hide
        Rajesh Taneja added a comment - - edited

        Works Great
        Thanks Marina, for improving filepicker.

        FYI: It is only integrated on master.

        Show
        Rajesh Taneja added a comment - - edited Works Great Thanks Marina, for improving filepicker. FYI: It is only integrated on master.
        Hide
        Sam Hemelryk added a comment -

        Congratulations are in order, you've made it, or at least your code has!
        It's now part of Moodle and both the git and cvs repositories have been updated.

        This issue is being marked as fixed and closed.

        Thank you.

        Show
        Sam Hemelryk added a comment - Congratulations are in order, you've made it, or at least your code has! It's now part of Moodle and both the git and cvs repositories have been updated. This issue is being marked as fixed and closed. Thank you.
        Hide
        David Mudrak added a comment -

        The so called improvement "If there is only one non-empty filearea within a module, do not show it and skip the extra subdirectory level;" is probably causing a regression in the workshop module. I have multiple fileareas there but if some of them contains just one itemid, it is automatically skipped. This leads to information loss because the item name (the name of the submission's author in case of workshop) is not displayed. This is difficult to reproduce now because there are other bugs that I have fixed on my machine. But I am open to discuss this or report as a separate issue.

        Show
        David Mudrak added a comment - The so called improvement "If there is only one non-empty filearea within a module, do not show it and skip the extra subdirectory level;" is probably causing a regression in the workshop module. I have multiple fileareas there but if some of them contains just one itemid, it is automatically skipped. This leads to information loss because the item name (the name of the submission's author in case of workshop) is not displayed. This is difficult to reproduce now because there are other bugs that I have fixed on my machine. But I am open to discuss this or report as a separate issue.
        Hide
        Stuart Lamour added a comment - - edited

        Some notes on drag & drop -
        What drag & drop upload does is remove some of the steps most people don't use in the upload workflow.

        The actual interaction of uploading via drag & drop is not in itself necessarily the thing most people like about it, but the fact that it reduces the number of clicks and form elements that need to be tackled to do what should be a simple task.

        I'm unsure from the wireframes if this addresses the issues of http://docs.moodle.org/dev/Files_usability_2.3#Solution_A2:_Add_.22quick_upload.22_buttons_that_bring_up_dialog_as_quickly_as_possible - but will be looking at it with interest to see if it does.

        Show
        Stuart Lamour added a comment - - edited Some notes on drag & drop - What drag & drop upload does is remove some of the steps most people don't use in the upload workflow. The actual interaction of uploading via drag & drop is not in itself necessarily the thing most people like about it, but the fact that it reduces the number of clicks and form elements that need to be tackled to do what should be a simple task. I'm unsure from the wireframes if this addresses the issues of http://docs.moodle.org/dev/Files_usability_2.3#Solution_A2:_Add_.22quick_upload.22_buttons_that_bring_up_dialog_as_quickly_as_possible - but will be looking at it with interest to see if it does.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: