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

Unable to download recent file in file picker with JS turned off

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Filepicker
    • Labels:
    • Testing Instructions:
      Hide
      1. Turn off browser Javascript
      2. Add file resource a course
      3. In "select files" section, add a file from 'recent files'
      4. Choose 'download'

      Make sure there's no error display.

      Show
      Turn off browser Javascript Add file resource a course In "select files" section, add a file from 'recent files' Choose 'download' Make sure there's no error display.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Replication steps:

      1. Be sure to have JS turned off at browser and site/user level
      2. Go to course and add "resource > file"
      3. In "Select files" section choose "Add", then "Recent files"
      4. Choose a file listed and click "Select"
      5. select the download button, the following error message occurs:

      Notice: Undefined property: stdClass::$contextid in /moodle/lib/filestorage/file_storage.php on line 953

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              marina Marina Glancy added a comment -

              Rossie, I have just received another bug MDL-33335. Can you take a look please, it seems like the similar problem.

              Show
              marina Marina Glancy added a comment - Rossie, I have just received another bug MDL-33335 . Can you take a look please, it seems like the similar problem.
              Hide
              rwijaya Rossiani Wijaya added a comment -

              post patch.

              Please make sure MDL-33196 patch has been applied before using this patch.

              Sending for peer review. Ankit?

              Show
              rwijaya Rossiani Wijaya added a comment - post patch. Please make sure MDL-33196 patch has been applied before using this patch. Sending for peer review. Ankit?
              Hide
              nebgor Aparup Banerjee added a comment -

              looks good.
              -replication instructions could be rephrased into proper test instructions.

              • if you're generous, you could update (repository/filepicker.php) this old style

                $user_context = get_context_instance(CONTEXT_USER, $USER->id);


                to the class based call.

              Show
              nebgor Aparup Banerjee added a comment - looks good. -replication instructions could be rephrased into proper test instructions. if you're generous, you could update (repository/filepicker.php) this old style $user_context = get_context_instance(CONTEXT_USER, $USER->id); to the class based call.
              Hide
              marina Marina Glancy added a comment - - edited

              Hi Rossie,
              there was a change in the content of files.source field. It became a serialised object now. Apparently DS forgot to put those changes in non-js filepicker.
              In your patch please remove changes in lib/filelib.php and use in repository/filepicker.php:

              $record->source = serialize((object)array('source' => $thefile['url']));

              as it is done in repository/repository_ajax.php.
              Also please set all the other fields as well as repository_ajax.php does it: timecreated, timemodified, userid

              Show
              marina Marina Glancy added a comment - - edited Hi Rossie, there was a change in the content of files.source field. It became a serialised object now. Apparently DS forgot to put those changes in non-js filepicker. In your patch please remove changes in lib/filelib.php and use in repository/filepicker.php: $record->source = serialize((object)array('source' => $thefile['url'])); as it is done in repository/repository_ajax.php. Also please set all the other fields as well as repository_ajax.php does it: timecreated, timemodified, userid
              Hide
              marina Marina Glancy added a comment -

              Rossie,
              regarding your comment that when file is picked from recent files, the size is 0:

              I looked at the code and it appears that it calls the wrong function and picking files from any moodle repository (server files, course legacy files, private files and recent files) should not work. And also it does not work in 2.2 either.
              So, can you create another issue please (I did not check in 2.1, most likely it does not work there either).

              As for this issue, can you please add the following, so the strict standards are followed:

               
              diff --git a/repository/draftfiles_manager.php b/repository/draftfiles_manager.php
              index b3f1dde..0bbc3a3 100644
              --- a/repository/draftfiles_manager.php
              +++ b/repository/draftfiles_manager.php
              @@ -333,7 +333,8 @@ default:
               
                               $home_url->param('action', 'browse');
                               $home_url->param('draftpath', $file->filepath);
              -                $foldername = trim(array_pop(explode('/', trim($file->filepath, '/'))), '/');
              +                $filepathchunks = explode('/', trim($file->filepath, '/'));
              +                $foldername = trim(array_pop($filepathchunks), '/');
                               echo html_writer::link($home_url, $foldername);
               
                               $home_url->param('draftpath', $file->filepath);

              Show
              marina Marina Glancy added a comment - Rossie, regarding your comment that when file is picked from recent files, the size is 0: I looked at the code and it appears that it calls the wrong function and picking files from any moodle repository (server files, course legacy files, private files and recent files) should not work. And also it does not work in 2.2 either. So, can you create another issue please (I did not check in 2.1, most likely it does not work there either). As for this issue, can you please add the following, so the strict standards are followed:   diff --git a/repository/draftfiles_manager.php b/repository/draftfiles_manager.php index b3f1dde..0bbc3a3 100644 --- a/repository/draftfiles_manager.php +++ b/repository/draftfiles_manager.php @@ -333,7 +333,8 @@ default: $home_url->param('action', 'browse'); $home_url->param('draftpath', $file->filepath); - $foldername = trim(array_pop(explode('/', trim($file->filepath, '/'))), '/'); + $filepathchunks = explode('/', trim($file->filepath, '/')); + $foldername = trim(array_pop($filepathchunks), '/'); echo html_writer::link($home_url, $foldername); $home_url->param('draftpath', $file->filepath);
              Hide
              rwijaya Rossiani Wijaya added a comment -

              Hi Marina,

              Thank you for your feedback and patch.

              Note:
              There's a regression issue for using/downloading file from Moodle's repositories (local, user, recent, course files). In order to avoid a complex files conflicts, the proper fix to use file from these repositories, will be address on MDL-33473.

              Thanks.

              Show
              rwijaya Rossiani Wijaya added a comment - Hi Marina, Thank you for your feedback and patch. Note: There's a regression issue for using/downloading file from Moodle's repositories (local, user, recent, course files). In order to avoid a complex files conflicts, the proper fix to use file from these repositories, will be address on MDL-33473 . Thanks.
              Hide
              nebgor Aparup Banerjee added a comment -

              its great! sending up for integration

              ps: i've turned my strict standards back on now.

              Show
              nebgor Aparup Banerjee added a comment - its great! sending up for integration ps: i've turned my strict standards back on now.
              Hide
              poltawski Dan Poltawski added a comment -

              I'm not entirely clear about this source field thing. Linking MDL-33513

              Show
              poltawski Dan Poltawski added a comment - I'm not entirely clear about this source field thing. Linking MDL-33513
              Hide
              poltawski Dan Poltawski added a comment -

              I've integrated this now. As noted the source field is really confusing - but that is addressed in the other issue.

              Show
              poltawski Dan Poltawski added a comment - I've integrated this now. As noted the source field is really confusing - but that is addressed in the other issue.
              Hide
              poltawski Dan Poltawski added a comment -

              Tested and passed, thanks!

              Show
              poltawski Dan Poltawski added a comment - Tested and passed, thanks!
              Hide
              dougiamas Martin Dougiamas added a comment -

              Yep this is OK for now (fixing the serialised thing) but in MDL-33513 we are actually going to get rid of the serialised thing everywhere.

              I'll make a note there that this line here that serialises the source field should be UNDONE again.

              Show
              dougiamas Martin Dougiamas added a comment - Yep this is OK for now (fixing the serialised thing) but in MDL-33513 we are actually going to get rid of the serialised thing everywhere. I'll make a note there that this line here that serialises the source field should be UNDONE again.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

              Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

              Many thanks for your collaboration!

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    25/Jun/12