Moodle

activity block should detect no course and return.

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9.4, 2.0
  • Fix Version/s: 1.9.5, 2.0
  • Component/s: Blocks
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE

Description

Somehow this block managed to get on the admin pagetype for a site I'm helping with and it caused rebuild_course_cache to get rebuilt for the entire site... 24,000 queries!

this teeny patch fixes it:

diff --git a/blocks/activity_modules/block_activity_modules.php b/blocks/activity_modules/block_activity_modules.php
index 14e9239..9f4e5f7 100644
— a/blocks/activity_modules/block_activity_modules.php
+++ b/blocks/activity_modules/block_activity_modules.php
@@ -24,6 +24,9 @@ class block_activity_modules extends block_list { $course = get_record('course', 'id', $this->instance->pageid); }

+ if (empty($course)) { + return ''; + }
require_once($CFG->dirroot.'/course/lib.php');

$modinfo = get_fast_modinfo($course);

which I will commit to head & stable if nobody stops me

Activity

Hide
Petr Škoda (skodak) added a comment -

+1 for commit

Show
Petr Škoda (skodak) added a comment - +1 for commit
Hide
Penny Leach added a comment -

committed to head & stable
thanks petr

Show
Penny Leach added a comment - committed to head & stable thanks petr
Hide
Dan Poltawski added a comment -

[Landed here from dev chat catchup]
Hmm hmm hmm, I think i've seen this same problem elsewhere, possibly when I was digging around in MDL-18264

I wonder if some sort of developer warning in rebuild_course_cache might be useful to catch out times when this sort of thing is inadvertently happening.

Show
Dan Poltawski added a comment - [Landed here from dev chat catchup] Hmm hmm hmm, I think i've seen this same problem elsewhere, possibly when I was digging around in MDL-18264 I wonder if some sort of developer warning in rebuild_course_cache might be useful to catch out times when this sort of thing is inadvertently happening.
Hide
Tim Hunt added a comment -

Also, this is not a very robust fix. If you are not on the course page, then $this->instance->pageid is essentially a random number, which may just happen to be the ID of a valid course. Shouldn't we have a check on $this->instance->pagetype?

Show
Tim Hunt added a comment - Also, this is not a very robust fix. If you are not on the course page, then $this->instance->pageid is essentially a random number, which may just happen to be the ID of a valid course. Shouldn't we have a check on $this->instance->pagetype?
Hide
Penny Leach added a comment -

Yes, probably.

In this case it was admin, where there is none, but I agree.

Show
Penny Leach added a comment - Yes, probably. In this case it was admin, where there is none, but I agree.
Hide
Penny Leach added a comment -

Actually

It seems like overkill to even have this fix in the block - blocklib should check pagetype against applicable_formats.

Pagelib kills kittens.

Show
Penny Leach added a comment - Actually It seems like overkill to even have this fix in the block - blocklib should check pagetype against applicable_formats. Pagelib kills kittens.
Hide
Penny Leach added a comment -

blocklib too

Show
Penny Leach added a comment - blocklib too

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: