jpahullo / moodle-tool_mergeusers

Merge users script for Moodle
https://moodle.org/plugins/view.php?plugin=tool_mergeusers
22 stars 50 forks source link

Is "DELETE FROM mdl_grade_grades" the correct thing to do? #52

Open luisdev opened 10 years ago

luisdev commented 10 years ago

Using moodle-tool_mergeusers $plugin->version = 2014070900;

I merged profile 43493 into profile 44175 on my test site and I noticed the following DELETE statement on the results page, /admin/tool/mergeusers/index.php:

DELETE FROM mdl_grade_grades WHERE id IN (580471, 580470, 580909, 580875, 580828, 580640, 580677, 577710, 577709, 577854, 577713, 577798, 577801, 577858, 577805, 577816, 580469)

Why is the plugin DELETING records from mdl_grade_grades?

Surely we want to keep those grade_grades records and move them onto the profile during the merge, instead of deleting them? In other words, shouldn't it be doing a UPDATE instead of a DELETE?

Example: UPDATE mdl_grade_grades SET userid = WHERE id IN (580471, 580470, 580909, 580875, 580828, 580640, 580677, 577710, 577709, 577854, 577713, 577798, 577801, 577858, 577805, 577816, 580469)

My test Moodle site is running on a Windows Server 2008 R2 virtual machine with: IIS 7.5 PHP 5.5.12 NTS MSSQL 2008 R2 and Moodle $release = '2.5.6+ (Build: 20140529)';.

jpahullo commented 10 years ago

HI @luisdev

You should have into account that both users may have records for the same compound index, so that in those cases you have to remove some of them (check MergeUserTool:mergeCompoundIndex: https://github.com/ndunand/moodle-tool_mergeusers/blob/master/lib/mergeusertool.php#L543 ).

To actually answer your question, you should verify that there are either deletes and updates to grade_grades table. In other words, that both users have records for the same compound index https://github.com/ndunand/moodle-tool_mergeusers/blob/master/config/config.php#L56 before merging.

Saludos,

Jordi

luisdev commented 10 years ago

What columns make up that compound index?

jpahullo commented 10 years ago

Hi Luis,

I do no understand your question. But, if you are refering to the colums that compound the index on grade_grades, you can:

  1. Check the union of all column name appearing on 'userfield' and 'otherfields' of the configuration setting 'compoundindexes' from https://github.com/ndunand/moodle-tool_mergeusers/blob/master/config/config.php#L56. In this case, 'userid' and 'itemid', and
  2. In general, check your database schema and Moodle assumptions in code to confirm that.

Did I help you?

Saludos,

Jordi

jpahullo commented 10 years ago

@luisdev , are all things clear? Could we close this issue?

Thanks in advance!

Jordi

luisdev commented 10 years ago

Umm... Let me test it again (and work out what those compound indexes mean).

luisdev commented 10 years ago

Ok, I'm still trying to understand those compound indexes. What happens if one user has duplicate profiles in Moodle and this happens:

Logs in on 10 February 2014 with the mdl_user.id = 123 profile and completes "Quiz_ABC" in Course 12345 and gets 50%.

Then he logs in again on 11 February 2014, this time with the mdl_user.id = 789 profile, and completes the same "Quiz_ABC" in Course 12345 again and this time gets 80%.

In other words:

Before the profile merge the gradebook will show the two different quiz grades under the two different profiles. But what grade will Moodle show in the gradebook for the "profile to keep" AFTER the merge?

The Grade Report must still reflect BOTH the quiz attempts, but they must now both be linked to the "profile to keep".

Do you agree? Is that what happens after you merge the profiles?

luisdev commented 10 years ago

Does or should the Merge script use the grading method for the quiz to determine which of the two quiz attempt results should be displayed in the gradebook for the "profile to keep" after the merge (i.e. Last Attempt, Highest Grade, etc.)?

jpahullo commented 10 years ago

I think we are mixing things: This issue was related to mdl_grade_grades, but in your last comments you referred to quiz_attempts.

quiz_attempts table has also a compound index (user, quiz and attempt, if I remember well). If attempts for a given quiz and course were made by only one user, they are maintained.

However, if attempts for the scenario you are talking about, this tool provides the following options (seee README):

Special case # 10: quiz_attempts table has a three-field unique index, including userid, quiz and attempt. This table and related quiz_grades and quiz_grades_history are processed as specified in the plugin settings. This plugin provides you several options when merging quiz attempts from two users:

  1. Merge attempts from both users and renumber. Attempts from the old user are merged with the ones of the new user and renumbered by the time they were started.
  2. Keep attempts from the new user. Attempts from the old user are removed. Attempts from the new user are kept, since this option considers them as the most important.
  3. Keep attempts from the old user. Attempts from the new user are removed. Attempts from the old user are kept, since this option considers them as the most important.
  4. Do nothing: do not merge nor delete (by default). Attempts are not merged nor deleted, remaining related to the user who made them. This is the most secure action, but merging users from user A to user B or B to A may produce different quiz grades.

You're are proposing somehow a wizard with which a Moodle administrator (according to the criteria of teacher of the given course?) could check (one by one?) which attempts to maintain per quiz and course.

The above options (showed in the README) provide automatic options for merging. By now, they are sufficient for the current use. If you like to have additional manual merging options, you could propose it in a sepparated issue.

Hoping this helps!

Saludos,

Jordi

luisdev commented 10 years ago

Sorry, I'm still trying to get me head around this... Let's take this example:

Student_123 (mdl_user.id = 123) has completed an activity, quiz_x (mdl_quiz.id = 555), on 1 May 2014. That quiz attempt has a corresponding record in the mdl_grade_grades table for that grade that he obtained in that quiz.

The same student has completed the SAME activity (quiz_x, mdl_quiz.id = 555) on a different Moodle profile as Student_987 (mdl_user.id = 987) on a different date, 1 June 2014.

So you have two quiz attempts for the same quiz done under two Moodle profiles on two different dates. What happens after the merge? Is the profile_to_keep now still linked to both the two quiz attempts?

And what result does the gradebook show for that quiz attempt? Is the option that has been selected as Grading method in the quiz settings used to determine which of the two attempts shows in the gradebook?

If I'm just being stupid and the above scenario does not result in any data loss during the merge then you can close this issue. :-)

jpahullo commented 10 years ago

I think I got you are referring to.

Let me think about it and I'll be back then.

Jordi

anieminen commented 8 years ago

Hi,

I came across this issue when merging two user accounts recently:

I noticed that the wrong grade in gradebook can be "fixed" simply by editing the user's grade and saving changes in the gradebook / grader report. After that also the gradebook shows correct grade 80 for user B for the activity.

So I think the problem here is only related to gradebook. And maybe it could be resolved by triggering some update to the gradebook after merging mdl_grade_grades table? Quiz activity is a bit more complicated case, but I've had no trouble merging quiz attempts. Quiz activity uses the selected grading method in quiz settings to determine the final quiz grade, which is then shown in the gradebook.

jpahullo commented 5 years ago

I think that regrading quizzes as the plugin does, it fixes the problem on grading quizzes and the scenario that @luisdev exposed before.

However, this issue still applies for the general gradebook. And there are other open issues related to the gradebook too.

We should face somehow how to deal with regrading after merging users.