Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-28564

implement support for extra answer fields in question export and import

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.1
    • Fix Version/s: 2.1.2
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      1. First run all the question unit tests. That gives this code a thorough work-out.

      2. Then, as a double-check, make sure you can export and import some questions in Moodle XML format using the question bank user-interface.

      Please check tricky things, like question texts containing < and &.

      Show
      1. First run all the question unit tests. That gives this code a thorough work-out. 2. Then, as a double-check, make sure you can export and import some questions in Moodle XML format using the question bank user-interface. Please check tricky things, like question texts containing < and &.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Gliffy Diagrams

        Issue Links

          Activity

          Hide
          timhunt Tim Hunt added a comment -

          Jamie, is this a duplicate of MDL-24408?

          Show
          timhunt Tim Hunt added a comment - Jamie, is this a duplicate of MDL-24408 ?
          Hide
          jamiesensei Jamie Pratt added a comment -

          Yes, it is. Sorry didn't realize there was an outstanding issue.

          Show
          jamiesensei Jamie Pratt added a comment - Yes, it is. Sorry didn't realize there was an outstanding issue.
          Hide
          jamiesensei Jamie Pratt added a comment -

          Actually this issue just addresses the import and export of extra answer fields. MDL-24408 seems to address a lot of other aspects of extra answer fields.

          Show
          jamiesensei Jamie Pratt added a comment - Actually this issue just addresses the import and export of extra answer fields. MDL-24408 seems to address a lot of other aspects of extra answer fields.
          Hide
          jamiesensei Jamie Pratt added a comment -

          I have a patch for this :

          https://github.com/jamiepratt/moodle/compare/master...MDL-28564_wip

          Currently import and export in the varnumeric question type depends on this patch. I could move all the functionality into the question type and not call the parent class if it is undecided what the best way is to implement extra answer fields.

          Show
          jamiesensei Jamie Pratt added a comment - I have a patch for this : https://github.com/jamiepratt/moodle/compare/master...MDL-28564_wip Currently import and export in the varnumeric question type depends on this patch. I could move all the functionality into the question type and not call the parent class if it is undecided what the best way is to implement extra answer fields.
          Hide
          jamiesensei Jamie Pratt added a comment -

          Here is the related bug in the OU tracker : http://ltsredmine.open.ac.uk/issues/1171

          And my patch on github for the question type plug in code : https://github.com/jamiepratt/moodle-qtype_varnumeric/commit/b196bc1e1dc404e408e748eb875699e742ee4a07

          Show
          jamiesensei Jamie Pratt added a comment - Here is the related bug in the OU tracker : http://ltsredmine.open.ac.uk/issues/1171 And my patch on github for the question type plug in code : https://github.com/jamiepratt/moodle-qtype_varnumeric/commit/b196bc1e1dc404e408e748eb875699e742ee4a07
          Hide
          salvetore Michael de Raadt added a comment -

          Thanks for reporting this and providing a solution.

          Show
          salvetore Michael de Raadt added a comment - Thanks for reporting this and providing a solution.
          Hide
          timhunt Tim Hunt added a comment -

          Some code-review comments:

          1. don't use static methods like wrap_html_special_chars without good reason. This should be a normal method.

          2. do write PHP doc comments for any new methods you create.

          3. However, this method should not exist at all. It is close to duplicating the writetext method in the format class. So, the method you want belongs in the format class, and you should refactor to avoid duplication.

          I am about to make a revised patch.

          Show
          timhunt Tim Hunt added a comment - Some code-review comments: 1. don't use static methods like wrap_html_special_chars without good reason. This should be a normal method. 2. do write PHP doc comments for any new methods you create. 3. However, this method should not exist at all. It is close to duplicating the writetext method in the format class. So, the method you want belongs in the format class, and you should refactor to avoid duplication. I am about to make a revised patch.
          Hide
          timhunt Tim Hunt added a comment -

          In a sense this is a new feature, so it should not go on the 2.1 branch.

          On the other hand, it would be very helpful for the OU to have this in 2.1, and it will not break or change any existing functionality. It will only make it easier for people writing third-parth question types. And the unit tests verify that nothing is broken. Therefore, please merge this onto the 2.1 branch as well as master. Thanks.

          Show
          timhunt Tim Hunt added a comment - In a sense this is a new feature, so it should not go on the 2.1 branch. On the other hand, it would be very helpful for the OU to have this in 2.1, and it will not break or change any existing functionality. It will only make it easier for people writing third-parth question types. And the unit tests verify that nothing is broken. Therefore, please merge this onto the 2.1 branch as well as master. Thanks.
          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
          samhemelryk Sam Hemelryk added a comment -

          Thanks guys, this has been integrated to 21 and master.
          Just noting that the first commit message could be made clearer, certainly the quoting is interesting.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks guys, this has been integrated to 21 and master. Just noting that the first commit message could be made clearer, certainly the quoting is interesting. Cheers Sam
          Hide
          rwijaya Rossiani Wijaya added a comment -

          Exporting and importing question works great. Question texts are also tested with the following chars:

          • single and double quotes
          • & (and sign)
          • <> (greater and less than signs)

          Test passed.

          Show
          rwijaya Rossiani Wijaya added a comment - Exporting and importing question works great. Question texts are also tested with the following chars: single and double quotes & (and sign) <> (greater and less than signs) Test passed.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay!

          Closing, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                10/Oct/11