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

Reuse get_all_sections() function in get_array_of_activities()

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Won't Fix
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3.2
    • Component/s: Course
    • Labels:

      Description

      This could potentially mean less DB queries (which is always a good thing), also promotes code reuse.

      The only problem I could see is due to get_all_sections() caching the sections where get_array_of_activities() may be expected to reload them. Testing has brought up no issues however.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            You might have to wait a bit for a review of this. At least while we are working towards 2.3 release.

            Show
            salvetore Michael de Raadt added a comment - You might have to wait a bit for a review of this. At least while we are working towards 2.3 release.
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for working on this, by the way.

            Sorry for being so curt.

            Show
            salvetore Michael de Raadt added a comment - Thanks for working on this, by the way. Sorry for being so curt.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Nice simple improvement Aaron. Putting this up for integration now.

            Show
            samhemelryk Sam Hemelryk added a comment - Nice simple improvement Aaron. Putting this up for integration now.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Aaron, got this off the long list of issues for integration..

            Show
            poltawski Dan Poltawski added a comment - Thanks Aaron, got this off the long list of issues for integration..
            Hide
            poltawski Dan Poltawski added a comment -

            Tested by added new sections, switched between section modes etc. All seemed to be working ok.

            thanks

            Show
            poltawski Dan Poltawski added a comment - Tested by added new sections, switched between section modes etc. All seemed to be working ok. thanks
            Hide
            poltawski Dan Poltawski added a comment -

            Turns out my testing wasn't good enough. I have reverted it:

            This failed phpunit tests

            Test Result (3 failures / +3)
            conditionlib_testcase.test_section_modinfo
            conditionlib_testcase.test_is_available
            conditionlib_testcase.test_section_is_available

            Show
            poltawski Dan Poltawski added a comment - Turns out my testing wasn't good enough. I have reverted it: This failed phpunit tests Test Result (3 failures / +3) conditionlib_testcase.test_section_modinfo conditionlib_testcase.test_is_available conditionlib_testcase.test_section_is_available
            Hide
            sry_not4sale Aaron Barnes added a comment -

            Abandoning this change as there appears to be a high possibility of edge cases and resetting the cache could be difficult.

            Show
            sry_not4sale Aaron Barnes added a comment - Abandoning this change as there appears to be a high possibility of edge cases and resetting the cache could be difficult.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Sep/12