Going through database unique keys...
for the user table
- should email be unique?
- should idnumber be unique?
we do enforce email uniqueness at the app level, but having in in SQL gives us depth (if our PHP code leaks a few duplicates) and also speeds up lookups, because the planner knows that the index is unique.
for the courses table
- should idnumber be unique?
Yes, yes, and yes, as far as I can see. 
Given the feedback below and the variety of uses that people have for this field I think we have to keep email in the current situation: force uniqueness at the app level but not at the database level. It seems to work best that way.
The main reason for uniqueness at all is for password recovery when using internal authentication methods. With external authentication, of course, such password recovery is not needed anyway so email uniqueness is not important.
The main reason for uniqueness at all is for password recovery when using internal authentication methods. With external authentication, of course, such password recovery is not needed anyway so email uniqueness is not important.
Great! I'll leave email uniqueness alone then ;)
In the next few days I'll be preparing a patch for HEAD to cleanup idnumber and change the index to unique for mysql and postgres. It will also cover find_form_errors() so users get a friendly msg.
Should course idnumber be unique too? Enrolment code seems to work on that assumption, but I don't recall seeing it enforced anywhere.
In the next few days I'll be preparing a patch for HEAD to cleanup idnumber and change the index to unique for mysql and postgres. It will also cover find_form_errors() so users get a friendly msg.
Should course idnumber be unique too? Enrolment code seems to work on that assumption, but I don't recall seeing it enforced anywhere.
In reply to Martín Langhoff
Re: Database unique keys for user and course...
Uhm,
while idnumbers should be unique (by definition), what the *reason* to have unique email addresses? Recover password? Duplicate mails from forums?
Couldn't be that "feature" useful for some teachers with different login but the same email (not really sure, only making a supposition...).
Ciao
while idnumbers should be unique (by definition), what the *reason* to have unique email addresses? Recover password? Duplicate mails from forums?
Couldn't be that "feature" useful for some teachers with different login but the same email (not really sure, only making a supposition...).
Ciao
Since email addresses need to be unique on the moodle server, can you use noreply@localhost multiple times? 
If that is the case you could have the emails numbered as such: 1@localhost, 2@localhost, 3@localhost, ... etc. You can then select to have the email non-active under the user's settings.
If that is the case you could have the emails numbered as such: 1@localhost, 2@localhost, 3@localhost, ... etc. You can then select to have the email non-active under the user's settings.
I vote for emailadresses to be unique. To often studenst forget their account/password and try to make a new account. I don't know why they don't like to push the forgot passwordbutton (here they don't like the generated paswords 
)
Since they can't create a duplicate account with the same emailadress, they have to go trough the forgot password hassle (or create a new e-mailadres like some of them do).
There's a possible disagreement here between users who create the accounts themselves and users using the email based authentication.
Generating the textfile with unique email adresses is very easy using excell (auto increases the numbers)
)Since they can't create a duplicate account with the same emailadress, they have to go trough the forgot password hassle (or create a new e-mailadres like some of them do).
There's a possible disagreement here between users who create the accounts themselves and users using the email based authentication.
Generating the textfile with unique email adresses is very easy using excell (auto increases the numbers)
Maybe the textfile import could accept empty email addresses or a conventional entry ("null")?
I am happy with the present state: unique at app level and not unique at db level
BTW did you know that if you set "0" as email it works as NULL, after first login student must enter email
(could be useful for bulk uploads)
BTW did you know that if you set "0" as email it works as NULL, after first login student must enter email
In reply to Eloy Lafuente (stronk7)
Re: Database unique keys for user and course...
Nā Zbigniew Fiedorowicz -
Email should definitely not be unique. I already have lots of bogus/test users with email addresses nobody@mysite. I expect there may be lots of other Moodle sites with nonunique email addresses imported via the uploaduser.php script, which doesn't check for email uniqueness. Forcing uniqueness of email addresses would force Moodle upgrades to fail at such sites.
In reply to Zbigniew Fiedorowicz
Re: Database unique keys for user and course...
Nā Martín Langhoff -
We would have to find a way to deal with those.
Hmmm. Those users can't edit their profile without being forced to change their email. Similar to having an empty email ;)
Hmmm. Those users can't edit their profile without being forced to change their email. Similar to having an empty email ;)
In reply to Eloy Lafuente (stronk7)
Re: Database unique keys for user and course...
Nā Martín Langhoff -
Ok idnumber is unique. Happy :D
Don't know about the design decision of having email being unique, Martin? I personally like it, but I haven't thought of the wider implications.
Just to elaborate the idea a bit more: Right now email is (a) required and (b) unique at the app level, not the DB level.
Clearly, making it required (not null) at the DB level would break user imports from file uploads and such. So I won't go there.
I am still keen on having email unique at the DB level. It will:
- improve consistency: if you have 2 users with the same email, neither can change their profile data without changing their email address!
- improve performance. limited to email address searches, so not really important.
Would it break stuff?
Don't know about the design decision of having email being unique, Martin? I personally like it, but I haven't thought of the wider implications.
Just to elaborate the idea a bit more: Right now email is (a) required and (b) unique at the app level, not the DB level.
Clearly, making it required (not null) at the DB level would break user imports from file uploads and such. So I won't go there.
I am still keen on having email unique at the DB level. It will:
- improve consistency: if you have 2 users with the same email, neither can change their profile data without changing their email address!
- improve performance. limited to email address searches, so not really important.
Would it break stuff?
In reply to Martín Langhoff
Re: Database unique keys for user and course...
Nā Zbigniew Fiedorowicz -
Pardon my ignorance about sql/mysql. What would happen if you have a user table with nonunique email and the upgrade tried to ALTER the table to force the field to be unique? I thought that this would cause the ALTER operation to fail? If not, would the altered table be considered as corrupted and in need of repair?
For your information, the uploaduser script does require a nonblank email field but doesn't check it for uniqueness.
For your information, the uploaduser script does require a nonblank email field but doesn't check it for uniqueness.
In reply to Zbigniew Fiedorowicz
Re: Database unique keys for user and course...
Trying to create UNIQUE keys in a table containing non-unique values will throw an:
#1062 - Duplicate entry 'value' for key... error.
and the index won't be created. But no corruption or problem at all.
Anyway, I think that, independently from the UNIQUE existence, check must be done at application level always. If we rely in DB only, we'll get an DB (low level) error without showing the user the readable cause of his error, isn't it?
Ciao
#1062 - Duplicate entry 'value' for key... error.
and the index won't be created. But no corruption or problem at all.
Anyway, I think that, independently from the UNIQUE existence, check must be done at application level always. If we rely in DB only, we'll get an DB (low level) error without showing the user the readable cause of his error, isn't it?
Ciao
In reply to Eloy Lafuente (stronk7)
Re: Database unique keys for user and course...
Nā Martín Langhoff -
Agreed, check at both levels! It's just that we aren't checking everywhere at the app level ;)
In reply to Zbigniew Fiedorowicz
Re: Database unique keys for user and course...
Nā Martín Langhoff -
"What would happen if you have a user table with nonunique email and the upgrade tried to ALTER the table to force the field to be unique?"
It would fail ;)
In those cases, what we do is we cleanup duplicates before attempting to ALTER the table. Duplicate emails would be set to null, which would have the same effect as having them duplicated, but will mean much cleaner data.
We can always combine this with changes in uploaduser to accept something like 'null' or just empty.
It boils down to what should the data look like. A design issue, I guess.
It would fail ;)
In those cases, what we do is we cleanup duplicates before attempting to ALTER the table. Duplicate emails would be set to null, which would have the same effect as having them duplicated, but will mean much cleaner data.
We can always combine this with changes in uploaduser to accept something like 'null' or just empty.
It boils down to what should the data look like. A design issue, I guess.
Wasn't sure where to post this, but I think this is the most relevant place even though nobody has been here in 2 years 
.
The edit course page does not check to ensure that the idnumber a user enters is unique.
The lack of checking allowed a duplicate idnumber entry; combine this with a bug in LDAP auto course creation and you get a big mess
We had one course being continually created about 100+ times in total before catching the problem.
I have posted this as Bug #5393 along with two diffs; one for /enrol/ldap/enrol.php and a second for /course/edit.php which.
. The edit course page does not check to ensure that the idnumber a user enters is unique.
The lack of checking allowed a duplicate idnumber entry; combine this with a bug in LDAP auto course creation and you get a big mess
We had one course being continually created about 100+ times in total before catching the problem.I have posted this as Bug #5393 along with two diffs; one for /enrol/ldap/enrol.php and a second for /course/edit.php which.