jpahullo / moodle-tool_mergeusers

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

Problem concerning the plugin update to Moodle 4.1 #243

Closed xDavox21 closed 1 year ago

xDavox21 commented 1 year ago

Hi, the problem I'm having is with the merge method, declared inside the DuplicatedData interface. This method is called inside the GenericDuplicatedDataMerger class, but it has 3 parameters inside ($data, $fromuser, $touser), while in the base method it is declared with only two parameters. I noticed this error while upgrading the plugin to build 2023040401 (for Moodle 4.1), I'm using PHP 8.1. Also I noticed that the GenericDuplicatedDataMerger class is not being used. So I wanted to ask if this error concerns the PHP version I'm using, and consequently change something, or can i ignore the error and update the plugin? Kind regards.

jpahullo commented 1 year ago

Thanks @xDavox21 for your feedback.

All existing tests passed and I didn't face with this problem.

Let me have some time to investigate.

Thanks,

Jordi

jpahullo commented 1 year ago

Hi @xDavox21,

I set up a clean Moodle 4.1 and php 8.1 locally. I build a test course, and merged two users. All went ok as you can see:

Merge user accounts
1. Choose users to merge / ► 2. Confirm users to merge / ► 3. Merging results and log

Merged «tool_generator_000001» (user ID = 3) into «tool_generator_000002» (user ID = 4)

For further reference, these results are recorded in the log id 1.
Here are the queries that have been sent to the DB:

For logging or security reasons we are skipping my_pages, user_info_data, user_preferences, user_private_key.
To remove these entries, delete the old user once this script has run successfully.
Started merging at Thursday, 13 April 2023, 10:18 PM
UPDATE m_favourite SET userid = '4' WHERE id IN (1)
UPDATE m_forum_posts SET userid = '4' WHERE id IN (14)
UPDATE m_logstore_standard_log SET relateduserid = '4' WHERE id IN (166, 266, 267)
UPDATE m_message_conversation_members SET userid = '4' WHERE id IN (1)
User data from table quiz_attempts, quiz_grades, quiz_grades_history are not updated.
DELETE FROM m_role_assignments WHERE id IN (1)
DELETE FROM m_user_enrolments WHERE id IN (1)
Finished merging at Thursday, 13 April 2023, 10:18 PM
Merge took 0 seconds

Merge successful

Could you please share the log you faced and how you get that log with that notice? I could't reproduce your PHP notice.

I'll go anyway to check that method signature and see what happens.

Thanks,

Jordi

jpahullo commented 1 year ago

I can see that currently GenericDuplicatedDataMerger is not used anywhere. And detected the API inconsistence.

So, next release I will remove this class. And I'll do for both latest plugin versions.

With this, I think, we will solve that issue.

Thanks,

Jordi

jpahullo commented 1 year ago

pushed new branches, waiting for CI. If all went ok, I'll create new tags.

jpahullo commented 1 year ago

Released new versions without that class.

Closing as fixed.

xDavox21 commented 1 year ago

Thanks for your response!

jpahullo commented 1 year ago

Thanks to you, @xDavox21 !