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

External function: update_categories does not update the path field when moving category

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Course, Web Services
    • Labels:
    • Testing Instructions:
      Hide

      Run the REST demo client
      Call core_course_update_categories(). You must move a category from a parent to another parent. Check that the moved category has a correct path. For example if it was moved from category1->id = 4 to category2->id = 5, then the path should have been updated from /4/X to /5/X

      You can check the path field in the DB table: course_categories

      Alternative test:
      you can take my phpunit test for this function in https://github.com/mouneyrac/moodle/commit/bae128d5e1124a191d00648e1aca57f6b407ca1c (MDL-33529) - it's done and it will be much faster test to call this unit test (if you do so, comment the other PHPunit test because lots of them are related to fix waiting to be integrated. If you don't comment other tests they will throw errors to you.)

      Show
      Run the REST demo client Call core_course_update_categories(). You must move a category from a parent to another parent. Check that the moved category has a correct path. For example if it was moved from category1->id = 4 to category2->id = 5, then the path should have been updated from /4/X to /5/X You can check the path field in the DB table: course_categories Alternative test: you can take my phpunit test for this function in https://github.com/mouneyrac/moodle/commit/bae128d5e1124a191d00648e1aca57f6b407ca1c ( MDL-33529 ) - it's done and it will be much faster test to call this unit test (if you do so, comment the other PHPunit test because lots of them are related to fix waiting to be integrated. If you don't comment other tests they will throw errors to you.)
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      fix_course_sortorder() called is overwritten by a later $DB->update call.

      To fix:

      // Get updated path by move_category().
      $category->path = $DB->get_field('course_categories', 'path',
                              array('id' => $category->id));

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Jerome, this has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Jerome, this has been integrated now
            Hide
            phalacee Jason Fowler added a comment -

            Tested using the phpunit test -

            Waiting for other test execution to complete...
            PHPUnit 3.6.10 by Sebastian Bergmann.

            Configuration read from /var/www/repos/imaster/moodle/phpunit.xml

            ....F.FEE

            Time: 02:01, Memory: 123.75Mb

            There were 2 errors:

            1) courseexternallib_testcase::test_get_course_contents
            Undefined index: modules

            /var/www/repos/imaster/moodle/course/tests/courseexternallib_test.php:523
            /var/www/repos/imaster/moodle/lib/phpunit/classes/advanced_testcase.php:76

            To re-run:
            phpunit courseexternallib_testcase course/tests/courseexternallib_test.php

            2) courseexternallib_testcase::test_duplicate_course
            Undefined index: modules

            /var/www/repos/imaster/moodle/course/tests/courseexternallib_test.php:568
            /var/www/repos/imaster/moodle/lib/phpunit/classes/advanced_testcase.php:76

            To re-run:
            phpunit courseexternallib_testcase course/tests/courseexternallib_test.php

            There were 2 failures:

            1) courseexternallib_testcase::test_create_courses
            Failed asserting that '0' matches expected '1'.

            /var/www/repos/imaster/moodle/course/tests/courseexternallib_test.php:378
            /var/www/repos/imaster/moodle/lib/phpunit/classes/advanced_testcase.php:76

            To re-run:
            phpunit courseexternallib_testcase course/tests/courseexternallib_test.php

            2) courseexternallib_testcase::test_get_courses
            Failed asserting that '0' matches expected '1'.

            /var/www/repos/imaster/moodle/course/tests/courseexternallib_test.php:469
            /var/www/repos/imaster/moodle/lib/phpunit/classes/advanced_testcase.php:76

            To re-run:
            phpunit courseexternallib_testcase course/tests/courseexternallib_test.php

            FAILURES!
            Tests: 9, Assertions: 39, Failures: 2, Errors: 2.

            Show
            phalacee Jason Fowler added a comment - Tested using the phpunit test - Waiting for other test execution to complete... PHPUnit 3.6.10 by Sebastian Bergmann. Configuration read from /var/www/repos/imaster/moodle/phpunit.xml ....F.FEE Time: 02:01, Memory: 123.75Mb There were 2 errors: 1) courseexternallib_testcase::test_get_course_contents Undefined index: modules /var/www/repos/imaster/moodle/course/tests/courseexternallib_test.php:523 /var/www/repos/imaster/moodle/lib/phpunit/classes/advanced_testcase.php:76 To re-run: phpunit courseexternallib_testcase course/tests/courseexternallib_test.php 2) courseexternallib_testcase::test_duplicate_course Undefined index: modules /var/www/repos/imaster/moodle/course/tests/courseexternallib_test.php:568 /var/www/repos/imaster/moodle/lib/phpunit/classes/advanced_testcase.php:76 To re-run: phpunit courseexternallib_testcase course/tests/courseexternallib_test.php – There were 2 failures: 1) courseexternallib_testcase::test_create_courses Failed asserting that '0' matches expected '1'. /var/www/repos/imaster/moodle/course/tests/courseexternallib_test.php:378 /var/www/repos/imaster/moodle/lib/phpunit/classes/advanced_testcase.php:76 To re-run: phpunit courseexternallib_testcase course/tests/courseexternallib_test.php 2) courseexternallib_testcase::test_get_courses Failed asserting that '0' matches expected '1'. /var/www/repos/imaster/moodle/course/tests/courseexternallib_test.php:469 /var/www/repos/imaster/moodle/lib/phpunit/classes/advanced_testcase.php:76 To re-run: phpunit courseexternallib_testcase course/tests/courseexternallib_test.php FAILURES! Tests: 9, Assertions: 39, Failures: 2, Errors: 2.
            Hide
            poltawski Dan Poltawski added a comment -

            Ping Jerome!

            Show
            poltawski Dan Poltawski added a comment - Ping Jerome!
            Hide
            jerome Jérôme Mouneyrac added a comment -

            In the test instructions, I meant you need to comment the none related unit tests as they will throw errors (the fix are done but not in integration branch) But you don't need to retest it because no errors were thrown for update_categories() so all is good. You can mark it as passed, thanks.

            Show
            jerome Jérôme Mouneyrac added a comment - In the test instructions, I meant you need to comment the none related unit tests as they will throw errors (the fix are done but not in integration branch) But you don't need to retest it because no errors were thrown for update_categories() so all is good. You can mark it as passed, thanks.
            Hide
            phalacee Jason Fowler added a comment -

            oh, okay, I was really confused about what should be commented out with that, no probs, as soon as this is resest, I will pass it then

            Show
            phalacee Jason Fowler added a comment - oh, okay, I was really confused about what should be commented out with that, no probs, as soon as this is resest, I will pass it then
            Hide
            jerome Jérôme Mouneyrac added a comment -

            No problem Jason. I guess don't lose time to retest, from your log it worked. You can pass it

            Show
            jerome Jérôme Mouneyrac added a comment - No problem Jason. I guess don't lose time to retest, from your log it worked. You can pass it
            Hide
            phalacee Jason Fowler added a comment -

            All good thanks Jerome

            Show
            phalacee Jason Fowler added a comment - All good thanks Jerome
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Big thanks for the effort. This is now part of Moodle upstream. Let's wait for regressions, yay! LOL

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Big thanks for the effort. This is now part of Moodle upstream. Let's wait for regressions, yay! LOL Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Jun/12