Moodle

enrol/database/enrol_database_sync.php unenrolls everyone!

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 1.7.4, 1.8.5, 1.9
  • Fix Version/s: 1.8.6, 1.9.1
  • Component/s: Enrolments
  • Labels:
    None
  • Database:
    Any
  • Affected Branches:
    MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE

Description

After successfully assigning users in enrol/database/enrol.php's sync_enrolments(), the script procedes to role_unassign() all users managed by the database enrollment plugin. The SQL in the section following the comment "// prune enrolments to courses that are no longer in ext auth", uses a left outer join of role_assignments on a join of context on course. The outer join returns a resultset of all database role assignments, and passes these off for unassignment.

Line 351 of enrol/database/enrol.php (version 1.32) should read:
351 JOIN ({$CFG->prefix}context cn
not the current
351 LEFT OUTER JOIN ({$CFG->prefix}context cn

The above change will affect the stated purpose to "prune enrolments to courses that are no longer in ext auth".

I'm guessing that the intention of the outer join was probably to find roles associated with a missing course or context. If this is necessary, some other implementation will be needed.

Attachments

  1. Text File
    database_enrol.php.patch.txt
    05/May/08 5:27 AM
    2 kB
    Eloy Lafuente (stronk7)
  2. Text File
    enroll.php.patch
    10/May/08 4:03 AM
    2 kB
    Neil Streeter

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Nicolas Connault added a comment -

Another enrol one.

Show
Nicolas Connault added a comment - Another enrol one.
Hide
Permalink
C. Lopez added a comment -

I have been looking into this, and it is my feeling that the LEFT JOIN statement is theoretically correct, but the problem (in my case, anyway) is that versions pf MySQL before 5.0.1 do not support nested joins. Here we have a command of the form

SELECT * FROM RA LEFT JOIN (C, CN)

And this is a form that explicitly not allowed in MySQL prior to 5.0.1. Basically, these versions of MySQL will just ignore the parentheses. That's what's resulting in this behavior.

Since Moodle is supposed to support MySQL 4.1, the query will have to be rewritten. I think a query of the form

SELECT * FROM RA LEFT JOIN CN LEFT JOIN C

That is,

"SELECT ra.roleid, ra.userid, ra.contextid
FROM {$CFG->prefix}role_assignments ra
LEFT JOIN {$CFG->prefix}context cn
ON ra.contextid = cn.id AND cn.contextlevel = ".CONTEXT_COURSE."
LEFT JOIN {$CFG->prefix}course c ON cn.instanceid = c.id
WHERE ra.enrol = 'database'" .....

will do what was intended here, though it may not be formally equivalent.

Show
C. Lopez added a comment - I have been looking into this, and it is my feeling that the LEFT JOIN statement is theoretically correct, but the problem (in my case, anyway) is that versions pf MySQL before 5.0.1 do not support nested joins. Here we have a command of the form SELECT * FROM RA LEFT JOIN (C, CN) And this is a form that explicitly not allowed in MySQL prior to 5.0.1. Basically, these versions of MySQL will just ignore the parentheses. That's what's resulting in this behavior. Since Moodle is supposed to support MySQL 4.1, the query will have to be rewritten. I think a query of the form SELECT * FROM RA LEFT JOIN CN LEFT JOIN C That is, "SELECT ra.roleid, ra.userid, ra.contextid FROM {$CFG->prefix}role_assignments ra LEFT JOIN {$CFG->prefix}context cn ON ra.contextid = cn.id AND cn.contextlevel = ".CONTEXT_COURSE." LEFT JOIN {$CFG->prefix}course c ON cn.instanceid = c.id WHERE ra.enrol = 'database'" ..... will do what was intended here, though it may not be formally equivalent.
Hide
Permalink
Martin Dougiamas added a comment -

Hmm, Martin is this still a problem?

Show
Martin Dougiamas added a comment - Hmm, Martin is this still a problem?
Hide
Permalink
Prabir Choudhury added a comment -

there is a bug in Mysql 5.0.3a. to get details plz visit http://bugs.mysql.com/bug.php?id=25575

Show
Prabir Choudhury added a comment - there is a bug in Mysql 5.0.3a. to get details plz visit http://bugs.mysql.com/bug.php?id=25575
Hide
Permalink
C. Lopez added a comment -

I don't think this MySQL bug is relevant to this Moodle bug.

Show
C. Lopez added a comment - I don't think this MySQL bug is relevant to this Moodle bug.
Hide
Permalink
Neil Streeter added a comment -

It is indeed still a problem. I should have looked here first :-/ Serves me right...

I am using a version of mysql prior to 5.0.1.... I'll take a look at what C. Lopez has suggested..

http://moodle.org/mod/forum/discuss.php?d=94586

ns

Show
Neil Streeter added a comment - It is indeed still a problem. I should have looked here first :-/ Serves me right... I am using a version of mysql prior to 5.0.1.... I'll take a look at what C. Lopez has suggested.. http://moodle.org/mod/forum/discuss.php?d=94586 ns
Hide
Permalink
Eloy Lafuente (stronk7) added a comment - - edited

Well,

IMO that script shouldn't try to "fix" (or clean) enrolments in orphan contexts or courses (that's another problem out from the database enrol itself).

So I've transformed the problematic query into one common INNER JOIN so it simply returns the role assignments in moodle courses that aren't any more in external DB. Just that.

I haven't tested code (patch attached for Moodle 1.9) but should work fine, AFAIK.

If somebody can test it I'll commit it to as many releases as possible.

TIA and ciao

Show
Eloy Lafuente (stronk7) added a comment - - edited Well, IMO that script shouldn't try to "fix" (or clean) enrolments in orphan contexts or courses (that's another problem out from the database enrol itself). So I've transformed the problematic query into one common INNER JOIN so it simply returns the role assignments in moodle courses that aren't any more in external DB. Just that. I haven't tested code (patch attached for Moodle 1.9) but should work fine, AFAIK. If somebody can test it I'll commit it to as many releases as possible. TIA and ciao
Hide
Permalink
Petr Škoda (skodak) added a comment -

my +1, but did not test it either

Show
Petr Škoda (skodak) added a comment - my +1, but did not test it either
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Thanks for feedback, Petr!

Can anybody apply the patch above against 1.9 and check it fixes the "exceed of deletions" problem?

TIA and ciao

Show
Eloy Lafuente (stronk7) added a comment - Thanks for feedback, Petr! Can anybody apply the patch above against 1.9 and check it fixes the "exceed of deletions" problem? TIA and ciao
Hide
Permalink
Neil Streeter added a comment -

Initial patch has an extra '(' character on the first JOIN line. This simply fixes that. --ns

Show
Neil Streeter added a comment - Initial patch has an extra '(' character on the first JOIN line. This simply fixes that. --ns
Hide
Permalink
Neil Streeter added a comment -

I tested the fix (quickly - not extensively) and it works perfectly once the extra '(' is removed.

Thanks!
Neil

Show
Neil Streeter added a comment - I tested the fix (quickly - not extensively) and it works perfectly once the extra '(' is removed. Thanks! Neil
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Cool, you really tested it, Neil! :-P

Going to apply the (fixed) patch to 18_STABLE, 19_STABLE and HEAD:

Thanks for feedback! Ciao

Show
Eloy Lafuente (stronk7) added a comment - Cool, you really tested it, Neil! :-P Going to apply the (fixed) patch to 18_STABLE, 19_STABLE and HEAD: Thanks for feedback! Ciao
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Done patch has been applied to 18_STABLE, 19_STABLE and HEAD.

Resolving as fixed. Thanks and ciao

Show
Eloy Lafuente (stronk7) added a comment - Done patch has been applied to 18_STABLE, 19_STABLE and HEAD. Resolving as fixed. Thanks and ciao
Hide
Permalink
Petr Škoda (skodak) added a comment -

reviewed, closing, thanks!

Show
Petr Škoda (skodak) added a comment - reviewed, closing, thanks!

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.