Moodle

$cm from get_fast_modinfo should include all fields

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.0.2
  • Fix Version/s: 2.0.3
  • Component/s: Course
  • Labels:
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

There are a few variables which are included in $cm if you get it using get_record from the course_modules table (& similar approaches), but are not included if you get it via get_fast_modinfo.

These are:

  • Three completion settings (this causes problems if you try to call some completion-related functions using a $cm from get_fast_modinfo)
  • $cm->section (section id; it does have section number) - this used to cause a problem in nav block but it was fixed a different way because not really necessary there, however there might be other areas of code where this would cause a problem if we called specific functions with a cm_info
  • $cm->added (date added)
  • $cm->score (value appears to be unused)
  • $cm->module (module id)
  • $cm->visibleold (previous value of visibility for individual item when section is hidden)

In order to allow third-party code (and potentially core code) to reliably use get_fast_modinfo to create $cm objects, I would like to add support for passing through these parameters.

In addition I would like to change the existing logic (it's in the same code) around rebuilding the course cache if user changes the $CFG->enableavailability. IMO it is better to do this when changing the admin setting. I added a new option to admin setting base class so that you can set if a particular admin setting affects modinfo and therefore requires this rebuild.

Note that these changes unfortunately slightly increase the size of modinfo. Most of these values default to 0/empty so will not take any space but section, module, added, and visibleold will usually be set, adding 4 data items to each modinfo entry.

  1. File
    temp.php
    12/Mar/11 1:46 AM
    0.3 kB
    Sam Marshall

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Sam Marshall added a comment - - edited

I have done code for this and tested it using the following script:

$testcm = $DB->get_record('course_modules', array('id'=>1));
$comparecm = $modinfo->get_cm(1);
foreach ($testcm as $property=>$value) {
    print_object("$property = $value / " . $comparecm->{$property});
}

(Also see temp.php for more complete version of same script)

Some notes about this implementation:

  • The values that were not present before will default to zero/empty until after course cache is rebuilt. Consequently it is not advisable for core code to rely on these values soon.
  • For this reason perhaps it might be best to include an upgrade script with rebuild_course_cache in (note: rebuilding course cache is a cheap operation, assuming you use the 'eh just build it next time somebody asks' parameter).
  • If it isn't appropriate to make that change in 2.0.3 then 2.1 maybe?
  • I documented and included the 'score' field, but this doesn't seem to do anything, so I marked it as deprecated.

By the way, the immediate reason I coded this is that it is causing a bug in ForumNG on our 2.0 dev system and I don't really want to fix it by adding in a get_record (which is obviously the easy fix but it sucks). It occurred to me that having just a few missing parts of $cm that are not in cm_info is unhelpful in general and likely to lead to other bugs.

Show
Sam Marshall added a comment - - edited I have done code for this and tested it using the following script: $testcm = $DB->get_record('course_modules', array('id'=>1)); $comparecm = $modinfo->get_cm(1); foreach ($testcm as $property=>$value) { print_object( "$property = $value / " . $comparecm->{$property}); } (Also see temp.php for more complete version of same script) Some notes about this implementation: The values that were not present before will default to zero/empty until after course cache is rebuilt. Consequently it is not advisable for core code to rely on these values soon. For this reason perhaps it might be best to include an upgrade script with rebuild_course_cache in (note: rebuilding course cache is a cheap operation, assuming you use the 'eh just build it next time somebody asks' parameter). If it isn't appropriate to make that change in 2.0.3 then 2.1 maybe? I documented and included the 'score' field, but this doesn't seem to do anything, so I marked it as deprecated. By the way, the immediate reason I coded this is that it is causing a bug in ForumNG on our 2.0 dev system and I don't really want to fix it by adding in a get_record (which is obviously the easy fix but it sucks). It occurred to me that having just a few missing parts of $cm that are not in cm_info is unhelpful in general and likely to lead to other bugs.
Hide
Permalink
Sam Marshall added a comment -

BASIC TEST STEPS
----------------

1. Look at a course page. Verify that it appears correct and no warnings display even though debug is set to developer level.

2. Edit any activity on the course and save changes (to cause course cache to be rebuilt). Verify that course still appears correct and no warnings display.

SPECIAL BONUS TEST STEPS
------------------------

(these ones actually test the new feature rather than just 'um it didn't break everything completely')

1. Temporarily add attached file temp.php into the Moodle root directory.

2. Pick a course-module id, on the course used before, e.g. 1234.

3. In your browser load temp.php?id=1234

4. Verify that the value on the left of the / and the value on the right of the / is the same for all values.

5. Make sure completion and availability are turned on. Edit your activity and set those options, especially including the automatic completion 'require view' and 'require grade' options, expected completion date.

6. Reload the temp.php script. Verify that the values on left of / and right of / are still identical including for the completion values that you just changed.

Show
Sam Marshall added a comment - BASIC TEST STEPS ---------------- 1. Look at a course page. Verify that it appears correct and no warnings display even though debug is set to developer level. 2. Edit any activity on the course and save changes (to cause course cache to be rebuilt). Verify that course still appears correct and no warnings display. SPECIAL BONUS TEST STEPS ------------------------ (these ones actually test the new feature rather than just 'um it didn't break everything completely') 1. Temporarily add attached file temp.php into the Moodle root directory. 2. Pick a course-module id, on the course used before, e.g. 1234. 3. In your browser load temp.php?id=1234 4. Verify that the value on the left of the / and the value on the right of the / is the same for all values. 5. Make sure completion and availability are turned on. Edit your activity and set those options, especially including the automatic completion 'require view' and 'require grade' options, expected completion date. 6. Reload the temp.php script. Verify that the values on left of / and right of / are still identical including for the completion values that you just changed.
Hide
Permalink
Helen Foster added a comment -

Thanks Sam!

Show
Helen Foster added a comment - Thanks Sam!

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: