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

          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