Bug in Calculation of Course Progress

I'm working on extending CoursePress and I needed to calculate course progress, specifically course completion, in a wp-cron task.

Long story short, I found that when calculating for another user, "draft" units are used in the calculation when using
Student_Completion::is_course_complete( $student_id, $course_id )

I tracked the issue further down into the function get_units_from_course() in class.course.unit.php where $status is passed in, but inexplicably ignored within the function. This function calls the function get_units_with_modules() where the post_status is hard-coded to include both "publish" and "draft".

Somehow the calculations are correct (only counting published units) when calculating for the logged in user, but for the purposes of extending, the calculations should not take "draft" units into the total course progress calculation.

Hopefully this helps and can get resolved soon!

  • Dimitris

    Hey there madmen,

    hope you're doing good and thanks for reaching us!

    As far as I understand, there's something wrong in your implementation as CoursePress calculates correctly only the published units.

    Can you try to use the "get_completion()" function located in
    /wp-content/plugins/coursepress/includes/classes/class.course.completion.php#L296 instead?

    Either way, I already asked our CP devs on this and I'll let you know as soon as I've got some valuable feedback from them. :slight_smile:

    Take care,
    Dimitris

  • madmen

    Hi @dimitris_kalliris!

    It looks like the Course_Completion class is being deprecated in favor of the Student_Completion class, according to the comments in /wp-content/plugins/coursepress/includes/classes/class.course.completion.php#L18

    I don't want to dig too far into something that's going to be removed soon.. But in a little curious digging, it appears the deprecated functionality also suffered from the same calculation problem.

    The old class, Course_Completion, calculates completion = units completed / total units. Total units is inherited from the parent class function get_units() where the default status is 'any'.

    The new class, Student_Completion, calculates completion = units completed / total units. Total units is inherited from the parent class function get_units(). This time Student_Completion (function calculate_course_completion(), Line 447) passes $status = 'publish' to $course->get_units().

    So both classes get # of total units by a call to the function get_units() in class.course.php ...

    Which then calls Unit::get_units_from_course()

    Which then calls Course::get_units_with_modules()

    This last call to Course::get_units_with_modules() does not include the status because the function does not even accept status as a parameter.

    Specifically, /wp-content/plugins/coursepress/includes/classes/class.course.php#L262 has the status hardcoded to include units with status = publish and draft.

    Obviously this code is probably used elsewhere and there are larger implications than my bug, but the root of the problem appears to lie in the hardcoding of status in Course::get_units_with_modules()

  • Dimitris

    Hey there madmen,

    hope you're doing good today! :slight_smile:

    You're right on the Course_Completion class, let's leave that aside then.

    In the "get_units_with_modules()" function though, and specifically in
    /wp-content/plugins/coursepress/includes/classes/class.course.php#L353-357
    we can see the following code, which "surpasses" the
    $status = array( 'publish', 'draft' );
    part that you mentioned.

    if( ! current_user_can( 'manage_options' ) ) {
      return self::filter_units( 'publish', $units );
    } else {
      return $units;
    }

    I already pinged our CP team again to provide me with some more feedback.
    I'll keep you posted as soon as possible. Meanwhile your patience is highly appreciated! :slight_smile:

    Take care,
    Dimitris