Moodle

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

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Won't Fix
  • Affects Version/s: 1.7.4, 1.8.5, 1.9
  • Fix Version/s: 1.9.10
  • Labels:
    None
  • Affected Branches:
    MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

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.

Activity

Hide
David Mudrak added a comment -

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.

Show
David Mudrak added a comment - 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.
Hide
Eloy Lafuente (stronk7) added a comment -

Wow, sounds important! Assigning to Petr. Thanks!

Show
Eloy Lafuente (stronk7) added a comment - Wow, sounds important! Assigning to Petr. Thanks!
Hide
David Mudrak added a comment -

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

Show
David Mudrak added a comment - Oooops - the previous patch actually broke the upgrade - shame on me. I have removed it and here is (hopefully) good one.
Hide
David Mudrak added a comment -

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

Show
David Mudrak added a comment - 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
Hide
David Mudrak added a comment -

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

Show
David Mudrak added a comment - The same patch in unified format (I swear there was a time when MartinD recommended context diff format. But Eclipse guys won
Hide
David Mudrak added a comment -

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

Show
David Mudrak added a comment - 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
Hide
David Mudrak added a comment -

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

Show
David Mudrak added a comment - 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
Hide
David Mudrak added a comment -

Patches based on Petr's proposal attached in subtasks

Show
David Mudrak added a comment - Patches based on Petr's proposal attached in subtasks
Hide
David Mudrak added a comment -

Ready to be committed. Waiting for +2

Show
David Mudrak added a comment - Ready to be committed. Waiting for +2
Hide
David Mudrak added a comment -

Both 1.6 and 1.7 are not supported now and apparently nobody complains

Show
David Mudrak added a comment - Both 1.6 and 1.7 are not supported now and apparently nobody complains

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: