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

Images are missing in the grid boxes when restoring a course #172

Closed
neeesn opened this issue Jan 5, 2023 · 40 comments
Closed

Images are missing in the grid boxes when restoring a course #172

neeesn opened this issue Jan 5, 2023 · 40 comments
Assignees

Comments

@neeesn
Copy link

neeesn commented Jan 5, 2023

When we copy courses, the images in tiles are missing.
The images are in the "Master" but not in the new course.

The same issue if we import into an existing course, the images in the tiles are missing.

We are currently on Moodle 4.0.4+ (Build: 20221018)

@gjb2048
Copy link
Collaborator

gjb2048 commented Jan 5, 2023 via email

@gjb2048 gjb2048 self-assigned this Jan 5, 2023
@neeesn
Copy link
Author

neeesn commented Jan 6, 2023

The grid version is 400.2.0 - 2022072204

@gjb2048
Copy link
Collaborator

gjb2048 commented Jan 6, 2023

Copy course works (M4.0.5+ (Build: 20230105) - Grid 400.2.0). Importing won't as that is intentional as source images could overwrite destination images. Screen shot evidence of copy process along with steps to replicate please.

@gjb2048
Copy link
Collaborator

gjb2048 commented Jan 6, 2023

Course backup and restore into a new course also work.

@JRinggren
Copy link

We are experiencing that tiles images are only copied correctly if they are present in both image section and short description. Is that expected behavior?

@gjb2048
Copy link
Collaborator

gjb2048 commented Jan 11, 2023

No. And my test only had the images in the image section and not the section summary, thus cannot replicate.

And this used to be potentially possible with the M3.11- version, but has been fixed now. M4.0+ version is a re-write that does not have the issue in regards to the file area used for the image being the same as the one used in the section summary, and so if the same image was uploaded then could happen.

@gjb2048
Copy link
Collaborator

gjb2048 commented Jan 24, 2023

Possibly related to comments by Michael Woods on https://moodle.org/plugins/format_grid.

@gjb2048
Copy link
Collaborator

gjb2048 commented Mar 4, 2023

No reply in ages, closing.

@gjb2048 gjb2048 closed this as completed Mar 4, 2023
@JRinggren
Copy link

Hi again,
Sorry to open this up again.
It seems we have a site experiencing the same issues as Michael Woods like you suggested.
The scenario is that importing or restoring (while deleting existing content) into an existing course doesn't contain section images. You report that you will get around to thinking of a solution at some point. Do you have any time frame for this, that we can report to our siteowners?

@gjb2048 gjb2048 reopened this Mar 13, 2023
@gjb2048
Copy link
Collaborator

gjb2048 commented Mar 13, 2023

@JRinggren No time frame, I tend to look at issues for non-clients when I get a moment or need the challenge of solving the problem.

@JRinggren
Copy link

Hi again!
Okay. Thanks for the reply!

@aspark21
Copy link

aspark21 commented Aug 9, 2023

Hi Gareth,

happy to fund this

Al

@aspark21
Copy link

aspark21 commented Aug 9, 2023

@hovez701 can provide more details if you still have issues replicating

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 9, 2023

@aspark21 Thanks, but need to be convinced that the correct issue is being solved as there seems to be two different scenarios going on here. The one that I could potentially address is when the user requests that the target course has its contents deleted. However that won't affect the this issue as that relates to course duplication, which I cannot replicate. G.

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 9, 2023

Info:

Screenshot 2023-08-09 162130

@hovez701
Copy link

Hi @gjb2048 , we are getting this issue from our course rollover plugin, which creates a new blank course first and then restores the backup into that new course. We believe this matches your scenario 1.

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 10, 2023

@hovez701 Ok, so how can I programmatically detect that the target course is blank and that the contents require deleting? Do you tell the restore process what type of restore it is? Or rather, why don't you just instigate the duplication mechanism instead?

@aydevworks
Copy link

@hovez701 Ok, so how can I programmatically detect that the target course is blank and that the contents require deleting? Do you tell the restore process what type of restore it is? Or rather, why don't you just instigate the duplication mechanism instead?

Hi @gjb2048,

It's always a blank course and restore using the moodle restore_controller.
$restore = new restore_controller($backup['id'], $course->id, backup::INTERACTIVE_NO, backup::MODE_GENERAL, $USER->id, backup::TARGET_CURRENT_DELETING);

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 10, 2023

@aydevworks Thanks, I'll do some thinking. Where I can I get the 'course rollover plugin' from to test with please? Instructions etc. too.

@aydevworks
Copy link

@gjb2048 Thanks, but could you please try to resolve this scenario first?
"The one that I could potentially address is when the user requests that the target course has its contents deleted."

Then we can test it from our side as the course rollover plugin required a few other plugins, a bit tricky to setup too. 😄

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 10, 2023

@aydevworks Ok.

@aydevworks
Copy link

@gjb2048 Thank you! 🙏

@gjb2048 gjb2048 changed the title Bug: Import / copy of courses - images in tiles are missing Images are missing in the grid boxes when restoring a course Aug 10, 2023
@aydevworks
Copy link

Thanks @gjb2048, restoring backup into a blank course is the only use case for the 'course rollover plugin'. We should be fine without the merging being fixed.

@aspark21 @hovez701 what do you think?

gjb2048 added a commit that referenced this issue Aug 11, 2023
gjb2048 added a commit that referenced this issue Aug 11, 2023
gjb2048 added a commit that referenced this issue Aug 11, 2023
gjb2048 added a commit that referenced this issue Aug 11, 2023
@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 11, 2023

Dear @aydevworks,

After extensive research / trying to figure out how the restore API works, I've concluded that it is not possible to delete the existing files when deleting the content and before the images are restored from the backup, as no method that format can implement in its own restore classes will be called (unless there is a step / task that can be overridden - but I don't think so as the steps / tasks are course ones and not like for blocks / modules). This therefore also goes for a merge whereby the 'after_course' method is only called when the course settings change and thus on a merge to tidy up any restored files that are not used in the grid. Again, research wise, this does not seem to be the case when looking in the files table, however somehow when the 'delete content' option is chosen the files are used and work = odd! And on a merge when they are not, then they don't show in the files table, even though the restore mechanism -> 'process()' in 'backup/util/plan/restore_structure_step.class.php' indicates a 'files' restore process for the images that are in the backup file... unless, and I've yet to check, these were the old 'course/section' component/filearea files that are in my test courses before these recent changes and I've not breakpointed this in 'process' yet for a backup created from a new Grid course that doesn't have them.

G

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 11, 2023

P.S. Had to rebase as fixed a bug outside of this issue.

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 11, 2023

P.P.S. Nope - "And on a merge when they are not, then they don't show in the files table, even though the restore mechanism -> 'process()' in 'backup/util/plan/restore_structure_step.class.php' indicates a 'files' restore process for the images that are in the backup file... unless, and I've yet to check, these were the old 'course/section' component/filearea files that are in my test courses before these recent changes and I've not breakpointed this in 'process' yet for a backup created from a new Grid course that doesn't have them." - tested and the files are being parsed - not sure what's going on.

gjb2048 added a commit that referenced this issue Aug 13, 2023
gjb2048 added a commit that referenced this issue Aug 13, 2023
gjb2048 added a commit that referenced this issue Aug 13, 2023
gjb2048 added a commit that referenced this issue Aug 13, 2023
@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 15, 2023

@hovez701 Any news on this from your end please?

@aspark21
Copy link

aspark21 commented Aug 15, 2023

Feel free to continue looking into this for the other use cases @gjb2048

Seeing how painful this currently is, it may be useful to merge the partial fix for these use cases and expand on this in a further merge?

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 15, 2023

@aspark21 Thanks. I thought you were on holiday?

@aspark21
Copy link

back for one day only and then off again 😄

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 16, 2023

Released in https://moodle.org/mod/forum/discuss.php?d=449513 - with merge (when no image in destination and image in source, then source image gets used) and delete.

@gjb2048 gjb2048 closed this as completed Aug 16, 2023
@gjb2048 gjb2048 added the Fixed label Aug 16, 2023
@phette23
Copy link

Is there any chance for a backport to 4.0? We are also affected by this in a similar context (using local_course_template plugin) and are on 4.0.

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 16, 2023 via email

@phette23
Copy link

@gjb2048 How much would that cost? Should I email you about this?

@gjb2048
Copy link
Collaborator

gjb2048 commented Aug 17, 2023

@phette23 Between one to two hours at my current rate of 44 GBP per hour as I'll need to backport all of this:

Screenshot 2023-08-17 114732

too, in order to keep things in sync + a Moodle dot org release. Please use the eMail address at the top of Support.md (https://github.com/gjb2048/moodle-format_grid/blob/master/Support.md) - note that I'm in the UK and I see you're in the US, so please let me know of any issues pertaining to your use of international contractors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants