Moodle
  1. Moodle
  2. MDL-30592

Quiz statistics report - images in the question text do not show up

    Details

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

      1. Create a question with an image in the question text.
      2. Go to the question bank, and turn on "Show question text in the question list". Verify that the image appears in the question text.
      3. Add the question (and any others you like) to a new quiz, and attempt it as a student.
      4. As teacher, go to the statistics report for the quiz.
      5. Download the full report as XHTML. Verify that the images in the question texts show up.
      6. Click on the question name in the Quiz structure analysis. On the page you get to, verify that the image in the question text shows up.

      Note that the cherry-pick to 21 was not clean. In particular changes had to be merged in plugin_callback. Please also test the rating and comments systems, to verify that there are no regressions.

      Show
      1. Create a question with an image in the question text. 2. Go to the question bank, and turn on "Show question text in the question list". Verify that the image appears in the question text. 3. Add the question (and any others you like) to a new quiz, and attempt it as a student. 4. As teacher, go to the statistics report for the quiz. 5. Download the full report as XHTML. Verify that the images in the question texts show up. 6. Click on the question name in the Quiz structure analysis. On the page you get to, verify that the image in the question text shows up. Note that the cherry-pick to 21 was not clean. In particular changes had to be merged in plugin_callback. Please also test the rating and comments systems, to verify that there are no regressions.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Create a question with an image in the question text. Attempt the quiz a few times as different students, then go to the statistics report, and drill down into the detailed analysis of one question.

      The images does not appear (there is just a PLUGINFILE URL output).

        Gliffy Diagrams

          Issue Links

            Activity

            Tim Hunt created issue -
            Hide
            Tim Hunt added a comment -

            This also affects 'display question text' in the question bank.

            Show
            Tim Hunt added a comment - This also affects 'display question text' in the question bank.
            Tim Hunt made changes -
            Field Original Value New Value
            Link This issue has been marked as being related by MDL-29579 [ MDL-29579 ]
            Tim Hunt made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Testing Instructions 1. Create a question with an image in the question text.
            2. Go to the question bank, and turn on "Show question text in the question list". Verify that the image appears in the question text.
            3. Add the question (and any others you like) to a new quiz, and attempt it as a student.
            4. As teacher, go to the statistics report for the quiz.
            5. Download the full report as XHTML. Verify that the images in the question texts show up.
            6. Click on the question name in the Quiz structure analysis. On the page you get to, verify that the image in the question text shows up.
            Labels triaged
            Hide
            Tim Hunt added a comment -

            Petr, I would be most grateful if you could take a quick look at this and tell me whether you think this is sensible. Thanks.

            Show
            Tim Hunt added a comment - Petr, I would be most grateful if you could take a quick look at this and tell me whether you think this is sensible. Thanks.
            Tim Hunt made changes -
            Status Open [ 1 ] Waiting for peer review [ 10012 ]
            Pull Master Diff URL https://github.com/timhunt/moodle/compare/MDL-29579...MDL-30592
            Pull Master Branch MDL-30592
            Pull from Repository git://github.com/timhunt/moodle.git
            Peer reviewer skodak
            Hide
            Petr Skoda added a comment -

            I like the component related changes - component means any system subsystem or plugin type+name, I think it is better than separate $plugintype + $pluginname parameters. The rest makes sense too.

            Show
            Petr Skoda added a comment - I like the component related changes - component means any system subsystem or plugin type+name, I think it is better than separate $plugintype + $pluginname parameters. The rest makes sense too.
            Hide
            Tim Hunt added a comment -

            Thanks Petr. I plan to test some more tomorrow morning, then submit for integration.

            Show
            Tim Hunt added a comment - Thanks Petr. I plan to test some more tomorrow morning, then submit for integration.
            Hide
            Tim Hunt added a comment -

            I am now that this is ready to be integrated, but I want to wait until MDL-29579 is integrated, then rebase, before submitting this one for integration.

            Show
            Tim Hunt added a comment - I am now that this is ready to be integrated, but I want to wait until MDL-29579 is integrated, then rebase, before submitting this one for integration.
            Hide
            Tim Hunt added a comment -

            Submitting for integration now.

            Show
            Tim Hunt added a comment - Submitting for integration now.
            Tim Hunt made changes -
            Tim Hunt made changes -
            Testing Instructions 1. Create a question with an image in the question text.
            2. Go to the question bank, and turn on "Show question text in the question list". Verify that the image appears in the question text.
            3. Add the question (and any others you like) to a new quiz, and attempt it as a student.
            4. As teacher, go to the statistics report for the quiz.
            5. Download the full report as XHTML. Verify that the images in the question texts show up.
            6. Click on the question name in the Quiz structure analysis. On the page you get to, verify that the image in the question text shows up.
            1. Create a question with an image in the question text.
            2. Go to the question bank, and turn on "Show question text in the question list". Verify that the image appears in the question text.
            3. Add the question (and any others you like) to a new quiz, and attempt it as a student.
            4. As teacher, go to the statistics report for the quiz.
            5. Download the full report as XHTML. Verify that the images in the question texts show up.
            6. Click on the question name in the Quiz structure analysis. On the page you get to, verify that the image in the question text shows up.

            Note that the cherry-pick to 21 was not clean. In particular changes had to be merged in plugin_callback. Please also test the rating and comments systems, to verify that there are no regressions.
            Eloy Lafuente (stronk7) made changes -
            Currently in integration Yes [ 10041 ]
            Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            Hide
            Sam Hemelryk added a comment -

            Hi Tim,

            Things look nearly 100% spot on thanks and great to have the callback thing finally sorted.
            Sending this back at the moment so a couple of minor things that can be cleaned up.

            1. component_callback exceptions still refer to plugin_callback.
            2. upgrade.txt should be updated to include move from plugin_callback to component_callback
            3. Personally I think that existing plugin_callback should be upgraded as part of this change. I'm open to discussion on this, however there are only 10 so it's not a big task.

            As I imagine that this will be back up promptly feel free to ask Eloy to integrate it (I am 100% happy for this to be taken).

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi Tim, Things look nearly 100% spot on thanks and great to have the callback thing finally sorted. Sending this back at the moment so a couple of minor things that can be cleaned up. component_callback exceptions still refer to plugin_callback. upgrade.txt should be updated to include move from plugin_callback to component_callback Personally I think that existing plugin_callback should be upgraded as part of this change. I'm open to discussion on this, however there are only 10 so it's not a big task. As I imagine that this will be back up promptly feel free to ask Eloy to integrate it (I am 100% happy for this to be taken). Cheers Sam
            Hide
            Sam Hemelryk added a comment -

            Used the wrong button sorry for the spam

            Show
            Sam Hemelryk added a comment - Used the wrong button sorry for the spam
            Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Reopened [ 4 ]
            Hide
            Tim Hunt added a comment -

            Hmm. I was not planning to deprecate the old plugin_callback API, just to introduce a new one (and then refactor to eliminate duplicated code). Is the old plugin_callback so terrible that it needs to be killed? If not, can't we just have a new MDL for that in future (assigned to moodle.com).

            Show
            Tim Hunt added a comment - Hmm. I was not planning to deprecate the old plugin_callback API, just to introduce a new one (and then refactor to eliminate duplicated code). Is the old plugin_callback so terrible that it needs to be killed? If not, can't we just have a new MDL for that in future (assigned to moodle.com).
            Eloy Lafuente (stronk7) made changes -
            Link This issue has a non-specific relationship to MDL-30743 [ MDL-30743 ]
            Hide
            Eloy Lafuente (stronk7) added a comment -

            I've created MDL-30743... about to decide to deprecate & drop plugin_callback()

            Show
            Eloy Lafuente (stronk7) added a comment - I've created MDL-30743 ... about to decide to deprecate & drop plugin_callback()
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Getting this for integration...

            Show
            Eloy Lafuente (stronk7) added a comment - Getting this for integration...
            Eloy Lafuente (stronk7) made changes -
            Status Reopened [ 4 ] Waiting for integration review [ 10010 ]
            Integrator samhemelryk stronk7
            Fix Version/s 2.1.4 [ 11452 ]
            Fix Version/s 2.2.1 [ 11456 ]
            Fix Version/s STABLE backlog [ 10463 ]
            Eloy Lafuente (stronk7) made changes -
            Affects Version/s 2.3 [ 10657 ]
            Eloy Lafuente (stronk7) made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Hide
            Eloy Lafuente (stronk7) added a comment -

            This has been integrated (21, 22 and master), thanks!

            One small commit was added on top tidying phpdocs and exception message.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - This has been integrated (21, 22 and master), thanks! One small commit was added on top tidying phpdocs and exception message. Ciao
            Eloy Lafuente (stronk7) made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Rossiani Wijaya made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Tester rwijaya
            Hide
            Rossiani Wijaya added a comment -

            Hi Tim,

            While testing the statistics' report, the following error occurs (debug: developer mode).

            Notice: Undefined variable: context in /home/moodle/mod/quiz/report/statistics/report.php on line 88

            Looking at the patch it just needs a minor tweak to $this->context.

            Test failed.

            Show
            Rossiani Wijaya added a comment - Hi Tim, While testing the statistics' report, the following error occurs (debug: developer mode). Notice: Undefined variable: context in /home/moodle/mod/quiz/report/statistics/report.php on line 88 Looking at the patch it just needs a minor tweak to $this->context. Test failed.
            Rossiani Wijaya made changes -
            Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
            Hide
            Tim Hunt added a comment -

            Ah, the problem was that MDL-27183 added a new reference to $context, so when that change was merged with this, we got the error you report. I will do one more small commit to fix this in a couple of hours when I get into work.

            Show
            Tim Hunt added a comment - Ah, the problem was that MDL-27183 added a new reference to $context, so when that change was merged with this, we got the error you report. I will do one more small commit to fix this in a couple of hours when I get into work.
            Show
            Tim Hunt added a comment - Fixup commit ready for integration: https://github.com/timhunt/moodle/compare/master...MDL-30592_fixup https://github.com/timhunt/moodle/compare/MOODLE_22_STABLE...MDL-30592_fixup_22 https://github.com/timhunt/moodle/compare/MOODLE_21_STABLE...MDL-30592_fixup_21
            Eloy Lafuente (stronk7) made changes -
            Status Problem during testing [ 10007 ] Integration review in progress [ 10004 ]
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Re-integrated with the $this->context fixup.

            Show
            Eloy Lafuente (stronk7) added a comment - Re-integrated with the $this->context fixup.
            Eloy Lafuente (stronk7) made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Tim Hunt made changes -
            Summary Quiz statistics report - images in the quetsion text do not show up Quiz statistics report - images in the question text do not show up
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Uhm, SamH just noticed that this in 21_STABLE is leading to tons of debug warnings... looking back... it seems that surely we'll need to backport the changes in comments stuff done by MDL-29504 (commit 60c452b4) to avoid them asap.

            Show
            Eloy Lafuente (stronk7) added a comment - Uhm, SamH just noticed that this in 21_STABLE is leading to tons of debug warnings... looking back... it seems that surely we'll need to backport the changes in comments stuff done by MDL-29504 (commit 60c452b4) to avoid them asap.
            Sam Hemelryk made changes -
            Link This issue has been marked as being related by MDL-30791 [ MDL-30791 ]
            Hide
            Sam Hemelryk added a comment -

            Created MDL-30791 to fix those notices. Has been peer-reviewed by Tim and is just waiting integration now.

            Show
            Sam Hemelryk added a comment - Created MDL-30791 to fix those notices. Has been peer-reviewed by Tim and is just waiting integration now.
            Rossiani Wijaya made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            Rossiani Wijaya added a comment -

            This is working great.

            Thanks for fixing this Tim.

            Test passed.

            Show
            Rossiani Wijaya added a comment - This is working great. Thanks for fixing this Tim. Test passed.
            Rossiani Wijaya made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

            Now... disconnect, relax and enjoy the next days, yay!

            Closing...ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao
            Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 23/Dec/11

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: