[#MDL-14537] Modules Database and Forum do not convert 1.6 settings into 1.7 role overrides correctly

Issue Details (XML | Word | Printable)

Key:	MDL-14537
Type:	Bug
Status:	Open
Priority:	Minor
Assignee:	David Mudrak
Reporter:	David Mudrak
Votes:	1
Watchers:	2

Operations

Add/Edit UI Mockup to this issue

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

Moodle

Modules Database and Forum do not convert 1.6 settings into 1.7 role overrides correctly

Created: 24/Apr/08 06:54 AM Updated: 26/Nov/09 12:22 AM

Component/s:

Database activity module, Forum

Affects Version/s:

1.7.4, 1.8.5, 1.9

Fix Version/s:

1.9.8

Participants:	David Mudrak and Eloy Lafuente (stronk7)
Security Level:	None
Affected Branches:	MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE
Fixed Branches:	MOODLE_19_STABLE

Sub-Tasks:

All

Open

Sub-Task Progress:

1.	upgrade of editingteacherroles: fix database activity module		Open	David Mudrak
2.	upgrade of editingteacherroles: fix forum activity module		Open	David Mudrak

Description

« Hide

Both modules (and maybe some others) convert some 1.6 settings into role overrides during upgrade or restore from 1.6 to a higher version. During this process, the list of legacy teacher roles is used. But, only moodle/legacy:teacher is used at the moment, while the roles with moodle/legacy:editingteacher should be used as well.

I have not tested, but reading the code, the capabilities are set to non-editing teachers only.

All

Comments

Change History

Version Control

Sort Order:

[ Permalink | « Hide ]

David Mudrak added a comment - 24/Apr/08 07:13 AM

Patch attached. It adds editing teachers into the $teacherroles. The casting is there because get_roles_with_capability() may return "false" and PHP can't concatenate boolean with array.

[ Permalink | « Hide ]

Eloy Lafuente (stronk7) added a comment - 24/Apr/08 08:42 AM

Wow, sounds important! Assigning to Petr. Thanks!

[ Permalink | « Hide ]

David Mudrak added a comment - 29/Apr/08 03:20 AM

Oooops - the previous patch actually broke the upgrade - shame on me. I have removed it and here is (hopefully) good one.

[ Permalink | « Hide ]

David Mudrak added a comment - 07/May/08 02:59 AM

The patch is based on the following PHP casting feature:
"Converting NULL to an array results in an empty array."
http://www.php.net/manual/en/language.types.array.php#language.types.array.casting

waiting for +2 approval

[ Permalink | « Hide ]

David Mudrak added a comment - 07/May/08 05:46 AM

The same patch in unified format (I swear there was a time when MartinD recommended context diff format. But Eclipse guys won

[ Permalink | « Hide ]

David Mudrak added a comment - 07/May/08 06:21 AM

The patch has been discarded. Petr proposed other approach:

change calls of notice() to notify() - we do not want to stop processing in case of missing roles
change the interface of data_convert_to_roles() and forum_convert_to_roles() to accept additional parameter $editingteacherroles
take (!defined('RESTORE_SILENTLY')) into a count

I am going to prepare the patch

[ Permalink | « Hide ]

David Mudrak added a comment - 07/May/08 03:21 PM

Affected files:

Index: mod/data/restorelib.php
Index: mod/data/db/mysql.php
Index: mod/data/db/postgres7.php
Index: mod/forum/restorelib.php
Index: mod/forum/db/mysql.php
Index: mod/forum/db/postgres7.php

[ Permalink | « Hide ]

David Mudrak added a comment - 07/May/08 04:46 PM

Patches based on Petr's proposal attached in subtasks

[ Permalink | « Hide ]

David Mudrak added a comment - 20/Jun/08 04:40 PM

Ready to be committed. Waiting for +2

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