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

Can't display quiz attempt when question has been deleted

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.1, 2.2
    • Fix Version/s: 2.1.2
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      1. Create a quiz containing at least two questions, and attempt at as a student.
      2. Go into the database, and delete one of the questions used. (That is, delete the row from the mdl_question table. Ideally also delete the linked data in other tables.)
      3. Review the quiz attempt using that question. Of course, you cannot expect to see the deleted questions, but as much as possible of the remaining information should still be visible and usable.

      (A less destructive way to do step 2. is just to edit the id in the mdl_question table. E.g. change id 123 to id 100123. Then you can easily put it back once you have finished testing.)

      Show
      1. Create a quiz containing at least two questions, and attempt at as a student. 2. Go into the database, and delete one of the questions used. (That is, delete the row from the mdl_question table. Ideally also delete the linked data in other tables.) 3. Review the quiz attempt using that question. Of course, you cannot expect to see the deleted questions, but as much as possible of the remaining information should still be visible and usable. (A less destructive way to do step 2. is just to edit the id in the mdl_question table. E.g. change id 123 to id 100123. Then you can easily put it back once you have finished testing.)
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Sorry, I need to file another question-related problem. Not sure whether this is only related to data migration.

      When a quiz attempt contains a deleted question, viewing the quiz attempt (mod/quiz/review.php) fails with an error message as below. The remainder of the page is not shown.

      Category ID is invalid

      More information about this error
      Stack trace:

      • line 429 of /lib/setuplib.php: moodle_exception thrown
      • line 1349 of /lib/questionlib.php: call to print_error()
      • line 696 of /mod/quiz/attemptlib.php: call to question_has_capability_on()
      • line 951 of /mod/quiz/attemptlib.php: call to quiz_attempt->get_display_options_with_edit_link()
      • line 182 of /mod/quiz/renderer.php: call to quiz_attempt->render_question()
      • line 54 of /mod/quiz/renderer.php: call to mod_quiz_renderer->questions()
      • line 259 of /mod/quiz/review.php: call to mod_quiz_renderer->review_page()

      This comes about as follows. In qtype_missingtype::make_deleted_instance(), the category id of the "fake" question is set to NULL. However, in lib/questionlib.php, function question_has_capability_on(), the question is tested on having a valid category. None is found, so an error is encountered.

      So far, I was able to reproduce this with migrated data only. Is Moodle 2.1 expected to deal with "deleted questions" or is this a problem of the migration process?

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            It is probably a migration only issues - or at least a 'result of an earlier bug'-only issue.

            I suggest we add an if (!emtpy($question->categoryid) somewhere. If this is a fake placeholder for a deleted question, there is no point offering an edit link.

            Show
            timhunt Tim Hunt added a comment - It is probably a migration only issues - or at least a 'result of an earlier bug'-only issue. I suggest we add an if (!emtpy($question->categoryid) somewhere. If this is a fake placeholder for a deleted question, there is no point offering an edit link.
            Hide
            bostelm Henning Bostelmann added a comment -

            This modification fixes it for me. Users will not have any capabilities on deleted questions, which seems consistent.

            Show
            bostelm Henning Bostelmann added a comment - This modification fixes it for me. Users will not have any capabilities on deleted questions, which seems consistent.
            Hide
            timhunt Tim Hunt added a comment -

            Thanks Rossi for starting the review of this. I will take it from here.

            Show
            timhunt Tim Hunt added a comment - Thanks Rossi for starting the review of this. I will take it from here.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            No worries Tim.

            All yours.

            Show
            rwijaya Rossiani Wijaya added a comment - No worries Tim. All yours.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Henning,

            Thanks for providing the patch to address the issue.

            Could you also provide some testing instructions to test this issue?

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Henning, Thanks for providing the patch to address the issue. Could you also provide some testing instructions to test this issue?
            Hide
            timhunt Tim Hunt added a comment -

            Two problems with the patch:

            1. You get simpler code by inserting the new code after the existing if (!is_object($question)) { block.

            2. In general, don't pollute the code with references to bug numbers. All that information is automatically tracked by git. On the other hand, the situation that makes this new code necessary is quite obscure, so a comment that explains why it is needed is a good idea.

            I am about to submit a revised fix for integration.

            Show
            timhunt Tim Hunt added a comment - Two problems with the patch: 1. You get simpler code by inserting the new code after the existing if (!is_object($question)) { block. 2. In general, don't pollute the code with references to bug numbers. All that information is automatically tracked by git. On the other hand, the situation that makes this new code necessary is quite obscure, so a comment that explains why it is needed is a good idea. I am about to submit a revised fix for integration.
            Hide
            timhunt Tim Hunt added a comment -

            Henning, if you could test my revised fix, that would be helpful. Thanks.

            Show
            timhunt Tim Hunt added a comment - Henning, if you could test my revised fix, that would be helpful. Thanks.
            Hide
            bostelm Henning Bostelmann added a comment -

            Thanks Tim. I have now tested this with my "real life" data on the MOODLE_21_STABLE branch (or rather your branch MDL-28687_21). It fixes the problem for me.

            Show
            bostelm Henning Bostelmann added a comment - Thanks Tim. I have now tested this with my "real life" data on the MOODLE_21_STABLE branch (or rather your branch MDL-28687 _21). It fixes the problem for me.
            Hide
            timhunt Tim Hunt added a comment -

            Great! Thanks for the confirmation. And I am impressed by the speed!

            Show
            timhunt Tim Hunt added a comment - Great! Thanks for the confirmation. And I am impressed by the speed!
            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
            nebgor Aparup Banerjee added a comment - - edited

            just checking: is this to be fixed in 2.0.x as well?

            Show
            nebgor Aparup Banerjee added a comment - - edited just checking: is this to be fixed in 2.0.x as well?
            Hide
            bostelm Henning Bostelmann added a comment -

            No, it's related to the new Question Engine only, which was introduced in 2.1.

            Show
            bostelm Henning Bostelmann added a comment - No, it's related to the new Question Engine only, which was introduced in 2.1.
            Hide
            nebgor Aparup Banerjee added a comment -

            Thanks! This has been integrated!

            Show
            nebgor Aparup Banerjee added a comment - Thanks! This has been integrated!
            Hide
            rwijaya Rossiani Wijaya added a comment -

            works great.

            Test passed

            Show
            rwijaya Rossiani Wijaya added a comment - works great. Test passed
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Oct/11