Moodle
  1. Moodle
  2. MDL-30854

Calculated question not added into the Quiz automatically

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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:
    • Rank:
      33854

      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

      1. smurf_master.xml
        3 kB
        Eloy Lafuente (stronk7)

        Activity

        Hide
        David Mudrak added a comment -

        Fixing a typo in the description, sorry for the spam

        Show
        David Mudrak added a comment - Fixing a typo in the description, sorry for the spam
        Hide
        Tim Hunt added a comment -

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

        Show
        Tim Hunt added a comment - This is indeed separate from MDL-30739 . Thanks for catching it.
        Hide
        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
        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
        Tim Hunt added a comment -

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

        Show
        Tim Hunt added a comment - Again, I am adding some plausible candidates to peer-review this as watchers.
        Hide
        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
        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
        Pierre Pichet added a comment -

        checkout MDL-30854, beginning tests

        Show
        Pierre Pichet added a comment - checkout MDL-30854 , beginning tests
        Hide
        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
        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
        Pierre Pichet added a comment -

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

        Show
        Pierre Pichet added a comment - Adding from the quiz interface, the questions were created at the page selected.
        Hide
        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
        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
        Tim Hunt added a comment -

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

        Show
        Tim Hunt added a comment - Thanks for the review and testing Pierre. I'm submitting this for integration now.
        Hide
        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
        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
        Tim Hunt added a comment -

        All three branches re-based.

        Show
        Tim Hunt added a comment - All three branches re-based.
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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: