Moodle
  1. Moodle
  2. MDL-37459

Allow "locked" and "advanced" checkboxes to be added to any admin setting.

    Details

    • Testing Instructions:
      Hide
      1. This change should have no effect on existing module forms because it only adds the functions but does not use them in any modules (see linked issues for implementations).
      2. So testing requires:
      3. Viewing the admin settings forms for quiz and verifying that the settings can be updated and saved with no errors
      4. Viewing the activity settings forms for a few different activities including assignment and quiz and verifying there are no errors when viewing/saving.
      Show
      This change should have no effect on existing module forms because it only adds the functions but does not use them in any modules (see linked issues for implementations). So testing requires: Viewing the admin settings forms for quiz and verifying that the settings can be updated and saved with no errors Viewing the activity settings forms for a few different activities including assignment and quiz and verifying there are no errors when viewing/saving.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-37459-master
    • Rank:
      47095

      Description

      This patch moves the implementation of the "advanced" and "locked" checkboxes for admin_settings to the base class - so any admin setting can use these flags. The current implementations of blah_with_advanced and blah_with_lock have been converted into wrappers which just set the flags on the base class.

      This allows any setting to use any combination of locked or advanced checkboxes and makes it easier to add new ones (hidden might be nice...).

      This change is 100% backward compatible and does not require any changes to existing subclasses of admin_settings.

      To enable an advanced checkbox - call $setting->set_advanced(true);
      To enable an locked checkbox - call $setting->set_lockable(true);

      To set the defaults for the checkboxes call

      $setting->set_advanced_flag_options(admin_setting::OPTION_ENABLED, true);
      or
      $setting->set_locked_flag_options(admin_setting::OPTION_ENABLED, false);

      Note: All modules will work with this patch with 0 changes - the config values set have the same names as the previous implementations.

      Discussion link:
      https://moodle.org/mod/forum/discuss.php?d=220263

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Feel free to put this up for peer review when you are ready.

          Show
          Michael de Raadt added a comment - Feel free to put this up for peer review when you are ready.
          Hide
          Damyon Wiese added a comment -

          With this patch integrated - the code to add a "locked" and "advanced" flag to a single setting for an activity (teamsubmission for mod_assign is shown) is this:

              $name = new lang_string('teamsubmission', 'mod_assign');
              $description = new lang_string('teamsubmission_help', 'mod_assign');
              $setting = new admin_setting_configcheckbox('assign/teamsubmission',
                                                              $name,
                                                              $description,
                                                              0);
              $setting->set_advanced(true);
              $setting->set_lockable(true);
              $settings->add($setting);
          
          

          in mod/assign/settings.php
          which adds the default setting to the list of admin settings.

          and this:

          $this->apply_admin_defaults();
          

          At the end of the constructor in mod/assign/mod_form.php

          Show
          Damyon Wiese added a comment - With this patch integrated - the code to add a "locked" and "advanced" flag to a single setting for an activity (teamsubmission for mod_assign is shown) is this: $name = new lang_string('teamsubmission', 'mod_assign'); $description = new lang_string('teamsubmission_help', 'mod_assign'); $setting = new admin_setting_configcheckbox('assign/teamsubmission', $name, $description, 0); $setting->set_advanced( true ); $setting->set_lockable( true ); $settings->add($setting); in mod/assign/settings.php which adds the default setting to the list of admin settings. and this: $ this ->apply_admin_defaults(); At the end of the constructor in mod/assign/mod_form.php
          Hide
          Damyon Wiese added a comment -

          Screenshots

          Show
          Damyon Wiese added a comment - Screenshots
          Hide
          Tim Hunt added a comment -

          I am just looking at the code, and spotted the following points. (Sorry, there seem to be a lot of these!)

          1. From the piont of view of someone who might use this API, I feel it would be more natural if the set_advanced method signature was differernt. I cannot think of any situation where I would call

              $setting->set_advanced(false);
          

          I can think that it would be nice to be able to do

              $setting->set_advanced(true);
              $setting->set_advanced_default(true); in one line of code.
          

          Therefore, I would make the API

              function set_advanced($default = admin_setting::DEFAULT_NOT_ADVANCED);
          

          and I would define the two constants

              const DEFAULT_NOT_ADVANCED = false;
              const DEFAULT_ADVANCED = true;
          

          because

              $setting->set_advanced(admin_setting::DEFAULT_ADVANCED);
          

          is more self-documenting.

          2. I think it would make more sense if the set_lockable methods were closer to the set_advanced methods, and comment 1 applies to them too.

          3. Actually, set_advanced and set_lockable are bad method names. We are not making the admin setting advanced or locable. We are saying that the admin setting should also have advanced and locable checkboxes. How about set_with_advanced or add_advanced_checkbox. (Is checkbox too much of an implementation detail?) So:

              $setting->add_advanced_option(admin_setting::DEFAULT_ADVANCED);
              $setting->add_locked_option();
          

          4. If you have an API, why are the admin_setting fields public?

          5. I think that admin_setting_configtext_with_advanced etc. should become @deprecated, with developer debug output to tell you the new better way to implement this.

          6. I think that you should do the extra commit to covert the existing quiz settings to your new classes.

          7. I think that the moodleform_mod.php changes should be guarded by a call to plugin_supports(FEATURE_ACTIVITY_SETTING_DEFAULTS); (but I might be wrong about this.)

          8. PHPdoc comments like /** @var bool $advanceddefault default for advanced checkbox */ are wrong. WIth @var you should not repeat the variable name (I think).

          9. $lockedsettings = false; in moodleform_mod.php. This variable is never used - or was there a plan to output some extra help text on screen only if there were locked settings, and that code is missing?

          Show
          Tim Hunt added a comment - I am just looking at the code, and spotted the following points. (Sorry, there seem to be a lot of these!) 1. From the piont of view of someone who might use this API, I feel it would be more natural if the set_advanced method signature was differernt. I cannot think of any situation where I would call $setting->set_advanced( false ); I can think that it would be nice to be able to do $setting->set_advanced( true ); $setting->set_advanced_default( true ); in one line of code. Therefore, I would make the API function set_advanced($ default = admin_setting::DEFAULT_NOT_ADVANCED); and I would define the two constants const DEFAULT_NOT_ADVANCED = false ; const DEFAULT_ADVANCED = true ; because $setting->set_advanced(admin_setting::DEFAULT_ADVANCED); is more self-documenting. 2. I think it would make more sense if the set_lockable methods were closer to the set_advanced methods, and comment 1 applies to them too. 3. Actually, set_advanced and set_lockable are bad method names. We are not making the admin setting advanced or locable. We are saying that the admin setting should also have advanced and locable checkboxes. How about set_with_advanced or add_advanced_checkbox. (Is checkbox too much of an implementation detail?) So: $setting->add_advanced_option(admin_setting::DEFAULT_ADVANCED); $setting->add_locked_option(); 4. If you have an API, why are the admin_setting fields public? 5. I think that admin_setting_configtext_with_advanced etc. should become @deprecated, with developer debug output to tell you the new better way to implement this. 6. I think that you should do the extra commit to covert the existing quiz settings to your new classes. 7. I think that the moodleform_mod.php changes should be guarded by a call to plugin_supports(FEATURE_ACTIVITY_SETTING_DEFAULTS); (but I might be wrong about this.) 8. PHPdoc comments like /** @var bool $advanceddefault default for advanced checkbox */ are wrong. WIth @var you should not repeat the variable name (I think). 9. $lockedsettings = false; in moodleform_mod.php. This variable is never used - or was there a plan to output some extra help text on screen only if there were locked settings, and that code is missing?
          Hide
          Tim Hunt added a comment -

          Sorry, and one more:

          10. Are you familiar with this bit of config-dist.php: https://github.com/moodle/moodle/blob/master/config-dist.php#L523. Will admins be able to control the _advanced and _locked options in this way? The answer should be yes, and you should update that bit of config-dist to explain how.

          Show
          Tim Hunt added a comment - Sorry, and one more: 10. Are you familiar with this bit of config-dist.php: https://github.com/moodle/moodle/blob/master/config-dist.php#L523 . Will admins be able to control the _advanced and _locked options in this way? The answer should be yes, and you should update that bit of config-dist to explain how.
          Hide
          Damyon Wiese added a comment -

          Thanks Tim,

          This feedback is very helpful - I'll update the branch to incorporate this.

          Show
          Damyon Wiese added a comment - Thanks Tim, This feedback is very helpful - I'll update the branch to incorporate this.
          Hide
          Damyon Wiese added a comment -

          I have updated the patch.

          Re each point made by Tim above:

          1 2 & 3. I refactored this version a bit - instead of bools in the setting - I made a class for "admin_setting_flag" and added an empty array of these classes to each setting. The first call to set_advanced_flag_options (for example) will create an instance of the flag and add it to the array.

          So the api becomes:

          $setting>set_advanced_flag_options(admin_setting_flag::OPTION_ENABLED, true);
          $setting>set_locked_flag_options(admin_setting_flag::OPTION_ENABLED, false);

          I left the first param there - because you might want to disable a setting in a subclass (maybe?).

          I left the second param as a boolean instead of using consts - because there is existing code that is passing these as bools (_with_advanced and friends) and I don't think it makes sense to force a refactor of all that code.

          locked and advanced are identical (I also had a need to add a new one for "enabled" - this was specifically to set the default for an optional date_time_selector).

          4. Now private.

          5. I thought about this and this would create a huge patch for no benefit. The old classes are still handy shortcuts to add a setting with an advanced checkbox in a single line.

          6. I'll do this for quiz in a separate linked issue (this one could be a meta).

          7. The moodleform_mod changes are already opt-in - I made it so you have to call apply_admin_defaults from the mod_form constructor if you want the defaults/flags set.

          8. Fixed.

          9. Yes (good ESP) I have removed this.

          10. Updated config-dist.php with an example of setting the flags on a plugin setting.

          Show
          Damyon Wiese added a comment - I have updated the patch. Re each point made by Tim above: 1 2 & 3. I refactored this version a bit - instead of bools in the setting - I made a class for "admin_setting_flag" and added an empty array of these classes to each setting. The first call to set_advanced_flag_options (for example) will create an instance of the flag and add it to the array. So the api becomes: $setting>set_advanced_flag_options(admin_setting_flag::OPTION_ENABLED, true); $setting>set_locked_flag_options(admin_setting_flag::OPTION_ENABLED, false); I left the first param there - because you might want to disable a setting in a subclass (maybe?). I left the second param as a boolean instead of using consts - because there is existing code that is passing these as bools (_with_advanced and friends) and I don't think it makes sense to force a refactor of all that code. locked and advanced are identical (I also had a need to add a new one for "enabled" - this was specifically to set the default for an optional date_time_selector). 4. Now private. 5. I thought about this and this would create a huge patch for no benefit. The old classes are still handy shortcuts to add a setting with an advanced checkbox in a single line. 6. I'll do this for quiz in a separate linked issue (this one could be a meta). 7. The moodleform_mod changes are already opt-in - I made it so you have to call apply_admin_defaults from the mod_form constructor if you want the defaults/flags set. 8. Fixed. 9. Yes (good ESP) I have removed this. 10. Updated config-dist.php with an example of setting the flags on a plugin setting.
          Hide
          Tim Hunt added a comment -

          Thanks Damyon. That is looking better.

          1, 2 & 3) That is a nicer API.

          7. apply_admin_defaults is a nice way to do this.

          I have a few new comments, but they are all pretty trivial, and in some cases just observations on code readability that occured to me while reading your code, not suggesting you change anything:

          11. const ADVANCED_FLAG = 0; - it is almost always better to make these values self-documenting when you do print_object debugging, so I would do const ADVANCED_FLAG = 'advanced'; (For example, all the PARAM_ constants used to be meaningless numbers, but now they are short meaningful strings.)

          12. Should set_flag_options be protected? Also, it it was me, I would be inclined to put the $shortname argument first, so if you are reading a the code of a call to that function, you can see what is being configured, before the details of the configuration, but is is probably not worth changing that.

          13. In output_setting_flags and write_setting_flags, should you have if ($flag->is_enabled()) inside the loop? Oh, I see, that code is in the admin_setting_flag class. That is an interesting observation on code readability: It was clear to me that get_setting_flag_defaults was correct code, but for the other too I had to dig deeper to verify it was correct. To make things clearer, you would either need to move the if, or rename the method to output_setting_flag_if_enabled.

          14. $defaults = array_merge($defaults, $setting->get_setting_flag_defaults()); - and alternative way to do this API would be
          $defaults = $setting->add_setting_flag_defaults($defaults);

          15. if (count($defaults)) { is a bad way to do if (!empty($defaults)) {. The second is more readable "If defaults is not empty".

          16. You should not add a blank line at the end of theme/base/style/course.css.

          I had one other thought, which we might think about in future, but which is not part of this issue: It is a pity that apply_admin_defaults() is only in moodleform_mod.php. It might be nice to have such easy defaults for other types of forms. For example there are defaults for the course settings form in the admin tree. Of course, a more generic apply_admin_defaults in the form base class would need to take more arguments in order to do the right tying. As I say, an idea for the future.

          Show
          Tim Hunt added a comment - Thanks Damyon. That is looking better. 1, 2 & 3) That is a nicer API. 7. apply_admin_defaults is a nice way to do this. I have a few new comments, but they are all pretty trivial, and in some cases just observations on code readability that occured to me while reading your code, not suggesting you change anything: 11. const ADVANCED_FLAG = 0; - it is almost always better to make these values self-documenting when you do print_object debugging, so I would do const ADVANCED_FLAG = 'advanced'; (For example, all the PARAM_ constants used to be meaningless numbers, but now they are short meaningful strings.) 12. Should set_flag_options be protected? Also, it it was me, I would be inclined to put the $shortname argument first, so if you are reading a the code of a call to that function, you can see what is being configured, before the details of the configuration, but is is probably not worth changing that. 13. In output_setting_flags and write_setting_flags, should you have if ($flag->is_enabled()) inside the loop? Oh, I see, that code is in the admin_setting_flag class. That is an interesting observation on code readability: It was clear to me that get_setting_flag_defaults was correct code, but for the other too I had to dig deeper to verify it was correct. To make things clearer, you would either need to move the if, or rename the method to output_setting_flag_if_enabled. 14. $defaults = array_merge($defaults, $setting->get_setting_flag_defaults()); - and alternative way to do this API would be $defaults = $setting->add_setting_flag_defaults($defaults); 15. if (count($defaults)) { is a bad way to do if (!empty($defaults)) {. The second is more readable "If defaults is not empty". 16. You should not add a blank line at the end of theme/base/style/course.css. I had one other thought, which we might think about in future, but which is not part of this issue: It is a pity that apply_admin_defaults() is only in moodleform_mod.php. It might be nice to have such easy defaults for other types of forms. For example there are defaults for the course settings form in the admin tree. Of course, a more generic apply_admin_defaults in the form base class would need to take more arguments in order to do the right tying. As I say, an idea for the future.
          Hide
          Damyon Wiese added a comment -

          Thanks Tim,

          Branch is updated again.

          11 - Those consts were left in and unused - I have stripped them out.

          12 - Yes protected makes sense - if you want to add a new flag you should subclass.

          13 - Yes agree - moved the check outside the function and into the loop

          14 - Yes agree - I don't like array_merge much.

          15 - Yes agree - changed

          16 - Oops - backtracked and amended the old commit.

          Re: apply_admin_defaults - I think we should take the modules first and then see how many other places this might be useful.

          Thanks for your time looking at this.

          Show
          Damyon Wiese added a comment - Thanks Tim, Branch is updated again. 11 - Those consts were left in and unused - I have stripped them out. 12 - Yes protected makes sense - if you want to add a new flag you should subclass. 13 - Yes agree - moved the check outside the function and into the loop 14 - Yes agree - I don't like array_merge much. 15 - Yes agree - changed 16 - Oops - backtracked and amended the old commit. Re: apply_admin_defaults - I think we should take the modules first and then see how many other places this might be useful. Thanks for your time looking at this.
          Hide
          Tim Hunt added a comment -

          Great! If you have done all that, then this must be nearly cooked.

          I am thinking that I have looked at this several times, and so it would be better if you got a fresh pair of eyes to give this a final once-over. Do you want to try to grab someone else from HQ for a final review?

          Show
          Tim Hunt added a comment - Great! If you have done all that, then this must be nearly cooked. I am thinking that I have looked at this several times, and so it would be better if you got a fresh pair of eyes to give this a final once-over. Do you want to try to grab someone else from HQ for a final review?
          Hide
          Damyon Wiese added a comment -

          Rebased

          Show
          Damyon Wiese added a comment - Rebased
          Hide
          Dan Poltawski added a comment -

          Hi Damyon,

          Thanks - this looks good to me for integration.

          1. Is it worth doing an an AMOS copy for the Enabled string from another file with that same string? (I'm not 100% certain what is best practice here, seems like it'd save translators work, but I guess there is risk of a bad string in different context).
          2. mod/assign/settings.php and mod/assign/mod_form.php have some unnecessary whitespace changes
          3. Personally i'd prefer to see some of the commits squashed (where they are telling the story of the review cycle, rather than the story of the code change in Moodle).
          Show
          Dan Poltawski added a comment - Hi Damyon, Thanks - this looks good to me for integration. Is it worth doing an an AMOS copy for the Enabled string from another file with that same string? (I'm not 100% certain what is best practice here, seems like it'd save translators work, but I guess there is risk of a bad string in different context). mod/assign/settings.php and mod/assign/mod_form.php have some unnecessary whitespace changes Personally i'd prefer to see some of the commits squashed (where they are telling the story of the review cycle, rather than the story of the code change in Moodle).
          Hide
          Ralf Hilgenstock added a comment -

          Is it possible to hide 'locked' settings for teachers by admin settings?
          This would offer the opportunity to reduce settings visibility for activities for teachers based on organizational definition.

          Is there a plan to make all activity settings configurable by admin settings?

          Ralf

          Show
          Ralf Hilgenstock added a comment - Is it possible to hide 'locked' settings for teachers by admin settings? This would offer the opportunity to reduce settings visibility for activities for teachers based on organizational definition. Is there a plan to make all activity settings configurable by admin settings? Ralf
          Hide
          Damyon Wiese added a comment -

          I think this should be left for 2.6 - there is not enough time left to implement this for a majority of modules in 2.5 and advanced settings may change a bit with the shortforms changes.

          Will resubmit early in 2.6 cycle.

          Show
          Damyon Wiese added a comment - I think this should be left for 2.6 - there is not enough time left to implement this for a majority of modules in 2.5 and advanced settings may change a bit with the shortforms changes. Will resubmit early in 2.6 cycle.
          Hide
          Damyon Wiese added a comment -

          Ralf, it was decided in the linked discussion that hiding locked settings would be a "bad" idea. Also - each module will require some small changes to implement this functionality - these will be tackled as part of the META MDL-37914.

          The dust has settled on short forms - I think it's time to rebase and submit this for integration.

          Show
          Damyon Wiese added a comment - Ralf, it was decided in the linked discussion that hiding locked settings would be a "bad" idea. Also - each module will require some small changes to implement this functionality - these will be tackled as part of the META MDL-37914 . The dust has settled on short forms - I think it's time to rebase and submit this for integration.
          Hide
          Damyon Wiese added a comment -

          Note - I rebased this and rechecked the branch (along with MDL-37621)

          Show
          Damyon Wiese added a comment - Note - I rebased this and rechecked the branch (along with MDL-37621 )
          Hide
          Marina Glancy added a comment -

          Hi Damyon,
          this is a great improvement and great work.
          I would substitute get_string() with new lang_string() in functions set_xxx_flag_options() so the tree is built faster.
          What do you think?

          Show
          Marina Glancy added a comment - Hi Damyon, this is a great improvement and great work. I would substitute get_string() with new lang_string() in functions set_xxx_flag_options() so the tree is built faster. What do you think?
          Hide
          Damyon Wiese added a comment -

          Hi Marina,

          Good suggestion about lang_string - I have added a patch to change this.

          I also fixed the git diff url.

          Show
          Damyon Wiese added a comment - Hi Marina, Good suggestion about lang_string - I have added a patch to change this. I also fixed the git diff url.
          Hide
          Damyon Wiese added a comment -

          Been discussing the patch and I need to add more info to the phpdocs for apply_admin_defaults about the way the defaults for dates are handled (currently relative to midnight).

          Show
          Damyon Wiese added a comment - Been discussing the patch and I need to add more info to the phpdocs for apply_admin_defaults about the way the defaults for dates are handled (currently relative to midnight).
          Hide
          Damyon Wiese added a comment -

          Added docs and fixed the bug in my lang_string changes - this is ready to look at again. I squashed the commits again - no sense in having lots of tiny changes.

          I also added one change to apply_admin_defaults from your suggestion Marina - it now accepts an optional array of dates that are used as the relative dates for the defaults. I slept on it and decided this was worth adding now even if it is not used.

          Show
          Damyon Wiese added a comment - Added docs and fixed the bug in my lang_string changes - this is ready to look at again. I squashed the commits again - no sense in having lots of tiny changes. I also added one change to apply_admin_defaults from your suggestion Marina - it now accepts an optional array of dates that are used as the relative dates for the defaults. I slept on it and decided this was worth adding now even if it is not used.
          Hide
          Marina Glancy added a comment -

          Thanks Damyon, this has been integrated.
          I also added label dev_docs_required because this is a change in modules API

          Show
          Marina Glancy added a comment - Thanks Damyon, this has been integrated. I also added label dev_docs_required because this is a change in modules API
          Hide
          Damyon Wiese added a comment -

          Somehow, unprintable characters made it into the last commit.

          Here is a patch to fix this:

          https://github.com/damyon/moodle/branches/MDL-37459-master-fix

          Show
          Damyon Wiese added a comment - Somehow, unprintable characters made it into the last commit. Here is a patch to fix this: https://github.com/damyon/moodle/branches/MDL-37459-master-fix
          Hide
          Marina Glancy added a comment -

          patch integrated. thanks.

          Show
          Marina Glancy added a comment - patch integrated. thanks.
          Hide
          Petr Škoda added a comment - - edited

          I believe this is not doing what it advertises to do, the problem is that the apply_admin_defaults() overrides existing current values.

          Scenario:
          1/ something is not locked - teacher select "true" and saves form
          2/ admin changes enables setting - locks and sets default to false
          3/ nothing changes for anybody yet
          4/ teacher open the form to verify what is the setting - they see false (??), they cancel edit
          5/ nothing changes
          6/ teacher opens the form again and just saves
          7/ the something is now really "false", wow

          My problems are:
          a/ this is not a "locking", it is "forcing new default after save"
          b/ I do not like the visual part much, line of checkboxes - the first checkbox has label on the left the rest on the right, I geuss there should be at least some CSS to visually deemphasise the lock and advanced flag

          Failing for now, please get feedback from MD or other integrators, sorry.

          Show
          Petr Škoda added a comment - - edited I believe this is not doing what it advertises to do, the problem is that the apply_admin_defaults() overrides existing current values. Scenario: 1/ something is not locked - teacher select "true" and saves form 2/ admin changes enables setting - locks and sets default to false 3/ nothing changes for anybody yet 4/ teacher open the form to verify what is the setting - they see false (??), they cancel edit 5/ nothing changes 6/ teacher opens the form again and just saves 7/ the something is now really "false", wow My problems are: a/ this is not a "locking", it is "forcing new default after save" b/ I do not like the visual part much, line of checkboxes - the first checkbox has label on the left the rest on the right, I geuss there should be at least some CSS to visually deemphasise the lock and advanced flag Failing for now, please get feedback from MD or other integrators, sorry.
          Hide
          Damyon Wiese added a comment -

          Thanks Petr,

          I'm looking at this now.
          a/ Is an easy fix
          b/ I'm thinking about (I agree with your comments).

          Show
          Damyon Wiese added a comment - Thanks Petr, I'm looking at this now. a/ Is an easy fix b/ I'm thinking about (I agree with your comments).
          Hide
          Damyon Wiese added a comment - - edited

          Comment deleted because it was wrong.

          Show
          Damyon Wiese added a comment - - edited Comment deleted because it was wrong.
          Hide
          Damyon Wiese added a comment - - edited

          Comment deleted because it was wrong.

          Show
          Damyon Wiese added a comment - - edited Comment deleted because it was wrong.
          Hide
          Damyon Wiese added a comment -

          Thinking... for existing modules, if an admin changes a setting, it does not make sense to force the modules to use the new setting value everywhere (locked or not). Some settings need to be applied when a module is first created, and cannot be modified after that (e.g. blind marking). Also - for date based settings this does not make sense at all - everytime a module would be updated, the date would change even though it was locked.

          So the correct solution is to:

          • Lock the values at the defaults on module create page (current solution)
          • On update, do not change any values - and only lock settings if they are the same as the admin default
          • Disallow locking of date/time settings
          Show
          Damyon Wiese added a comment - Thinking... for existing modules, if an admin changes a setting, it does not make sense to force the modules to use the new setting value everywhere (locked or not). Some settings need to be applied when a module is first created, and cannot be modified after that (e.g. blind marking). Also - for date based settings this does not make sense at all - everytime a module would be updated, the date would change even though it was locked. So the correct solution is to: Lock the values at the defaults on module create page (current solution) On update, do not change any values - and only lock settings if they are the same as the admin default Disallow locking of date/time settings
          Hide
          Marina Glancy added a comment - - edited

          Agree with first two. After you implement #2 from your list Damyon, the problem with date will disappear so I don't see any problems with allowing date locking. (Not 100% sure, if the locked duration is relative to the current date it will only be locked during module creation and won't be locked when editing module on the next day)

          Show
          Marina Glancy added a comment - - edited Agree with first two. After you implement #2 from your list Damyon, the problem with date will disappear so I don't see any problems with allowing date locking. (Not 100% sure, if the locked duration is relative to the current date it will only be locked during module creation and won't be locked when editing module on the next day)
          Hide
          Damyon Wiese added a comment -

          The problem with date locking is that - the admin setting is a duration and it is applied relative to either the time the module was created or another time supplied as a parameter. So at the time the module is updated - it will always be different and never locked - this is why it doesn't make sense to lock it in the first place.

          Show
          Damyon Wiese added a comment - The problem with date locking is that - the admin setting is a duration and it is applied relative to either the time the module was created or another time supplied as a parameter. So at the time the module is updated - it will always be different and never locked - this is why it doesn't make sense to lock it in the first place.
          Hide
          Marina Glancy added a comment -

          agree. After all, if module developer wants it to be potentially locked he can create duration form element instead of datetime

          Show
          Marina Glancy added a comment - agree. After all, if module developer wants it to be potentially locked he can create duration form element instead of datetime
          Hide
          Damyon Wiese added a comment -

          Fix pushed to

          https://github.com/damyon/moodle/compare/44df0d9aaa8e4905af73a24c28bdcd566461ea4c...MDL-37459-master-fix2

          That implements 1 and 2. (2 commits)

          I didn't to 3 (prevent locking date/time fields) because in this case the admin setting is a duration field and it might make sense to lock this if it is used for a different purpose. I think this should just be noted in the docs. I'll remove the uses of this in assignment in a new patch for MDL-37621.

          This fix also gives some default styling to the setting flags in the admin pages.

          Show
          Damyon Wiese added a comment - Fix pushed to https://github.com/damyon/moodle/compare/44df0d9aaa8e4905af73a24c28bdcd566461ea4c...MDL-37459-master-fix2 That implements 1 and 2. (2 commits) I didn't to 3 (prevent locking date/time fields) because in this case the admin setting is a duration field and it might make sense to lock this if it is used for a different purpose. I think this should just be noted in the docs. I'll remove the uses of this in assignment in a new patch for MDL-37621 . This fix also gives some default styling to the setting flags in the admin pages.
          Hide
          Petr Škoda added a comment -

          makes sense, thanks, please ping me when it is ready for testing

          Show
          Petr Škoda added a comment - makes sense, thanks, please ping me when it is ready for testing
          Hide
          Marina Glancy added a comment -

          Damyon, I have a couple comments, you may agree or disagree:

          • function apply_admin_defaults_after_data() should not be private
          • I would split the code and apply defaults in apply_admin_defaults() and lock/freeze elements in apply_admin_defaults_after_data(). It does not look right that you set defaults in definition_after_data().
          • "!empty($this->current->id)" does not seem the common way to check if we are updating or creating, it is usually checked as "if ($this->_cm)" in moodleform_mod.php

          The rest looks great. Thanks

          Show
          Marina Glancy added a comment - Damyon, I have a couple comments, you may agree or disagree: function apply_admin_defaults_after_data() should not be private I would split the code and apply defaults in apply_admin_defaults() and lock/freeze elements in apply_admin_defaults_after_data(). It does not look right that you set defaults in definition_after_data(). "!empty($this->current->id)" does not seem the common way to check if we are updating or creating, it is usually checked as "if ($this->_cm)" in moodleform_mod.php The rest looks great. Thanks
          Hide
          Damyon Wiese added a comment -

          Thanks Marina - I'll add a patch for these issues now.

          Show
          Damyon Wiese added a comment - Thanks Marina - I'll add a patch for these issues now.
          Hide
          Damyon Wiese added a comment -

          Extra patch is ready - I added a new patch to the same branch - feel free to squash them if you like.

          Show
          Damyon Wiese added a comment - Extra patch is ready - I added a new patch to the same branch - feel free to squash them if you like.
          Hide
          Marina Glancy added a comment -

          Thank you Damyon, it is integrated (master).
          Petr please re-test. There are now changes to base and bootstrap themes, can you check UI in both standard and clean themes please.

          Show
          Marina Glancy added a comment - Thank you Damyon, it is integrated (master). Petr please re-test. There are now changes to base and bootstrap themes, can you check UI in both standard and clean themes please.
          Hide
          Petr Škoda added a comment -
          reset() expects parameter 1 to be array, boolean given in /Users/skodak/server/workspace/moodle26/course/moodleform_mod.php on line 906
          

          when editing existing assignment - $name is alwaysshowdescription and value bool(true).

          Show
          Petr Škoda added a comment - reset() expects parameter 1 to be array, boolean given in /Users/skodak/server/workspace/moodle26/course/moodleform_mod.php on line 906 when editing existing assignment - $name is alwaysshowdescription and value bool(true).
          Hide
          Petr Škoda added a comment -

          btw, the right aligned checkboxes are a bit annoying on my 30" screen, but that can be addressed later, it is much better than before, thanks!

          Show
          Petr Škoda added a comment - btw, the right aligned checkboxes are a bit annoying on my 30" screen, but that can be addressed later, it is much better than before, thanks!
          Hide
          Damyon Wiese added a comment -

          Thanks Petr,

          There is a fix for the last bug you found here (branch is MDL-37459-fix3)

          https://github.com/damyon/moodle/commit/0a44c1ce813d0c2ed265195b2d75d1708803c322

          Show
          Damyon Wiese added a comment - Thanks Petr, There is a fix for the last bug you found here (branch is MDL-37459 -fix3) https://github.com/damyon/moodle/commit/0a44c1ce813d0c2ed265195b2d75d1708803c322
          Hide
          Marina Glancy added a comment -

          Thanks Damyon, fix integrated. Waiting for testing agin

          Show
          Marina Glancy added a comment - Thanks Damyon, fix integrated. Waiting for testing agin
          Hide
          David Monllaó added a comment -

          Hi,

          As commented in https://tracker.moodle.org/browse/MDL-37621?focusedCommentId=228132&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-228212, behat reports failures, both seems related with this issue. The site used is new for each test (same we site we have after a CLI install)

          ( ! ) Notice: Undefined property: stdClass::$timelimit_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 92
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$overduehandling_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 99
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$graceperiod_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 109
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$attempts_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 128
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$grademethod_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 135
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$shufflequestions_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 149
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$questionsperpage_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 184
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$navmethod_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 190
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$shuffleanswers_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 199
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$attemptonlast_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 218
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$showuserpicture_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 258
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$decimalpoints_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 269
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$questiondecimalpoints_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 280
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$showblocks_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 286
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$password_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 296
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$subnet_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 303
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$delay1_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 310
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$delay2_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 317
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          ( ! ) Notice: Undefined property: stdClass::$browsersecurity_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 326
          Call Stack
          #	Time	Memory	Function	Location
          1	0.0006	319872	{main}( )	../modedit.php:0
          2	0.1890	30080608	mod_quiz_mod_form->__construct( )	../modedit.php:248
          3	0.1890	30085232	moodleform_mod->moodleform_mod( )	../mod_form.php:53
          4	0.1893	30089064	moodleform->moodleform( )	../moodleform_mod.php:77
          5	0.1909	30174032	mod_quiz_mod_form->definition( )	../formslib.php:191
          
          Show
          David Monllaó added a comment - Hi, As commented in https://tracker.moodle.org/browse/MDL-37621?focusedCommentId=228132&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-228212 , behat reports failures, both seems related with this issue. The site used is new for each test (same we site we have after a CLI install) ( ! ) Notice: Undefined property: stdClass::$timelimit_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 92 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$overduehandling_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 99 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$graceperiod_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 109 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$attempts_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 128 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$grademethod_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 135 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$shufflequestions_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 149 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$questionsperpage_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 184 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$navmethod_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 190 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$shuffleanswers_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 199 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$attemptonlast_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 218 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$showuserpicture_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 258 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$decimalpoints_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 269 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$questiondecimalpoints_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 280 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$showblocks_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 286 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$password_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 296 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$subnet_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 303 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$delay1_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 310 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$delay2_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 317 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191 ( ! ) Notice: Undefined property: stdClass::$browsersecurity_adv in /home/davidm/Desktop/moodlecode/INTEGRATION/master/mod/quiz/mod_form.php on line 326 Call Stack # Time Memory Function Location 1 0.0006 319872 {main}( ) ../modedit.php:0 2 0.1890 30080608 mod_quiz_mod_form->__construct( ) ../modedit.php:248 3 0.1890 30085232 moodleform_mod->moodleform_mod( ) ../mod_form.php:53 4 0.1893 30089064 moodleform->moodleform( ) ../moodleform_mod.php:77 5 0.1909 30174032 mod_quiz_mod_form->definition( ) ../formslib.php:191
          Hide
          Damyon Wiese added a comment -

          Thanks David,

          So the cause of this error is that the _adv settings are not being set on a new install with this patch.

          Looking now...

          Show
          Damyon Wiese added a comment - Thanks David, So the cause of this error is that the _adv settings are not being set on a new install with this patch. Looking now...
          Hide
          Damyon Wiese added a comment -

          And a fix for it is here:

          Repo: git://github.com/damyon/moodle.git
          Branch: MDL-37459-master-fix4
          Diff: https://github.com/damyon/moodle/compare/e3b472e82c02cc4b7718e44196d4a52e0ca4c3f3...MDL-37459-master-fix4

          Can please pull this in Marina?

          Thanks!

          Show
          Damyon Wiese added a comment - And a fix for it is here: Repo: git://github.com/damyon/moodle.git Branch: MDL-37459 -master-fix4 Diff: https://github.com/damyon/moodle/compare/e3b472e82c02cc4b7718e44196d4a52e0ca4c3f3...MDL-37459-master-fix4 Can please pull this in Marina? Thanks!
          Hide
          Marina Glancy added a comment -

          integrated, thanks

          Show
          Marina Glancy added a comment - integrated, thanks
          Hide
          Petr Škoda added a comment -

          Everything seems to be working fine for me now, thanks!

          Show
          Petr Škoda added a comment - Everything seems to be working fine for me now, thanks!
          Hide
          Marina Glancy added a comment -

          Thanks for your awesome work! This has now become a part of Moodle.

          Closing as fixed!

          Show
          Marina Glancy added a comment - Thanks for your awesome work! This has now become a part of Moodle. Closing as fixed!
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as this is now documented in http://docs.moodle.org/26/en/Assignment_settings

          Show
          Mary Cooch added a comment - Removing docs_required label as this is now documented in http://docs.moodle.org/26/en/Assignment_settings

            People

            • Votes:
              9 Vote for this issue
              Watchers:
              18 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: