Moodle
  1. Moodle
  2. MDL-29044

Move course category creation to separate function

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.1
    • Fix Version/s: 2.3
    • Component/s: Course, Enrolments
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      Run supplied unit tests first:

      • Navigate to Settings -> Site Administration -> Courses -> Add/edit courses
      • Add new category
      • Create a new category at 'Top'
        • Name the category 'MDL-29044-1'
        • Confirm that the new category was created in the 'Top' course category
      • Add another new category
        • Name the category 'MDL-29044-2'
        • Create this new category under MDL-29044-1 to confirm that parenting works
        • Confirm that the new category was created within MDL-29044-1
      • Add another new category
      Show
      Run supplied unit tests first: Navigate to Settings -> Site Administration -> Courses -> Add/edit courses Add new category Create a new category at 'Top' Name the category ' MDL-29044 -1' Confirm that the new category was created in the 'Top' course category Add another new category Name the category ' MDL-29044 -2' Create this new category under MDL-29044 -1 to confirm that parenting works Confirm that the new category was created within MDL-29044 -1 Add another new category Name the category ' MDL-29044 -0' Create this new category under MDL-29044 -1 Confirm that the new category was created within MDL-29044 -1 Confirm that categories are listed in the order MDL-29044 -2, MDL-29044 -0
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-29044-master-3

      Description

      As requested by Petr in MDL-28516, we ideally need to move course category creation to a separate function to support creating course categories in enrolment plugins (and anywhere else too).

      This patch creates a new function in course/lib.php called create_course_category().
      At present it marks the created context as dirty, but does not call fix_course_sortorder. This is because course/editcategory.php (which is the only place which calls this code) calls this regardless of whether a course category is created, or edited.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Michael de Raadt added a comment -

            Hi, Dan and Petr.

            I've added you as watchers for this issue as you were involved in the linked issue that this follows on from.

            Show
            Michael de Raadt added a comment - Hi, Dan and Petr. I've added you as watchers for this issue as you were involved in the linked issue that this follows on from.
            Hide
            Dan Poltawski added a comment -

            Andrew perhaps if you got a chance you could rebase this and submit it for peer review, targeted for master

            Show
            Dan Poltawski added a comment - Andrew perhaps if you got a chance you could rebase this and submit it for peer review, targeted for master
            Hide
            Andrew Nicols added a comment -

            I've rebased on master.

            Please note that this patch changes the way in which sortorder is calculated for new categories.
            The original code set a fixed sortorder of 999 for new categories and then relied on fix_course_sortorder to correct this.
            The patch modifies this to not set a sortorder which is picked up by fix_course_sortorder as being an unsorted course order and moved to the end instead.

            Show
            Andrew Nicols added a comment - I've rebased on master. Please note that this patch changes the way in which sortorder is calculated for new categories. The original code set a fixed sortorder of 999 for new categories and then relied on fix_course_sortorder to correct this. The patch modifies this to not set a sortorder which is picked up by fix_course_sortorder as being an unsorted course order and moved to the end instead.
            Hide
            Andrew Davis added a comment -

            The code change looks fair enough. Consider adding a unit test to /course/simpletest/testcourselib.php.

            Possibly also expand the test instructions to include creating at least two categories with the same parent to check that the sorting works as you expect it to. Both the automatic fixing of sort order and manually altering the sort order.

            Show
            Andrew Davis added a comment - The code change looks fair enough. Consider adding a unit test to /course/simpletest/testcourselib.php. Possibly also expand the test instructions to include creating at least two categories with the same parent to check that the sorting works as you expect it to. Both the automatic fixing of sort order and manually altering the sort order.
            Hide
            Andrew Nicols added a comment -

            Updated to add phpunit tests and rebase against current master

            Show
            Andrew Nicols added a comment - Updated to add phpunit tests and rebase against current master
            Hide
            Andrew Nicols added a comment -

            Submitting for integration now that unit tests are written as requested

            Show
            Andrew Nicols added a comment - Submitting for integration now that unit tests are written as requested
            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
            Sam Hemelryk added a comment -

            Thanks Andrew + Andrew, this has been integrated now

            Show
            Sam Hemelryk added a comment - Thanks Andrew + Andrew, this has been integrated now
            Hide
            Ankit Agarwal added a comment -

            Hi ,
            PHP unit tests passed successfully

            Time: 13 seconds, Memory: 75.50Mb                                                                      
            OK (4 tests, 31 assertions)
            

            Rest things works as expected.
            Passing
            Thanks

            Show
            Ankit Agarwal added a comment - Hi , PHP unit tests passed successfully Time: 13 seconds, Memory: 75.50Mb OK (4 tests, 31 assertions) Rest things works as expected. Passing Thanks
            Hide
            Eloy Lafuente (stronk7) added a comment -

            UPDATE tracker_issues
               SET status = 'Closed',
                  comment = 'Thanks!'
            WHEN participants = 'Did a gorgeous work'
            

            This landed upstream some hours ago (some - me - developer fell slept in the sofa yesterday before spamming this).

            Show
            Eloy Lafuente (stronk7) added a comment - UPDATE tracker_issues SET status = 'Closed', comment = 'Thanks!' WHEN participants = 'Did a gorgeous work' This landed upstream some hours ago (some - me - developer fell slept in the sofa yesterday before spamming this).

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: