Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-65067

format_weeks detection wrongly relies on get_section_dates method presence

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      Note for the tester: This patch is really trivial. We still want to provide a way to test it. These testing instructions do not work for 3.6 so we can consider this test passed if it passed in master or 37

      Prerequisites

      1. Install https://moodle.org/plugins/format_topcoll
      2. Go to site admin > location > location settings. Ensure that the timezone value matches the timezone in your system
      3. Log out and log in again
      4. Create a course (the list below is sorted)
        1. with topcoll format
        2. enrol 1 user as student
        3. Go to Grades > Setup and select "Edit > Edit settings" for the grading category whose name equals the course short name. Click on "Show more" under "Category total" and set "Grade to pass" to "50.00". Save changes.
        4. Create a forum activity on section 1 (not on the top section with no week dates)
        5. Set the course start date to today at 00:00 and the end date at this exact minute.
      5. Create a course (the list below is sorted)
        1. with weeks format (with "Calculate the end date from the number of sections")
        2. enrol 1 user as student

      Test 1

      1. Execute the following CLI command

        php admin/tool/analytics/cli/guess_course_start_and_end.php --guessall --update
        

      2. You SHOULD see no errors nor warnings and you SHOULD see "Success" at the bottom

      Test 2

      1. Go to site admin > analytics and disable "analytics | onlycli" setting
      2. Go to site admin > analytics > analytics models
      3. Click on "New > Create model"
        • Select "Students at risk of not achieving the minimum grade to pass the course"
        • Select "Forum cognitive" and "Forum social" as indicators
        • Select "Single range" as time-splitting method
        • Save changes
      4. Click on "Evaluate" for the model "Students at risk of not achieving the minimum grade to pass the course" and select "Current time-splitting method" as the time-splitting method used during evaluation.
      5. You SHOULD see no PHP exceptions
      6. You SHOULD NOT see a "There is no data to evaluate the model"
      Show
      Note for the tester: This patch is really trivial. We still want to provide a way to test it. These testing instructions do not work for 3.6 so we can consider this test passed if it passed in master or 37 Prerequisites Install https://moodle.org/plugins/format_topcoll Go to site admin > location > location settings. Ensure that the timezone value matches the timezone in your system Log out and log in again Create a course (the list below is sorted) with topcoll format enrol 1 user as student Go to Grades > Setup and select "Edit > Edit settings" for the grading category whose name equals the course short name. Click on "Show more" under "Category total" and set "Grade to pass" to "50.00". Save changes. Create a forum activity on section 1 (not on the top section with no week dates) Set the course start date to today at 00:00 and the end date at this exact minute. Create a course (the list below is sorted) with weeks format (with "Calculate the end date from the number of sections") enrol 1 user as student Test 1 Execute the following CLI command php admin/tool/analytics/cli/guess_course_start_and_end.php --guessall --update You SHOULD see no errors nor warnings and you SHOULD see "Success" at the bottom Test 2 Go to site admin > analytics and disable "analytics | onlycli" setting Go to site admin > analytics > analytics models Click on "New > Create model" Select "Students at risk of not achieving the minimum grade to pass the course" Select "Forum cognitive" and "Forum social" as indicators Select "Single range" as time-splitting method Save changes Click on "Evaluate" for the model "Students at risk of not achieving the minimum grade to pass the course" and select "Current time-splitting method" as the time-splitting method used during evaluation. You SHOULD see no PHP exceptions You SHOULD NOT see a "There is no data to evaluate the model"
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_35_STABLE, MOODLE_36_STABLE, MOODLE_37_STABLE
    • Fixed Branches:
      MOODLE_36_STABLE, MOODLE_37_STABLE
    • Pull from Repository:
    • Pull 3.6 Branch:
    • Pull Master Branch:
      MDL-65067_master

      Description

      In /analytics/classes/local/community_of_inquiry_activity.php, at line 836; the format_weeks is detected using the mere existance of the 'get_section_dates' method:

              // When the course is using format weeks we use the week's end date.
              $format = course_get_format($activity->get_modinfo()->get_course());
              // We should change this in MDL-60702.
              if (method_exists($format, 'get_section_dates')) {
                  $dates = $format->get_section_dates($section);
      

      When format_topcoll is also installed, the get_section_dates($section) call will fail because format_topcoll's get_section_dates() takes three arguments:

          public function get_section_dates($section, $course, $tcsettings) {
      

      So what happened here is that:

      • format_base does not declare the get_section_dates() method; it's therefore OK for both format_weeks and format_topcoll to declare it, with any public interface as they see fit. (Point is, they take different inputs and output differently too).
      • the community_of_inquiry_activity class tries to detect format_weeks' presence by checking if the method exist. When the method exist, it will call it, with only one argument; and this breaks when the format implements it with a different API (such as format_topcoll).

      What can be done?

      • the detection of format_weeks in the community_of_inquiry_activity class should be changed to also check that the format is the expected one.
      • format_base could declare a new non-abstract get_section_dates() method.

      A symptom of that problem was that any run of the tool_analytics\task\train_models scheduled task threw a coding error:

      A lock was created but not released at:
      [dirroot]/analytics/classes/dataset_manager.php on line 133
       
      Code should look like:
        $factory = \core\lock\lock_config::get_lock_factory('type');
        $lock = $factory->get_lock(503195);
        $lock->release(); // Locks must ALWAYS be released like this.
      

      The lock was not released because the function call just exploded.

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                4 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  8/Jul/19

                  Time Tracking

                  Estimated:
                  Original Estimate - 0 minutes
                  0m
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 7 hours
                  7h