Moodle

I can confirm this for Import on current 2.0.3 master. The backup on my 2.0.3 master is currently broken, but I assume it is happening there also.

Here's a vote for this! I would still like to see topic summaries have checkboxes on both the import and restore pages, just like all the resources and activities do. Actually, I'd like to see check boxes for blocks as well (custom and HTML blocks in particular) - because these too are copied on restore (when deleting first) but not when importing.

Hello everyone,
summaries of the topics are not imported.
1) I did the tests making backup course and then restoring
2) I did the tests importing of the course
In both cases the summaries came empty.

Moodle Moodle 2.0.3+ (Build: 20110726)
PHP Version 5.3.5-0.dotdeb.0
Mysql 5.0.51.24.5

Just correcting the link type here.

I confirm this issue too for Moodle 2.1.1+ (Build: 20110803)

Anybody working on this?

We need to import from a very large amount of courses, soon (in the next few days)
And i am asking, before i start to figure it out on my own to see if someone is already figure it out or currently working on this issue.

Nadav

I am stuck and need some help, please
(I am using Moodle 2.2 Dev 20111012 git code)

I have used XDEBUG to try and figure out why Course Summaries are not Imported
and got to the "foreach" loop (line 72) that is not entered for the "course_sections" restore task.
(moodle/backup/util/helper/restore_decode_content.class.php)

public function process($processor) {
        if (!$processor instanceof restore_decode_processor) { // No correct processor, throw exception
            throw new restore_decode_content_exception('incorrect_restore_decode_processor', get_class($processor));
        }
        if (!$this->restoreid) { // Check restoreid is set
            throw new restore_decode_rule_exception('decode_content_restoreid_not_set');
        }

        // Get the iterator of contents
        $it = $this->get_iterator();
        foreach ($it as $itrow) {

I have checked the temporary backup folder on the disk,
and it has all the Section's proper information, including the Summery field.

Also, it appears $it->current = null after call to get_iterator that returned no records with the following SQL:

SELECT t.id, t.summary
FROM mdl_course_sections t
JOIN mdl_backup_ids_temp b ON b.newitemid = t.id
WHERE b.backupid = 'fb2cd5954dfba382ddc48b5373878997'
AND b.itemname = 'course_section'

Status update: no progress. still. MHQ, i'd love some help/guidance. if you have any time to spare (i know you do not )
Any tip will help. and i will keep digging myself.

A preferred behavior was discussed and implemented in 1.x versions of moodle, MDL-8848:

Current behaviour is: (1.X)

a) If we are restoring to a NEW course (not existing yet), section summaries ARE restored.
b) If we are restoring to an EXISTING course, DELETING previous data, section summaries ARE restored.
c) If we are restoring to an EXISTING course, KEEPING previous data, section summaries ARE RESTORED IF target section summary is empty.
d) If we are importing to an EXISTING course, section summaries ARE RESTORED IF target section summary is empty.

So section summaries are ALWAYS restored UNLESS there are some content in them.

No new setting nor more options. Just new behaviour.

Credit goes to Dan Poltawski for the patch (I forgot to credit you in the commit, Dan, sorry).

Ciao

Others suggested there be a check box for choosing if summaries should be imported or not.

I have a working patch that gives users an option to include section summaries during import.

Here's a summary of how it works for end users:

When using the import feature to copy course content from an old course
to a new course, there is now an option to "Include Section Summaries".
This option is checked by default.

With the "Include Section Summaries" option checked:
The section summary will be copied if both:
The course section is selected during import (checked)
The course section summary you are copying to is blank (It won't overwrite existing summaries)

• The section name is also copied in this process if it is not set.

The code is a fork off of MOODLE_21_STABLE (currently 2.1.3) and is available on github here:

https://github.com/mikehas/moodle/tree/MDL-27764_SECTION_SUMMARIES

I am new to the process of contributing upstream, but I am hoping someone has time to do a peer review and give feedback.

• Mike

I just made an update to include a file I missed on the initial upload.

https://github.com/mikehas/moodle/tree/MDL-27764_SECTION_SUMMARIES

UI with section summaries option.

Here are screenshots of the process.

Initial import - Select the course to import from:

Course settings for the import (Note new setting to include section summaries):

Section and item settings (Checking a section will now include that section's summary and title):

Confirmation screen:

I'm hoping to get some feedback, does this look like a reasonable interface? We are going to be running this code this upcoming quarter. Unfortunately, due to the way we provision users – they do not have the ability to add themselves to a course – a full backup and restore will lock them out of their course until the next day; Modifying the import was the preferred option.

I'm hoping I can get a few people take a look at this and potentially get the code, or some form of it, adopted into moodle for simpler long-term maintenance.

One current limitation is:

• If the course you are importing from has more sections than the course you are importing to, then the section names and summary information is not copied to the new course.

Any ideas or feedback would be helpful. Are others still interested?

Hey Mike.
Thanks for all the great work. Unfortunately I can't test your patch as I'm on moodle 2.2.
It'll probably hard to get your patch included unless you write it for the newest release (I could be wrong about that.)

The interface looks exactly like what I would want, and how most people would think it works.

The limitation you mentioned would be a big one for teachers trying to share content.
Here's something you might consider as well.
Instead of just including section summaries for all sections or none at all,
you might have it add a checkbox for each section so they could be handled separately. I imagine coding it this way would also get rid of limitation.
This would allow for importing activities/etc from one section without importing the summary, while importing a summary from another section at the same time.
Thanks for the great work!
Kory

Thanks for the feedback Kory.

I did look into adding a section summary option under each section, I opt'd not to for two reasons:

• It cluttered up the screen quite a bit with a new section summary option for each section.
• It was difficult to write the code for this behavior AND to make it look OK in the GUI ( displaying the option at the activity/block level when it is really a section level setting, if that makes sense). This could be overcome with a bit more time.

"The limitation you mentioned would be a big one for teachers trying to share content"

• "If the course you are importing from has more sections than the course you are importing to, then the section names and summary information is not copied to the new course."
• As a work around it would be possible to create extra sections, then perform the import.
• I am working on mimicking the behavior of activities. That is to append the summaries to first section if a corresponding section doesn't exist in the new course.

"Unfortunately I can't test your patch as I'm on moodle 2.2"

• I'm looking into implementing this in 2.2 and 2.3. If there aren't any conflicts, it should be simple.

"It was difficult to write the code for this behavior AND to make it look OK in the GUI ( displaying the option at the activity/block level when it is really a section level setting, if that makes sense). This could be overcome with a bit more time."

What if you put this checkbox (for whether or not to import the summary) next to the section title. e.g.
something like
General □ Include Summary □

Or is that what you meant?
Perhaps if it is too cluttered you could use an icon with hovertext explaining?

"As a work around it would be possible to create extra sections, then perform the import."
Definitely. Perhaps even the code could create these extra sections if needed?

"I am working on mimicking the behavior of activities. That is to append the summaries to first section if a corresponding section doesn't exist in the new course."

Wouldn't it make more sense to create another section and stick everything in it?

"I'm looking into implementing this in 2.2 and 2.3. If there aren't any conflicts, it should be simple."

As soon as you do let me know and you'll have your first beta tester

Kory

"It was difficult to write the code for this behavior AND to make it look OK in the GUI ( displaying the option at the activity/block level when it is really a section level setting, if that makes sense). This could be overcome with a bit more time."

What if you put this checkbox (for whether or not to import the summary) next to the section title. e.g.
something like
General □ Include Summary □

The way moodle2 displays backups settings is very structured. By default, when I added the setting to each section, it displayed directly under the section setting in the same format as the section itself (this was confusing, because it looked like a separate section itself). Modifying the way this displayed would be possible, but would require separate code for display, which may require CSS which is theme dependent for the most part. This was a light-weight solution.

When displayed as a sub-element like the activities, I wasn't able to make a dependency between the "include section summary" option and the "section included" setting. Another look at this with fresh eyes could produce another solution.

Or is that what you meant?
Perhaps if it is too cluttered you could use an icon with hovertext explaining?

"As a work around it would be possible to create extra sections, then perform the import."
Definitely. Perhaps even the code could create these extra sections if needed?

This would be possible, but then there would be a distinct difference from all other import options, (e.g. In this scenario when you import activities no new sections are created.)

"I am working on mimicking the behavior of activities. That is to append the summaries to first section if a corresponding section doesn't exist in the new course."

Wouldn't it make more sense to create another section and stick everything in it?

Again, this would be possible, but it would differ from the way all other import options work.

"I'm looking into implementing this in 2.2 and 2.3. If there aren't any conflicts, it should be simple."

As soon as you do let me know and you'll have your first beta tester

Kory

Will do, thanks Kory.

Section summaries patch for moodle 22.

Kory,

Here is the patch for moodle 2.2+ (Build: 20111209)

MDL-27764_section_summaries_moodle22.patch

You should be able to apply the patch with something like:

$git apply "MDL-27764_section_summaries_moodle22.patch"

Confirmed affects versions 2.1 and 2.2. Affected version only lists 2.0.3.

I noticed this behavior for some time. For large courses, it is VERY tedious to UN-check all the activities or blocks you want to import (from the INCLUDE" section, especially if you only want to import one or two activities.
A better approach is to have a "Check All" and an "UNCheck All"checkbox at the top of the page that would then allow me to manually pick only the one I want to import, instead of having to uncheck every activity in the course.

I agree the uncheck/check all feature would be nice. Note: if you uncheck the section, then it will essentially uncheck all activities and resources for that section during import.

Confirmed sections summaries patch for 2.1 ( https://github.com/mikehas/moodle/tree/MDL-27764_SECTION_SUMMARIES ) also functions correctly in 2.0.
(Note; I just applied the diff at the correct lines, actually merging was not a real option).

Good to hear Sebastian! I haven't seen any other recent activity on this. I'm not sure what triaged signifies.

Hi,

Michael just pinged me about this. I'll be on it, re-reading and discussing a bit here along the week (once we complete this week's integration).

Thanks for all the feedback and sorry for the delay, this was well hidden in my flood of issues.

In the mean time... just one reflexion...

I remember myself implementing the logic present in 1.9 ( MDL-8848, pasted by "mikehas" above) into the 2.x restore/import. In fact I can see how it's implemented @ restore_section_structure_step, always setting the section's name and summary if empty @ target course.

In which cases is this approach failing? Note I've only overviewed the issue comments and the proposed patch. Just want to know the exact situations when the "planned logic" is not working as expected.

Note that, surely I'm a bit reticent to include new settings if we can provide a settings-free alternative, mainly because the introduction of settings is like a virus and somebody could start arguing that the "include section summaries" should be spread to every section in the course, to decide about it. Or also somebody could argue that there should be 3 alternatives for the setting (ignore, set if empty, set always)... so if we can achieve a good solution without any new setting...

So, can you confirm the combinations where the current logic (mimicked from 1.9) is failing?

TIA and ciao, will be back soon.

In our case, the minimal example to show the failing logic is the following:

(tested with moodle 2.2.1 (Build: 20120109) )

1.) Create course with the following settings:

• Topics format
• 10 Topics

2.) Alter summaries of sections 1 and 2 ->

moodledb=# SELECT * FROM mdl_course_sections WHERE course = 2;

id | course | section | name | summary | summaryformat | sequence | visible
------------------------------------------------------------------------------------------------------------------------------------+--------
2 | 2 | 0 | | | 1 | 1 | 1
3 | 2 | 1 | | <p>Some test</p>\r | 1 | 2 | 1
: <p><span style="color: #ff0000;">Test Test Test</span></p>\r
: <p><span style="background-color: #ffff00;">Some other test</span></p>\r
: <p><img src="@@PLUGINFILE@@/IMG_0185.JPG" height="500" width="375" /></p>
4 | 2 | 2 | This is a test section | <p>Some test</p>\r | 1 | | 1
: <p>&</p>
5 | 2 | 3 | | | 1 | | 1
6 | 2 | 4 | | | 1 | | 1
7 | 2 | 5 | | | 1 | | 1
8 | 2 | 6 | | | 1 | | 1
9 | 2 | 7 | | | 1 | | 1
10 | 2 | 8 | | | 1 | | 1
11 | 2 | 9 | | | 1 | | 1
12 | 2 | 10 | | | 1 | | 1

3. Create another empty course, same settings as in 1.) ->

moodledb=# SELECT * FROM mdl_course_sections WHERE course = 3;

id | course | section | name | summary | summaryformat | sequence | visible
------------------------------------------------+--------
13 | 3 | 0 | | | 1 | 3 | 1

4.) Import contents of course with id=2 into course with id=3 ->

moodledb=# SELECT * FROM mdl_course_sections WHERE course = 3;

id | course | section | name | summary | summaryformat | sequence | visible
------------------------------------------------+--------
13 | 3 | 0 | | | 1 | 3 | 1
25 | 3 | 1 | | | 1 | 5 | 1
26 | 3 | 2 | | | 1 | | 1
27 | 3 | 3 | | | 1 | | 1
28 | 3 | 4 | | | 1 | | 1
29 | 3 | 5 | | | 1 | | 1
30 | 3 | 6 | | | 1 | | 1
31 | 3 | 7 | | | 1 | | 1
32 | 3 | 8 | | | 1 | | 1
33 | 3 | 9 | | | 1 | | 1
34 | 3 | 10 | | | 1 | | 1

Note that the summary for course id = 3 is blank (e.g. the query SELECT * FROM mdl_course_sections WHERE summary = '' AND course = 3 yields the same set of sections)

Hope this helps, please feel free to ask for more details.

Best regards & thanks for addressing this issue
Martin

Adding patches for 21_STABLE, 22_STABLE and master.

They, simply, restitute old Moodle 1.9 behavior, where:

• Restore always tries to use as much information from sections as possible.
• Section names and summaries are always restored, but
• If there is already information in the target section, it's not overwritten.

So, with the patch, any restore operation (course restore, import, to new or to existing courses, adding or deleting information) will follow the (1.9) behavior explained above.

It does not use any new setting at all, just applies the rules above.

Plz, comment/test it and I'll be happy to send this to integration.

Ciao

Thank you, Eloy. I tested it in 2.1.3 and it works. The only negative thing I noticed is that I ended up with two News forums in the zero section. I tested without the patch and did not end up with two News forums. This is minor but I wanted to mention it.

Aha, thanks Chris...

about the duped News forums, I'm 100% sure it's unrelated with the change I did to sections restore... perhaps creating a new issue is the best way about that.

So I'm sending this to integration... let's see if it lands this week or next one.

Ciao

PS: Many thanks to all by your support and patience, specially to "mikehas" !

Thanks Eloy - spot on 100% and integrated

Test result: Unsuccessful.

I was able to generate an error during a restore/import. I was able to isolate the cause. When a source course has a section with a name+summary and I restore+merge/import into a course where the corresponding section has no name (default name checked) but has a summary, the following error is thrown (two or three times).

Error: mdb->get_record() found more than one record!

    line 1335 of \lib\dml\moodle_database.php: call to debugging()
    line 1398 of \lib\dml\moodle_database.php: call to moodle_database->get_record_sql()
    line 156 of \backup\moodle2\restore_qtype_plugin.class.php: call to moodle_database->get_field_sql()
    line 131 of \backup\util\plan\restore_structure_step.class.php: call to restore_qtype_plugin->process_question_answer()
    line 103 of \backup\util\helper\restore_structure_parser_processor.class.php: call to restore_structure_step->process()
    line 125 of \backup\util\xml\parser\processors\grouped_parser_processor.class.php: call to restore_structure_parser_processor->dispatch_chunk()
    line 91 of \backup\util\helper\restore_structure_parser_processor.class.php: call to grouped_parser_processor->postprocess_chunk()
    line 148 of \backup\util\xml\parser\processors\simplified_parser_processor.class.php: call to restore_structure_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 105 of \backup\util\plan\restore_structure_step.class.php: call to progressive_parser->process()
    line 153 of \backup\util\plan\base_task.class.php: call to restore_structure_step->execute()
    line 148 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 310 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 46 of \backup\restore.php: call to restore_ui->execute()

On the page, the following information was given once (I'm not sure if it's pertinent)...

Debug info: Duplicate entry '3-24' for key 'mdl_gradgrad_useite_uix'
INSERT INTO mdl_grade_grades (userid,rawgrade,rawgrademax,rawgrademin,rawscaleid,usermodified,finalgrade,hidden,locked,locktime,exported,overridden,excluded,feedback,feedbackformat,information,informationformat,timecreated,timemodified,itemid) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
[array (
0 => '3',
1 => NULL,
2 => '100.00000',
3 => '0.00000',
4 => NULL,
5 => NULL,
6 => '64.50000',
7 => '0',
8 => '0',
9 => '0',
10 => '0',
11 => '0',
12 => '0',
13 => NULL,
14 => '0',
15 => NULL,
16 => '0',
17 => NULL,
18 => NULL,
19 => '24',
)]
Stack trace:

    line 413 of \lib\dml\moodle_database.php: dml_write_exception thrown
    line 901 of \lib\dml\mysqli_native_moodle_database.php: call to moodle_database->query_end()
    line 943 of \lib\dml\mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->insert_record_raw()
    line 222 of \backup\moodle2\restore_stepslib.php: call to mysqli_native_moodle_database->insert_record()
    line 131 of \backup\util\plan\restore_structure_step.class.php: call to restore_gradebook_structure_step->process_grade_grade()
    line 103 of \backup\util\helper\restore_structure_parser_processor.class.php: call to restore_structure_step->process()
    line 125 of \backup\util\xml\parser\processors\grouped_parser_processor.class.php: call to restore_structure_parser_processor->dispatch_chunk()
    line 91 of \backup\util\helper\restore_structure_parser_processor.class.php: call to grouped_parser_processor->postprocess_chunk()
    line 148 of \backup\util\xml\parser\processors\simplified_parser_processor.class.php: call to restore_structure_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 105 of \backup\util\plan\restore_structure_step.class.php: call to progressive_parser->process()
    line 153 of \backup\util\plan\base_task.class.php: call to restore_structure_step->execute()
    line 148 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 310 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 46 of \backup\restore.php: call to restore_ui->execute()

The restore is working properly still, despite the error.

Hi Michael,

I think the first notices are one "old" issue, happening when one question has 2 answers with the same text. It has been reported multiple times in the past (MDL-28539 for example) and I'm 99.99% sure it is unrelated with the change in sections restore performed here. We also have the (somehow) opposite issue (0 answers found, like MDL-26442). We need to find an alternative way to handle those repeated/missing answers, just I don't know how, yet.

And the second error is also an existing one, see MDL-31337 (and it sounds to me that there are more reports similar), about gradebook restore duplicating some grades already existing. Again, I think this is unrelated with the sections fix implemented here. Seems to be something happening when the same course is being restore multiple times onto the same target course (initial thought), but cannot tell much more about it for now.

So, IMO, we can safely consider this as passed if the (sections-centered) testing worked as expected.

Also, given that you've in your hands one backup file able to reproduce the 2 issues above, it would be great to have access to it, if it's not private. So we can try to fix those bugs using such backup as reference.

Ciao

reseting to "testing" status.

OK, it looks like a backup without any activities was still carrying over questions and question categories, and this was the source of the problem.

After repeating this with fresh course that did not have any questions/categories, the result is a big .

Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!).

icao_reverse('arreis olik rebemevon afla letoh ognat');

Closing, ciao