Moodle
  1. Moodle
  2. MDL-28687

Can't display quiz attempt when question has been deleted

    Details

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

      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?

        Activity

        Hide
        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
        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
        Henning Bostelmann added a comment -

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

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

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

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

        No worries Tim.

        All yours.

        Show
        Rossiani Wijaya added a comment - No worries Tim. All yours.
        Hide
        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
        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
        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
        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
        Tim Hunt added a comment -

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

        Show
        Tim Hunt added a comment - Henning, if you could test my revised fix, that would be helpful. Thanks.
        Hide
        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
        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
        Tim Hunt added a comment -

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

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

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

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

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

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

        Thanks! This has been integrated!

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

        works great.

        Test passed

        Show
        Rossiani Wijaya added a comment - works great. Test passed
        Hide
        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
        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: