Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Files API
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      All the pages where we have a simple"upload" to get import files should now use a filepicker:

      Please add any others to this list:

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              jerome Jérôme Mouneyrac added a comment -
              Show
              jerome Jérôme Mouneyrac added a comment - I added Course import ( http://docs.moodle.org/en/Community_hub#Import_course )
              Hide
              dongsheng Dongsheng Cai added a comment -

              Jerome, course import needs a new repository plugin, is it?

              Show
              dongsheng Dongsheng Cai added a comment - Jerome, course import needs a new repository plugin, is it?
              Hide
              dongsheng Dongsheng Cai added a comment -

              Petr,

              I created two funcitons: get_draft_files and delete_draft_files in formslib, get_draft_files will be used to read draft file info. Let me know if there is any problem.

              Thanks

              Show
              dongsheng Dongsheng Cai added a comment - Petr, I created two funcitons: get_draft_files and delete_draft_files in formslib, get_draft_files will be used to read draft file info. Let me know if there is any problem. Thanks
              Hide
              skodak Petr Skoda added a comment -

              hmm, this is exactly what I tried to prevent in the file api design, instead of reading the draft area you can simply get the file content or save the files somewhere. I agree we need some cleanup function for forms, but I think it should be something more general like $mform->dispose() which actually does all the necessary cleanup for all form elements and it should also clear the submitted flag in _POST.

              The problem is that the draft files would get discarded after the new proposed $mform->dispose().

              Why do you need this get_draft_files()? You just want to get one file, right? The correct way now is to do $mform->get_file_content($elname). As I said before the problem is that it is not suitable for large files, so you might need to actually use $mform->save_file() and store the content into temporary file.

              So in short:
              1/ instead of mform->delete_draft_files($elname) we need mform->dispose() imo
              2/ I do not think we need get_draft_files() when the necessary mform->get_file_content() and mform->save_file() already exists (it is compatible with old style upload and new file pickers btw)
              3/ +1 for the "|| $element instanceof MoodleQuickForm_filemanager"

              Petr

              Show
              skodak Petr Skoda added a comment - hmm, this is exactly what I tried to prevent in the file api design, instead of reading the draft area you can simply get the file content or save the files somewhere. I agree we need some cleanup function for forms, but I think it should be something more general like $mform->dispose() which actually does all the necessary cleanup for all form elements and it should also clear the submitted flag in _POST. The problem is that the draft files would get discarded after the new proposed $mform->dispose(). Why do you need this get_draft_files()? You just want to get one file, right? The correct way now is to do $mform->get_file_content($elname). As I said before the problem is that it is not suitable for large files, so you might need to actually use $mform->save_file() and store the content into temporary file. So in short: 1/ instead of mform->delete_draft_files($elname) we need mform->dispose() imo 2/ I do not think we need get_draft_files() when the necessary mform->get_file_content() and mform->save_file() already exists (it is compatible with old style upload and new file pickers btw) 3/ +1 for the "|| $element instanceof MoodleQuickForm_filemanager" Petr
              Hide
              dongsheng Dongsheng Cai added a comment -

              Hi, Petr,

              Thanks for review.

              I already used get_file_content for a few places, sometimes, if modules need file handler, save_file function will be used.

              The problem I have got here is question importing, it is trying to get the uploaded file name, in this case, get_file_content and save_file are not enough, I haven't looked into the question plugins yet, but this function can be useful when you want draft file info.

              Show
              dongsheng Dongsheng Cai added a comment - Hi, Petr, Thanks for review. I already used get_file_content for a few places, sometimes, if modules need file handler, save_file function will be used. The problem I have got here is question importing, it is trying to get the uploaded file name, in this case, get_file_content and save_file are not enough, I haven't looked into the question plugins yet, but this function can be useful when you want draft file info.
              Hide
              skodak Petr Skoda added a comment -

              You can use mform->get_new_filename() to get the file name, right? The mform->get_draft_files() is dangerous because it would encourage bad coding style in most of the cases. IMO we should not add it if there is no valid use case for it.

              Show
              skodak Petr Skoda added a comment - You can use mform->get_new_filename() to get the file name, right? The mform->get_draft_files() is dangerous because it would encourage bad coding style in most of the cases. IMO we should not add it if there is no valid use case for it.
              Hide
              dongsheng Dongsheng Cai added a comment -

              Thanks get_new_filename will solved the problem, I didn't notice this one.

              I will rename delete_draft_files and add "|| MoodleQuickForm_filemanager".

              Show
              dongsheng Dongsheng Cai added a comment - Thanks get_new_filename will solved the problem, I didn't notice this one. I will rename delete_draft_files and add "|| MoodleQuickForm_filemanager".
              Hide
              mudrd8mz David Mudrák added a comment -

              Hi Dong,

              I just realized that Outcomes import do not work at all, due to an exception thrown. As far as I can see, it is caused because the form submits into index.php which in turn includes import.php. In import.php, all environment (course, contexts, security) is repeated which leads to the exception (re-setting the course and therefore the theme).

              So I propose:
              1. either to copy the code from import.php directly into index.php
              2. or let the import form submit into import.php and after the successful process, redirect to index.php

              Show
              mudrd8mz David Mudrák added a comment - Hi Dong, I just realized that Outcomes import do not work at all, due to an exception thrown. As far as I can see, it is caused because the form submits into index.php which in turn includes import.php. In import.php, all environment (course, contexts, security) is repeated which leads to the exception (re-setting the course and therefore the theme). So I propose: 1. either to copy the code from import.php directly into index.php 2. or let the import form submit into import.php and after the successful process, redirect to index.php
              Hide
              dongsheng Dongsheng Cai added a comment -

              Hi, David
              Thanks, I am going to separate import form and index.php

              Show
              dongsheng Dongsheng Cai added a comment - Hi, David Thanks, I am going to separate import form and index.php
              Hide
              dongsheng Dongsheng Cai added a comment -

              the import course from hub will be tracked here: MDL-23148

              Show
              dongsheng Dongsheng Cai added a comment - the import course from hub will be tracked here: MDL-23148
              Hide
              dongsheng Dongsheng Cai added a comment -

              Please feel free to reopen it if you find more 'import' button needed to use filepicker

              Show
              dongsheng Dongsheng Cai added a comment - Please feel free to reopen it if you find more 'import' button needed to use filepicker

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    24/Nov/10