Moodle

Incorrectly Displaying New Admin Settings If Not Set

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7, 1.7.1, 1.8
  • Fix Version/s: 1.9
  • Component/s: Administration
  • Labels:
    None
  • Database:
    MySQL
  • Affected Branches:
    MOODLE_17_STABLE, MOODLE_18_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

Settings using the new admin interface will incorrectly appear as though they are set to their default value even if they have not been created.

This could be reproduced in a number of ways, from deleting one of the values for the config table to accidentally clicking away during the upgrade:

  • Upgrade a moodle
  • upgrade core takes place
  • prompted to upgrade settings
  • click away from the settings page (for example the admin link, rather than save settings)
  • module upgrade continues
  • upgrade finished
  • View a setting which should've been set (e.g. enablerecordcache in performance, gradebookroles in appearance)
  • the settings will look like its set to the default, but the setting doesn't exist.

A way round this would perhaps be more agreessive redirection to the uprgrade settings screen if any settings don't exist? Or perhaps something which indicates is showing the default rather than what it is set to?

Activity

Hide
Dan Poltawski added a comment -

Attaching a patch which will look check for admin settings for any which aren't initialised from admin/index.php, and redirect to the new settings screen if any aren't found. Being slighly more agressive at redirecting than the previous one-time chance it had on upgrades.

I don't know if searching for new settings every time we hit admin/index.php is a bit heavyweight, though..

Show
Dan Poltawski added a comment - Attaching a patch which will look check for admin settings for any which aren't initialised from admin/index.php, and redirect to the new settings screen if any aren't found. Being slighly more agressive at redirecting than the previous one-time chance it had on upgrades. I don't know if searching for new settings every time we hit admin/index.php is a bit heavyweight, though..
Hide
Dan Poltawski added a comment -

Hi Petr,

I wonder if you could review this patch i've created. Basically the idea is to try and prevent the condition where by a site admin doesn't properly setup new admin settings.

This causes a problem as it means the settings will just display their default values when examined (despite not actually being set).

In order to combat this i've added a redirect to the new settings screen if any admin settings aren't set..

Dan

Show
Dan Poltawski added a comment - Hi Petr, I wonder if you could review this patch i've created. Basically the idea is to try and prevent the condition where by a site admin doesn't properly setup new admin settings. This causes a problem as it means the settings will just display their default values when examined (despite not actually being set). In order to combat this i've added a redirect to the new settings screen if any admin settings aren't set.. Dan
Hide
Petr Škoda (skodak) added a comment -

I would move the new function to lib/adminlib,php

My +1 to commit it into HEAD, and maybe backporting to 1.8.x after some more testing

Show
Petr Škoda (skodak) added a comment - I would move the new function to lib/adminlib,php My +1 to commit it into HEAD, and maybe backporting to 1.8.x after some more testing
Hide
Petr Škoda (skodak) added a comment -

I do not think we need the
unset($setting)
there

Show
Petr Škoda (skodak) added a comment - I do not think we need the unset($setting) there
Hide
Dan Poltawski added a comment -

I've commited this to HEAD

Show
Dan Poltawski added a comment - I've commited this to HEAD

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: