General developer forum

Hi Jeff -

You are correct. The intent was to return the display code from the function. I will update the description.

For now, this is just a first step / simple step to separate the view code. The other elements you've discussed really need a broader scope around making course formats more modular and expandable. I think we should set up a page on the dev wiki for this. I will get that started.

mike

Hi Jeff and Mike,

I would just like to add a vote for a way to get the current URL of where the module is being printed. Either pass it as a parameter or ensure that the $PAGE object is set (I think the standard page object has base URL and a way to add parameters? Not entirely sure). This way modules can submit to the same page or go off, do something great and come back.

Anyhoo, looking forward to this new API

Cheers,
Mark

Hi Tim -

I'm here with him and have discussed it. Thanks.

mike

Yep, we've been talking about it and I think it's a great idea.

Is it not a good idea to keep discussion about the API here?

It's not a bug , it's of general interest, and you want it searchable as part of developer's design discussions.

I'm also wondering if we shouldn't use this opportunity to create a standard module class...

YES! plus one from me.

I had intended to go into each module and create a [module]_view function that would return the display code,

Do you mean "return php code to be evaluated"? I don't think that's a good idea -- we'll have to start coding "in strings" which is horrid.

At a very basic level, I think what I need to change is the plan.

Watching the process -- yep, I agree.

So, I now think the correct thing to do is to instead create a new function called [module]_get_content (like blocks).

The blocks approach is very problematic WRT performance.

I'd suggest using functions or a light OOP approach that follows roughly the pattern we have in the "good" pages. If right now we have

• mod/bla/foo.php -- stuff that needs to happen before we print any HTML, then prints header, foo.html, footer
• mod/bla/foo.html

We could move to having * $mod->page_foo_prepare() -- stuff that needs to happen before we print html * $mod->page_foo_display() -- returns html * mod/bla/foo.php instantiates the object, calls _prepare(), prints header, prints the output of _display(), prints footer.

A couple of notes...

This won't be trivial because the way we are doing it right now we are counting on foo.php and foo.html to be in the same (global) scope. If we do this with a light OOP approach, we can store var we want to set in _prepare() and use in _display() as an object property. This will lead to better (and safer!) code, but takes a lot fo work!

I really like the idea of moving to OOP modules (w00t!) with one important caveat: let's have the object be the 'module library', able to handle many 'module instances' without making them object instances. This is what we have now in the function-oriented code and it allows us to optimise performance a lot. A purist OOP would instantiate one object per-module-instance but it would lose the scalability we have now. We need to retain the ability to deal with many mod instances with one call that does smart SQL in the background. Otherwise we'll be in a lot of pain performance-wise...

Should the '_get_content' function handle whether or not the viewer has the right to do this, or should that be left up to whatever's calling it?

The _prepare() part should do all the footwork of figuring out rights, retrieving the data that we are going to display. _view() should not need to call has_capability(), as _prepare() will have made those calles and set appropriate obj properties tracking that.

There are exceptions though

It all maps pretty closely to what we are doing now where we have the page.php and page.html split.

Edit: And yet, as Petr says - I don't think we can change the modules API in the 1.x series... unless we can come up with a smart way to make it backwards compatible... maybe?

Further, Moodle has a lot of code to manage all of its plugins and with each new plugin, it gets larger (from what I have seen). This would hopefully be a way to create a single plugin system.

Just an afterthought, this could basically be solved by providing a way to register a directory as a plugins directory and have admin/index.php call upgrade_plugins() for all registered plugin directories.

Cheers,
Mark

This post is a bunch of my random thoughts. Sorry

I think the planned scope of this change is too small. If changing modules in 2.0 we should do it properly - which is to realise that modules are the same as blocks (they just happen to display differently - but that won't even be true after this change you're suggesting) and create a single interface for both.

I would recommend throwing away both existing interfaces, or keeping them as-is for legacy support but adding a completely new unified interface for all new development. This means all existing code can continue unmodified but core modules/blocks (the important ones) can be implemented using the new system, and obviously features that require the new system will only be available for those blodules (See what I did there?).

I think the idea of having a standard function that prints the 'module display' code is a good idea and leads in to potentially embedding modules within other modules (some teaching content followed by a quick quiz question) although that might be a long way off.

One thing to bear in mind is that the various modules need to have the capacity to create actions which 'know where they are'. For example, let's say I'm at a page using the new magic course view format that includes content from the resourcepage module (whoops, it's OU-specific, but never mind) and I click the eye icon next to an item to hide it.

• The eye is a form, which posts to a (say) visibility.php in the resourcepage folder. It probably does this using a relative link. Whoops! That won't work any more because the PHP script is coming from /course/view.php. So all these links need to be changed to absolute.
• Once you get to the form, it probably changes the object visibility in the database then redirects straight back... to resourcepage/view.php. Whoops! This won't work any more because it needs to go to course/view.php. (I trust nobody will suggest using REFERER headers - just don't go there.) There needs to be a standard Moodle way to specify the context (maybe just the part of the path following $CFG->wwwroot) as a form parameter so that we can safely return there.
  
• If there is an error then ideally the Continue button would also return to the same context. If we used a standard parameter name for context, then perhaps it would be possible for error() to do this completely automatically.

One other thing is that in order to make a significant difference, modules using this function should receive standard stuff already done for them. For example it should contain parameters $course, $cm, $context (possibly $module but not sure about that) and at least the course access permissions of the user should have already been checked [as mentioned]. However the header should not have been printed, as noted by Martin L. What's more, I know that every time I call print_header I need to use some slightly weird variety of it... If we are making a function that returns stuff then it can simply return a $object like:

$object->content
$object->header->navigation
$object->header->buttons
etc.

(And if the header's already been printed then this is helpfully ignored. As somebody else noted there would need to be a $embedded parameter which told you whether to add your own buttons inside the content.)

By the way apart from using a new function to replace view.php, one possibility is to create a module-display.php script which would be included instead. (Include it, and capture output if needed.) This is a bit more ghetto Moodley but does have some advantages - in particular if it's ever important to display output progressively. (Obviously a function return will always buffer the module's output.)

--sam

Mike - one thing I realised thinking about this is that the changes I outlined above can be made without OOP, and just on a per-page basis. They are good changes, and they'll make the subsequent change to OOP much easier. It would look like:

 $state = forum_page_bla_prepare();
 print_header();
 forum_page_bla_view($stage);
 print_footer();