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:
    • Rank:
      33376

      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).

        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 Škoda 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 Škoda 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: