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

          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