Moodle
  1. Moodle
  2. MDL-29563

Duplicate runs on modules that do not support backup/restore

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1.1, 2.2, 2.3
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Backup
    • Labels:
    • Testing Instructions:
      Hide
      1. Download and install a contrib module without backup2 support, for example: http://moodle.org/plugins/view.php?plugin=mod_videochat (throws a few notices but it's ok for this testing)
      2. Create / goto a course and add an assignment and a instance of the "module without backup2 support"
      3. Only the assignment instance should have the duplicate icon
      4. Copy the URL of the duplicate action of the assignment instance and change the instance id for the "module without backup2 support" instance id
      5. At the next screen click continue
      6. There should appear an error message explaining the situation and a continue button to return to the course view page
      Show
      Download and install a contrib module without backup2 support, for example: http://moodle.org/plugins/view.php?plugin=mod_videochat (throws a few notices but it's ok for this testing) Create / goto a course and add an assignment and a instance of the "module without backup2 support" Only the assignment instance should have the duplicate icon Copy the URL of the duplicate action of the assignment instance and change the instance id for the "module without backup2 support" instance id At the next screen click continue There should appear an error message explaining the situation and a continue button to return to the course view page
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-29563_master
    • Rank (Obsolete):
      19126

      Description

      Some modules may not support backup/restore. If they do not, then you get a fatal error about it's backup class does not exist at backup/util/factories/backup_factory.class.php on line 107

        Issue Links

          Activity

          Hide
          Mark Nielsen added a comment -

          The attached patch suppresses the copy icon in the course icons and also displays a nice error on modduplicate.php. You may want to move the error to mod.php (display it before the confirmation of duplication).

          Show
          Mark Nielsen added a comment - The attached patch suppresses the copy icon in the course icons and also displays a nice error on modduplicate.php. You may want to move the error to mod.php (display it before the confirmation of duplication).
          Hide
          Michael de Raadt added a comment -

          Thanks for spotting that and providing a patch.

          We'll need to find a contributed module that does not have backup support to test this. Do you have any suggestions, Mark?

          Show
          Michael de Raadt added a comment - Thanks for spotting that and providing a patch. We'll need to find a contributed module that does not have backup support to test this. Do you have any suggestions, Mark?
          Hide
          Mark Nielsen added a comment -

          Sorry I don't

          Show
          Mark Nielsen added a comment - Sorry I don't
          Hide
          David Monllaó added a comment -

          Thanks for the patch Mark. Adapting the patch to 2.2, 2.3 and master and adding pull branches

          Show
          David Monllaó added a comment - Thanks for the patch Mark. Adapting the patch to 2.2, 2.3 and master and adding pull branches
          Hide
          Rajesh Taneja added a comment -

          Thanks for the patch Mark and David,

          Patch looks nice, although you might want to consider using print_error at https://github.com/dmonllao/moodle/compare/master...MDL-29563_master#L1R64

          As we don't expect this to execute in normal case (Duplicate icon will not rendered after this patch), IMO this should be handled by print_error.

          Rest all look good.

          Show
          Rajesh Taneja added a comment - Thanks for the patch Mark and David, Patch looks nice, although you might want to consider using print_error at https://github.com/dmonllao/moodle/compare/master...MDL-29563_master#L1R64 As we don't expect this to execute in normal case (Duplicate icon will not rendered after this patch), IMO this should be handled by print_error. Rest all look good.
          Hide
          David Monllaó added a comment -

          Thanks for the comments Raj, I've updated the pull branches with the proposed change. Requesting peer review again

          Show
          David Monllaó added a comment - Thanks for the comments Raj, I've updated the pull branches with the proposed change. Requesting peer review again
          Hide
          Rajesh Taneja added a comment -

          Thanks David,
          Patch looks good now, pushing for integration review.

          Show
          Rajesh Taneja added a comment - Thanks David, Patch looks good now, pushing for integration review.
          Hide
          Dan Poltawski 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
          Dan Poltawski 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
          David Monllaó added a comment -

          Hi Dan,

          Branches rebased

          Show
          David Monllaó added a comment - Hi Dan, Branches rebased
          Hide
          Sam Hemelryk added a comment -

          Awesome thanks David, this has been integrated now.

          Show
          Sam Hemelryk added a comment - Awesome thanks David, this has been integrated now.
          Hide
          Adrian Greeve added a comment -

          Tested with 2.2, 2.3 and master. The patch works on all the listed versions.
          No problems encountered.
          Test Passed

          Show
          Adrian Greeve added a comment - Tested with 2.2, 2.3 and master. The patch works on all the listed versions. No problems encountered. Test Passed
          Hide
          Aparup Banerjee added a comment -

          yay, it works!

          This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week.

          Thank you all for taking the time to get us here.

          cheers!

          Show
          Aparup Banerjee added a comment - yay, it works! This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week. Thank you all for taking the time to get us here. cheers!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: