Moodle
  1. Moodle
  2. MDL-32570

Deleting "Miscellaneous" category broke the ability to use IMS Enterprise to create courses

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.1, 2.4.6, 2.5.2
    • Fix Version/s: 2.4.7, 2.5.3
    • Component/s: Enrolments
    • Labels:
    • Testing Instructions:
      Hide

      2.5 and master:

      1. Run phpunit tests

      2.4:

      1. Download the attached IMS enterprise enrolment file and put it in your moodledata directory
      2. Delete the 'Miscelaneous category' in your site.
      3. VERIFY: that going to /course/category.php?id=1 returns 'Unknown category'
      4. Go to Home / Site administration / Plugins / Enrolments
      5. Enable the IMS enterprise enrolment plugin
      6. In the settings, provide the path to the file you uploaded in File location
      7. Ensure 'Create new (hidden) course categories if not found in Moodle' is disabled
      8. Ensure 'Create new (hidden) courses if not found in Moodle' is enabled
      9. Save settings
      10. Go to the link 'perform an IMS Enterprise import right now'
      11. VERIFY: the IMS enrolment works ok
      12. VERIFY: You have a new course in your Moodle called 'MOODLE104'
      13. VERIFY: no debugging is experienced
      Show
      2.5 and master: Run phpunit tests 2.4: Download the attached IMS enterprise enrolment file and put it in your moodledata directory Delete the 'Miscelaneous category' in your site. VERIFY: that going to /course/category.php?id=1 returns 'Unknown category' Go to Home / Site administration / Plugins / Enrolments Enable the IMS enterprise enrolment plugin In the settings, provide the path to the file you uploaded in File location Ensure 'Create new (hidden) course categories if not found in Moodle' is disabled Ensure 'Create new (hidden) courses if not found in Moodle' is enabled Save settings Go to the link 'perform an IMS Enterprise import right now' VERIFY: the IMS enrolment works ok VERIFY: You have a new course in your Moodle called 'MOODLE104' VERIFY: no debugging is experienced
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-32570-master
    • Story Points (Obsolete):
      2
    • Sprint:
      BACKEND Sprint 5

      Description

      We deleted the "miscellaneous" category in Moodle 2.1 and ran our IMS Enterprise script. No courses were created. The incident sounds similar to http://tracker.moodle.org/browse/MDL-21470 and this posting http://moodle.org/mod/forum/discuss.php?d=200639

      If we remove the one section in the script that is really a term, but has the same tag as a course with no category specified, then all of the other courses are created just fine. When we created the "miscellaneous" category again then the script runs without a problem. An empty course with the name "Spring 2012" (which is the name of our category) is created in the "Miscellaneous" category. The real courses are then created in the "Spring 2012" category.

      I don't know if it matters but we are on PHP Version 5.3.5

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Petr Skoda added a comment -

            thanks for the report

            Show
            Petr Skoda added a comment - thanks for the report
            Hide
            Barbara Taylor added a comment -

            I hit add too soon. My questions are: Do we have to keep and hide the miscellaneous category? If we aren't using it we'd like to delete it but don't want the course creation problem. In the script we have <orgunit>Spring 2012</orgunit> code one of the <group> sections but not in the other <group> section. Does that make a difference?

            Show
            Barbara Taylor added a comment - I hit add too soon. My questions are: Do we have to keep and hide the miscellaneous category? If we aren't using it we'd like to delete it but don't want the course creation problem. In the script we have <orgunit>Spring 2012</orgunit> code one of the <group> sections but not in the other <group> section. Does that make a difference?
            Hide
            Dan Poltawski added a comment -

            Requesting peer review.

            Notes:

            1. This causes pretty horrible situation where you can't even get to the course to delete it.
            2. The unit tests are new in 2.5
            3. I added a static cache here without MUC. I know this isn't ideal, but I think its overkill to add a defined MUC cache here, i'd prefer to get the default category just once, but its impossible to pass this around neatly. So, i've ended up with the static cache.
            Show
            Dan Poltawski added a comment - Requesting peer review. Notes: This causes pretty horrible situation where you can't even get to the course to delete it. The unit tests are new in 2.5 I added a static cache here without MUC. I know this isn't ideal, but I think its overkill to add a defined MUC cache here, i'd prefer to get the default category just once, but its impossible to pass this around neatly. So, i've ended up with the static cache.
            Hide
            Ankit Agarwal added a comment -

            Hi Dan,
            The patch looks good. However am just wondering would it be better to improve get_children() instead so it doesn't do an extra DB query if it has all the categories in cache?

            Also there is an extra empty line in the phpunit file.

            Thanks

            Show
            Ankit Agarwal added a comment - Hi Dan, The patch looks good. However am just wondering would it be better to improve get_children() instead so it doesn't do an extra DB query if it has all the categories in cache? Also there is an extra empty line in the phpunit file. Thanks
            Hide
            Dan Poltawski added a comment -

            Hi Ankit,

            You are right to raise the issue about the static cache, its a bit ugly and I know it. But...

            1. I don't want to get into the internals of changing the category things in this issue. I would also need to fix it differently in 2.4 and 2.5 and 2.6.
            2. It is very likely for ims enrolment to be doing lots of these course creates at once.
            3. Its hard to pass the state around the plugin without that becoming even more ugly..

            Perhaps the nicest solution from my point of view would be to do no caching at all and just accept that it might double the query count, but i'm not keen on this.

            Another solution i'd prefer to use an ad-hoc muc cache even cos then at least we'd get phpunit reseting, but i've been discouraged from using ad hoc caches like this. Also, I don't feel like this will be a massive phpunit reset problem

            So, as discussed i'm gonna try my luck with the integrators, I agree its not ideal at all.

            I've fixed up the extra line (although there are many problems in this file, there is an open issue to fix them)

            Show
            Dan Poltawski added a comment - Hi Ankit, You are right to raise the issue about the static cache, its a bit ugly and I know it. But... I don't want to get into the internals of changing the category things in this issue. I would also need to fix it differently in 2.4 and 2.5 and 2.6. It is very likely for ims enrolment to be doing lots of these course creates at once. Its hard to pass the state around the plugin without that becoming even more ugly.. Perhaps the nicest solution from my point of view would be to do no caching at all and just accept that it might double the query count, but i'm not keen on this. Another solution i'd prefer to use an ad-hoc muc cache even cos then at least we'd get phpunit reseting, but i've been discouraged from using ad hoc caches like this. Also, I don't feel like this will be a massive phpunit reset problem So, as discussed i'm gonna try my luck with the integrators, I agree its not ideal at all. I've fixed up the extra line (although there are many problems in this file, there is an open issue to fix them)
            Hide
            Dan Poltawski 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
            Dan Poltawski 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
            Damyon Wiese added a comment -

            Ok - tested and it would be nice if coursecat cache used it's cache properly - but agreed that is a separate issue.

            I think that would be the nicest solution - and remove this static cache when it's done. I created MDL-42193 for this.

            Thought about abusing $CFG->defaultrequestcategory for this purpose and agreed with Dan that would not be correct.

            I wonder if we should be creating the courses at all if the category doesn't exist? Putting the courses in the top of the category tree seems a bit random to me (but changing this behaviour wouldn't make sense for stables).

            Show
            Damyon Wiese added a comment - Ok - tested and it would be nice if coursecat cache used it's cache properly - but agreed that is a separate issue. I think that would be the nicest solution - and remove this static cache when it's done. I created MDL-42193 for this. Thought about abusing $CFG->defaultrequestcategory for this purpose and agreed with Dan that would not be correct. I wonder if we should be creating the courses at all if the category doesn't exist? Putting the courses in the top of the category tree seems a bit random to me (but changing this behaviour wouldn't make sense for stables).
            Hide
            Damyon Wiese added a comment -

            Thanks Dan,

            Integrated to 24, 25 and master.

            Created follow-up issue to improve coursecat caching and afterwards, remove this static var.

            Show
            Damyon Wiese added a comment - Thanks Dan, Integrated to 24, 25 and master. Created follow-up issue to improve coursecat caching and afterwards, remove this static var.
            Hide
            Andrew Nicols added a comment -

            Dan Poltawski, any chance you could add the test file to the issue for 24 testing?

            Cheers,

            Andrew

            Show
            Andrew Nicols added a comment - Dan Poltawski , any chance you could add the test file to the issue for 24 testing? Cheers, Andrew
            Hide
            Dan Poltawski added a comment -

            Doh, sorry.

            Show
            Dan Poltawski added a comment - Doh, sorry.
            Hide
            Andrew Nicols added a comment -

            Passing.
            Ran all unit tests, and manual tests for 2.4

            Show
            Andrew Nicols added a comment - Passing. Ran all unit tests, and manual tests for 2.4
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Yes, it's happening (somewhere in the French Polynesia, right now). And you did it, raising Moodle to new excellency levels.

            Or, if you prefer, yes, you fixed that boring issue.

            Thanks anyway! Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Yes, it's happening (somewhere in the French Polynesia, right now). And you did it, raising Moodle to new excellency levels. Or, if you prefer, yes, you fixed that boring issue. Thanks anyway! Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Agile