Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: DEV backlog
    • Fix Version/s: 2.1
    • Component/s: Lesson
    • Labels:
      None

      Gliffy Diagrams

        Issue Links

          Activity

          Hide
          Aparup Banerjee added a comment -

          1) process_lesson() php_doc mentions MOD/CHOICE
          2) process_lesson_pages() -> process_lesson_page() as we're processing just the one page. also in get_path()
          3) process_lesson_answers() -> process_lesson_page_answer() as we're processing a single answer thats within a page. also that call to write_xml needs to be checked, i'm not sure about providing the 4th arg hardcoded.

          have you tested your xml file with the unit test ? (described in http://docs.moodle.org/en/Development:Backup_1.9_conversion_for_developers#Setting-up_the_development_environment )

          Show
          Aparup Banerjee added a comment - 1) process_lesson() php_doc mentions MOD/CHOICE 2) process_lesson_pages() -> process_lesson_page() as we're processing just the one page. also in get_path() 3) process_lesson_answers() -> process_lesson_page_answer() as we're processing a single answer thats within a page. also that call to write_xml needs to be checked, i'm not sure about providing the 4th arg hardcoded. have you tested your xml file with the unit test ? (described in http://docs.moodle.org/en/Development:Backup_1.9_conversion_for_developers#Setting-up_the_development_environment )
          Hide
          Rossiani Wijaya added a comment -

          Hi Apu,

          Thanks for reviewing.
          1) fixed
          2) renaming it to process_lesson_page()
          3) renaming it to process_lesson_answer(), and removing the 3rd and 4th arguments (it was there for my testing purpose only)

          I have tested the xml file with the unit test. I will attach the xml file.

          Note: updated diff url

          Show
          Rossiani Wijaya added a comment - Hi Apu, Thanks for reviewing. 1) fixed 2) renaming it to process_lesson_page() 3) renaming it to process_lesson_answer(), and removing the 3rd and 4th arguments (it was there for my testing purpose only) I have tested the xml file with the unit test. I will attach the xml file. Note: updated diff url
          Hide
          Rossiani Wijaya added a comment -

          Assigning David as peer review.

          Show
          Rossiani Wijaya added a comment - Assigning David as peer review.
          Hide
          David Mudrak added a comment -

          Hi Rossi. Thanks for your work. Just details:

          1) please replace the get_moduleid() call with the new one as shown in the template.
          2) please get rid of the trailing whitespace

          Show
          David Mudrak added a comment - Hi Rossi. Thanks for your work. Just details: 1) please replace the get_moduleid() call with the new one as shown in the template. 2) please get rid of the trailing whitespace
          Hide
          Rossiani Wijaya added a comment -

          Updated and submit github pull request for David:
          https://github.com/mudrd8mz/moodle/pull/5

          Show
          Rossiani Wijaya added a comment - Updated and submit github pull request for David: https://github.com/mudrd8mz/moodle/pull/5
          Hide
          David Mudrak added a comment -

          Why isn't the field answertext renamed to answer_text via a recipe in get_paths() ? Then you can use write_xml() wrapper for the whole data easily without using the manual foreach loop.

          Show
          David Mudrak added a comment - Why isn't the field answertext renamed to answer_text via a recipe in get_paths() ? Then you can use write_xml() wrapper for the whole data easily without using the manual foreach loop.
          Hide
          Rossiani Wijaya added a comment -

          Hi David,
          I fixed the patch as you suggested.

          Resubmitted patch for github pull request:
          https://github.com/mudrd8mz/moodle/pull/6

          Show
          Rossiani Wijaya added a comment - Hi David, I fixed the patch as you suggested. Resubmitted patch for github pull request: https://github.com/mudrd8mz/moodle/pull/6
          Hide
          David Mudrak added a comment -

          Thanks Rossie, lookign good now.

          Show
          David Mudrak added a comment - Thanks Rossie, lookign good now.
          Hide
          David Mudrak added a comment -

          Merged into the pre-integration branch. Thanks Rossie

          Show
          David Mudrak added a comment - Merged into the pre-integration branch. Thanks Rossie
          Hide
          David Mudrak added a comment -

          Rosie, can you please review all steps in mod/lesson/db/upgrade.php and make sure they all are reflected in moodle1 converter? For example the upgrade step 2010121400 does not seem to replayed. Thanks.

          Show
          David Mudrak added a comment - Rosie, can you please review all steps in mod/lesson/db/upgrade.php and make sure they all are reflected in moodle1 converter? For example the upgrade step 2010121400 does not seem to replayed. Thanks.
          Hide
          Aparup Banerjee added a comment - - edited

          Thanks David, i'll put that bit in soon, looking at it now. (Rosie, i'm just grabbing this bit while your away)

          Show
          Aparup Banerjee added a comment - - edited Thanks David, i'll put that bit in soon, looking at it now. (Rosie, i'm just grabbing this bit while your away)
          Hide
          Aparup Banerjee added a comment -

          fyi: i've got a branch up at https://github.com/nebgor/moodle/compare/backup-convert...MDL-27446 , still checking the rest of upgrade steps.

          Show
          Aparup Banerjee added a comment - fyi: i've got a branch up at https://github.com/nebgor/moodle/compare/backup-convert...MDL-27446 , still checking the rest of upgrade steps.
          Hide
          Aparup Banerjee added a comment -
          Show
          Aparup Banerjee added a comment - David, i've created https://github.com/mudrd8mz/moodle/pull/12
          Hide
          David Mudrak added a comment -

          Apu's recent changes merged into the pre-integration branch. Thanks!

          Show
          David Mudrak added a comment - Apu's recent changes merged into the pre-integration branch. Thanks!
          Hide
          Aparup Banerjee added a comment - - edited

          David, i can't see that the <jumpto/> issue fix pulled into your backup-convert branch yet. I've merged it into my current working branch for MDL-27819.

          Show
          Aparup Banerjee added a comment - - edited David, i can't see that the <jumpto/> issue fix pulled into your backup-convert branch yet. I've merged it into my current working branch for MDL-27819 .
          Hide
          David Mudrak added a comment -

          Oops sorry my bad. I just cherry-picked the submitted patch, please re-fetch.

          Show
          David Mudrak added a comment - Oops sorry my bad. I just cherry-picked the submitted patch, please re-fetch.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: