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
    • Rank:
      18575

      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.

        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: