Moodle
  1. Moodle
  2. MDL-25631

Course Import does not import Legacy Course Files

    Details

    • Testing Instructions:
      Hide

      Using the legacy-file.mbz
      #Restore into a new course.
      #Confirm Legacy course files shows up, and contains Eris.jpg
      #Create a new course
      #Restore into current course - Overwrite course config: No
      #Confirm Legacy course files shows up, and contains Eris.jpg
      #Create a new course
      #Restore over current course (deleting first) - Overwrite course config: No
      #Confirm Legacy course files shows up, and contains Eris.jpg
      #Create a new course
      #Do an import from one of the previous courses.
      #Confirm Legacy course files shows up, and contains Eris.jpg

      Using the no-legacy-file.mbz
      #Restore into a new course.
      #Confirm Legacy course files does not shows up
      #Restore into current course - Overwrite course config: No
      #Confirm Legacy course files does not shows up
      #Restore over current course (deleting first) - Overwrite course config: No
      #Confirm Legacy course files does not shows up
      #Do an import from one of the previous courses without legacy files.
      #Confirm Legacy course files does not shows up

      Test on demand migration of imported course.

      1. Restore legacy-file.mbz into a new course.
      2. Create a new page activity in the course.
      3. Update the page activity in the database:
        update mdl_page set legacyfiles = 2, content='<img src="@@PLUGINFILE@@/Eris.jpg" />' where id = XXX;
        
      4. Do not look at the page activity - go directly to a different course
      5. Import the legacy-file course with the page activity.
      6. View the page activity and verify that the image is not broken.
      Show
      Using the legacy-file.mbz #Restore into a new course. #Confirm Legacy course files shows up, and contains Eris.jpg #Create a new course #Restore into current course - Overwrite course config: No #Confirm Legacy course files shows up, and contains Eris.jpg #Create a new course #Restore over current course (deleting first) - Overwrite course config: No #Confirm Legacy course files shows up, and contains Eris.jpg #Create a new course #Do an import from one of the previous courses. #Confirm Legacy course files shows up, and contains Eris.jpg Using the no-legacy-file.mbz #Restore into a new course. #Confirm Legacy course files does not shows up #Restore into current course - Overwrite course config: No #Confirm Legacy course files does not shows up #Restore over current course (deleting first) - Overwrite course config: No #Confirm Legacy course files does not shows up #Do an import from one of the previous courses without legacy files. #Confirm Legacy course files does not shows up Test on demand migration of imported course. Restore legacy-file.mbz into a new course. Create a new page activity in the course. Update the page activity in the database: update mdl_page set legacyfiles = 2, content='<img src= "@@PLUGINFILE@@/Eris.jpg" />' where id = XXX; Do not look at the page activity - go directly to a different course Import the legacy-file course with the page activity. View the page activity and verify that the image is not broken.
    • Workaround:
      Hide

      Instead of importing, make the backup of the required resources in the source course and restore it inside the target course. The backup / restore process (on contrary to the import process) does transfer the Legacy course files area so the on-demand-migration works.

      Show
      Instead of importing, make the backup of the required resources in the source course and restore it inside the target course. The backup / restore process (on contrary to the import process) does transfer the Legacy course files area so the on-demand-migration works.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-25631_m24
    • Pull Master Branch:
    • Rank:
      15081

      Description

      A course that is upgrade from 1.9 to 2.0 will contain a Legacy Course Files area which is necessary for the "on-the-fly" migration of files into a resource (for example, HTML mini sites). A backup/restore of such a course will correctly include the Legacy Course Files section. However, doing an Import of a course does not give the ability to migrate the Legacy Course Files. This means that resources end up with broken links because it can't continue the live migration as files are accessed.

      This is a significant issue since some users will rely heavily on the Import tool to move content from their upgraded 1.9 courses into new 2011 courses.

        Issue Links

          Activity

          Hide
          Richard Jones added a comment -

          This may be related - our site upgraded from 1.9 to 2.0 and now 2.02. Importing OR restoring an old course from the upgraded site with files referenced in html areas such as Moodle pages is inconsistent - not all files are successfully transferred or imported from the legacy files area to the server files area in the new course - some are, some are not. I tracked one such file that has an original link code ending:

          draft/427336261/Unit_2_FLASH/u5_10_analyze_flash_guide_a4.pdf in the original course but
          draft/858727418/Unit_2_FLASH/u5_10_analyze_flash_guide_a4.pdf after import.

          It exists in the old course legacy files area, but is not seen in the new course under server files. The file has multiple entries in the mdl_files table as you might expect.

          Some of our courses have been completely constructed in this way which means (as is said above) a great deal of work to fix on a file by file basis.

          Show
          Richard Jones added a comment - This may be related - our site upgraded from 1.9 to 2.0 and now 2.02. Importing OR restoring an old course from the upgraded site with files referenced in html areas such as Moodle pages is inconsistent - not all files are successfully transferred or imported from the legacy files area to the server files area in the new course - some are, some are not. I tracked one such file that has an original link code ending: draft/427336261/Unit_2_FLASH/u5_10_analyze_flash_guide_a4.pdf in the original course but draft/858727418/Unit_2_FLASH/u5_10_analyze_flash_guide_a4.pdf after import. It exists in the old course legacy files area, but is not seen in the new course under server files. The file has multiple entries in the mdl_files table as you might expect. Some of our courses have been completely constructed in this way which means (as is said above) a great deal of work to fix on a file by file basis.
          Hide
          Jason Ilicic added a comment -

          It looks like legacy course files are only restored if restoring to a new course or if the overwrite configuration option is set to yes. If the conditions are otherwise, then the process to import course legacy files are skipped.

          Show
          Jason Ilicic added a comment - It looks like legacy course files are only restored if restoring to a new course or if the overwrite configuration option is set to yes. If the conditions are otherwise, then the process to import course legacy files are skipped.
          Hide
          William Stites added a comment -

          @Jason - Where is the overwrite configuration option found?

          Show
          William Stites added a comment - @Jason - Where is the overwrite configuration option found?
          Hide
          William Stites added a comment -

          Found it in the Common Repository Settings.

          Show
          William Stites added a comment - Found it in the Common Repository Settings.
          Hide
          Daniel Falk added a comment -

          Even with that setting some links will not work due to the changes in the file api... What it seems we are going to have to do is relink all of the images to the legacy files directory for restored courses from 1.9 unless someone else has found an solution.

          Show
          Daniel Falk added a comment - Even with that setting some links will not work due to the changes in the file api... What it seems we are going to have to do is relink all of the images to the legacy files directory for restored courses from 1.9 unless someone else has found an solution.
          Hide
          Martin Dougiamas added a comment -

          David are you able to help by looking at this and working out the correct solution?

          Show
          Martin Dougiamas added a comment - David are you able to help by looking at this and working out the correct solution?
          Hide
          David Mudrak added a comment -

          I'm on it

          Show
          David Mudrak added a comment - I'm on it
          Hide
          David Mudrak added a comment -

          Confirmed. I am able to reproduce the bug with a bit of hacking.

          1. Create a course with Legacy files support enabled
          2. Add a Page resource without any images embedded
          3. Upload file "image.gif" to the Legacy course files
          4. Directly in the database, update the field "content" of the created Page record so that it contains link to a non-existing embedded file:

          UPDATE mdl_page SET content = '<img src="@@PLUGINFILE@@/image.gif" />' WHERE id = xxx;
          

          5. Do not open the Page now - the on-demand migration would migrate the image.git from legacy files to the page content area (if you do open the Page by accident, remove the relevant created records from mdl_files table)
          6. Create another course
          7. Import the Page from the first course

          Expected result:
          The referenced image.git should be migrated and embedded into the new page.

          What actually happens:
          Legacy files are not transferred to the new course so the on-demand-migration via resourcelib_try_file_migration() fails

          Show
          David Mudrak added a comment - Confirmed. I am able to reproduce the bug with a bit of hacking. 1. Create a course with Legacy files support enabled 2. Add a Page resource without any images embedded 3. Upload file "image.gif" to the Legacy course files 4. Directly in the database, update the field "content" of the created Page record so that it contains link to a non-existing embedded file: UPDATE mdl_page SET content = '<img src= "@@PLUGINFILE@@/image.gif" />' WHERE id = xxx; 5. Do not open the Page now - the on-demand migration would migrate the image.git from legacy files to the page content area (if you do open the Page by accident, remove the relevant created records from mdl_files table) 6. Create another course 7. Import the Page from the first course Expected result: The referenced image.git should be migrated and embedded into the new page. What actually happens: Legacy files are not transferred to the new course so the on-demand-migration via resourcelib_try_file_migration() fails
          Hide
          David Mudrak added a comment -

          Providing Workaround. Lowering priority - because the workaround solution is known and there is no real data-loss (the files are still there in the source location), this can't be cosidered as blocker or critical issue.

          I am continuing working on proper fix, indeed.

          Show
          David Mudrak added a comment - Providing Workaround. Lowering priority - because the workaround solution is known and there is no real data-loss (the files are still there in the source location), this can't be cosidered as blocker or critical issue. I am continuing working on proper fix, indeed.
          Hide
          David Mudrak added a comment -

          Hmm, this is actually pretty tricky. The solution does not seem to be any trivial because of the current architecture of moodle2 backup/restore scheme.

          • Just to remind, the import process is basically one backup process (with mode set to backup::MODE_IMPORT) followed by one restore process (see backup/import.php)
          • During the backup phase of the import, temporary backup structure of type backup::TYPE_1COURSE is created. This type of backup includes the legacy files described in files.xml.
          • During the restore phase of the import, the method restore_course_structure_step::after_execute() is responsible for restoring the legacy course files. But restore_course_task includes this restore_course_structure_step if and only if
            // Executed conditionally if restoring to new course or if overwrite_conf setting is enabled
            if ($this->get_target() == backup::TARGET_NEW_COURSE || $this->get_setting_value('overwrite_conf') == true) {
                $this->add_step(new restore_course_structure_step('course_info', 'course.xml'));
            }
            

            Of course, during the import neither we restore into a new course nor we overwrite the target course's configuration. So the restore_course_structure_step is not executed and and legacy course files are not restored.

          This has other implications, too. For example, if someone makes one activity backup (backup::TYPE_1ACTIVITY) of a resource that refers to a non-migrated file (by "non-migrated" I mean a file that is registered only in Legacy course files area and not in some resource's own area), that file is not be included in the backup.

          Possible solution might be to separate legacy files from the course structure step. That is, make legacy course files included in both 1COURSE and 1ACTIVITY backup types. And then, on restore, always restore the included Legacy files. But that implies a serious danger of overwriting current legacy course files, should they already exist in the target course.

          This needs to be discussed with Eloy and Petr at least.

          Show
          David Mudrak added a comment - Hmm, this is actually pretty tricky. The solution does not seem to be any trivial because of the current architecture of moodle2 backup/restore scheme. Just to remind, the import process is basically one backup process (with mode set to backup::MODE_IMPORT) followed by one restore process (see backup/import.php) During the backup phase of the import, temporary backup structure of type backup::TYPE_1COURSE is created. This type of backup includes the legacy files described in files.xml. During the restore phase of the import, the method restore_course_structure_step::after_execute() is responsible for restoring the legacy course files. But restore_course_task includes this restore_course_structure_step if and only if // Executed conditionally if restoring to new course or if overwrite_conf setting is enabled if ($ this ->get_target() == backup::TARGET_NEW_COURSE || $ this ->get_setting_value('overwrite_conf') == true ) { $ this ->add_step( new restore_course_structure_step('course_info', 'course.xml')); } Of course, during the import neither we restore into a new course nor we overwrite the target course's configuration. So the restore_course_structure_step is not executed and and legacy course files are not restored. This has other implications, too. For example, if someone makes one activity backup (backup::TYPE_1ACTIVITY) of a resource that refers to a non-migrated file (by "non-migrated" I mean a file that is registered only in Legacy course files area and not in some resource's own area), that file is not be included in the backup. Possible solution might be to separate legacy files from the course structure step. That is, make legacy course files included in both 1COURSE and 1ACTIVITY backup types. And then, on restore, always restore the included Legacy files. But that implies a serious danger of overwriting current legacy course files, should they already exist in the target course. This needs to be discussed with Eloy and Petr at least.
          Hide
          David Mudrak added a comment -

          Ping for Petr and Eloy (who is on holiday now). I would appreciate of you find some time and think about this VIP issue. TIA

          Show
          David Mudrak added a comment - Ping for Petr and Eloy (who is on holiday now). I would appreciate of you find some time and think about this VIP issue. TIA
          Hide
          Petr Škoda added a comment - - edited

          my +1 to not import legacy files because new activities should not use them and the existing activities should be gradually migrated. So instead of working on this we should imo create the migrations code for legacy files. But first we need a way to actually manage the new file areas in tinymce, ideally there should be also some two-pane file manager that works on top of file_browser abstraction.

          Show
          Petr Škoda added a comment - - edited my +1 to not import legacy files because new activities should not use them and the existing activities should be gradually migrated. So instead of working on this we should imo create the migrations code for legacy files. But first we need a way to actually manage the new file areas in tinymce, ideally there should be also some two-pane file manager that works on top of file_browser abstraction.
          Hide
          David Mudrak added a comment - - edited

          Thanks Petr. Yet another solution came to my mind. In 1.9, one can choose to include Course files during the import (with all consequences like eventual rewrite of existing files). Maybe we could introduce similar option for 2.x import - "Include Legacy course files". Of course, if and only if both courses have them enabled and there are some Legacy files present in the source course.

          Show
          David Mudrak added a comment - - edited Thanks Petr. Yet another solution came to my mind. In 1.9, one can choose to include Course files during the import (with all consequences like eventual rewrite of existing files). Maybe we could introduce similar option for 2.x import - "Include Legacy course files". Of course, if and only if both courses have them enabled and there are some Legacy files present in the source course.
          Hide
          Jason Ilicic added a comment -

          David, I had the same idea myself. Course legacy files should not need to be brought over all the time, but only if the option is chosen. Is it possible to create an extra process to import the legacy files to the new course? I understand that this file area is to be gradually phased out but some courses which have been migrated from Moodle 1.9 require all of these files to be re-uploaded in to the new file area in the relative activities - this is a lot of work for some as some courses contain hundreds of files.

          Show
          Jason Ilicic added a comment - David, I had the same idea myself. Course legacy files should not need to be brought over all the time, but only if the option is chosen. Is it possible to create an extra process to import the legacy files to the new course? I understand that this file area is to be gradually phased out but some courses which have been migrated from Moodle 1.9 require all of these files to be re-uploaded in to the new file area in the relative activities - this is a lot of work for some as some courses contain hundreds of files.
          Hide
          Tim Lock added a comment -

          Hi David,

          Any progress on this issue?

          We are seeing this issue most affecting universities with Question Images within Question Text.

          Show
          Tim Lock added a comment - Hi David, Any progress on this issue? We are seeing this issue most affecting universities with Question Images within Question Text.
          Hide
          Paula Clough added a comment -

          Any progress on this issue?

          This really is a big inconvenience for University courses. In order to have final course grades and official transcripts, we have to use courses set up by our system for each semester and find a way to import in the structure of the course. Though we are finding that most of the information is coming through with a backup and restore to the course created by the system, we do not have access to all the files in the legacy files folder in the new course. It is quite empty. As a work around, I have downloaded the legacy file and then uploaded it into the system files for the course before running the backup, but it increases the size of the back up. Sure would like to know where those files are hiding out in the system. They must be there somewhere because the links still work and the pictures still show up. Help with this would be greatly appreciated.

          Thanx, Paula

          Show
          Paula Clough added a comment - Any progress on this issue? This really is a big inconvenience for University courses. In order to have final course grades and official transcripts, we have to use courses set up by our system for each semester and find a way to import in the structure of the course. Though we are finding that most of the information is coming through with a backup and restore to the course created by the system, we do not have access to all the files in the legacy files folder in the new course. It is quite empty. As a work around, I have downloaded the legacy file and then uploaded it into the system files for the course before running the backup, but it increases the size of the back up. Sure would like to know where those files are hiding out in the system. They must be there somewhere because the links still work and the pictures still show up. Help with this would be greatly appreciated. Thanx, Paula
          Hide
          David Mudrak added a comment -

          Sorry, no progress on this. We do not seem to be able to agree on a clear solution for this. Comments and suggestions from more developers are welcome.

          Show
          David Mudrak added a comment - Sorry, no progress on this. We do not seem to be able to agree on a clear solution for this. Comments and suggestions from more developers are welcome.
          Hide
          Eric Merrill added a comment -

          This is big for us too.

          I know this ticket is specifically listed as import, but I think the issue is more generic than that.

          When people to restores, we can't have them overwriting the course config, because course names, dates, etc are set by an external system, but this mean that when restoring 1.9 backups, or 2.0 backups that used legacy files, resources may be broken.

          I really think that there should be an option during a restore, on the same page as 'overwrite course config', or during an import, that allows you to select 'Restore legacy files'.

          Show
          Eric Merrill added a comment - This is big for us too. I know this ticket is specifically listed as import, but I think the issue is more generic than that. When people to restores, we can't have them overwriting the course config, because course names, dates, etc are set by an external system, but this mean that when restoring 1.9 backups, or 2.0 backups that used legacy files, resources may be broken. I really think that there should be an option during a restore, on the same page as 'overwrite course config', or during an import, that allows you to select 'Restore legacy files'.
          Hide
          David Mudrak added a comment -

          Sorry, I am unassigning this from myself and dropping the "triaged" label so that it can be re-triaged again. This issue was discussed with several Moodle core gurus but we did not reach a consensus of how to deal with this. This needs more attention from other HQ people.

          Show
          David Mudrak added a comment - Sorry, I am unassigning this from myself and dropping the "triaged" label so that it can be re-triaged again. This issue was discussed with several Moodle core gurus but we did not reach a consensus of how to deal with this. This needs more attention from other HQ people.
          Hide
          Eric Merrill added a comment - - edited

          In MDL-32598, I broke legacy files out of the course structure step during the restore. This should make it relatively easy to make an import option to import legacy files.

          Show
          Eric Merrill added a comment - - edited In MDL-32598 , I broke legacy files out of the course structure step during the restore. This should make it relatively easy to make an import option to import legacy files.
          Hide
          Mark Drechsler added a comment -

          Really hoping that the Core Devs can collaborate to find a solution to this - it is something which causes a great deal of problems for our large clients upgrading from 1.9 to 2.x. I note that Jason provided a suggestion for a resolution which wasn't responded to - has it been considered as a potential solution?

          Sorry for being pushy, but this really does have a significant impact in terms of data loss for our large clients.

          Show
          Mark Drechsler added a comment - Really hoping that the Core Devs can collaborate to find a solution to this - it is something which causes a great deal of problems for our large clients upgrading from 1.9 to 2.x. I note that Jason provided a suggestion for a resolution which wasn't responded to - has it been considered as a potential solution? Sorry for being pushy, but this really does have a significant impact in terms of data loss for our large clients.
          Hide
          Adam Olley added a comment -

          Here's my take on a fix for this issue. It implements the "Include legacy files" option as suggested/discussed by David and Jason in this issue.

          When importing (MODE_IMPORT) the option is given. If selected, legacy files are brought across. This defaults to unchecked, as is current moodle behaviour.

          I'll rebase this onto other Moodle stable branches if it's deemed a suitable solution. But for anyone interested, the commit /should/ apply cleanly to any version of moodle after M2.0.9 (and possibly earlier versions of M2.0.x as well).

          Show
          Adam Olley added a comment - Here's my take on a fix for this issue. It implements the "Include legacy files" option as suggested/discussed by David and Jason in this issue. When importing (MODE_IMPORT) the option is given. If selected, legacy files are brought across. This defaults to unchecked, as is current moodle behaviour. I'll rebase this onto other Moodle stable branches if it's deemed a suitable solution. But for anyone interested, the commit /should/ apply cleanly to any version of moodle after M2.0.9 (and possibly earlier versions of M2.0.x as well).
          Hide
          Ashley Holman added a comment -

          Have any core devs reviewed Adam's patch yet? This is a much desired feature, and there is code here waiting to be reviewed.

          Much appreciated if someone can take a look.

          Show
          Ashley Holman added a comment - Have any core devs reviewed Adam's patch yet? This is a much desired feature, and there is code here waiting to be reviewed. Much appreciated if someone can take a look.
          Hide
          Dan Marsden added a comment -

          assigning this to me - I'll peer review/test it tonight as it's affecting one of our clients too - if it all looks good I'll bounce it up for integration. (expecting that integrators might have an opinion on the implementation and reject if needed)

          Show
          Dan Marsden added a comment - assigning this to me - I'll peer review/test it tonight as it's affecting one of our clients too - if it all looks good I'll bounce it up for integration. (expecting that integrators might have an opinion on the implementation and reject if needed)
          Hide
          Dan Marsden added a comment -

          the patch is almost there - it restores legacy course files but doesn't set the course to show legacy files so any links to the files from content in the course are broken until the course enables legacy files.

          The code should check if any legacy files are included in the import and set 'courselegacyfiles' to Yes for that course.

          Ashley/Adam - is this something you guys can do with the patch? - I can then peer review and send up for integration?

          Show
          Dan Marsden added a comment - the patch is almost there - it restores legacy course files but doesn't set the course to show legacy files so any links to the files from content in the course are broken until the course enables legacy files. The code should check if any legacy files are included in the import and set 'courselegacyfiles' to Yes for that course. Ashley/Adam - is this something you guys can do with the patch? - I can then peer review and send up for integration?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          U P S T R E A M I Z E D !

          Many thanks, this is now available in all the repos (git & cvs).

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks, this is now available in all the repos (git & cvs). Closing, ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Doh,

          somehow this issue was closed incorrectly when processing all the integrated issues this week. (sort of most voted and current in integration filters mix). Apologies for the confusion, reseting to previous status!

          Ciao, Eloy

          Show
          Eloy Lafuente (stronk7) added a comment - Doh, somehow this issue was closed incorrectly when processing all the integrated issues this week. (sort of most voted and current in integration filters mix). Apologies for the confusion, reseting to previous status! Ciao, Eloy
          Hide
          Daniel Kaelin added a comment -

          Any updates on when this will be incorporated into the Moodle code now that a patch has been developed?

          Show
          Daniel Kaelin added a comment - Any updates on when this will be incorporated into the Moodle code now that a patch has been developed?
          Hide
          B. Friesen added a comment -

          I don't actually find that the 'workaround' works. When I restore one course into another, the legacy files are not copied over.

          Show
          B. Friesen added a comment - I don't actually find that the 'workaround' works. When I restore one course into another, the legacy files are not copied over.
          Hide
          Michael Woods added a comment -

          This issue is big for us as well. We routinely backup/restore hundreds of courses every semester. We would like to move away from legacy course files at some point, but for now, we need to be able to support them a little bit longer. Restoring into existing courses blows them away. One significant area where things break is in the assignment descriptions, where many teachers (in Moodle 1.9.x) hyperlinked to a document describing the assignment. These hyperlinks were not auto-migrated to the pluginfile.php method during the 1.9.x to 2.x upgrade, so that is the primary area where legacy files are still needed for backwards compatibility. What's the easiest way forward?

          Show
          Michael Woods added a comment - This issue is big for us as well. We routinely backup/restore hundreds of courses every semester. We would like to move away from legacy course files at some point, but for now, we need to be able to support them a little bit longer. Restoring into existing courses blows them away. One significant area where things break is in the assignment descriptions, where many teachers (in Moodle 1.9.x) hyperlinked to a document describing the assignment. These hyperlinks were not auto-migrated to the pluginfile.php method during the 1.9.x to 2.x upgrade, so that is the primary area where legacy files are still needed for backwards compatibility. What's the easiest way forward?
          Hide
          Luciano Silva added a comment -

          Big problem for us, too. Many teachers complaining. Any solution ahead? Unfortunately we still need these files, and importing a course broken all links.

          Show
          Luciano Silva added a comment - Big problem for us, too. Many teachers complaining. Any solution ahead? Unfortunately we still need these files, and importing a course broken all links.
          Hide
          Dan Marsden added a comment -

          updated branches based on Adams patch

          Show
          Dan Marsden added a comment - updated branches based on Adams patch
          Hide
          Dan Marsden added a comment -

          bouncing this up for integration - if we need to automatically set the new course to enable legacy files lets do that on a separate patch.

          Show
          Dan Marsden added a comment - bouncing this up for integration - if we need to automatically set the new course to enable legacy files lets do that on a separate patch.
          Hide
          Dan Marsden added a comment -

          oops - I see MDL-32598 has a differnt patch that also sets legacy files in the new course to enabled - it has review from Eloy already too - have pinged Eloy asking him which patch he prefers.

          Show
          Dan Marsden added a comment - oops - I see MDL-32598 has a differnt patch that also sets legacy files in the new course to enabled - it has review from Eloy already too - have pinged Eloy asking him which patch he prefers.
          Hide
          Luciano Silva added a comment -

          We tested Adams patch, importing old courses, and worked ok. It seems to be a very good solution.

          Show
          Luciano Silva added a comment - We tested Adams patch, importing old courses, and worked ok. It seems to be a very good solution.
          Hide
          Damyon Wiese 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.

          Thanks!

          Show
          Damyon Wiese 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. Thanks!
          Hide
          Damyon Wiese added a comment -

          This patch looks OK to me - the language string could be changed to say "Legacy course files" as that is consistent with the language string elsewhere.

          Stopping review to see if Eloy has comments re: MDL-32598.

          Show
          Damyon Wiese added a comment - This patch looks OK to me - the language string could be changed to say "Legacy course files" as that is consistent with the language string elsewhere. Stopping review to see if Eloy has comments re: MDL-32598 .
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi,

          I really think that MDL-32598 is more "aligned" with the real cause/problem of this issue:

          1) It does not modify backup at all. legacy files are always included, unconditionally. No need for new setting in backup/import at all.

          2) It applies to any restore operation, this is restricted only to IMPORT type operations (where there are other operations - restoring to existing course, no overwrite, other types... - that also have the same problem).

          3) While I agree with Petr's comments above about the need to kill those legacy files (we cannot live forever with them), I also think that, if present... they should be restored always, just for consistency (problem is that you may be restoring way too many UNUSED files, agree, but the restored/imported course will work). Other tools are needed to take rid of those files (perhaps some mega-searcher looking for old file.php calls, or registering every use somewhere...), but that's out of the goals for this issue.

          So my opinion is that we should try to fix this by something like MDL-32598, moving the restoration of legacy files to another, independent step that should do something like:

          0) delete current call making the restoration of course legacy files.
          1) are there any course legacy files awaiting us (in the temp files table)? If yes:

          • show setting allowing to opt-out.
          • restore them based on the setting decision.
          • mark the course legacyfiles = 2 (so the area will be visible)

          Basically what I expressed @ https://tracker.moodle.org/browse/MDL-32598?focusedCommentId=166899&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-166899 (what a pity that one became stalled).

          So, summary, current patch here:

          1) Is it better than nothing. YES.
          2) Is it covering all the possibilities. NO.
          3) Does it help a lot of people (surely yes, but surely not everybody + is missing the ability to update the course, that is important).

          And where does my vote go:

          I would ask if anybody can come with the modifications needed to make this better along the current week. And then, no matter of the results, send it to integration again. Just a last chance call for improvement of current solution (that is good, but...).

          And I also would close MDL-32598 to continue the work/progress here. Both are clearly trying to fix the same problems, just this has more people involved.

          Hope this helps, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi, I really think that MDL-32598 is more "aligned" with the real cause/problem of this issue: 1) It does not modify backup at all. legacy files are always included, unconditionally. No need for new setting in backup/import at all. 2) It applies to any restore operation, this is restricted only to IMPORT type operations (where there are other operations - restoring to existing course, no overwrite, other types... - that also have the same problem). 3) While I agree with Petr's comments above about the need to kill those legacy files (we cannot live forever with them), I also think that, if present... they should be restored always, just for consistency (problem is that you may be restoring way too many UNUSED files, agree, but the restored/imported course will work). Other tools are needed to take rid of those files (perhaps some mega-searcher looking for old file.php calls, or registering every use somewhere...), but that's out of the goals for this issue. So my opinion is that we should try to fix this by something like MDL-32598 , moving the restoration of legacy files to another, independent step that should do something like: 0) delete current call making the restoration of course legacy files. 1) are there any course legacy files awaiting us (in the temp files table)? If yes: show setting allowing to opt-out. restore them based on the setting decision. mark the course legacyfiles = 2 (so the area will be visible) Basically what I expressed @ https://tracker.moodle.org/browse/MDL-32598?focusedCommentId=166899&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-166899 (what a pity that one became stalled). So, summary, current patch here: 1) Is it better than nothing. YES. 2) Is it covering all the possibilities. NO. 3) Does it help a lot of people (surely yes, but surely not everybody + is missing the ability to update the course, that is important). And where does my vote go: I would ask if anybody can come with the modifications needed to make this better along the current week. And then, no matter of the results, send it to integration again. Just a last chance call for improvement of current solution (that is good, but...). And I also would close MDL-32598 to continue the work/progress here. Both are clearly trying to fix the same problems, just this has more people involved. Hope this helps, ciao
          Hide
          Eric Merrill added a comment -

          Since I wrote the original patch in MDL-32598, I'll take a stab at this in the morning (about 12 hrs)

          -eric

          Show
          Eric Merrill added a comment - Since I wrote the original patch in MDL-32598 , I'll take a stab at this in the morning (about 12 hrs) -eric
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Oh, that's great, Eric, thanks! Feel free to ping/ask/contact for anything!

          Show
          Eloy Lafuente (stronk7) added a comment - Oh, that's great, Eric, thanks! Feel free to ping/ask/contact for anything!
          Hide
          Dan Marsden added a comment -

          great! - re-assigning this to Eric - would be great to finally get a patch that sorts this upstream.

          Show
          Dan Marsden added a comment - great! - re-assigning this to Eric - would be great to finally get a patch that sorts this upstream.
          Hide
          Eric Merrill added a comment -

          Been working on this for a while now. There seems to be an issue that at the point that the settings are created (define_settings()), no information that can be used to determine if there are legacy files are loaded yet. Only root settings and a few pieces of core course info are loaded.

          Also, there doesn't seem to be a way of adding a opt in/out setting to import w/o modifying the backup step, unless I'm missing something there.

          Please take a look at https://github.com/merrill-oakland/moodle/compare/MDL-25631 and give me some feedback. I can work on this more ASAP.

          Show
          Eric Merrill added a comment - Been working on this for a while now. There seems to be an issue that at the point that the settings are created (define_settings()), no information that can be used to determine if there are legacy files are loaded yet. Only root settings and a few pieces of core course info are loaded. Also, there doesn't seem to be a way of adding a opt in/out setting to import w/o modifying the backup step, unless I'm missing something there. Please take a look at https://github.com/merrill-oakland/moodle/compare/MDL-25631 and give me some feedback. I can work on this more ASAP.
          Hide
          Eric Merrill added a comment -

          The quick option since we have the code freeze we are running up against:

          Always show the 'include files' dropdown, but set to No, during restores. If the files aren't present when it gets later in the process, the step is just skipped.
          On import, import the files if they are present.

          Show
          Eric Merrill added a comment - The quick option since we have the code freeze we are running up against: Always show the 'include files' dropdown, but set to No, during restores. If the files aren't present when it gets later in the process, the step is just skipped. On import, import the files if they are present.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi, thanks a lot Eric!

          My +1 goes to kill the setting. If there are legacy files, let's restore them unconditionally. Will that lead to an excess of files, surely. But is it consistent, yes, always.

          As commented above, other tools will have to handle/cleanup those legacy files, not restore.

          Of course, the course (yay, redundancy) only will be updated (legacyfiles=2) if there are any legacy file, but it seems your current patch already does that ok, so I have not said anything.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi, thanks a lot Eric! My +1 goes to kill the setting. If there are legacy files, let's restore them unconditionally. Will that lead to an excess of files, surely. But is it consistent, yes, always. As commented above, other tools will have to handle/cleanup those legacy files, not restore. Of course, the course (yay, redundancy) only will be updated (legacyfiles=2) if there are any legacy file, but it seems your current patch already does that ok, so I have not said anything. Ciao
          Hide
          Eric Merrill added a comment -

          Attaching files for testing.

          Show
          Eric Merrill added a comment - Attaching files for testing.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          +1 super!

          Any feedback / opinion / experience will be welcome!

          Thanks Eric et all!

          Show
          Eloy Lafuente (stronk7) added a comment - +1 super! Any feedback / opinion / experience will be welcome! Thanks Eric et all!
          Hide
          Damyon Wiese added a comment -

          Looking at this now - it appears the page activity will still not try to on-demand migrate the files because the legacyfiles column of the activity is not set to RESOURCELIB_LEGACYFILES_ACTIVE.

          (Still looking).

          Show
          Damyon Wiese added a comment - Looking at this now - it appears the page activity will still not try to on-demand migrate the files because the legacyfiles column of the activity is not set to RESOURCELIB_LEGACYFILES_ACTIVE. (Still looking).
          Hide
          Damyon Wiese added a comment -

          Ah - I understanding better now. (just writing this note to explain better to myself).

          Restoring from 1.9 to 2 will restore the activities with the legacyfiles flag set to RESOURCELIB_LEGACYFILES_ACTIVE. Backup and restore that activity before the ondemand migration is complete and the flag will be included (because not all the files have been moved yet). Now that the legacy files are included always the migration will work correctly.

          Will update the test instructions as I don't think they fully cover the reported issue.

          Show
          Damyon Wiese added a comment - Ah - I understanding better now. (just writing this note to explain better to myself). Restoring from 1.9 to 2 will restore the activities with the legacyfiles flag set to RESOURCELIB_LEGACYFILES_ACTIVE. Backup and restore that activity before the ondemand migration is complete and the flag will be included (because not all the files have been moved yet). Now that the legacy files are included always the migration will work correctly. Will update the test instructions as I don't think they fully cover the reported issue.
          Hide
          Damyon Wiese added a comment -

          Thanks all, this change looks good.

          Integrated to 23, 24 and master now.

          Show
          Damyon Wiese added a comment - Thanks all, this change looks good. Integrated to 23, 24 and master now.
          Hide
          Rajesh Taneja added a comment -

          Thanks for fixing this Eric and providing excellent testing instructions.

          Works as described.

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Eric and providing excellent testing instructions. Works as described.
          Hide
          Damyon Wiese added a comment -

          Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

          Show
          Damyon Wiese added a comment - Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).
          Hide
          Mary Cooch added a comment -

          (Housekeeping) Removing docs_required label as I added a reference to this issue in http://docs.moodle.org/en/Import_course_data for 2.4 and 2.3

          Show
          Mary Cooch added a comment - (Housekeeping) Removing docs_required label as I added a reference to this issue in http://docs.moodle.org/en/Import_course_data for 2.4 and 2.3

            People

            • Votes:
              50 Vote for this issue
              Watchers:
              46 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: