Details

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

      Gliffy Diagrams

        Attachments

          Issue Links

            Activity

            Hide
            nebgor 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
            nebgor 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
            rwijaya 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
            rwijaya 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
            rwijaya Rossiani Wijaya added a comment -

            Assigning David as peer review.

            Show
            rwijaya Rossiani Wijaya added a comment - Assigning David as peer review.
            Hide
            mudrd8mz David Mudrák 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
            mudrd8mz David Mudrák 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
            rwijaya Rossiani Wijaya added a comment -

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

            Show
            rwijaya Rossiani Wijaya added a comment - Updated and submit github pull request for David: https://github.com/mudrd8mz/moodle/pull/5
            Hide
            mudrd8mz David Mudrák 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
            mudrd8mz David Mudrák 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
            rwijaya 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
            rwijaya 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
            mudrd8mz David Mudrák added a comment -

            Thanks Rossie, lookign good now.

            Show
            mudrd8mz David Mudrák added a comment - Thanks Rossie, lookign good now.
            Hide
            mudrd8mz David Mudrák added a comment -

            Merged into the pre-integration branch. Thanks Rossie

            Show
            mudrd8mz David Mudrák added a comment - Merged into the pre-integration branch. Thanks Rossie
            Hide
            mudrd8mz David Mudrák 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
            mudrd8mz David Mudrák 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
            nebgor 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
            nebgor 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
            nebgor 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
            nebgor 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
            nebgor Aparup Banerjee added a comment -
            Show
            nebgor Aparup Banerjee added a comment - David, i've created https://github.com/mudrd8mz/moodle/pull/12
            Hide
            mudrd8mz David Mudrák added a comment -

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

            Show
            mudrd8mz David Mudrák added a comment - Apu's recent changes merged into the pre-integration branch. Thanks!
            Hide
            nebgor 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
            nebgor 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
            mudrd8mz David Mudrák added a comment -

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

            Show
            mudrd8mz David Mudrák 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:
                  Fix Release Date:
                  1/Jul/11