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

Calculated question not added into the Quiz automatically

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.6, 2.1.4, 2.2.1, 2.3
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Quiz
    • Labels:
    • Testing Instructions:
      Hide

      Because of the mess that was found while fixing this, the fix need to be tested for each of

      A. A normal simple question (e.g. True/false)
      B. A calculated question.
      C. A calculatedmulti question.

      For each of those question types:

      0. On the Edit quiz page for a quiz.
      1. Click the 'Add a question ...' button on a page of the quiz.
      2. Choose to create a question of that type, and work through the editing form until you are don.
      3. Verify that you get back to the edit quiz page, and that the new question has been added to the page you chose.

      (Note that there is a separate, know, issue where sometimes a random empty page is added to the quiz too. Don't worry about that for now. It will be fixed separately.)

      4. Also make sure there are no problems if you create a question of this type directly in the question bank (If necessary, click the [Show] link where it says Question bank contents.)
      5. Go to the question bank for the course (using the link in the course settings block, not in the quiz settings), and make sure there are no regressions when creating questions of the given type there either.

      D. We also need to test random questions.

      1. Back on the quiz edit page, add a random question, picking from an existing category, somewhere in the quiz.
      2. Then click the 'Configure question' link.
      3. Change something on that form (or don't) and click save.
      4. Verify that there are no errors.

      Show
      Because of the mess that was found while fixing this, the fix need to be tested for each of A. A normal simple question (e.g. True/false) B. A calculated question. C. A calculatedmulti question. For each of those question types: 0. On the Edit quiz page for a quiz. 1. Click the 'Add a question ...' button on a page of the quiz. 2. Choose to create a question of that type, and work through the editing form until you are don. 3. Verify that you get back to the edit quiz page, and that the new question has been added to the page you chose. (Note that there is a separate, know, issue where sometimes a random empty page is added to the quiz too. Don't worry about that for now. It will be fixed separately.) 4. Also make sure there are no problems if you create a question of this type directly in the question bank (If necessary, click the [Show] link where it says Question bank contents.) 5. Go to the question bank for the course (using the link in the course settings block, not in the quiz settings), and make sure there are no regressions when creating questions of the given type there either. D. We also need to test random questions. 1. Back on the quiz edit page, add a random question, picking from an existing category, somewhere in the quiz. 2. Then click the 'Configure question' link. 3. Change something on that form (or don't) and click save. 4. Verify that there are no errors.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      How to reproduce

      1. When editing quiz, click the "Add a question..." at the bottom of a page frame
      2. Choose "Calculated" question type
      3. Define the question and save it

      Expected result

      • The question is added into the quiz at the place where the add button was pressed (like other questions do - tested with the Numerical)

      What actually happens

      • The question is created in the bank only and must be added to the quiz manually

      note: this was discovered while testing MDL-30739 though it does not seem to be linked anyhow

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            mudrd8mz David Mudrák added a comment -

            Fixing a typo in the description, sorry for the spam

            Show
            mudrd8mz David Mudrák added a comment - Fixing a typo in the description, sorry for the spam
            Hide
            timhunt Tim Hunt added a comment -

            This is indeed separate from MDL-30739. Thanks for catching it.

            Show
            timhunt Tim Hunt added a comment - This is indeed separate from MDL-30739 . Thanks for catching it.
            Hide
            timhunt Tim Hunt added a comment -

            OK, here is another one for peer review. Once I started looking into this, I found a big of a mess that needed to be cleaned up.

            Show
            timhunt Tim Hunt added a comment - OK, here is another one for peer review. Once I started looking into this, I found a big of a mess that needed to be cleaned up.
            Hide
            timhunt Tim Hunt added a comment -

            Again, I am adding some plausible candidates to peer-review this as watchers.

            Show
            timhunt Tim Hunt added a comment - Again, I am adding some plausible candidates to peer-review this as watchers.
            Hide
            ppichet Pierre Pichet added a comment -

            At first sight, I think I understand your modifications at least on the calculated types
            My next time window is monday-tuesday next week.
            Let's hope that the window will remain valid.

            Show
            ppichet Pierre Pichet added a comment - At first sight, I think I understand your modifications at least on the calculated types My next time window is monday-tuesday next week. Let's hope that the window will remain valid.
            Hide
            ppichet Pierre Pichet added a comment -

            checkout MDL-30854, beginning tests

            Show
            ppichet Pierre Pichet added a comment - checkout MDL-30854 , beginning tests
            Hide
            ppichet Pierre Pichet added a comment -

            I have created the suggested question types adding calculated simple
            in the various settings suggested.
            The questions were all created in the correct category i.e.
            default for the given quiz ( the context i.e. the course)when the quiz is created.
            This is also the category that is shown when you click on [SHOW]
            The random were also created in the good category.
            there was no problems when adding questions from the SHOW list.

            As far a I could see the category parameter is not lost in the different steps.

            Thanks for fixing the historical mess and for implementing full PHP 5 coding.

            Should I also test the 2.1 and 2.2 versions ?

            Show
            ppichet Pierre Pichet added a comment - I have created the suggested question types adding calculated simple in the various settings suggested. The questions were all created in the correct category i.e. default for the given quiz ( the context i.e. the course)when the quiz is created. This is also the category that is shown when you click on [SHOW] The random were also created in the good category. there was no problems when adding questions from the SHOW list. As far a I could see the category parameter is not lost in the different steps. Thanks for fixing the historical mess and for implementing full PHP 5 coding. Should I also test the 2.1 and 2.2 versions ?
            Hide
            ppichet Pierre Pichet added a comment -

            Adding from the quiz interface, the questions were created at the page selected.

            Show
            ppichet Pierre Pichet added a comment - Adding from the quiz interface, the questions were created at the page selected.
            Hide
            timhunt Tim Hunt added a comment -

            No need to test 2.1 and 2.2 separately, I think. Thanks for looking at it. I will submit it for integration in time for next week.

            Show
            timhunt Tim Hunt added a comment - No need to test 2.1 and 2.2 separately, I think. Thanks for looking at it. I will submit it for integration in time for next week.
            Hide
            timhunt Tim Hunt added a comment -

            Thanks for the review and testing Pierre. I'm submitting this for integration now.

            Show
            timhunt Tim Hunt added a comment - Thanks for the review and testing Pierre. I'm submitting this for integration now.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            timhunt Tim Hunt added a comment -

            All three branches re-based.

            Show
            timhunt Tim Hunt added a comment - All three branches re-based.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Attaching smurf xml file, just because I detected some false positives on it and need it available here.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Attaching smurf xml file, just because I detected some false positives on it and need it available here.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks!

            Note: Sure you know, what is being done by you are deleting a varying number of hidden fields here and there and adding the same 7 always from the wizard class. Also, you've killed a lot of defaults apparently... testing will say, surely.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks! Note: Sure you know, what is being done by you are deleting a varying number of hidden fields here and there and adding the same 7 always from the wizard class. Also, you've killed a lot of defaults apparently... testing will say, surely.
            Hide
            timhunt Tim Hunt added a comment -

            Correct. The original bug was really that there should be the same set if hidden fields in all four forms, and over time that had got broken. Therefore, refactoring to make one method to add the hidden fields was the right solution.

            The set default calls were wrong. These values all get set by setData.

            Show
            timhunt Tim Hunt added a comment - Correct. The original bug was really that there should be the same set if hidden fields in all four forms, and over time that had got broken. Therefore, refactoring to make one method to add the hidden fields was the right solution. The set default calls were wrong. These values all get set by setData.
            Hide
            nebgor Aparup Banerjee added a comment -

            A->0-3 ok,
            B->0-3 ok,
            C->0-3 ok,
            A->4-5 ok (used 'course admin'->'question bank' for step 5s)
            B->4-5 ok,
            C->4-5 ok,

            note : for B's and C's, i didn't really get back to the edit quiz page, i had the page 'choose wildcards dataset properties' and so on but i did end up at the edit quiz page after that.

            D is fine too.

            Test passed for me, yay!

            Show
            nebgor Aparup Banerjee added a comment - A->0-3 ok, B->0-3 ok, C->0-3 ok, A->4-5 ok (used 'course admin'->'question bank' for step 5s) B->4-5 ok, C->4-5 ok, note : for B's and C's, i didn't really get back to the edit quiz page, i had the page 'choose wildcards dataset properties' and so on but i did end up at the edit quiz page after that. D is fine too. Test passed for me, yay!
            Hide
            timhunt Tim Hunt added a comment -

            What you are seeing for B and C is correct. These question types are edited using a three-page wizard interface. The original bug was really that the returnurl, and other information, got lost between pages 1 and 2 of the wizard.

            Thanks for completing the long testing instructions.

            Show
            timhunt Tim Hunt added a comment - What you are seeing for B and C is correct. These question types are edited using a three-page wizard interface. The original bug was really that the returnurl, and other information, got lost between pages 1 and 2 of the wizard. Thanks for completing the long testing instructions.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories.

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories. Closing, ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Mar/12