Description

Version 2.0 deletes all grades information for students unenrolled from a course. This is opposite from previous behavior and IMO very undesireable. A moderate number of times each term a student will get accidentally unenrolled from a course. Retention of grades information permits them to be re-enrolled without issue.

Activity

This is an issue at my school as well, not just when a student is accidentally unenrolled but when a student is moved from one course to another. For example, a student in Algegra Honors may be moved to a non honors Algebra course based on her first quarter grade. Loosing the first quarter grade history is a problematic.
It would be helpful if the student can be removed as a participant in the course does not show up in the student's course list, so the student does not receive emails of announcements and so calendar entries do not show up in the student's Upcoming Events block.
It would be helpful to keep grade the student's grade history until such time as the course is reset but not have the student appear in the teacher's gradebook or any grade exports.

Edwinna Lucyk
added a comment - 17/Dec/10 1:07 AM This is an issue at my school as well, not just when a student is accidentally unenrolled but when a student is moved from one course to another. For example, a student in Algegra Honors may be moved to a non honors Algebra course based on her first quarter grade. Loosing the first quarter grade history is a problematic.
It would be helpful if the student can be removed as a participant in the course does not show up in the student's course list, so the student does not receive emails of announcements and so calendar entries do not show up in the student's Upcoming Events block.
It would be helpful to keep grade the student's grade history until such time as the course is reset but not have the student appear in the teacher's gradebook or any grade exports.

James Scully
added a comment - 17/Dec/10 3:10 AM Sorry for the second message - the classification of priority as minor is most inappropriate. This is exactly the kind of behaviour that should be preserved as matter of policy in any upgrade.

Bob Puffer
added a comment - 17/Dec/10 5:52 AM I don't presume to set the priority of an issue when I post it and this issue hasn't yet been reviewed by moodle HQ so the priority of "minor" is hopefully temporary.

Petr Skoda
added a comment - 17/Dec/10 5:27 PM This is intentional, we do need to remove user data from the course after unenrolling especially for performance reasons.
The new enrolment framework already prevents some types of "accidental" unernolments, if you want to keep student history then just disable the enrolment.
Petr

We can add a new gradebook option that would try to recover previous grades from the grade_grades history table, that way you would get the grades back when you reenrol the student (simple change, could be in 2.0.1).

Another topic is the prevention of unenrolment, maybe we could replace the current unenrol action with "disable enrolment + remove all course roles", that way people could not accidentally unenrol users. The current "unernol with data purging" would have to be hidden a bit somewhere else.

Petr Skoda
added a comment - 17/Dec/10 6:20 PM The currrent enrolment logic is:
1/ unenrol == purge data (groups, gradebook, subscriptions, etc.)
2/ disable enrolment == keep data, but prevent student access to course
We can add a new gradebook option that would try to recover previous grades from the grade_grades history table, that way you would get the grades back when you reenrol the student (simple change, could be in 2.0.1).
Another topic is the prevention of unenrolment, maybe we could replace the current unenrol action with "disable enrolment + remove all course roles", that way people could not accidentally unenrol users. The current "unernol with data purging" would have to be hidden a bit somewhere else.
Petr

I've taken this discussion up in the forum because it will involve a greater cross-section of interested parties and I see by the votes there are many interested parties. I've noted there an instance of how a single decision made in the enrollment plugin setup (the options noted by Petr) doesn't really work because there are numerous reasons why someone would not be in the enrollment authority. Removing the data associated with an unenrollment only works when the unenrollment is intentional and not likely to be reversed. I'm confident we can get this worked out but I don't want to trivialize how big a change in expected behavior this really is. Having spent a lot of hours looking at gradebook code I don't feel very confident that grades history will be accurate in all cases.

Bob Puffer
added a comment - 18/Dec/10 3:37 AM I've taken this discussion up in the forum because it will involve a greater cross-section of interested parties and I see by the votes there are many interested parties. I've noted there an instance of how a single decision made in the enrollment plugin setup (the options noted by Petr) doesn't really work because there are numerous reasons why someone would not be in the enrollment authority. Removing the data associated with an unenrollment only works when the unenrollment is intentional and not likely to be reversed. I'm confident we can get this worked out but I don't want to trivialize how big a change in expected behavior this really is. Having spent a lot of hours looking at gradebook code I don't feel very confident that grades history will be accurate in all cases.

Dean Thayer
added a comment - 18/Dec/10 3:40 AM Explain this bit about the enrollment plugins more fully, please. Does the unenrollment of a student by an enrollment plugin necessarily remove the student's data from the course?

I agree there are potential problems in the user interface and we have to try a lot harder to prevent unintentional unenrolemnts, but the fundamental rules are:
a/ unenrol == purge user data from course with the exception of items that are related to work of other users (such as removing of forum posts would break discussion threading)
b/ suspend user enrolment == keep all user data and prevent user access to course

This is not really a big change because we were removing some user data already in 1.9 such as forum subscriptions, group membership. etc.

The proposed solution is:
1/ educate users that unenrol means purge course data for given suer
2/ use disabling of user enrolments+removing of all roles as a default option instead of full unenrol
3/ implement optional reconstruction of grades from the grades history table after reenrolling of user

Petr Skoda
added a comment - 18/Dec/10 4:27 AM I agree there are potential problems in the user interface and we have to try a lot harder to prevent unintentional unenrolemnts, but the fundamental rules are:
a/ unenrol == purge user data from course with the exception of items that are related to work of other users (such as removing of forum posts would break discussion threading)
b/ suspend user enrolment == keep all user data and prevent user access to course
This is not really a big change because we were removing some user data already in 1.9 such as forum subscriptions, group membership. etc.
The proposed solution is:
1/ educate users that unenrol means purge course data for given suer
2/ use disabling of user enrolments+removing of all roles as a default option instead of full unenrol
3/ implement optional reconstruction of grades from the grades history table after reenrolling of user
Do the points 1,2,3 above solve the problem for all of you?

2) Student course swapping. 2 weeks into semester after submitting their first assignment a student decides to drop course 1 and do course 2 instead. A week later they decide to swap back into course 1.

We need to be able to reenroll the student in both cases and have their grade data reappear. To deal number 2 in particular I feel like we need to either make the immediate purging of grade data either not happen or be automatically reversed upon re-enrollment.

Andrew Davis
added a comment - 18/Dec/10 12:18 PM - edited There are two scenarios we need to be able to recover from:
1) Admin error. Admin accidently unenrolls user B when they meant to unenroll user A.
2) Student course swapping. 2 weeks into semester after submitting their first assignment a student decides to drop course 1 and do course 2 instead. A week later they decide to swap back into course 1.
We need to be able to reenroll the student in both cases and have their grade data reappear. To deal number 2 in particular I feel like we need to either make the immediate purging of grade data either not happen or be automatically reversed upon re-enrollment.

Anything that requires the user to remember that when unenrolling a user they should not press unenroll seems doomed to failure. I dont think we can rely on this.

The ability to rebuild grades after re-enrollment sounds like the way to go. Short of adding a flag to each of the user's grades to indicate that they should be ignored and then purged after the course is over.

Andrew Davis
added a comment - 18/Dec/10 12:36 PM "educate users that unenrol means purge course data for given suer"
Anything that requires the user to remember that when unenrolling a user they should not press unenroll seems doomed to failure. I dont think we can rely on this.
The ability to rebuild grades after re-enrollment sounds like the way to go. Short of adding a flag to each of the user's grades to indicate that they should be ignored and then purged after the course is over.

I don't think you should ever delete student grading data from a course except perhaps with a special purge-data script that is run manually. It simply is an insignificant amount of data to keep around given that at some point you may want to go back and see what has happened with a given student.

The one time you should purge this data automatically is when you delete a course as at that point you have lost the context of anything that is in the grading table.

This is especially an important feature at the start of terms where teachers or students may unenroll themselves from the last term or while changing courses just to find that grading issues come up. But frankly I have found it helpful to go back to a course I taught a few years ago to see how a few students performed in deciding whether to change assignments. The ability to simply reenroll them for a short period of time and see how the course went for them has proved very useful.

I see no reason to look at 1.9 and earlier behavior in this case as being undesirable. Except for the very largest institutions (who can use a special purge script) the affect on database size or search speed is just insignificant.

Gary Anderson
added a comment - 20/Dec/10 2:08 AM I don't think you should ever delete student grading data from a course except perhaps with a special purge-data script that is run manually. It simply is an insignificant amount of data to keep around given that at some point you may want to go back and see what has happened with a given student.
The one time you should purge this data automatically is when you delete a course as at that point you have lost the context of anything that is in the grading table.
This is especially an important feature at the start of terms where teachers or students may unenroll themselves from the last term or while changing courses just to find that grading issues come up. But frankly I have found it helpful to go back to a course I taught a few years ago to see how a few students performed in deciding whether to change assignments. The ability to simply reenroll them for a short period of time and see how the course went for them has proved very useful.
I see no reason to look at 1.9 and earlier behavior in this case as being undesirable. Except for the very largest institutions (who can use a special purge script) the affect on database size or search speed is just insignificant.

The grade history tables were designed to keep the data when we remove the user from course, change gradebook structure or do anything else. It is not important in which table the data actually is as long as it is possible to get the grades before the unenrolment.

The data amount may be insignificant (in some sites it definitely is huge due to a bug in early 1.9.x), but during the recalculation of all grades (which needs to be done as fast as possible) we need to know which grades have to be recalculated - we can do that simply by JOINing to the enrolment table now which should be a lot faster. There many complains that gradebook queries are slow, this is one of the way to get some performance improvements.

Reenrolling users is really bad idea because you do not want to "enrol" them === be able to access the course! Instead we should finally create the report for grade history views...

So far nobody demonstrated valid reason why the grades should not be moved to the grades history table during unenrolment, I have proposed solutions that are not just workarounds for some missing functionality.

1/ reenrolling students just to get grades - grade history view and optional copying of grades from history
2/ accidental unerol/reenrol - changes in enrolment UI, making the suspend enrolment default option
3/ complex enrol workflows - customised enrol plugin, the new enrol framework is a lot more flexible

Does anybody have any other unenrol data purging related issue? Do the proposed changes above solve your problems?

Petr Skoda
added a comment - 20/Dec/10 3:40 AM - edited The grade history tables were designed to keep the data when we remove the user from course, change gradebook structure or do anything else. It is not important in which table the data actually is as long as it is possible to get the grades before the unenrolment.
The data amount may be insignificant (in some sites it definitely is huge due to a bug in early 1.9.x), but during the recalculation of all grades (which needs to be done as fast as possible) we need to know which grades have to be recalculated - we can do that simply by JOINing to the enrolment table now which should be a lot faster. There many complains that gradebook queries are slow, this is one of the way to get some performance improvements.
Reenrolling users is really bad idea because you do not want to "enrol" them === be able to access the course! Instead we should finally create the report for grade history views...
So far nobody demonstrated valid reason why the grades should not be moved to the grades history table during unenrolment, I have proposed solutions that are not just workarounds for some missing functionality.
1/ reenrolling students just to get grades - grade history view and optional copying of grades from history
2/ accidental unerol/reenrol - changes in enrolment UI, making the suspend enrolment default option
3/ complex enrol workflows - customised enrol plugin, the new enrol framework is a lot more flexible
Does anybody have any other unenrol data purging related issue? Do the proposed changes above solve your problems?

Though I may be mistaken or have missed something along the way of the 2.0 design discussions that took place in the open, it seems to me the decision to change functionality which has been expected behavior for a very long time was arrived at in a fairly short time with little discussion taking place outside core development. To summarize what Gary Anderson put so eloquently, "what's the use of changing something so obviously important for such little gain in performance?" I stand by my early contention that this change is a mistake and the decision on its reversal should not be rushed through over a weekend in the holiday season.

Bob Puffer
added a comment - 20/Dec/10 6:53 AM Though I may be mistaken or have missed something along the way of the 2.0 design discussions that took place in the open, it seems to me the decision to change functionality which has been expected behavior for a very long time was arrived at in a fairly short time with little discussion taking place outside core development. To summarize what Gary Anderson put so eloquently, "what's the use of changing something so obviously important for such little gain in performance?" I stand by my early contention that this change is a mistake and the decision on its reversal should not be rushed through over a weekend in the holiday season.

There seems to be a confusion between internal technical implementation and user interface aspects. We can tweak the UI so that it seems to work the same way as in 1.9.

I am changing the title, because no grades are deleted - they are just moved to different table! Please stop panicking, there will be soon an option to get the original grades automatically after user reenrolment.

Petr Skoda
added a comment - 20/Dec/10 7:43 AM There seems to be a confusion between internal technical implementation and user interface aspects. We can tweak the UI so that it seems to work the same way as in 1.9.
I am changing the title, because no grades are deleted - they are just moved to different table! Please stop panicking, there will be soon an option to get the original grades automatically after user reenrolment.

From MDL-20617 " Petr Škoda (skodak) added a comment - 15/Dec/10 4:52 PM :
...only enrolled users are supposed to have grades, all grades for not-enrolled should be dropped at some time; when unenrolling users we now delete the grades; in any case you should not add grades for users that are not enrolled"

I don't agree with renaming of the issue. I don't agree that this is an "internal technical implementation" issue and therefore (somehow) doesn't deserve Moodle community participation. I believe my comments to-date indicate a desire for careful decision, not panic. I am, however starting to feel a bit incensed at the steps that are apparently being taken to cover the tracks of a poorly reasoned decision.

Bob Puffer
added a comment - 20/Dec/10 8:03 AM From MDL-20617 " Petr Škoda (skodak) added a comment - 15/Dec/10 4:52 PM :
...only enrolled users are supposed to have grades, all grades for not-enrolled should be dropped at some time; when unenrolling users we now delete the grades; in any case you should not add grades for users that are not enrolled"
I don't agree with renaming of the issue. I don't agree that this is an "internal technical implementation" issue and therefore (somehow) doesn't deserve Moodle community participation. I believe my comments to-date indicate a desire for careful decision, not panic. I am, however starting to feel a bit incensed at the steps that are apparently being taken to cover the tracks of a poorly reasoned decision.

I am patiently trying to explain how the gradebook works internally and what are the options to resolve the issue, I would personally prefer a more constructive approach. It seems that I can not persuade you with my words so I will try to do that with my code instead.

Petr Skoda
added a comment - 20/Dec/10 8:10 AM I am patiently trying to explain how the gradebook works internally and what are the options to resolve the issue, I would personally prefer a more constructive approach. It seems that I can not persuade you with my words so I will try to do that with my code instead.

I'm with you, Petr on desiring a more constructive conversation – one that hopefully involves all interested parties. Andrew's suggestion of rebuilding a "re-enrolled" student's grades seems like an equivalent to existing functionality IMO. I'd encourage more broad discussion before final action is taken and we somehow deem this important issue "laid to rest".

Bob Puffer
added a comment - 20/Dec/10 8:52 AM I'm with you, Petr on desiring a more constructive conversation – one that hopefully involves all interested parties. Andrew's suggestion of rebuilding a "re-enrolled" student's grades seems like an equivalent to existing functionality IMO. I'd encourage more broad discussion before final action is taken and we somehow deem this important issue "laid to rest".

I think the best thing to do at this point is to carefully explain exactly what happens when the following events occur, comparing and contrasting 1.9 and 2.0:

1) A manually enrolled user with grades is unenrolled and then re-enrolled via the Moodle administrative interface.
2) A user enrolled by an enrollment plugin with grades is unenrolled and re-enrolled by that plugin.

Any administrative options that affect the results in situations 1 and 2 need also be carefully explained. Let's make sure that everyone understands the issue completely before we proceed with discussion or code changes. A complete and accurate response here would be more valuable than a quick one. I personally recommend enhancing http://docs.moodle.org/en/Enrolment and perhaps http://docs.moodle.org/en/Enrolment_FAQ with this information and resuming work on this issue once that it completed.

Dean Thayer
added a comment - 20/Dec/10 9:56 PM I think the best thing to do at this point is to carefully explain exactly what happens when the following events occur, comparing and contrasting 1.9 and 2.0:
1) A manually enrolled user with grades is unenrolled and then re-enrolled via the Moodle administrative interface.
2) A user enrolled by an enrollment plugin with grades is unenrolled and re-enrolled by that plugin.
Any administrative options that affect the results in situations 1 and 2 need also be carefully explained. Let's make sure that everyone understands the issue completely before we proceed with discussion or code changes. A complete and accurate response here would be more valuable than a quick one. I personally recommend enhancing http://docs.moodle.org/en/Enrolment and perhaps http://docs.moodle.org/en/Enrolment_FAQ with this information and resuming work on this issue once that it completed.

In the past we have had students un-enroll themselves from a course (we use the Banner Luminis Plugin). They then get dropped from the course in Moodle.

When they change their minds a few days later faculty here are very grateful that quiz information and grade information is still there when the student re-enrolls themselves into the course. This is a HUGE change in expected behavior and a VERY big problem since students are very fickle in their course choices. In a case like this un-enrollment is intentional.

Rosalyn Metz
added a comment - 20/Dec/10 10:05 PM In the past we have had students un-enroll themselves from a course (we use the Banner Luminis Plugin). They then get dropped from the course in Moodle.
When they change their minds a few days later faculty here are very grateful that quiz information and grade information is still there when the student re-enrolls themselves into the course. This is a HUGE change in expected behavior and a VERY big problem since students are very fickle in their course choices. In a case like this un-enrollment is intentional.

The internal rules are really simple:
1/ we keep all course data for all enrolled users - both active and suspended
2/ during full unenrolment we delete as much as possible - this is decided usually by activities (we can not delete forum posts because other posts my be replays)
3/ suspended enrolment means that user can not enter the course unless there is guest access (designed for both future and historic enrolments)
4/ suspended users are usually hidden in the list of participants, reports, etc. (there can be a switch to show all enrolled users, not just the active ones)
5/ gradebook keeps full history of all grades, so nothing is actually completely deleted unless you disable the grade history

Internally there is API that does full unenrol with data purging and seprate suspend (aka unenrol while keeping all user data), it is not really important what we call it in the UI, we can change the actions or better we can make it configurable.

Rosalyn: the self enrolment is now a separate plugin and fully abstracted, it is not hardcoded in core any more. Anybody can easily tweak it so that it suspends users only, and then let them reenrol without loosing any data at all (including group membership, forum subscriptions, gradebook...). We can even add more options that specify what to do when user requests self-unenrolment. The proposed solution for optional recovering of grades after reenrolment solves your problem in a different way.

The enrolment framework in 2.0 is completely redesigned, it is now a lot more flexible and allows developers to create pretty much anything. Many old problems from 1.7-1.9 were already solved, unfortunately there are still some missing features or unexplained changes. If there were more people testing 2.0dev we could have addressed some of these issue already and teachers would not even realised that something was changed internally. This is the first 2.0.0 release, as with each new major release there are some pending problems, if you do not like that please keep using 1.9.x for now.

2.0 is a big step, developers can not expect that everything works the same internally, the best way how to learn more about the changes is to actually study the new db tables and read the existing enrol plugins - no spec or design document is going to explain everything.

I personally hope this issue will be resolved quickly. And yes we need a new documentation page describing the new enrolments, we need user docs for most new features in 2.0.

Petr Skoda
added a comment - 20/Dec/10 10:58 PM The internal rules are really simple:
1/ we keep all course data for all enrolled users - both active and suspended
2/ during full unenrolment we delete as much as possible - this is decided usually by activities (we can not delete forum posts because other posts my be replays)
3/ suspended enrolment means that user can not enter the course unless there is guest access (designed for both future and historic enrolments)
4/ suspended users are usually hidden in the list of participants, reports, etc. (there can be a switch to show all enrolled users, not just the active ones)
5/ gradebook keeps full history of all grades, so nothing is actually completely deleted unless you disable the grade history
Internally there is API that does full unenrol with data purging and seprate suspend (aka unenrol while keeping all user data), it is not really important what we call it in the UI, we can change the actions or better we can make it configurable.
Rosalyn: the self enrolment is now a separate plugin and fully abstracted, it is not hardcoded in core any more. Anybody can easily tweak it so that it suspends users only, and then let them reenrol without loosing any data at all (including group membership, forum subscriptions, gradebook...). We can even add more options that specify what to do when user requests self-unenrolment. The proposed solution for optional recovering of grades after reenrolment solves your problem in a different way.
The enrolment framework in 2.0 is completely redesigned, it is now a lot more flexible and allows developers to create pretty much anything. Many old problems from 1.7-1.9 were already solved, unfortunately there are still some missing features or unexplained changes. If there were more people testing 2.0dev we could have addressed some of these issue already and teachers would not even realised that something was changed internally. This is the first 2.0.0 release, as with each new major release there are some pending problems, if you do not like that please keep using 1.9.x for now.
2.0 is a big step, developers can not expect that everything works the same internally, the best way how to learn more about the changes is to actually study the new db tables and read the existing enrol plugins - no spec or design document is going to explain everything.
I personally hope this issue will be resolved quickly. And yes we need a new documentation page describing the new enrolments, we need user docs for most new features in 2.0.

Ive raised MDLSITE-1196 as, as others have mentioned, it doesn't feel like the enrollment documentation is both up to date and complete.

We need to hammer out what the desired behavior is before we worry about implementation details. Is http://docs.moodle.org/en/Development:New_enrolments_in_2.0 the definitive design document for the 2.0 enrollment system? There are a few enrollment documents around that don't look to have received much attention.

Andrew Davis
added a comment - 07/Jan/11 3:52 PM Ive raised MDLSITE-1196 as, as others have mentioned, it doesn't feel like the enrollment documentation is both up to date and complete.
We need to hammer out what the desired behavior is before we worry about implementation details. Is http://docs.moodle.org/en/Development:New_enrolments_in_2.0 the definitive design document for the 2.0 enrollment system? There are a few enrollment documents around that don't look to have received much attention.

These new changes are not difficult to overcome if you are currently using 2.0 and it sounds like Petr's proposed changes make the enrollment subsystem in 2.0 incredibly flexible, while still maintaining the simplicity and predicability of 1.9.

We too rely on the ability to maintain user data in 1.9 when un-enrolling and re-enrolling, but we have our own enrollment plugins, so for us the move to 2.0 means re-mapping un-enroll to suspend enrollment, a minor change in the larger scheme of things. We don't allow users to self un-enroll or faculty to un-enroll students, as this was a HUGE problem early on.

We'll probably add a cron hook in to remove suspended enrollments after some (committee agreed upon) specified time period.

I love the idea of the enrollment hooks! I cannot wait to use them. Thanks again.

Robert Russo
added a comment - 14/Jan/11 9:30 PM Kudos to the Moodle team for all the enrollment changes done in 2.0!
These new changes are not difficult to overcome if you are currently using 2.0 and it sounds like Petr's proposed changes make the enrollment subsystem in 2.0 incredibly flexible, while still maintaining the simplicity and predicability of 1.9.
We too rely on the ability to maintain user data in 1.9 when un-enrolling and re-enrolling, but we have our own enrollment plugins, so for us the move to 2.0 means re-mapping un-enroll to suspend enrollment, a minor change in the larger scheme of things. We don't allow users to self un-enroll or faculty to un-enroll students, as this was a HUGE problem early on.
We'll probably add a cron hook in to remove suspended enrollments after some (committee agreed upon) specified time period.
I love the idea of the enrollment hooks! I cannot wait to use them. Thanks again.

After some discussion about the various issues with Petr, I think the solution here for 2.0.x might be this:

1) There is a field in the database already called "active" which allows enrollments to be inactive/suspended so it doesn't show up on participation lists, but still technically in the course with all grades and user data intact. Currently the only way to flip this switch is to use the little "hand edit" icon on the enrolment page.

We could make this more visible by changing the confirmation page for when you try to delete/remove the enrolment, so that instead of just accept/cancel we have three options: "Disable this enrolment but retain all grades", "Remove this enrolment completely and delete all grades", "Cancel". This allows people to understand the difference and make that choice.

2) When adding a new enrolment manually, we should add a checkbox to "restore old grades from this user if they exist". If this checkbox is enabled, then the enrolment code will parse the history table and re-instate any old grades if they can be found.

Martin Dougiamas
added a comment - 28/Jan/11 10:45 AM After some discussion about the various issues with Petr, I think the solution here for 2.0.x might be this:
1) There is a field in the database already called "active" which allows enrollments to be inactive/suspended so it doesn't show up on participation lists, but still technically in the course with all grades and user data intact. Currently the only way to flip this switch is to use the little "hand edit" icon on the enrolment page.
We could make this more visible by changing the confirmation page for when you try to delete/remove the enrolment, so that instead of just accept/cancel we have three options: "Disable this enrolment but retain all grades", "Remove this enrolment completely and delete all grades", "Cancel". This allows people to understand the difference and make that choice.
2) When adding a new enrolment manually, we should add a checkbox to "restore old grades from this user if they exist". If this checkbox is enabled, then the enrolment code will parse the history table and re-instate any old grades if they can be found.
Does this sound OK with everyone?

Just for the record I believe the "active" field that Martin is referring to above is the status column in the user_enrolments table. The status column can hold either of the below values defined in /lib/enrollib.php (and possibly other values but these seem to be the two we're concerned with here)

Andrew Davis
added a comment - 04/Feb/11 3:03 PM - edited Just for the record I believe the "active" field that Martin is referring to above is the status column in the user_enrolments table. The status column can hold either of the below values defined in /lib/enrollib.php (and possibly other values but these seem to be the two we're concerned with here)
define('ENROL_USER_ACTIVE', 0);
define('ENROL_USER_SUSPENDED', 1);

Petr (or anyone else who knows), is there a reason why the below lines are reproduced exactly in both enrol/users.php and enrol/locallib.php? In the 'edit' branch of the switch user.php retrieves $instance and $plugin, runs some checks then calls $manager->edit_enrolment() which also loads $instance and $plugin and performs exactly the same checks.

By the looks of it the checks within users.php are used to prevent the edit form from being shown to users who don't pass the checks.
The checks in locallib.php are used to ensure the user can do what they are asking to do. Presently however the only place that the edit_enrolment method is being used if within users.php so yes the checks are being run twice.

Looking over the present situation within the code, the enrolment manager methods all check the associated capabilities for the actions they are performing. So remove it from users.php and people who arrive at the URL but don't have the caps will be able to see the form but won't be able to actually submit. Remove it from locallib.php and the method will differ in design from all the other enrolment manager methods.

Sam Hemelryk
added a comment - 08/Feb/11 11:03 AM By the looks of it the checks within users.php are used to prevent the edit form from being shown to users who don't pass the checks.
The checks in locallib.php are used to ensure the user can do what they are asking to do. Presently however the only place that the edit_enrolment method is being used if within users.php so yes the checks are being run twice.
Looking over the present situation within the code, the enrolment manager methods all check the associated capabilities for the actions they are performing. So remove it from users.php and people who arrive at the URL but don't have the caps will be able to see the form but won't be able to actually submit. Remove it from locallib.php and the method will differ in design from all the other enrolment manager methods.

To get to the UI involved go into a course and go to Users -> Enrolled Users in Course Administration. Click on the cross next to a student's enrollment in the Enrollment Methods column. From the confirmation screen you should now be able to either suspend or delete the student's enrollment.

Clicking on the Enrol Users button should bring up a dialog that now contains a checkbox that will cause any users you enrol to have grades retrieved from the grade history tables.

Andrew Davis
added a comment - 09/Feb/11 4:10 PM re those duplicate checks Ive left both of them but have added comments to describe their subtly different functions.
Ready for peer review.
repo: git://github.com/andyjdavis/moodle.git
branch: MDL-25718 _enrol_grades
diff: https://github.com/andyjdavis/moodle/compare/master...MDL-25718_enrol_grades
To get to the UI involved go into a course and go to Users -> Enrolled Users in Course Administration. Click on the cross next to a student's enrollment in the Enrollment Methods column. From the confirmation screen you should now be able to either suspend or delete the student's enrollment.
Clicking on the Enrol Users button should bring up a dialog that now contains a checkbox that will cause any users you enrol to have grades retrieved from the grade history tables.

Andrew Davis
added a comment - 11/Feb/11 4:02 PM - edited After a chat with Sam Ive put the checkbox and text in a div and span with a class to allow them to be styled.
I had also previously used a mixture of "recover grades" and "restore grades". Ive settled on recover grades and am now using this everywhere.

Still not quite sure how to handle #2. simply deleting any already inserted grades seems bad. Returning an error message to the user is fairly pointless as its not likely to be an error they can do much about. Not sure what to do.

Andrew Davis
added a comment - 16/Feb/11 2:18 PM Ive implemented suggestion #1.
https://github.com/andyjdavis/moodle/compare/master...MDL-25718_enrol_grades_alternative
Still not quite sure how to handle #2. simply deleting any already inserted grades seems bad. Returning an error message to the user is fairly pointless as its not likely to be an error they can do much about. Not sure what to do.

Ive added a safety check that will skip grade recovery if the user already has any grades. It just outputs a debug message although it could throw an exception (or similar) just as easily. Not sure how severe an error we want to produce.

Andrew Davis
added a comment - 16/Feb/11 6:21 PM Ive added a safety check that will skip grade recovery if the user already has any grades. It just outputs a debug message although it could throw an exception (or similar) just as easily. Not sure how severe an error we want to produce.

Ive mostly fixed the items outlined in PULL-371. I havent done the following yet.

1) grade_recover_history_grades() should probably also trigger grade refetching from activities, otherwise that data would be out of sync. You would probably use function grade_update_mod_grades($modinstance, $userid=0) for that. Some activities manually sync grades (push grades into gradebook). If the student was deleted by the time this was done their grades may not have been moved over.

2) "Why $fetchgradegradefromdb = false; when it is never used as variable?"
Purely for readability. Ive removed it.

3) "What happens when gradebook roles are empty? I disagree that is_enrolled() is not enough. The graded roles just mean "show user in gradebook", but we keep grades for all enrolled users. Your approach would create problems when you first enrol without role and then you get student role later."
Not sure I understand what you mean.

Andrew Davis
added a comment - 08/Mar/11 3:37 PM Ive mostly fixed the items outlined in PULL-371. I havent done the following yet.
1) grade_recover_history_grades() should probably also trigger grade refetching from activities, otherwise that data would be out of sync. You would probably use function grade_update_mod_grades($modinstance, $userid=0) for that. Some activities manually sync grades (push grades into gradebook). If the student was deleted by the time this was done their grades may not have been moved over.
2) "Why $fetchgradegradefromdb = false; when it is never used as variable?"
Purely for readability. Ive removed it.
3) "What happens when gradebook roles are empty? I disagree that is_enrolled() is not enough. The graded roles just mean "show user in gradebook", but we keep grades for all enrolled users. Your approach would create problems when you first enrol without role and then you get student role later."
Not sure I understand what you mean.

Would it have made more sense to have allowed handling of grades for the unenrolled to remain unchanged from 1.9? How much effort is being expended for how much savings in data space? If possible, I'd like to see a realistic analysis of just "rolling back" to where we were and where consensus agrees we should be.

Bob Puffer
added a comment - 08/Mar/11 10:21 PM Would it have made more sense to have allowed handling of grades for the unenrolled to remain unchanged from 1.9? How much effort is being expended for how much savings in data space? If possible, I'd like to see a realistic analysis of just "rolling back" to where we were and where consensus agrees we should be.

Robert, I agree there is a fair amount of flailing going on here trying to get things up to scratch. The practicalities of trying to roll back work on one sub-system of Moodle would likely be even more complex than continuing on this path :|

Update: made a few more minor alterations after chatting with Petr. I've also asked Sam to have another look at this today or tomorrow.

Andrew Davis
added a comment - 10/Mar/11 2:56 PM - edited Ok. I believe Ive dealt with those remaining 3 issues. Ill message Petr to get this looked at again hopefully before the next integration review cycle.
repo: git://github.com/andyjdavis/moodle.git
branch: MDL-25718 _enrol_grades_alternative2
diff: https://github.com/andyjdavis/moodle/compare/master...MDL-25718_enrol_grades_alternative2
Robert, I agree there is a fair amount of flailing going on here trying to get things up to scratch. The practicalities of trying to roll back work on one sub-system of Moodle would likely be even more complex than continuing on this path :|
Update: made a few more minor alterations after chatting with Petr. I've also asked Sam to have another look at this today or tomorrow.

I just noticed something. One of my test users had his grade change after being deleted and then having his grades recovered. This was due to an assignment he previously had no grade for gaining a grade of 0.00 after grade_grab_course_grades() was called.

I think this may be correct. Maybe he should have had a grade of 0.00 before and it just hadnt been pushed to gradebook yet. Alternatively, it could be incorrect.

Either way the students course grade changed as the course was an average and the 0 dragged the students average down. This was a little surprising as I was watching the students grades very closely but isnt necessarily incorrect behaviour. I will test further.

Andrew Davis
added a comment - 10/Mar/11 5:43 PM - edited I just noticed something. One of my test users had his grade change after being deleted and then having his grades recovered. This was due to an assignment he previously had no grade for gaining a grade of 0.00 after grade_grab_course_grades() was called.
I think this may be correct. Maybe he should have had a grade of 0.00 before and it just hadnt been pushed to gradebook yet. Alternatively, it could be incorrect.
Either way the students course grade changed as the course was an average and the 0 dragged the students average down. This was a little surprising as I was watching the students grades very closely but isnt necessarily incorrect behaviour. I will test further.

After doing some more testing there is definitely something funny going on in the lesson module. Even in a lesson the student has never touched calling grade_grab_course_grades() causes a grade of 0.00 to appear in the gradebook. It should be null to avoid affecting grade averages. Ill raise a bug.

Andrew Davis
added a comment - 11/Mar/11 11:13 AM - edited After doing some more testing there is definitely something funny going on in the lesson module. Even in a lesson the student has never touched calling grade_grab_course_grades() causes a grade of 0.00 to appear in the gradebook. It should be null to avoid affecting grade averages. Ill raise a bug.
update: raised MDL-26768

Sorry for late info, just found this while trying to work out what happened to our 2.0 gradebook. We have a scenario that was severely impacted by this issue.

We enrol students into groups based on tutorial timetable allocation, so we use the web-based file upload to make and populate the groups. Students can change tutorials of course, but the web-based upload system doesn't cater for removing users from groups. The easiest method we came up with was to delete everyone from a course with the flatfile plugin and then run the new upload. This worked well in 1.9. Using the same method in 2.0, we found an empty gradebook. (In fact, the new enrolment permissions blocked us from our established proces, so we had to make minor changes to the 2.0 code to make this work the way it used to in 1.9. We know: every time you hack core, God kills a kitten. But we needed a solution for our trial of 2.0 that didn't require lots of hours of coding and it's hopefully not a permanent situation).

Are the solutions above going to help us here, or will we have to re-think our whole user upload process? (Bonus points for suggesting a groups solution )

Marcus Leonard
added a comment - 15/Mar/11 8:57 AM Sorry for late info, just found this while trying to work out what happened to our 2.0 gradebook. We have a scenario that was severely impacted by this issue.
We enrol students into groups based on tutorial timetable allocation, so we use the web-based file upload to make and populate the groups. Students can change tutorials of course, but the web-based upload system doesn't cater for removing users from groups. The easiest method we came up with was to delete everyone from a course with the flatfile plugin and then run the new upload. This worked well in 1.9. Using the same method in 2.0, we found an empty gradebook. (In fact, the new enrolment permissions blocked us from our established proces, so we had to make minor changes to the 2.0 code to make this work the way it used to in 1.9. We know: every time you hack core, God kills a kitten. But we needed a solution for our trial of 2.0 that didn't require lots of hours of coding and it's hopefully not a permanent situation).
Are the solutions above going to help us here, or will we have to re-think our whole user upload process? (Bonus points for suggesting a groups solution )

Andrew Davis
added a comment - 15/Mar/11 11:01 AM Marcus, this fix should help you. Once this has been integrated when you delete a user and re-enroll them their grades will be recovered from the grade history table.

Andrew Davis
added a comment - 21/Mar/11 10:20 AM Following a discussion we're not going to add an option to remove the user's role assignments when they are unenrolled so Ive closed MDL-26813 . Instead they are always going to be removed.
We will then add the ability to recover a user's role assignments in the same way as we are recovering grades. MDL-26907

See the diff link to see where I'm at. https://github.com/andyjdavis/moodle/compare/master...MDL-25718_enrol_grades_alternative2
Ive added the removal of user roles when the user's last active enrollment is suspended. The problem is that right now there is no way aside from manually going in and setting them up for the user to get their roles back if they are unsuspended. That's the point of MDL-26907. The question is whether we're happy for this issue to be integrated with the understanding that role assignment recovery will be brought in later.

Even then theres no way that Im aware of of knowing what role assignments should be reconstructed for a given enrollment ie the user has 2 enrolments, both suspended and 1 is unsuspended.

I'm slowly learning more about enrollments and roles. Some of my findings are below for both myself and others:

On enrol lib/enrollib.php enrol_user() does role assignment ie the enrol code inserts role assignments.

On delete of enrolment lib/enrollib.pp unenrol_user() deletes role assignments.

On suspend of enrolment we're now deleting associated role assignments in /enrol/users.php using role_unassign_all() from accesslib.php

On un-suspend => in future we will reconstruct RAs. Grades arent touched when suspending enrollment.

Andrew Davis
added a comment - 23/Mar/11 2:56 PM - edited See the diff link to see where I'm at. https://github.com/andyjdavis/moodle/compare/master...MDL-25718_enrol_grades_alternative2
Ive added the removal of user roles when the user's last active enrollment is suspended. The problem is that right now there is no way aside from manually going in and setting them up for the user to get their roles back if they are unsuspended. That's the point of MDL-26907 . The question is whether we're happy for this issue to be integrated with the understanding that role assignment recovery will be brought in later.
Even then theres no way that Im aware of of knowing what role assignments should be reconstructed for a given enrollment ie the user has 2 enrolments, both suspended and 1 is unsuspended.
I'm slowly learning more about enrollments and roles. Some of my findings are below for both myself and others:
On enrol lib/enrollib.php enrol_user() does role assignment ie the enrol code inserts role assignments.
On delete of enrolment lib/enrollib.pp unenrol_user() deletes role assignments.
On suspend of enrolment we're now deleting associated role assignments in /enrol/users.php using role_unassign_all() from accesslib.php
On un-suspend => in future we will reconstruct RAs. Grades arent touched when suspending enrollment.

Ok, Ive had a chat with Petr. He's suggested splitting the syncing of roles after enrollment suspension into a separate issue which I have done. MDL-26941. We can either do the role assignment removal centrally using role_unassign_all() (or similar) or add suspend/unsuspend functions to enrolment plugins and make them responsible for their own role assignment management (more flexible but more complex).

Recovery of role assignments after an enrollment is un-suspended is currently meant to be done as part of MDL-26907.

Andrew Davis
added a comment - 23/Mar/11 4:51 PM Ok, Ive had a chat with Petr. He's suggested splitting the syncing of roles after enrollment suspension into a separate issue which I have done. MDL-26941 . We can either do the role assignment removal centrally using role_unassign_all() (or similar) or add suspend/unsuspend functions to enrolment plugins and make them responsible for their own role assignment management (more flexible but more complex).
Recovery of role assignments after an enrollment is un-suspended is currently meant to be done as part of MDL-26907 .
Trying to avoid this issue getting any bigger than it is already.

After a lot of chatter I think Im ok to call this resolved as the remaining work has all been pushed into other MDLs and require more consideration as to exactly how we're going to fix the remaining problems.

Andrew Davis
added a comment - 24/Mar/11 2:46 PM After a lot of chatter I think Im ok to call this resolved as the remaining work has all been pushed into other MDLs and require more consideration as to exactly how we're going to fix the remaining problems.
PULL-518

Rejecting for the same reasons like last week PULL-421, I think the part for "recovery of grades when enrolling" is ok and could be integrated, but I believe the "suspend x unenrol" part needs more work.

Andrew Davis
added a comment - 29/Mar/11 11:34 AM This has been rejected by Petr once again. The reasons in PULL-518 are below:
Please file two separate PULLs for:
1/ grade recovery during enrol
2/ suspend and unenrolment improvement
the role assignments history/recovery would be 3/
Rejecting for the same reasons like last week PULL-421, I think the part for "recovery of grades when enrolling" is ok and could be integrated, but I believe the "suspend x unenrol" part needs more work.

Andrew Davis
added a comment - 31/Mar/11 3:24 PM - edited Petr, the reasons in PULL-421 were discussed throughout the week (with you). It was my understanding they were going to be done in MDL-26941 and MDL-26907 . NOT in this issue.

Andrew Davis
added a comment - 08/Apr/11 3:22 PM After yet more discussion I have stripped out the changes that made the process to suspend enrollments more obvious and am again marking this resolved.
Pull requests for 2.0 stable and the 2.1 development branches.
PULL-605
PULL-606

I filed a new tracker because grade recovery is possible, but just on manual enrollments. When using cohorts or metacourses for enrolment there's no possibility for grade recovery. I would really like to see this functionality being added to Moodle.

Richard van Iwaarden
added a comment - 13/Nov/12 11:27 PM - edited Sorry for bringing this issue up after so long, but I filed a new tracker regarding this issue:
http://tracker.moodle.org/browse/MDL-36024
I filed a new tracker because grade recovery is possible, but just on manual enrollments. When using cohorts or metacourses for enrolment there's no possibility for grade recovery. I would really like to see this functionality being added to Moodle.

I see that this has been heavily discussed, and I as Richard said, I also hate to bring this issue up after so long. However, I believe this may be a similar issue I've recently seen with 2.4.5 recently. I could be wrong, and please let me know if I should submit a new tracker ticket, or if Moodle is behaving normally. However, I do not feel that Moodle is behaving normally – as this could possibly only be an issue with quiz and gradebook as that's the only way I've seen it.

What I've seen recently on 2 different sites: students are un-enrolled, accidentally, grades maintain within the quiz once the student is re-enrolled. However, the grade does not seem to populate back into the gradebook. I have found a workaround by going into the quiz, selecting to regrade all attempts. This seems to repopulate all grades back into the gradebook. However, without this step grades are not visible within the gradebook upon re-enrollment.

Tayla Craig
added a comment - 24/Aug/13 4:37 AM - edited I see that this has been heavily discussed, and I as Richard said, I also hate to bring this issue up after so long. However, I believe this may be a similar issue I've recently seen with 2.4.5 recently. I could be wrong, and please let me know if I should submit a new tracker ticket, or if Moodle is behaving normally. However, I do not feel that Moodle is behaving normally – as this could possibly only be an issue with quiz and gradebook as that's the only way I've seen it.
What I've seen recently on 2 different sites: students are un-enrolled, accidentally, grades maintain within the quiz once the student is re-enrolled. However, the grade does not seem to populate back into the gradebook. I have found a workaround by going into the quiz, selecting to regrade all attempts. This seems to repopulate all grades back into the gradebook. However, without this step grades are not visible within the gradebook upon re-enrollment.
Please advise.
Thank you!

Andrew Davis
added a comment - 26/Aug/13 12:57 AM Hello. I believe this issue has been fixed in MDL-36024 however it appears that the fix only went in Moodle 2.5 and later. Is anyone running 2.5+ able to verify that this is resolved?