Hi Aaron,
This looks good.
+1 for integration.
It should be back-ported to stables as well.
Thanks

Hi Aaron,

Looks good but could we:
a) Have some testing instructions for this to check for regression
b) Perhaps use the oportunity to move that embeded variable to a placeholder

thanks,
dan

Dan,

Could you change the status of this back to development in progress?

Thanks,
Aaorn

Reopened (back to development) as asked

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Thanks Aaron, changes look spot on and have been integrated now. Thanks!

Tested in master and 22, marking as failed because the expected behavior explained in the testing instruction is not archieved, but I'm not sure since there are no aggregations repeated.

Tested with 3 courses with course completion tracking, 2 with self completion enabled and the other one using these first 2 courses as prerequisistes

• Results in master
  • First cron execution

    Aggregating completions
    Aggregating completions for user 7 in course 2
    Aggregating completions for user 8 in course 2
    Aggregating completions for user 9 in course 2
    Aggregating completions for user 10 in course 2
    Aggregating completions for user 7 in course 3
    Aggregating completions for user 8 in course 3

  • Second cron execution

    Aggregating completions
    Aggregating completions for user 9 in course 3
    Aggregating completions for user 7 in course 4
    Aggregating completions for user 8 in course 4
    Aggregating completions for user 9 in course 4
    Aggregating completions for user 10 in course 4

  • Third cron execution

    Aggregating completions

• Results in MOODLE_22_STABLE
  • First cron execution

    Aggregating completions

  • Second cron execution

    Aggregating completions
    Aggregating completions for user 7 in course 2
    Aggregating completions for user 8 in course 2
    Aggregating completions for user 9 in course 2
    Aggregating completions for user 10 in course 2
    Aggregating completions for user 7 in course 3
    Aggregating completions for user 8 in course 3
    Aggregating completions for user 9 in course 3
    Aggregating completions for user 10 in course 3
    Aggregating completions for user 7 in course 4
    Aggregating completions for user 8 in course 4
    Aggregating completions for user 9 in course 4

  • Third cron exeuction

    Aggregating completions

Hi David,

I've just been looking at your output now.

Just to confirm you had the following set up when testing:

In both master and 22:

• Course.id=2 and course.id=3 each had self completion enabled
• course.id=4 had id=2 and id=3 as prerequisites

In master you had the following enrolments:

• course.id=2 -> Users 7,8,9,10
• course.id=3 -> Users 7,8,9
• course.id=4 -> Users 7,8,9,10

In 22 you had the following enrolments:

• course.id=2 -> Users 7,8,9,10
• course.id=3 -> Users 7,8,9,10
• course.id=4 -> Users 7,8,9

Assuming that is correct then the output looks correct.
The only thing I can't explain is why the aggregation process in master was split between the two cron runs.
Again assuming the setup I described above is accurate I don't think it is a bug caused by this issue and that this issue could be passed, however perhaps it is worth creating an issue to explore that split even if someone more knowledgeable in completion code comes along and tells us its normal/expected.

Cheers
Sam

Up for testing again, thanks David.

It passes. Same results in master with the same scenario, attaching Sam chat comments:

The completion record is being created by cron, would happen if you enabled that setting, but you would have also had more output with lines like "Marked started user '.$prev->userid.' in course '.$prev->course"

There are these lines.

• First cron execution

  Marking users as started
  Marked started user 7 in course 3
  Marked started user 8 in course 3
  Marked started user 9 in course 3
  Marked started user 7 in course 4
  Marked started user 8 in course 4
  Marked started user 9 in course 4
  Marked started user 7 in course 5
  Marked started user 8 in course 5
  Marked started user 9 in course 5
  Running completion_criteria_date->cron()
  Running completion_criteria_activity->cron()
  Running completion_criteria_duration->cron()
  Running completion_criteria_grade->cron()
  Running completion_criteria_course->cron()
  Aggregating completions
  Aggregating completions for user 7 in course 3
  Aggregating completions for user 8 in course 3
  Aggregating completions for user 9 in course 3
  Aggregating completions for user 7 in course 4
  Aggregating completions for user 8 in course 4
  Aggregating completions for user 9 in course 4
  Aggregating completions for user 7 in course 5

• Second cron execution

  Marking users as started
  Running completion_criteria_date->cron()
  Running completion_criteria_activity->cron()
  Running completion_criteria_duration->cron()
  Running completion_criteria_grade->cron()
  Running completion_criteria_course->cron()
  Aggregating completions
  Aggregating completions for user 8 in course 5
  Aggregating completions for user 9 in course 5

Hi guys,

The split in the first results causing the aggregation to occur over two cron runs is a known and logged issue.

Thanks for your help!
Aaron

*Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

Congratulations

!

You've made into Moodle

+

I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

cheers!