[#MDL-14326] Moodle does not delete empty course_modules after the restore

Issue Details (XML | Word | Printable)

Key:	MDL-14326
Type:	Bug
Status:	Closed
Resolution:	Fixed
Priority:	Major
Assignee:	David Mudrak
Reporter:	David Mudrak
Votes:	10
Watchers:	18

Operations

Add/Edit UI Mockup to this issue

If you were logged in you would be able to see more operations.

Moodle

Moodle does not delete empty course_modules after the restore

Created: 12/Apr/08 06:56 AM Updated: 25/Jun/09 05:43 PM

Component/s:

Backup

Affects Version/s:

1.8.5, 1.9

Fix Version/s:

1.8.7, 1.9.3

File Attachments:

MDL-14326-commited.patch (1 kB)
2.

restorelib.patch (3 kB)

Issue Links:

Dependency

This issue will help resolve:

MDL-10993 Restore creates duplicate activity/resource icons with no text

Database:	Any
Participants:	David Mudrak, Eloy Lafuente (stronk7), Frédéric Hoogstoel, Jeffrey Silverman, John T. Macklin, Kenneth Newquist, Petr Skoda, Robert Levy, Susan Mangan and Tim Hunt
Security Level:	None
QA Assignee:	Petr Skoda
Resolved date:	21/Aug/08
Affected Branches:	MOODLE_18_STABLE, MOODLE_19_STABLE
Fixed Branches:	MOODLE_18_STABLE, MOODLE_19_STABLE

Description

« Hide

PROBLEM
========
During the restore process, an empty course_module is created by backup/restorelib.php (see around line 1000). "Empty" means there is no instance assigned into this module (slot) yet.

If a module restore fails for some reason, the instance record is not created. User can then see just an icon of an module with no activity title.

SOLUTION/PROPOSAL
=================
As a last step of restore, Moodle should delete all course_modules without instance:

DELETE FROM course_modules where instance=0;

Please note, the empty module is deleted anyway, if user clicks on the icon.

All

Comments

Change History

Version Control

Sort Order:

[ Permalink | « Hide ]

David Mudrak added a comment - 12/Apr/08 02:02 PM

Please, forget the last sentence of the Description. The module has to be deleted manually. This is the reply:

The required instance of this module didn't exist. Module deleted.

[ Permalink | « Hide ]

Petr Skoda added a comment - 28/Apr/08 07:01 AM

David, do you feel like making a patch for MOODLE_19_STABLE?

[ Permalink | « Hide ]

David Mudrak added a comment - 02/May/08 04:20 AM

Finally it's just a single line of code that implements the proposed check. Works for me at MOODLE_19_STABLE @ Postgres 8.1

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 02/May/08 08:14 AM

Looks ok for me (I guess a lot of module-related data will be there - partially restore), but at least course won't show fake activities. +1

[ Permalink | « Hide ]

David Mudrak added a comment - 12/May/08 03:13 PM

I am waiting for another +1 from Petr or Martin. The I will commit into both STABLEs.

[ Permalink | « Hide ]

Petr Skoda added a comment - 13/May/08 04:02 AM

I heard some ppl are having activities without entries in course_module table (ask sam from OU if needed

but the opposite should never happen

+1 for 1.8 and 1.9 branches

[ Permalink | « Hide ]

Petr Skoda added a comment - 13/May/08 04:09 AM

hmmm, maybe we could limit it to current course only and add upgrade code with clean up of all existing courses

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 14/May/08 06:58 AM

100% agree with Petr last comment.

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 14/May/08 07:01 AM

Oh, I think it's enough for now.

Hehe, I don't use the web interface at all, at least for me (triaging everyone else bugs) the client is really better! B-)

Ciao

[ Permalink | « Hide ]

Jeffrey Silverman added a comment - 11/Jun/08 07:12 PM

Hi, all. I am experiencing this issue. Quick question: what does the "mdl_course_modules.instance" column signify?

Thanks!

[ Permalink | « Hide ]

Jeffrey Silverman added a comment - 11/Jun/08 07:27 PM

Never mind, figured it out. I'll answer here anyway for the legions of interested people flocking to this page.

The "instance" column is the id of the module, pointing to the modulename table. So, for example, a "label":

mdl_course_module.instance joins to mdl_label.id

[ Permalink | « Hide ]

David Mudrak added a comment - 11/Jun/08 07:59 PM

Yes Jeffrey, you are right. The "instance" is the ID of the module record in its own table. In "mdl_course_modules" we define just "slots" where module instances can be put into.

Ad this issue: I have not committed yet, because I want to check yet another approach: the restorelib should remember which slots (i.e. course_modules) were created during the restore process. At the end of the restore, only empty slots created in this restore process are deleted. This should prevent potential problems with concurrent (parallel) restore processes. In the current proposal, one running restore process could remove empty course_modules slots created by another (and still running!) restore process.

[ Permalink | « Hide ]

Kenneth Newquist added a comment - 13/Aug/08 11:03 PM

I'm experiencing this problem in Moodle 1.9.2 (Build: 20080711). Is there a patch available for this yet? Faculty are starting to hit Moodle as they ramp up for the fall semester and are encountering this problem in droves. If it helps, I can confirm that "DELETE FROM course_modules where instance=0" gets ride of the ghost activities.

[ Permalink | « Hide ]

Kenneth Newquist added a comment - 13/Aug/08 11:34 PM

To clarify, what I'm seeing is the behavior reported in <a href="http://tracker.moodle.org/browse/MDL-10993"> MDL-10993: Restore creates duplicate activity/resource icons with no text</a>. We get these duplicates when importing a course into a new blank course (rather than restoring from a backup). It appears to be intermittent; I can reproduce it on my development system but not my production one, yet I have reports of this happening in production as well.

The "DELETE FROM" line removed the duplicates however, as the tracker notes, fixing this will likely help with my problem as well.

[ Permalink | « Hide ]

David Mudrak added a comment - 15/Aug/08 04:10 AM

Increasing the priority. I am on the holiday at the moment but I shall post a patch next week.

[ Permalink | « Hide ]

John T. Macklin added a comment - 19/Aug/08 08:24 AM

I have had the same issues on multiple sites and this patch should resolve the issue with duplicate icons being created during Backup/Restore or import to new or existing course. moodle/backup/restorelib.php patch...

[ Permalink | « Hide ]

John T. Macklin added a comment - 19/Aug/08 08:26 AM

A work around for the issue with duplication ...

[ Permalink | « Hide ]

David Mudrak added a comment - 21/Aug/08 06:31 PM

I am going to commit the patch today

[ Permalink | « Hide ]

David Mudrak added a comment - 21/Aug/08 07:24 PM

~~MDL-14326~~-commited.patch - modified John's patch. It removes all course_modules that remained empty (i.e. with instance==0) after the particular restore process. The patch does not contain other Remote Learner modifications from the John's patch (i.e. ini_setting max executable time and ignoring mod/resource restore failures). Track these modification in a separate issue if needed.

[ Permalink | « Hide ]

David Mudrak added a comment - 21/Aug/08 08:22 PM

Fixed in CVS

[ Permalink | « Hide ]

Petr Skoda added a comment - 27/Aug/08 02:16 AM

thanks, closing

[ Permalink | « Hide ]

Petr Skoda added a comment - 03/Sep/08 06:23 AM

fixed the old DML syntax in HEAD, sorry for missing that

[ Permalink | « Hide ]

Robert Levy added a comment - 25/Sep/08 02:52 AM

Hi. Has this patch been included in the 1.9.2x version of moodle?

[ Permalink | « Hide ]

David Mudrak added a comment - 25/Sep/08 05:13 PM

Hi Robert. Yes, it has. Use the most recent wekly build of 1.9.2.

[ Permalink | « Hide ]

Tim Hunt added a comment - 22/Dec/08 01:29 PM

Hmm. does this leave orphan contexts floating around? That is, should be be calling delete_course_module from course/lib.php rather than calling delete records?

[ Permalink | « Hide ]

John T. Macklin added a comment - 23/Dec/08 12:44 AM

Tim,

The issue here was additional modules instances if you check grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$modulename,
'iteminstance'=>$cm->instance, 'courseid'=>$cm->course))) against one of these additional instances you will not find any grade_items or
events since the $cm->instance is invalid. Hence why I did not use delete_course_module from course/lib.php .

[ Permalink | « Hide ]

Susan Mangan added a comment - 15/May/09 06:57 AM

We are running version 1.8.8 and this problem still exists. Not sure about during restore but it occurred during import. After importing contents, empty resources appeared in various weeks and when clicking them the following error occurs:The required instance of this module didn't exist. Module deleted. But it doesn't actually delete. I could go back in and manually delete the empty modules, but why does this still occur? Same bug or different one?

[ Permalink | « Hide ]

Frédéric Hoogstoel added a comment - 25/Jun/09 05:43 PM

Hi !

I get this problem with version 1.8.5. But the biggest problemn is not that the icons of non-existing modules appear, but that thsese modules have not been created by the restore !
I did not find any issue about this : the restore stops with no error message. This problem is reported by several users in "Using Moodle" forum and in "Assistance Technique" forum of course "Moodle en Français" :
See http://moodle.org/mod/forum/discuss.php?d=121608#p554771
Should I create a new (related ?) issue ?

Thank you.

All content on this web site is made available under the GNU General Public License, unless otherwise stated.