Moodle
  1. Moodle
  2. MDL-28690

Meta courses and $CFG->longtimenosee can cause repeated welcome messages

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.13
    • Fix Version/s: 1.9.14
    • Component/s: Course
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      1) Set the $CFG->longtimenosee setting (any small value)
      2) Create a meta course and at least one child course
      3) Enrol a test user onto one of the child courses
      4) Ensure meta course is updated by running cron to update the user's enrolment into the meta course

      5) Wait $CFG->longtimenosee days (or manually change the table mdl_user_lastaccess, so that the record for that user and the metacourse has a timeaccess more than $CFG->longtimenosee days ago)

      6) Test that the test user isn't unenrolled from the metacourse and only from the child course.

      Show
      1) Set the $CFG->longtimenosee setting (any small value) 2) Create a meta course and at least one child course 3) Enrol a test user onto one of the child courses 4) Ensure meta course is updated by running cron to update the user's enrolment into the meta course 5) Wait $CFG->longtimenosee days (or manually change the table mdl_user_lastaccess, so that the record for that user and the metacourse has a timeaccess more than $CFG->longtimenosee days ago) 6) Test that the test user isn't unenrolled from the metacourse and only from the child course.
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      18350

      Description

      Replication instructions:

      1. Set the $CFG->longtimenosee setting (any value)
      2. Create a meta course and at least one child course
      3. Add a 'welcome' message to the meta course
      4. Enrol a user onto one of the child courses
      5. Run cron to update the user's enrolment into the meta course
      6. Wait $CFG->longtimenosee days (or manually change the table mdl_user_lastaccess, so that the record for that user and the metacourse has a timeaccess more than $CFG->longtimenosee days ago)

      Expected result: nothing happens, as the user is still enroled in the child course, so should stay enroled in the meta course as well.

      Actual result: every time admin/cron.php runs, there is a 20% chance the user will be unenroled from the meta course (as they have not visited it recently enough), they will then immediately be enroled back into it again (as they are still enroled in the child course), at which point they will receive a welcome message. The user will keep receiving 'welcome' emails multiple times an hour until they visit the meta course and reset the 'timeaccess' value.

      Proposed solution:
      In cron.php - check to see if the course is a metacourse before unenroling after $CFG->longtimenosee (the user will already be automatially unenroled, once they have been unenroled from the child courses, so we should not have to worry that they will never leave the meta course)

      $rs = get_recordset_sql ("SELECT id, userid, courseid
                                  FROM {$CFG->prefix}user_lastaccess
                                 WHERE courseid != ".SITEID."
                                   AND timeaccess < $cuttime ");
      

      Should become:

      $rs = get_recordset_sql ("SELECT la.id, la.userid, la.courseid
                                  FROM {$CFG->prefix}user_lastaccess la
                                  JOIN {$CFG->prefix}course c
                                    ON c.id = la.courseid
                                 WHERE la.courseid != ".SITEID."
                                   AND la.timeaccess < $cuttime 
                                   AND c.metacourse = 0");
      

      An alternative solution might be to update the 'timeaccess' field in the metacourse when the child course is visited.

      I have not fully investigated, but I suspect that M2.0 may have the same problem.

        Activity

        Hide
        Michael de Raadt added a comment -

        Hi, Davo. Good to hear from you.

        Thanks for reporting this and providing a solution.

        Show
        Michael de Raadt added a comment - Hi, Davo. Good to hear from you. Thanks for reporting this and providing a solution.
        Hide
        Aparup Banerjee added a comment -

        Hi Davo,
        Thanks for the report!

        The alternative might introduce a few more queries for a statistic that really can be easily mined.

        I'll put up patches with your query fix. (if you're not up to putting one git repo up yourself that is)

        cheers,
        Aparup

        Show
        Aparup Banerjee added a comment - Hi Davo, Thanks for the report! The alternative might introduce a few more queries for a statistic that really can be easily mined. I'll put up patches with your query fix. (if you're not up to putting one git repo up yourself that is) cheers, Aparup
        Hide
        Davo Smith added a comment -

        I can put together a git repo, if you prefer, but not until later this evening.

        Show
        Davo Smith added a comment - I can put together a git repo, if you prefer, but not until later this evening.
        Hide
        Aparup Banerjee added a comment -

        That'll be great Davo. Its just that the authorship information recorded would be more accurate, especially since we can do this now that we're on git.

        When you have a branch with that commit, i can pick (cherry-pick) your commits across to other needy branches.

        Show
        Aparup Banerjee added a comment - That'll be great Davo. Its just that the authorship information recorded would be more accurate, especially since we can do this now that we're on git. When you have a branch with that commit, i can pick (cherry-pick) your commits across to other needy branches.
        Hide
        Davo Smith added a comment -

        Added patch to Github repository

        Show
        Davo Smith added a comment - Added patch to Github repository
        Hide
        Aparup Banerjee added a comment -

        Thanks Davo!

        just checked and the fix isn't needed for 2.x as that bit was rewritten.

        putting this up for integration.

        Integrators: Davo's fix is also available rebased @
        git://github.com/nebgor/moodle.git
        https://github.com/nebgor/moodle/compare/MOODLE_19_STABLE...MDL-28690_m19

        Show
        Aparup Banerjee added a comment - Thanks Davo! just checked and the fix isn't needed for 2.x as that bit was rewritten. putting this up for integration. Integrators: Davo's fix is also available rebased @ git://github.com/nebgor/moodle.git https://github.com/nebgor/moodle/compare/MOODLE_19_STABLE...MDL-28690_m19
        Hide
        Eloy Lafuente (stronk7) added a comment -

        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

        Show
        Eloy Lafuente (stronk7) added a comment - 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
        Hide
        Aparup Banerjee added a comment -

        thanks, rebased my repo.

        Show
        Aparup Banerjee added a comment - thanks, rebased my repo.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        I've added one more commit adding the "la" prefix to all the fields. Unnecessary right now but better to avoid potential future collisions.

        Show
        Eloy Lafuente (stronk7) added a comment - I've added one more commit adding the "la" prefix to all the fields. Unnecessary right now but better to avoid potential future collisions.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
        Hide
        Rajesh Taneja added a comment -

        Works Great.
        Thanks for fixing this Davo and Aparup

        Show
        Rajesh Taneja added a comment - Works Great. Thanks for fixing this Davo and Aparup
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

        Show
        Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: