Difference between revisions of "Module visibility and display"
|Line 96:||Line 96:|
This class has three states:
This class has three states:
* BASIC_DATA: class has the data from database,
* BASIC_DATA: class has the data from database , not the .
* DYNAMIC_DATA: class has all data that is available for most pages.
* DYNAMIC_DATA: class has all data that is available for most pages.
* VIEW_DATA: class also has the data that is needed only on the view page.
* VIEW_DATA: class also has the data that is needed only on the view page
=== Existing properties ===
=== Existing properties ===
Revision as of 13:26, 22 December 2010
This document is only a proposal. The functionality described doesn't exist yet in Moodle.
- 1 DRAFT 2
- 2 Summary
- 3 API improvement
- 4 Removing existing hacks
- 5 get_fast_modinfo change
- 6 course_modinfo
- 7 cm_info class
- 8 Modify _get_coursemodule_info
- 9 Extend dynamic calculation
- 10 New module API
- 11 Access permissions
This is the second draft of the proposal, with changes based on Petr's comments. As well as/because of adding all the class stuff, this version no longer requires rebuild_course_cache and should be 100% backward compatible with all existing uses of modinfo.
- Petr wanted dynamic/lazy computation of some of the access stuff - this is necessary for the forum unread data etc, but not feasible to do in this version for any of the data that is already included in modinfo, because of the requirement for 100% BC which is not possible with __get methods; so all the existing property values must be initialised (see below). This also means it's a less scary change, i.e. more moving code around and less new code.
- I wanted a class for the overall thing, which I called course_modinfo (because it's basically from the modinfo field of the course table). Then I neeed a name for the class for an individual module instance info. Petr suggested module_info but I thought cm_info was a bit closer (module_info might sound like it was about a whole module, not one instance). Anyhow, both names subject to change if anyone has better suggestions.
- Linking the cm_info to its parent course_modinfo gives a nice framework to do caching/lazy-initialisation stuff. For instance when we calculate forum unread data in the mod_forum_cm_info_view function, we will get a request for a single $cminfo, but can actually retrieve the data for all forum instances on the course at once (same as at present).
- I didn't specify exactly how to address modinfo caching issues (...if there are any) but because this proposal suggests moving the 'work out the values' stuff into the class, and leaving get_fast_modinfo to deal only with caching, it should be a whole lot simpler to see how the caching works and alter it in future.
We would like to create a generic API that allows the following:
- Module display on front page can be customised, for example making it possible to create another module that behaves like Label (displaying arbitrary html rather than a link to activity view.php) - currently this is hardcoded hack for Label. This change should apply to other areas such as navigation block as well as course page.
- Some modules can provide dynamic text, such as forum displaying unread messages. At present this is hardcoded so that only forum can do it.
- Modules can be hidden completely, or greyed out, from the view of particular students according to either custom module behaviour, or (if not specified by module) default behaviour regarding a new capability moodle/course:viewactivity and the existing option for whether a non-available module is greyed out or hidden entirely.
This should take advantage of the existing modinfo cache in order that performance is not adversely affected.
Following Petr's suggestions, we would also like to improve the API by switching the modinfo in-memory data structures to use defined classes instead of anonymous stdClass objects. This achieves the following:
- Provides a location for documentation of these structures. (Currently they are undocumented.)
- In future, allows functions to specify the required type (i.e. some functions require the information from modinfo, not just a row from the table; they will now be able to specify this). There is no plan to change function definitions immediately.
- Where the type is defined, makes metadata available to IDEs, allowing code completion and automatic documentation viewing.
The following requirements should also be met:
- 100% backward compatibility for existing use of these structures by core and third-party modules. (Obviously, where these want to support the new features, such as the navigation supporting other label-like modules, there do need to be changes which are included in these patch. But if we forget something or it's third-party, it should keep working as at present.)
- 100% backward compatibility for existing modinfo data (in database).
Removing existing hacks
- Existing hacks regarding label in all areas of code (e.g. navigation, etc) will be changed from the logic 'is this a label?' to the logic 'does this activity have a view page?' (which will work for label too)
- Existing hacks regarding forum unread data will be removed and the forum unread code will be moved into the new API function. The code will be written in such a way as to have the same performance characteristics.
The get_fast_modinfo function will be changed to return a new object of type course_modinfo, which will be compatible with the existing $modinfo return value.
While constructing this object, in addition to current behaviour, the system will:
- support new values defined in _get_coursemodule_info
- extend dynamic per-user calculation (that checks is something is visible to current user, ->uservisible) with additional checks
- call the new module API (if provided) after calculating modinfo, to get dynamic information
The new class course_modinfo will contain properties:
- courseid, userid (ints)
- sections (array of int => array of int)
- cms (array of int => cm_info)
- instances (array of string => array of int => cm_info)
- groups - at present I can't tell what this is for as it seems to be empty for me even though there are groups on the course and I'm a member of one of them - hmmm - anyhow I will figure it out and implement it
These are identical to the values currently returned by modinfo, except for the use of cm_info class instead of bare stdClass for the information about modules.
There are three options for handling this class:
- Make these properties public so that they can be accessed exactly as at present.
- Make them private and use PHP magic __set and __get functions so that they can be read as normal but any attempt to write them would (while working) also cause a developer debug warning.
- Make the properties public, but deprecate them, and create separate get methods (get_courseid, get_userid, etc) which are recommended for future use.
I initially favoured the second option but unfortunately, PHP being PHP, this doesn't quite work properly: specifically, you can't use the empty() function on such values. Since there might be places in the code that call empty(), maybe this isn't a good idea.
Consequently I tend to the third one; we should make better get methods such as get_cm($cmid) as well as just get_cms(), which may help make new code more readable.
The code that creates this information should largely be moved to this class (either in a constructor or in init methods etc) from its current location in get_fast_modinfo. get_fast_modinfo should remain responsible only for caching these objects.
The new class cm_info will be constructed with a $parent (course_modinfo). Knowing its parent allows the class to carry out operations with regard to the whole course, where this is beneficial for performance, without needing extra parameters. There will be a get_modinfo() method to return this.
This class has three states:
- BASIC_DATA: class has the data from database modinfo table, and automatically completed data that is not related to the current user.
- DYNAMIC_DATA: class has all data that is available for most pages, including data related to the current request from the current user's permissions and the _cm_info_dynamic function if applicable.
- VIEW_DATA: class also has the data that is needed only on the course view page (or similar pages) from the _cm_info_view function if applicable (basically only used for forum unread data and stuff like that).
When get_fast_modinfo returns, all returned objects will be in DYNAMIC_DATA state. This will be sufficient for determining whether the current user has access to the activity and for displaying the basic link etc. Basically the only thing it doesn't include is supplemental data that is only needed on the course page, such as forum unread data (get_after_link function). This data takes time to calculate so is generated on request.
The intention is that getting to DYNAMIC_DATA state shouldn't require any database calls (certainly for the core modules). Getting to VIEW_DATA state may require database calls, e.g. to calculate the number of unread forum posts.
These properties are in current modinfo:
- id, instance, course, idnumber, visible, groupmode, groupingid, groupmembersonly, indent, completion, availablefrom, availableuntil, showavailability - data from course_modules row
- extra, icon, iconcomponent, modname, name, sectionnum, conditionscompletion, conditionsgrade, modplural (wtf is this there when the singular name isn't and both should be available with get_string?!) - data from modinfo which was computed when generating the cache
- availableinfo, available, uservisible - data relating to the current user which is computed dynamically when obtaining the modinfo object
This is the list of properties currently available in modinfo objects, so will be implemented as-is. The same question regarding use of __set, __get applies as above, but in this case it's perhaps more likely that people might call empty() on the existing properties; again, I propose retaining the above as public properties, but deprecated, and providing get_x methods for new use. Any new properties would be private and available only with get_ methods.
Some of the get methods might have a slightly different definition to the raw properties. For example get_icon should use the provided data to return a moodle_icon object, not a string.
New data / get methods
The new data and corresponding get methods are:
- has_view() - true if the module should have a link to its view.php shown in navigation, be included in lists of 'did the user visit this module' stats, etc. (Basically this is the 'is not a label' method.)
- get_url() - returns moodle_url to module view.php, or null if has_view is false (this reduces code duplication in various places)
- get_content() # - returns HTML content to be displayed on the course page where this module is placed; appears below the link, if present (this is how Label displays content on course page)
- get_extra_classes() # - returns additional CSS classes to be added to the A or DIV tag(s) for this item on main course page.
- get_custom_data() - returns an optional mixed value containing custom data for this module, which needs to be available course-wide via the get_fast_modinfo function.
- get_after_link() # - returns HTML code which displays after the link.
- get_after_edit_icons() # - returns HTML code which displays after the standard icons (hide, edit, delete, etc) when editing.
Functions marked # require 'view information'. This is additional information intended for use on the course view page and not in other places that use modinfo: the most obvious example (and the only one that affects core) is forum unread data. Consequently it is only obtained on request (see state information above). Obtaining view info requires calling the module's _cm_info_view function if it has one.
Set methods are available for use only by _cm_info_dynamic and _cm_info_view functions; see below.
There will be a change to the existing module API function _get_coursemodule_info which is used to update information before storing it in the database modinfo field. This change will retain backward compatibility.
The function returns an object which is currently a stdClass object. This possibility will be retained for backward compatibility, with unchanged behaviour. All existing fields are supported:
- name: name of instance or (for labels only) content of instance
- icon: name of icon or special weird string
- iconcomponent: component of icon, possibly works
- extra: extra data inserted somewhere horrible in the html
These fields are all optional and may be left unset to accept the defaults.
However a new class cached_cm_info can be returned instead.
The class cached_cm_info has the following properties:
- name, icon, iconcomponent: as before
- content: HTML content to be displayed on the course page where this module is placed; appears below the link, if present (this is how a Label-like module can display content on course page)
- customdata: A place to store a string or object containing custom data for this module, which needs to be available course-wide via the get_fast_modinfo function. If present, this data should be small in size.
- extraclasses: Extra CSS classes to add to the item on course page display, if required.
(It does not support extra, which is deprecated on account of being stupid, at least unless I figure out some existing use case that really needs to be carried forward into a non-deprecated system.)
Returning a stdClass object should result in identical content of the modinfo field in the database to present.
Return cached_cm_info results in similar content but with extra fields ->content and ->customdata. These fields are only stored if they are non-null, i.e. it won't bulk up the database with empty 'content=nothing' type data against every module.
Reading label data
The new content field is different to existing behaviour of the Label module, which stores its content in the 'name' field as noted. When reading this data or processing it (for example in cm_info class), the system takes the following approach:
- if the modname is 'label' and there is no 'content' field, then copy the 'name' field into 'content'.
Note that this behaviour retains backward compatibility; the label still has a silly 'name' value. For new code, has_view() returns false so it won't use name; for old code, the existing hard-coded exceptions for label will continue to work.
Extend dynamic calculation
Currently the modinfo code makes the following checks that apply dynamically per-request (and do not directly come from the cache) in order to create the ->uservisible member variable.
- If ->groupmembersonly is set, checks if the user belongs to group or has accessallgroups.
- If availability restrictions (date, grade, completion) are set, checks these (also even if available to current user, stores information into ->availableinfo, ->available for information when editing).
My proposal is:
- Make this part of the code (that 'specialises' a single mod value for the current user/request) into a separate function within cm_info class, just to simplify it. This function should be the one that changes the cm_info state from BASIC_DATA to DYNAMIC_DATA.
- Add a check for the moodle/course:viewactivity capability; if user doesn't have this capability, set ->uservisible to false. Also check the option about what to do with hidden activities; if this is set to the default 'grey it out', then set ->inactive to true.
- Note: The default value for moodle/course:viewactivity should be true for all roles, even guest. This maintains existing behaviour. Sites that don't want guests to view activities can change the main role definition for guest.
- Call the _cm_info_dynamic function (below) if the current module supplies one.
PERFORMANCE CONCERNS: Minimal. No new database queries are required, just a capability check and a function existence check.
New module API
Two new module API functions will be defined. They both have one parameter: the cm_info object for this module, containing all the data from the modinfo cache.
The functions are:
- _cm_info_dynamic - add basic data that is always required (must be fast)
- _cm_info_view - add data that is required for the course view page (can be slower)
The functions both call set methods in the cm_info object. Examples:
- set_user_visible(false) - hide activity entirely
- set_available(false) - grey out activity while it remains visible
- set_available_info('not available because you suck') - add explanation for why it's not available
- set_after_link('16 unread') # - add html code to appear after link
- set_edit_icons($html) # - add some code to appear when editing
- set_has_view(false) - make module not have link, navigation, etc even if it has a view.php
- set_content($html) # - add/change code to appear below the link
- set_name($text) - change text of the link
- set_extra_classes('whatever classes') # - add CSS classes
The bold functions set cm_info data that cannot be set anywhere else (such as by system defaults or modinfo classes). This is basically because there didn't seem any general need to cache this data, particularly bearing in mind that modules can cache anything they like using the customdata property of cached_cm_info.
_cm_info_view is restricted: for example, you cannot change user_visible status in view (because then it wouldn't work when checking access etc on other pages). You can only change the 'view' properties. This can be enforced by making the set methods throw exceptions if called when the cm_info is in its already-defined state.
Should it be necessary, this function can access the existing information in the cm_info object and also in the parent course_modinfo object (such as the user id) by calling get_modinfo on the cm_info.
PERFORMANCE CONCERNS: None. Of standard modules, only the forum will implement this and only in the 'view' version used just on course page; it will use the same code as at present. Custom modules will need to be written carefully in a similar manner so that they also perform well (ie do any queries once for whole course and store in global cache, not once per module).
When calling require_login with a $cm parameter, this will do get_fast_modinfo and check the is_user_visible() method in order to process per-user access restrictions:
PERFORMANCE CONCERNS: Because require_login like this is used on all module pages, we should use a profiler to evaluate before/after performance of this change and ensure that it isn't worsened. I suspect it won't be, see below.
NOTE: The way modules currently initialise is a bit inefficient, especially if get_fast_modinfo is to be used (and I suspect it already is on most pages, given the navigation block probably uses it). The current sequence on most module pages is something like:
- get course module (from id, supplied)
- get course
- get instance
we could change this to
- get course (from simple join using cmid, supplied)
- use get_fast_modinfo to get cm
- get instance
Not suggesting that as part of this change (and it probably shouldn't be in this document), just saying...