Moodle
  1. Moodle
  2. MDL-29058

no export / import from xml for specific feedback or hint files

    Details

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

      1) Ideally: create one of each type of question, putting a different image into the HTML of each field that uses the HTML editor.

      (You may feel that this is overkill. An alternative would be to do this with one match, multichoice and numerical question.)

      2) Export the questions as Moodle XML format.

      3) Import into a new course.

      4) Verify the images were correctly transferred.

      5) Run all the unit tests in /question.

      Show
      1) Ideally: create one of each type of question, putting a different image into the HTML of each field that uses the HTML editor. (You may feel that this is overkill. An alternative would be to do this with one match, multichoice and numerical question.) 2) Export the questions as Moodle XML format. 3) Import into a new course. 4) Verify the images were correctly transferred. 5) Run all the unit tests in /question.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      18584

      Description

      The Moodle xml format import / export code is not importing / exporting files for correct feedback / partially correct / incorrect or for hints.

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          What makes you think that? Looking at the code, the code to handle files in hints and combined feedback is there, and I am pretty sure I remember testing it.

          Show
          Tim Hunt added a comment - What makes you think that? Looking at the code, the code to handle files in hints and combined feedback is there, and I am pretty sure I remember testing it.
          Hide
          Jamie Pratt added a comment -

          Hi,

          During testing the ddimagetoimage question type I exported a file with images embedded in all html editors. When I imported the question I found that the images in the three specific feedback elements and the hints did not display, instead there was a broken link.

          Looking at the code I don't think there is code there to export files embedded in these html editors.

          I do see the code for import is there.

          Jamie

          Show
          Jamie Pratt added a comment - Hi, During testing the ddimagetoimage question type I exported a file with images embedded in all html editors. When I imported the question I found that the images in the three specific feedback elements and the hints did not display, instead there was a broken link. Looking at the code I don't think there is code there to export files embedded in these html editors. I do see the code for import is there. Jamie
          Hide
          Tim Hunt added a comment -

          OK. Obviously I did not test this properly.

          Show
          Tim Hunt added a comment - OK. Obviously I did not test this properly.
          Hide
          Tim Hunt added a comment -

          Jamie, I think this does it. Sadly, it cannot work without an API change to pass through some more information (contextid and questionid) so I think this has to be HEAD only.

          Please could you take a look and see if you think this works.

          Show
          Tim Hunt added a comment - Jamie, I think this does it. Sadly, it cannot work without an API change to pass through some more information (contextid and questionid) so I think this has to be HEAD only. Please could you take a look and see if you think this works.
          Hide
          Jamie Pratt added a comment -

          Looks good Tim. I tested export and import with image files in the html editor and it seems to work great now.

          Show
          Jamie Pratt added a comment - Looks good Tim. I tested export and import with image files in the html editor and it seems to work great now.
          Hide
          Tim Hunt added a comment -

          Thanks Jamie. Submitting for integration.

          Dear integrators. I am just submitting this for 2.2, since it requires a small API change. We could, however, consider adding this to 2.1. That would fix core qtypes.

          It would meed that third-party qtypes generated Missing function argument warnings on export, but the export would still work other wise (well third-party qtypes would still fail to export files, since the missing contextid would be treated as 0) but they would be no more broken than now, and third-party qtype authors would be able to fix the export of their questions, which is not possible now. If we do this, we would need to update question/type/upgrade.txt.

          Note too that this fix follows on from the changes in MDL-29739, so please integrate that other fix first.

          Show
          Tim Hunt added a comment - Thanks Jamie. Submitting for integration. Dear integrators. I am just submitting this for 2.2, since it requires a small API change. We could, however, consider adding this to 2.1. That would fix core qtypes. It would meed that third-party qtypes generated Missing function argument warnings on export, but the export would still work other wise (well third-party qtypes would still fail to export files, since the missing contextid would be treated as 0) but they would be no more broken than now, and third-party qtype authors would be able to fix the export of their questions, which is not possible now. If we do this, we would need to update question/type/upgrade.txt. Note too that this fix follows on from the changes in MDL-29739 , so please integrate that other fix first.
          Hide
          Jamie Pratt added a comment -

          Just a note to say that exports will fail if the user has display error messages on and the notices are displayed. This breaks the download as then headers cannot be output after the notices.

          Show
          Jamie Pratt added a comment - Just a note to say that exports will fail if the user has display error messages on and the notices are displayed. This breaks the download as then headers cannot be output after the notices.
          Hide
          Tim Hunt added a comment -

          Oh bum! I had forgotten that stupid change the the Moodle 2.x files API forced on us.

          Show
          Tim Hunt added a comment - Oh bum! I had forgotten that stupid change the the Moodle 2.x files API forced on us.
          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 -

          This has been integrated into master only.

          Tim,
          I too think that adding it to 2.1 would be a good idea too for the overall improvement if not for the fact that it doesn't really fix but enables a fix (but really that is IMHO). I can't assume this is desired by everyone (zero votes too ).

          I've stuck with putting this into master only, those who really need to fix (therefore break it first) can always pull this patch over to their 21 or upgrade or

          If anyone still feels strongly about backporting this we can always raise another issue as needed.

          Show
          Aparup Banerjee added a comment - This has been integrated into master only. Tim, I too think that adding it to 2.1 would be a good idea too for the overall improvement if not for the fact that it doesn't really fix but enables a fix (but really that is IMHO). I can't assume this is desired by everyone (zero votes too ). I've stuck with putting this into master only, those who really need to fix (therefore break it first) can always pull this patch over to their 21 or upgrade or If anyone still feels strongly about backporting this we can always raise another issue as needed.
          Hide
          Ankit Agarwal added a comment -

          all things working fine!
          Test passed
          Thanks!

          Show
          Ankit Agarwal added a comment - all things working fine! Test passed Thanks!
          Hide
          Tim Hunt added a comment -

          The problem is that this patch makes significant API changes, which will break all third-party question types. We can't do that on a stable branch.

          Show
          Tim Hunt added a comment - The problem is that this patch makes significant API changes, which will break all third-party question types. We can't do that on a stable branch.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: