New Course Formats and Blocks

New Course Formats and Blocks

by Mike Churchward -
Number of replies: 23
Picture of Core developers Picture of Plugin developers Picture of Testers
Jon - I've run into a problem with blocks and course formats. I want to create a new format, based on the weekly format. When I do this, I can't add blocks because its not one of the blocks recognized formats. To make it recognized, I have to edit 'blocklib.php', 'moodleblock.class.php' and any blocks that don't allow my format but I want to use. I think there is a better way. We should define a generic course format ('COURSE_FORMAT_ALL') that is the default. Then, any blocks that need or want to restrict themselves can still do so. We would also need to change the use of 'applicable_formats', and instead use a method like 'is_applicable_format($format)'. My proposed changes are as follows:
  1. blocklib.php:
    • Add: define ('COURSE_FORMAT_ALL', 0xFF);
    • Change 'block_method_result' as follows:
      function block_method_result($blockname, $method, $parameters=null) {
      if(($block = block_instance($blockname, null)) === false) {
      return NULL;
      }
      return ($block->$method($parameters));
      }

  2. moodleblock.class.php:
    • Add: is_applicable_format($format), as follows:
      function is_applicable_format($format) {
      // Default case: the block can be used in all course types
      return (($this->applicable_formats() == COURSE_FORMAT_ALL) || ($this->applicable_formats() == $format));
      }
    • Change: applicable_formats(), as follows:
      function applicable_formats() {
      // Default case: the block can be used in all course types
      return COURSE_FORMAT_ALL;
      }
  3. course/view.php:
    • Change the default for $course->format switch statement, as follows:
      switch($course->format) {
      case 'weeks':
      $courseformat = COURSE_FORMAT_WEEKS;
      break;
      case 'topics':
      $courseformat = COURSE_FORMAT_TOPICS;
      break;
      case 'social':
      $courseformat = COURSE_FORMAT_SOCIAL;
      break;
      default:
      $courseformat = COURSE_FORMAT_ALL;
      }
    • Change the missing_blocks code as follows:
      blocks_used($allblocks, $recblocks);
      if($editing && $recblocks) {
      foreach($recblocks as $recblock) {
      // If it's not hidden or displayed right now...
      if(!in_array($recblock->id, $allblocks) && !in_array(-($recblock->id), $allblocks)) {
      // And if it's applicable for display in this format...
      if(block_method_result($recblock->name, 'is_applicable_format', $courseformat)) {
      // Add it to the missing blocks
      $missingblocks[] = $recblock->id;
      }
      }
      }
      }

This would allow us to add new course formats without having to edit the 'blocks' code, something I'm sure you're in favour of. Thoughts?

I've also logged this as a feature request in the bug tracker.

mike

Average of ratings: Useful (1)
In reply to Mike Churchward

Re: New Course Formats and Blocks

by Mike Churchward -
Picture of Core developers Picture of Plugin developers Picture of Testers
Sorry, one error above...

The 'is_applicable_format()' should return:
return ($this->applicable_formats() & $format);
instead of what I have up there.

Do you want me to package this up and submit it?

mike
In reply to Mike Churchward

Re: New Course Formats and Blocks

by John Papaioannou -
OK, makes good sense. One thing though: I was going to comment on the is_applicable_format() thing, but you "fixed" it. wink

However, what's the point of having is_applicable_format() at all? The whole idea behind using bitwise-different define() constants was to make the check so trivial (as you demonstrate) that no "wrapper" function would be needed. I propose getting rid of it and inlining the check. Which may mean that block_method_result() needs no tweaking. If it ain't broke, don't fix it.

Other than that, looks OK to me.

Jon
In reply to John Papaioannou

Re: New Course Formats and Blocks

by Mike Churchward -
Picture of Core developers Picture of Plugin developers Picture of Testers
While this works, it still doesn't allow us the most control. New course formats don't have checks and just end up accepting all blocks by default (since they fall under the new 'COURSE_FORMAT_ALL').

Although the bitwise check is nice, in may be better to use a list of format names to check against, rather than the bitwise checks.

So instead 'applicable_formats' would return a list of valid formats. We would have something like:

$validformats = block_method_result($recblock->name, 'applicable_formats');
if ((strpos($validformats, COURSE_FORMAT_ALL) !== false) or (strpos($validformats, $course->format) !== false)) { ...

Its not as fast but much more flexible. We would need to change all of the existing 'applicable_format' methods in the existing blocks, though.

mike
In reply to Mike Churchward

Re: New Course Formats and Blocks

by John Papaioannou -
Mike, now you got me stumped.

If a block returns COURSE_FORMAT_ALL from applicable_formats(), then courses falling into this category will accept all blocks. This means you won't be able to easily restrict existing core blocks from being displayed in home-grown formats.

If COURSE_FORMAT_ALL is not returned, then we just have to give names to all course formats (both standard and home-grown) and add these to the applicable_formats() functions of all blocks we want to show is the new formats.

I don't see how you can get around both of these cases, really. It's either one or the other. And in fact I think that your code above doesn't fix the issue: any block that allows FORMAT_ALL would be accepted.

Have I misunderstood something?

In reply to John Papaioannou

Re: New Course Formats and Blocks

by Mike Churchward -
Picture of Core developers Picture of Plugin developers Picture of Testers
Sorry Jon. I don't think I explained it well.

There are two issues with creating a new course format. The first is that any new course format won't be accepted by the standard 'applicable_formats' call, because it is unknown. My first proposal dealt with this by assigning a catch-all format called COURSE_FORMAT_ALL as the default for the 'applicable_formats' method. This meant, that if a block did not specifically restrict a course format, it could be used by any course format and not just the standard ones.

The next problem is that the 'course/view.php' function assigns specific bit values to the three standard formats, but won't recognize any other course formats. I solved this by defaulting any unknown course formats to the COURSE_FORMAT_ALL code. While this allows any new course format to access any block (the desired affect), it removes the ability for a block to restrict itself if necessary. An example would be the 'social_activities' block which really is only applicable to the social format.

If we used a list of format names for the applicable course formats, and included 'course_format_all', 'course/view.php' would not have to know all of the possible formats. They would be provided by the blocks themselves as necessary. This would mean redoing the code for blocks that have already provided an 'applicable_formats' method however.

So, what I'm trying to do is allow blocks to be available to all custom course formats, and still allow blocks to be selective if necessary. Blocks that have no reason to be selective should either return 'course_format_all' or simply not implement the method.

Make sense?

mike
In reply to Mike Churchward

Re: New Course Formats and Blocks

by W Page -
Hi Mike!

Just for my own clarification as I am trying to learn php, are you suggesting this change to make it easier for new course formats to be accepted?

WP1
In reply to W Page

Re: New Course Formats and Blocks

by Mike Churchward -
Picture of Core developers Picture of Plugin developers Picture of Testers
Sort of, yes.

The problem with the current blocks implementation is that if you create a new course format that doesn't call itself one of the three standard ones, than you can't add any blocks to it. My proposal is to create a way to allow new formats to use existing blocks.

mike
In reply to Mike Churchward

Re: New Course Formats and Blocks

by W Page -
Hi Mike!

Thanks for the explanation.  I can understand this better now.  This will encourage others to develop more course formats.  I can follow this thread better now.

Back to lurking. smile

WP1
In reply to John Papaioannou

Re: New Course Formats and Blocks

by Mike Churchward -
Picture of Core developers Picture of Plugin developers Picture of Testers
Jon -

So what I propose is to do away with the bit codes, and just use the course format name string to test against.

In 'moodleblock.class.php', we would change 'applicable_formats()' to return the string 'allformats' by default:

function applicable_formats() {
// Default case: the block can be used in all course types
  return 'allformats';
}

 Any block that wishes to restrict its display to specific formats, would only need to override this method and return a list of format names.

Then, in 'course/view.php', we would do away with the case statement at the top that determines which course format is being used. We will just use the name of the format instead - that way, any and all course formats can be checked. Where the check for a valid format takes places (where 'applicable_formats' is called), change the code as follows:

$appformat = block_method_result($recblock->name, 'applicable_formats');
if($appformat == 'allformats' or strpos($appformat, $course->format) !== false) {
    // Add it to the missing blocks
    $missingblocks[] = $recblock->id;
}

This allows the block to be included if it is allowed in all formats, or if it has specifically allowed itself for whatever course format is being displayed.

Make sense?

mike
In reply to Mike Churchward

Re: New Course Formats and Blocks

by John Papaioannou -
What about

define('COURSE_FORMAT_ALL', 0xff);

and

if($appformat == COURSE_FORMAT_ALL || $appformat & $courseformat) {
// Add it to the missing blocks
}

Won't this work just as well? You could even code your blocks to do things like

function applicable_formats() {
// All course formats (current and future) except social
return COURSE_FORMAT_ALL ^ COURSE_FORMAT_SOCIAL;
}

What do you think?

In reply to John Papaioannou

Re: New Course Formats and Blocks

by Mike Churchward -
Picture of Core developers Picture of Plugin developers Picture of Testers
I like the idea - it was what I originally wanted to do (allow exclude as well as include). The one weakness is that it still doesn't include new course formats.

Because the 'course/view.php' has explicit assign statements based on known course formats (weeks, topics, social), the variable '$courseformat' won't ever be any new formats. That's why I was suggesting the string. Every course format has a name.

Ideally, we need to come up with a way of assigning a course format code to every new format. Any ideas?

mike
In reply to Mike Churchward

Re: New Course Formats and Blocks

by John Papaioannou -
Well, if someone can figure out how to create one and wants it explicitly recognized, then I don't think that expecting them to add a switch statement in course/view.php is unreasonable.

As for auto-assigning course format codes, well, that would probably mean db storage. Personally I don't think it's worth it right now because course formats are not that pluggable, like for example modules and blocks are.

One thing we could do right now, however, is just recognize a couple of "magic" format names like "extformat1" internally in the code without telling anyone... that way, you can develop your own course format under that name with minimum fuss.

PS: What's your new format like? I 'm curious! smile
In reply to John Papaioannou

Re: New Course Formats and Blocks

by Mike Churchward -
Picture of Core developers Picture of Plugin developers Picture of Testers
Well, if someone can figure out how to create one and wants it explicitly recognized, then I don't think that expecting them to add a switch statement in course/view.php is unreasonable.

Yes, but then you're stuck managing custom code. I really want to work on making formats more pluggable. I find them a great way of creating custom functionality without having to splinter the code. Let me see if I can come up with something. In the meantime, I'll reimplement what you said above.

I've done several formats, some are just variations on others.

One is a communication format that resembles the site page. 'n' number of posts to the news group are displayed in full in the main section.

Another interesting one I put together for a fellow here in Toronto (his design). It is a weekly format that hides section zero, displays a new post in its place, and has a weekly nav bar across the top to navigate to each week (image attached). You can use the nav bar to select the week (or all weeks), and it highlights accordingly. This format also used several new custom blocks, which was the reason for all this trouble... wink

mike
Attachment week_format_2.jpg
In reply to Mike Churchward

Re: New Course Formats and Blocks

by Martin Dougiamas -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
Course formats (and blocks) should be more pluggable ... I'll be happy with whatever you two guys can agree on for this.   smile
In reply to Mike Churchward

Re: New Course Formats and Blocks

by John Papaioannou -
Mike, I may have found the holy grail! big grin

I have an idea which I think will satisfy all of these in a quite clean way. Basically, it involves requiring each course format to set a variable which identifies itself, something like:
$myformatcode = COURSE_FORMAT_SOCIAL;

which of course it will define itself right before that. We 'll specify in the docs a range of valid values for these format codes (e.g. 0x0100 to 0xff00) so that externally developed formats will never conflict with built-in formats (e.g. 0x0000 to 0x00ff).

We 'll also keep everything discussed relating to COURSE_FORMAT_ALL, which means that by changing the built-in blocks code to use COURSE_FORMAT_ALL or something like COURSE_FORMAT_ALL ^ COURSE_FORMAT_SOCIAL, the builtin blocks could be used in totally unknown course formats without any problems.

If you want to restrict the displaying of builtin blocks in a way that explicitly involves custom formats you are stuck with custom code, but then you wouldn't be able to get around this in any case.

If you use two or more custom formats and these have conflicting $formatcodes, a simple one-line change [in a define()] would fix the problem. Also, in this case you are "maintaining" custom code in any case.

How does it sound? Give me a few days to actually code it and test it out!
In reply to John Papaioannou

Re: New Course Formats and Blocks

by W Page -
Hi Jon!

Would this mean that each format different than the standard 3 would be able to
  1. call itself easily in the code for display with minimal hacking or actually no code hacking at all?
  2. be able to have the ability [with minimal or no hacking]
    1. call customized blocks?
    2. call standard blocks?
  3. still allow for one of the 3 standard formats to be loaded?

BTW, Have you been able to attend any of the Olympic competitions.

WP1

    1.  
In reply to Mike Churchward

Re: New Course Formats and Blocks

by Martin Dougiamas -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
Hey Jon,

What's the problem with Mike's suggestion here?  Are you just worried about CPU cycles of comparing strings rather than bit codes?

I like the simplicity of just being able to drop in a new format and having it work without needing any special codes (like other Moodle plugins).
In reply to Martin Dougiamas

Re: New Course Formats and Blocks

by John Papaioannou -
I was thinking about a way to remove the "special codes" requirement from my solution in order to answer this, but I ended up thinking of something that makes Mike's suggestion "perfect":

function applicable_formats() { return 'topics+weeks'; }
function applicable_formats() { return 'all-social'; }

The core would do something like:

$formats = preg_split('/[+\-]/', $str, -1, PREG_SPLIT_OFFSET_CAPTURE);
foreach($formats as $format) {
if($format[1] > 0 && substr ($format[0], $format[1] - 1) {
// not allowed in $format
}
}

etc. etc.

If you think this is a good solution, you 've won me over...
In reply to John Papaioannou

Re: New Course Formats and Blocks

by Martin Dougiamas -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
Sorry, I missed this somehow earlier this week.  The preg_split stuff is a little funky but if it works I'm all for it.  Put it in.

I guess the base class will have:

function applicable_formats() { return 'all'; }

In reply to Mike Churchward

Re: New Course Formats and Blocks

by John Papaioannou -
All right!

I 've coded it and will commit it today.

Here's a summary:

  • All the COURSE_FORMAT_ constants are gone.
  • We 'll be referring to formats by name, plus the all and site "magic words".
  • applicable_formats() now should return an array with an arbitrary number of items, in the form $format => boolean. For example, the default now is return array('all' => true);
  • You can do more complex things like return array('all' => true, 'social' => false);
  • It seems that trying to descibe the formats like "all-social", which is equal to the above example, is bad since it doesn't give a computer-usable representation right away. However, if you guys think that's the way it should go, I have included function block_formats_to_array() in blocklib.php which does the conversion. Right now it doesn't get called from anywhere.
  • Updated all blocks code etc, etc, goes without saying. I 've removed applicable_formats() from some blocks which just returned everything, and let them default to the inherited version of the method which does the same thing.

Hope I didn't forget anything...

P.S.: There's a README that needs updating now, and also that block example in contrib/ (is it even still alive?). I 'll leave that for now, hope you 'll forgive me! wide eyes
Jon
In reply to John Papaioannou

Re: New Course Formats and Blocks

by Mike Churchward -
Picture of Core developers Picture of Plugin developers Picture of Testers
Sorry, I was away on vacation this week...

That sounds reasonable. I assume in '/course/view.php' you will check if the index with the same name as the current format is not 'false'?

mike
In reply to Mike Churchward

Re: New Course Formats and Blocks

by John Papaioannou -
Copy paste from the code:

if( isset($formats[$course->format]) ? $formats[$course->format] : !empty($formats['all'])) {
    // Allowed
}

// Translation: if the course format is explicitly accepted/rejected, use
// that setting. Otherwise, fallback to the 'all' format. The empty() test
// uses the trick that empty() fails if 'all' is either !isset() or false.

Jon