Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-35793

Admin options for when a user starts a preview

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.5
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      1. As admin, go to Admin -> Plugins -> Question types -> Question preview defaults and set some options.
      2. Create a new user 'teacher', and enrol that user as a Teacher in a course.
      3. Log in as 'teacher' and go to the course question bank.
      4. Preview a question (create on if necessary).
      5. Verify that the display options used are the ones the admin set.
      6. Change some options, and click 'Start again with these options'
      7. Log out.
      8. Log back in as 'teacher', go back to the course, and preview another question. The preferences used should be the ones you set in step 6.

      Show
      1. As admin, go to Admin -> Plugins -> Question types -> Question preview defaults and set some options. 2. Create a new user 'teacher', and enrol that user as a Teacher in a course. 3. Log in as 'teacher' and go to the course question bank. 4. Preview a question (create on if necessary). 5. Verify that the display options used are the ones the admin set. 6. Change some options, and click 'Start again with these options' 7. Log out. 8. Log back in as 'teacher', go back to the course, and preview another question. The preferences used should be the ones you set in step 6.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      At the moment, when a new user first previews a question, the display option are taken from some hard-coded values in question_preview_options::__construct().

      Instead, we would like admin options for these.

      This would require a new admin settings page

      • Site administration
        • Plugins
          • Question types
            • Manage question types
            • Default settings <-- this is the new one.

      For now there will be one group of settings here, with title "Default display options when previewing a question." There will be those 7 settings from the question preview window.

      These settings should be stored in the config_plugings table, with plugin = 'question'.

        Gliffy Diagrams

          Activity

          Hide
          dtle Thanh Le added a comment - - edited

          I've coded a solution and applied it to Moodle MASTER branch.

          See:
          https://github.com/dtle/moodle/tree/MDL-35793

          Diff:
          1) https://github.com/dtle/moodle/commit/9c377a27837d025c83a54361e6f83907921cb57e (initial commit)
          2) https://github.com/dtle/moodle/commit/3bffa18285d538c47a4ddc82a3992bb9ef75b0ef (correction to default values)
          3) https://github.com/dtle/moodle/commit/3bffa18285d538c47a4ddc82a3992bb9ef75b0ef (minor new file header correction)

          Before continuing, might be worth you reviewing it. I dont see this as adding much value because as soon as the user previews a question and changes any of the preview option, all the preview options are saved as user preferences. The way the code is written, config settings are overrided by user defaults, then by url settings as in these line of code in /question/preview.php:

          $options = new question_preview_options($question);
          $options->load_user_defaults();
          $options->set_from_request();

          ... This means once user preferences are saved, they will always supercede the config settings in all question previews - so the config settings only apply in one case only... the first time a user (with no save user preferences) tries to preview a question.

          Show
          dtle Thanh Le added a comment - - edited I've coded a solution and applied it to Moodle MASTER branch. See: https://github.com/dtle/moodle/tree/MDL-35793 Diff: 1) https://github.com/dtle/moodle/commit/9c377a27837d025c83a54361e6f83907921cb57e (initial commit) 2) https://github.com/dtle/moodle/commit/3bffa18285d538c47a4ddc82a3992bb9ef75b0ef (correction to default values) 3) https://github.com/dtle/moodle/commit/3bffa18285d538c47a4ddc82a3992bb9ef75b0ef (minor new file header correction) Before continuing, might be worth you reviewing it. I dont see this as adding much value because as soon as the user previews a question and changes any of the preview option, all the preview options are saved as user preferences. The way the code is written, config settings are overrided by user defaults, then by url settings as in these line of code in /question/preview.php: $options = new question_preview_options($question); $options->load_user_defaults(); $options->set_from_request(); ... This means once user preferences are saved, they will always supercede the config settings in all question previews - so the config settings only apply in one case only... the first time a user (with no save user preferences) tries to preview a question.
          Hide
          timhunt Tim Hunt added a comment -

          You are right that this does not seem very valuable, but actually it is, because it affects what people see when they first start using Moodle, and it is important to start people off using the right settings.

          I don't think it is right to create the new file admin/qdefaults.php. However, I am not sure where the best place to put it is. I will consult with some others.

          The main point is that you have to be very careful about performance in these settings files, because the tree of admin settings appear on every page to admins. Therefore, you need to avoid things like:

          • require_once($CFG->libdir . '/../question/engine/lib.php'); - where that then also requires a lot of other files.
          • any database queries.

          Techniques to avoid these problems include:

          1. hard-code values, rather than using the defined constant, if that constant is in a big library file.
          2. move constant definitions so they can be included cheaply
          3. For admin_setting_configselect, make a sub-class and override the load_choices method, so we only have to populate the choices if they are really needed.

          I am going to make some of these changes, and then ask you to review them.

          Show
          timhunt Tim Hunt added a comment - You are right that this does not seem very valuable, but actually it is, because it affects what people see when they first start using Moodle, and it is important to start people off using the right settings. I don't think it is right to create the new file admin/qdefaults.php. However, I am not sure where the best place to put it is. I will consult with some others. The main point is that you have to be very careful about performance in these settings files, because the tree of admin settings appear on every page to admins. Therefore, you need to avoid things like: require_once($CFG->libdir . '/../question/engine/lib.php'); - where that then also requires a lot of other files. any database queries. Techniques to avoid these problems include: hard-code values, rather than using the defined constant, if that constant is in a big library file. move constant definitions so they can be included cheaply For admin_setting_configselect, make a sub-class and override the load_choices method, so we only have to populate the choices if they are really needed. I am going to make some of these changes, and then ask you to review them.
          Hide
          timhunt Tim Hunt added a comment -

          Yay! admin_setting_question_behaviour already exists

          I think I am going to rename these settings to question_preview/, rather than question/. They are very specific, and only needed in one place, so better not to lump them in with the other question settings that are loaded in more places.

          Show
          timhunt Tim Hunt added a comment - Yay! admin_setting_question_behaviour already exists I think I am going to rename these settings to question_preview/, rather than question/. They are very specific, and only needed in one place, so better not to lump them in with the other question settings that are loaded in more places.
          Hide
          timhunt Tim Hunt added a comment -

          I think this is the way I would do it: https://github.com/timhunt/moodle/compare/MDL-35793. Comments?

          Show
          timhunt Tim Hunt added a comment - I think this is the way I would do it: https://github.com/timhunt/moodle/compare/MDL-35793 . Comments?
          Hide
          quen Sam Marshall added a comment -

          Tim: Looks good to me, some notes:

          1. I see you aren't using the actual constants but have referred to them in comments. I guess there is a policy to not do require_once calls in settings.php?

          2. This will add an extra db query to the load_user_defaults function, assuming it is only usually called once a page which I guess is likely, then it's probably fine.

          basically +1

          Show
          quen Sam Marshall added a comment - Tim: Looks good to me, some notes: 1. I see you aren't using the actual constants but have referred to them in comments. I guess there is a policy to not do require_once calls in settings.php? 2. This will add an extra db query to the load_user_defaults function, assuming it is only usually called once a page which I guess is likely, then it's probably fine. basically +1
          Hide
          timhunt Tim Hunt added a comment -

          Submitting for integration review.

          I would like this to go into MOODLE_24_STABLE because we then see the benefits more quickly at the OU, but that might not be a good enough reason.

          Show
          timhunt Tim Hunt added a comment - Submitting for integration review. I would like this to go into MOODLE_24_STABLE because we then see the benefits more quickly at the OU, but that might not be a good enough reason.
          Hide
          poltawski Dan Poltawski added a comment -

          I've added integration_held as this seems to be a new feature.

          Show
          poltawski Dan Poltawski added a comment - I've added integration_held as this seems to be a new feature.
          Hide
          poltawski Dan Poltawski added a comment -

          Integrated to master only (as this is a new feature).

          Thanks Tim

          Show
          poltawski Dan Poltawski added a comment - Integrated to master only (as this is a new feature). Thanks Tim
          Hide
          rwijaya Rossiani Wijaya added a comment -

          This works as expected.

          Test passed.

          Show
          rwijaya Rossiani Wijaya added a comment - This works as expected. Test passed.
          Hide
          poltawski Dan Poltawski added a comment -

          Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

          Show
          poltawski Dan Poltawski added a comment - Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!
          Hide
          tsala Helen Foster added a comment -

          Apologies for the delay in documenting this improvement, however it is finally done: https://docs.moodle.org/en/Managing_questions

          If you notice that the documentation can be improved, please feel free to log in to the wiki and edit the page. Help in keeping Moodle documentation accurate and up-to-date is much appreciated.

          Show
          tsala Helen Foster added a comment - Apologies for the delay in documenting this improvement, however it is finally done: https://docs.moodle.org/en/Managing_questions If you notice that the documentation can be improved, please feel free to log in to the wiki and edit the page. Help in keeping Moodle documentation accurate and up-to-date is much appreciated.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                14/May/13