Issue Details (XML | Word | Printable)

Key: MDL-14537
Type: Bug Bug
Status: Open Open
Priority: Minor 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: 

 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: Ascending order - Click to sort in descending order
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.

Eloy Lafuente (stronk7) added a comment - 24/Apr/08 08:42 AM
Wow, sounds important! Assigning to Petr. Thanks!

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.

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


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

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


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


David Mudrak added a comment - 07/May/08 04:46 PM
Patches based on Petr's proposal attached in subtasks

David Mudrak added a comment - 20/Jun/08 04:40 PM
Ready to be committed. Waiting for +2