Moodle
  1. Moodle
  2. MDL-32690

Restore 1.9 backup into 2.X fails on missing assignment type

    Details

    • Rank:
      39645

      Description

      Steps to reproduce:

      • Backup a 1.9 course that has an assignment type that doesn't exist in 2.X (EG: some third party plugin)
      • Restore that backup into 2.X
      • Here is the error:
        error/unsupported_subplugin
        
        More information about this error
        Stack trace:
        
        line 197 of /mod/assignment/backup/moodle1/lib.php: moodle1_convert_exception thrown
        line 132 of /mod/assignment/backup/moodle1/lib.php: call to moodle1_mod_assignment_handler->get_subplugin_handler()
        line 111 of /mod/assignment/backup/moodle1/lib.php: call to moodle1_mod_assignment_handler->handle_assignment_subplugin()
        line 299 of /backup/converter/moodle1/lib.php: call to moodle1_mod_assignment_handler->process_assignment()
        line 737 of /backup/converter/moodle1/lib.php: call to moodle1_converter->process_chunk()
        line 125 of /backup/util/xml/parser/processors/grouped_parser_processor.class.php: call to moodle1_parser_processor->dispatch_chunk()
        line 148 of /backup/util/xml/parser/processors/simplified_parser_processor.class.php: call to grouped_parser_processor->postprocess_chunk()
        line 92 of /backup/util/xml/parser/processors/progressive_parser_processor.class.php: call to simplified_parser_processor->process_chunk()
        line 169 of /backup/util/xml/parser/progressive_parser.class.php: call to progressive_parser_processor->receive_chunk()
        line 253 of /backup/util/xml/parser/progressive_parser.class.php: call to progressive_parser->publish()
        line ? of unknownfile: call to progressive_parser->end_tag()
        line 158 of /backup/util/xml/parser/progressive_parser.class.php: call to xml_parse()
        line 137 of /backup/util/xml/parser/progressive_parser.class.php: call to progressive_parser->parse()
        line 150 of /backup/converter/moodle1/lib.php: call to progressive_parser->process()
        line 128 of /backup/converter/convertlib.php: call to moodle1_converter->execute()
        line 215 of /backup/util/helper/convert_helper.class.php: call to base_converter->convert()
        line 408 of /backup/controller/restore_controller.class.php: call to convert_helper::to_moodle2_format()
        line 35 of /backup/restore.php: call to restore_controller->convert()
        

      Ideally, the moodle1 conversion process would just skip the unknown assignment type, but I haven't found a straight forward way to accomplish this.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          This is a known issue. That's not to say we should not try to do something about it.

          According to David, however, this might not be possible...

          http://tracker.moodle.org/browse/MDL-27960?focusedCommentId=115833&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-115833

          Show
          Michael de Raadt added a comment - This is a known issue. That's not to say we should not try to do something about it. According to David, however, this might not be possible... http://tracker.moodle.org/browse/MDL-27960?focusedCommentId=115833&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-115833
          Hide
          David Mudrak added a comment -

          Correct. As I said in the referenced comment, the assignment type is detected later in the linear processing of the moodle.xml file. So other information (like course module definition) has already been converted and there is no easy way to undo it. Throwing an exception was the most responsible way to inform the user that this restore would not be possible...

          The pity part is that the user can't easily skip the problematic module as the selection can happen only after the conversion. So the only way I can see at the moment is to restore the backup in 1.9 and re-backup it without the assignment. Which is far from anything acceptable, I agree.

          The solution might be to read moodle.xml twice. In the first iteration, we would detect this problem (among other useful things possibly) and after that, during the second iteration, the actual conversion would happen. This solution would need a bit of redesign of moodle2 restore API and UI so that the conversion is given a chance for the user's input (ie it is interactive). That is not possible at the moment, too.

          Show
          David Mudrak added a comment - Correct. As I said in the referenced comment, the assignment type is detected later in the linear processing of the moodle.xml file. So other information (like course module definition) has already been converted and there is no easy way to undo it. Throwing an exception was the most responsible way to inform the user that this restore would not be possible... The pity part is that the user can't easily skip the problematic module as the selection can happen only after the conversion. So the only way I can see at the moment is to restore the backup in 1.9 and re-backup it without the assignment. Which is far from anything acceptable, I agree. The solution might be to read moodle.xml twice. In the first iteration, we would detect this problem (among other useful things possibly) and after that, during the second iteration, the actual conversion would happen. This solution would need a bit of redesign of moodle2 restore API and UI so that the conversion is given a chance for the user's input (ie it is interactive). That is not possible at the moment, too.
          Hide
          Kris Stokking added a comment -

          David, I think you hit on the ideal solution from my perspective - notify the user of the problem, ask them how to proceed, and act accordingly. I would think that would be a pretty large change, and would require regression testing. Also, the problem exists for upgrades as well.

          In the interim, this is how we're planning to tackle the problem:

          1. Allow unsupported sub-plugins to convert, which should function similarly to how upgrades handle the problem.
          2. Hide unsupported sub-plugins, so the problem won't be visible to students (by default).
          3. Change the error message in assignment view of unsupported sub-plugins to be a bit more graceful.

          We'll be happy to share the patch if you think it would be useful.

          Show
          Kris Stokking added a comment - David, I think you hit on the ideal solution from my perspective - notify the user of the problem, ask them how to proceed, and act accordingly. I would think that would be a pretty large change, and would require regression testing. Also, the problem exists for upgrades as well. In the interim, this is how we're planning to tackle the problem: Allow unsupported sub-plugins to convert, which should function similarly to how upgrades handle the problem. Hide unsupported sub-plugins, so the problem won't be visible to students (by default). Change the error message in assignment view of unsupported sub-plugins to be a bit more graceful. We'll be happy to share the patch if you think it would be useful.
          Hide
          David Mudrak added a comment -

          Patch would be great! Thanks in advance.

          Show
          David Mudrak added a comment - Patch would be great! Thanks in advance.
          Hide
          Mark Nielsen added a comment -

          Added the code, please note that this has not run by our QA but I have tested it. Let me know what you think.

          Show
          Mark Nielsen added a comment - Added the code, please note that this has not run by our QA but I have tested it. Let me know what you think.
          Hide
          David Mudrak added a comment -

          Hi Mark. Thanks for your work on this. I have not tested the patch but I think I understand the overall intention. The following came to my mind: what about keeping moodle1_assignment_subplugin_handler as an abstract class and introduce new moodle1_assignment_unsupported_subplugin_handler (or so) subclass extending it. That it would be more obvious IMHO and would also let us check instanceof eventually in the future.

          Show
          David Mudrak added a comment - Hi Mark. Thanks for your work on this. I have not tested the patch but I think I understand the overall intention. The following came to my mind: what about keeping moodle1_assignment_subplugin_handler as an abstract class and introduce new moodle1_assignment_unsupported_subplugin_handler (or so) subclass extending it. That it would be more obvious IMHO and would also let us check instanceof eventually in the future.
          Hide
          Mark Nielsen added a comment -

          Good suggestion, implemented.

          Show
          Mark Nielsen added a comment - Good suggestion, implemented.
          Hide
          David Mudrak added a comment -

          This issue was assigned to me, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

          Show
          David Mudrak added a comment - This issue was assigned to me, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment
          Hide
          Nadav Kavalerchik added a comment -

          Any progress on this?

          We are in the middle of large migration into Moodle 2.5 and "error/unsupported_subplugin" keep occurring on various courses that uses assignment/type/team or assignment/type/peerreview. If I could only disable them on the automatic (cron) Moodle 1.9 backup, which will run once and generate backups without those problematic plugins, that would a good enough workaround for me. Any Ideas?

          Show
          Nadav Kavalerchik added a comment - Any progress on this? We are in the middle of large migration into Moodle 2.5 and "error/unsupported_subplugin" keep occurring on various courses that uses assignment/type/team or assignment/type/peerreview. If I could only disable them on the automatic (cron) Moodle 1.9 backup, which will run once and generate backups without those problematic plugins, that would a good enough workaround for me. Any Ideas?
          Hide
          Mark Nielsen added a comment -

          Re-created the branch based on master. Please re-review and submit for integration if things look good. Thanks!

          Show
          Mark Nielsen added a comment - Re-created the branch based on master. Please re-review and submit for integration if things look good. Thanks!
          Hide
          Dan Poltawski added a comment -

          Hi Mark,

          Thanks for recreating that branch. I'm going to look at this in more detail tomorrow, but one tiny thing is the missing () on the require once here:
          https://github.com/moodlerooms/moodle/compare/MDL-32690_restoreMissingAssignment_master#L3R3956

          cheers,
          dan

          Show
          Dan Poltawski added a comment - Hi Mark, Thanks for recreating that branch. I'm going to look at this in more detail tomorrow, but one tiny thing is the missing () on the require once here: https://github.com/moodlerooms/moodle/compare/MDL-32690_restoreMissingAssignment_master#L3R3956 cheers, dan
          Hide
          Mark Nielsen added a comment -

          Fixed

          Show
          Mark Nielsen added a comment - Fixed
          Hide
          Dan Poltawski added a comment -

          Attached a sample backup file which can be used for testing.

          Show
          Dan Poltawski added a comment - Attached a sample backup file which can be used for testing.
          Hide
          Dan Poltawski added a comment -

          While restoring, we get the following debugging notice:

          Incorrect assignment type: nanogong
          line 3625 of /mod/assignment/lib.php: call to debugging()
          line 941 of /course/lib.php: call to assignment_get_coursemodule_info()
          line 1498 of /lib/modinfolib.php: call to get_array_of_activities()
          line 479 of /backup/moodle2/restore_stepslib.php: call to rebuild_course_cache()
          line 34 of /backup/util/plan/restore_execution_step.class.php: call to restore_rebuild_course_cache->define_execution()
          line 153 of /backup/util/plan/base_task.class.php: call to restore_execution_step->execute()
          line 163 of /backup/util/plan/base_plan.class.php: call to base_task->execute()
          line 157 of /backup/util/plan/restore_plan.class.php: call to base_plan->execute()
          line 315 of /backup/controller/restore_controller.class.php: call to restore_plan->execute()
          line 147 of /backup/util/ui/restore_ui.class.php: call to restore_controller->execute_plan()
          line 50 of /backup/restore.php: call to restore_ui->execute()
          
          Show
          Dan Poltawski added a comment - While restoring, we get the following debugging notice: Incorrect assignment type: nanogong line 3625 of /mod/assignment/lib.php: call to debugging() line 941 of /course/lib.php: call to assignment_get_coursemodule_info() line 1498 of /lib/modinfolib.php: call to get_array_of_activities() line 479 of /backup/moodle2/restore_stepslib.php: call to rebuild_course_cache() line 34 of /backup/util/plan/restore_execution_step.class.php: call to restore_rebuild_course_cache->define_execution() line 153 of /backup/util/plan/base_task.class.php: call to restore_execution_step->execute() line 163 of /backup/util/plan/base_plan.class.php: call to base_task->execute() line 157 of /backup/util/plan/restore_plan.class.php: call to base_plan->execute() line 315 of /backup/controller/restore_controller.class.php: call to restore_plan->execute() line 147 of /backup/util/ui/restore_ui.class.php: call to restore_controller->execute_plan() line 50 of /backup/restore.php: call to restore_ui->execute()
          Hide
          Dan Poltawski added a comment -

          Well, I think that this solution makes sense and i've tested it works nicely.

          It'll remove a big blocker from some course backups from Moodle 1.9. So I think its good for integration.

          Mark, please could you create branches for 2.4 and 2.5 too? Thanks

          Show
          Dan Poltawski added a comment - Well, I think that this solution makes sense and i've tested it works nicely. It'll remove a big blocker from some course backups from Moodle 1.9. So I think its good for integration. Mark, please could you create branches for 2.4 and 2.5 too? Thanks
          Hide
          Dan Poltawski added a comment -

          (As far as I recall, old assignment types couldn't create other db tables or anything anyway either, right - so the existing restore logic is fine I think?)

          Show
          Dan Poltawski added a comment - (As far as I recall, old assignment types couldn't create other db tables or anything anyway either, right - so the existing restore logic is fine I think?)
          Hide
          Dan Poltawski added a comment -

          Looks like i'm wrong on that point: http://docs.moodle.org/dev/Assignment_types#Updating_the_database

          Hmm, so in that case - should we be restoring these assignment types at all? (I mean in the case of my nanogong one, what happens with Mark's patch means that it could potentially be upgraded, but with some plugins we might restore and loose data).

          To be honest, I think that just not blocking the restore despite what happens to the assignment type is an improvement..

          Show
          Dan Poltawski added a comment - Looks like i'm wrong on that point: http://docs.moodle.org/dev/Assignment_types#Updating_the_database Hmm, so in that case - should we be restoring these assignment types at all? (I mean in the case of my nanogong one, what happens with Mark's patch means that it could potentially be upgraded, but with some plugins we might restore and loose data). To be honest, I think that just not blocking the restore despite what happens to the assignment type is an improvement..
          Hide
          Mark Nielsen added a comment -

          Rebased master and it was a clean cherry pick to 2.4 and 2.5.

          Show
          Mark Nielsen added a comment - Rebased master and it was a clean cherry pick to 2.4 and 2.5.
          Hide
          Mark Nielsen added a comment -

          To be honest, I think that just not blocking the restore despite what happens to the assignment type is an improvement..

          Yes, either way you could have data loss. At least with this method, basic assignment types can be used once the code has been put in place without rebuilding courses.

          Show
          Mark Nielsen added a comment - To be honest, I think that just not blocking the restore despite what happens to the assignment type is an improvement.. Yes, either way you could have data loss. At least with this method, basic assignment types can be used once the code has been put in place without rebuilding courses.
          Hide
          Dan Poltawski added a comment -

          I'm pushing this for integration (with full awareness that this might be rejected).

          I'm with Mark, I think that this is a useful improvement - I know many schools who had mountains of course templates as 1.9 backup files and I think this issue would be blocking each one of those files from being restored now. Thats a lot of lost work and in those scenarios its just not practical that someone is going to come along and regenerate the backup file for use.

          Given we don't restore user data anyway I don't think the impact is huge.

          Show
          Dan Poltawski added a comment - I'm pushing this for integration (with full awareness that this might be rejected). I'm with Mark, I think that this is a useful improvement - I know many schools who had mountains of course templates as 1.9 backup files and I think this issue would be blocking each one of those files from being restored now. Thats a lot of lost work and in those scenarios its just not practical that someone is going to come along and regenerate the backup file for use. Given we don't restore user data anyway I don't think the impact is huge.
          Hide
          Nadav Kavalerchik added a comment -

          A Workaround I use:
          I am attaching a dummy plugin (template) which is currently set to emulate a scenario as if an assignment sub module "Team" is installed on Moodle 2+. Which enables the backup process to show the list of modules that about to be imported/restored from a Moodle 1.9.x backup file. And enables the user to uncheck (remove the selection) of the problematic "Team" assignment off the list and continue with the restore process.

          Even selecting the dummy assignment type "Team" will result in a successful restore of the course, but then, the user should remove the "Team" assignment from the course. (Although opening it will not result in an error, but a non functional "Offline" assignment)

          The dummy "Team" assignment type is a bare clone of "Offline" assignment type. With only the necessary functions to allow clean install and successful restore.

          It can be very easily adapted to other problematic assignment subtypes, by any admin.

          Enjoy

          Show
          Nadav Kavalerchik added a comment - A Workaround I use: I am attaching a dummy plugin (template) which is currently set to emulate a scenario as if an assignment sub module "Team" is installed on Moodle 2+. Which enables the backup process to show the list of modules that about to be imported/restored from a Moodle 1.9.x backup file. And enables the user to uncheck (remove the selection) of the problematic "Team" assignment off the list and continue with the restore process. Even selecting the dummy assignment type "Team" will result in a successful restore of the course, but then, the user should remove the "Team" assignment from the course. (Although opening it will not result in an error, but a non functional "Offline" assignment) The dummy "Team" assignment type is a bare clone of "Offline" assignment type. With only the necessary functions to allow clean install and successful restore. It can be very easily adapted to other problematic assignment subtypes, by any admin. Enjoy
          Hide
          Damyon Wiese added a comment -

          Thanks Mark,

          I agree this is much nicer. If the assignment sub type does not exist - there is no way to restore user data for this assignment, but it should not block the rest of the course. They still have the backup file in this case so they can restore it later once they install the correct assignment subtype.

          Integrated to 24, 25 and master.

          Show
          Damyon Wiese added a comment - Thanks Mark, I agree this is much nicer. If the assignment sub type does not exist - there is no way to restore user data for this assignment, but it should not block the rest of the course. They still have the backup file in this case so they can restore it later once they install the correct assignment subtype. Integrated to 24, 25 and master.
          Hide
          David Monllaó added a comment -

          All passing, tested in 24, 25 and master, including the bonus

          Show
          David Monllaó added a comment - All passing, tested in 24, 25 and master, including the bonus
          Hide
          Dan Poltawski added a comment -

          Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke:

          A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"

          Show
          Dan Poltawski added a comment - Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke: A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"

            People

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

              Dates

              • Created:
                Updated:
                Resolved: