Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements preliminary version of Moodle's backup/restore feature #41

Closed
wants to merge 1 commit into from

Conversation

juho-jaakkola
Copy link

@juho-jaakkola juho-jaakkola commented Mar 29, 2016

This is already able to make a backup of the database and restore it afterwards.

  • Figure out why image files and other media do not get restored from the backup package


// This regex already matches all jpg files in such way:
$search = "/\"path\":\"images\/([^\"]+)\",\"mime\"/";
$content = preg_replace($search, '@@PLUGINFILE@@/${1}","mime"', $content);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I tried replacing this:

"path":"images/imageSlideBackground-56c5a34e5743e.jpg"

with this:

"path":"@@PLUGINFILE@@/imageSlideBackground-56c5a34e5743e.jpg"

but it has no effect on the backup. There's either something I'm missing, or I just don't really understand how the backup feature is supposed to include the files.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've most likely misunderstood this part. Apparently the backup feature does not check existence of files in the text fields, but instead fetches them from the mdl_files table directly.

@juho-jaakkola
Copy link
Author

I've also asked help from the Moodle community: https://moodle.org/mod/forum/discuss.php?d=334643

@juho-jaakkola
Copy link
Author

I have today:

  • Rebased this on latest master
  • Created a follow issue for finishing the work: Include libraries into activity backup #107
  • Marked the backup feature as unsupported (FEATURE_BACKUP_MOODLE2: false) so no-one accidentally uses the feature and ends up with a broken backup

So I think it is fine to merge this, and get back to finishing the missing parts later.

@juho-jaakkola juho-jaakkola changed the title WIP: Implements Moodle's backup feature Implements preliminary version of Moodle's backup/restore feature Aug 12, 2016
* @subpackage backup-moodle2
* @copyright 2010 onwards Eloy Lafuente (stronk7) {@link http://stronk7.com}
* @package mod_hvp
* @copyright 2016 Mediamaisteri Oy {@link http://www.mediamaisteri.com}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an OS lawyer, so I'm not completely sure who should be mentioned on this line. :P Perhaps there should be at least a mention of the original author? Or perhaps this isn't really a problem in the first place.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would depend on how "original" it is. This seems to have been lifted from the choice module for the purpose of bootstrapping the back up library, but only from the perspective of being a template / saving typing, and not to build on top of the choice module, so you could view it as being "original".

However since you're commit history actually shows you started from the choice module, it could be viewed as a derivative work, so you may wish to reference the original authors (even if the file is now 99% different!).

Next time type it all from scratch and it's "yours" :-)

'timemodified',
));

$hvp_libraries = new backup_nested_element('libraries');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix indentation

@juho-jaakkola
Copy link
Author

The previously indented lines have now been aligned.

@vaughany
Copy link
Contributor

vaughany commented Jan 6, 2017

👍

@vaughany
Copy link
Contributor

Hi everyone. I'm keen to get a handle on where the H5P Moodle plugin is regarding backup and restore, mainly because I'm going to need it very soon.

Having a look at @juho-jaakkola 's PR it seems that all of it already exists within the H5P Moodle plugin master branch, despite this PR not being merged. I can see that the backup works but attached media doesn't (I had a H5P consisting solely of a pie chart which worked fine, but a collage in which the data loaded but the images obviously failed).

As my employer loves H5P and we're going to be using it a lot rather soon, I offer my services (and that of another developer @cgreenwood - we're that keen to get this working!) to get backup and restore working, if this still a community effort.

@icc
Copy link
Member

icc commented Jan 16, 2017

Hm, not sure why the PR isn't merged. All the changes should be available on master.

Any effort to try and fix the restore media/file issue is very welcome. I had set off some time to debug the exported files, but other issues had to come first.

@vaughany
Copy link
Contributor

Any pointers or info you could give us to get us going in the right direction? I'm not really sure what the issues even are yet. I'm coming at this from almost no knowledge of Moodle's backup and restore code, only the knowledge that we really need it working! :)

@icc
Copy link
Member

icc commented Jan 17, 2017

I'm sorry, my knowledge on the issue is quite limited as well. My guess is that there's something wrong when creating the export.

@vaughany
Copy link
Contributor

Thanks, @icc.

Do you have any pointers @juho-jaakkola ?

@vaughany
Copy link
Contributor

Please note that @cgreenwood and I have implemented a solution and submitted a PR: #131.

@icc
Copy link
Member

icc commented Jan 19, 2017

Great! I'll have a look and do some testing.

@fnoks fnoks closed this May 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants