Moodle

Lots of config settings don't have values in the database table mdl_config until they are changed

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.9.2
  • Fix Version/s: 1.9.3
  • Component/s: Database SQL/XMLDB
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

I'm trying to compare some Moodles to make sure they are set up identically by running SQL that selects name, value from mdl_config.

Some settings, even though they are true or false, have three settings, one before you do anything, and then 2 more depending on what you choose when you save the settings.

example: forum_usermarksread, starts blank, if you save the settings page that contains it without touching that dropdown the value in the database becomes 0, if you change it specificaly to YES you get 1.

This will probably give rise to obscure situations were updating a related setting results in the database value changing, and potentially some, but not all code treating that value as different from the null value. This bit us recently when anyone who updated their profile was given a timezone from half the world away.

There's several/many like this from simply doing select name, value from mdl_config where value = ''; the ones that jump out at me as not being text fields but rather choices from a defined list are: cachetype, resource_popup, register_auth, notifyloginfailures

Activity

Hide
Petr Škoda (skodak) added a comment -

Hello,
this is the result of admin settings changes introduced after 1.7, in general we do not use comparison for settings with true/false meaning - we use empty() and !empty() which works fine for '1', '0' and ''. Sometimes we even treat legacy string values as true/false.

Petr

Show
Petr Škoda (skodak) added a comment - Hello, this is the result of admin settings changes introduced after 1.7, in general we do not use comparison for settings with true/false meaning - we use empty() and !empty() which works fine for '1', '0' and ''. Sometimes we even treat legacy string values as true/false. Petr
Hide
David Scotson added a comment -

Three points:

1) not all of these are true/false e.g. notifyloginfailures has three choices: Nobody, Administrator, All Administrators.

2) This has caused an actual bug, because when you set an HTML form from the database you can get two different values that mean the same thing.

3) as I mentioned some are missing altogether e.g. rcache which doesn't exist until you save the performance page

Show
David Scotson added a comment - Three points: 1) not all of these are true/false e.g. notifyloginfailures has three choices: Nobody, Administrator, All Administrators. 2) This has caused an actual bug, because when you set an HTML form from the database you can get two different values that mean the same thing. 3) as I mentioned some are missing altogether e.g. rcache which doesn't exist until you save the performance page
Hide
Howard Miller added a comment -

I just took it that these where oversights in setting default values for things. I've fixed a few of these in 1.9. I was going to take a dig and check. Am I missing the point?

Show
Howard Miller added a comment - I just took it that these where oversights in setting default values for things. I've fixed a few of these in 1.9. I was going to take a dig and check. Am I missing the point?
Hide
Petr Škoda (skodak) added a comment -

aaah

1/ bug - should be fixed
3/ rcache is a special case, some settings are initialised in setup.php

Please report any inconsistencies or missing values here, thanks

Show
Petr Škoda (skodak) added a comment - aaah 1/ bug - should be fixed 3/ rcache is a special case, some settings are initialised in setup.php Please report any inconsistencies or missing values here, thanks
Hide
Petr Škoda (skodak) added a comment -

no more problems reported, closing - thanks!

Show
Petr Škoda (skodak) added a comment - no more problems reported, closing - thanks!
Hide
Tim Hunt added a comment -

Closing, as per last comment from Petr.

Show
Tim Hunt added a comment - Closing, as per last comment from Petr.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: