Database schema inconsistent with clean install

Question

Database schema inconsistent with clean install

aspark21 opened this issue 4 years ago · 24 comments

Run: php admin/cli/check_database_schema.php

Database schema inconsistent with clean install. Running this for a core issue in 3.9 where there were some steps missing from the db/upgrade.php compared to the install.xml (MDL-69049) we have picked up on the following inconsistencies with this plugin:

-------------------------------------------------------------------------------
plagiarism_turnitin_files
 * column 'student_read' should allow NULL (I)
-------------------------------------------------------------------------------
plagiarism_turnitin_users
 * column 'userid' should be NOT NULL (I)
 * column 'turnitin_uid' should be NOT NULL (I)
 * column 'turnitin_utp' should be NOT NULL (I)
-------------------------------------------------------------------------------
plagiarism_turnitin_courses
 * column 'courseid' should be NOT NULL (I)
 * column 'ownerid' should be NOT NULL (I)
 * column 'turnitin_ctl' should be NOT NULL (X)
 * column 'turnitin_cid' should be NOT NULL (I)

Answer 1 · 2020-07-22T03:02:39.000Z

Yeah, looks like this inconsistence comes from these commits:
8148c47
21883cf

The plagiarism_turnitin_files table is fixed in this commit:
24c781a

Answer 2 · 2020-09-14T11:47:54.000Z

@aspark21 - see #542

Answer 3 · 2020-09-17T00:20:47.000Z

Sorry, haven't really had a chance to go through this in any detail and about to go on leave but this seems to roughly be doing what's needed - i.e. either fix install.xml or upgrade.php for fields and then have an upgrade script which cleans things up.

Would it be worth us upgrading on one of our test sites?
If so, @cybernotic can pick up while I'm away

Answer 4 · 2020-10-26T20:29:07.000Z

I've just run an upgrade with develop branch of plagiarism plugin (+ PR 553 which is not relevant to this issue but is for our own pre-3.9 testing)

plagiarism_turnitin_users

column 'userid' should be NOT NULL (I)

plagiarism_turnitin_courses

column 'courseid' should be NOT NULL (I)

Answer 5 · 2020-10-26T20:43:32.000Z

Just had a look back at https://github.com/turnitin/moodle-plagiarism_turnitin/pull/542/files#diff-fcb317575e9e6245fdf1590e51a86685b61c2cc9573a9df8034323ccb3ef614fR447 and it does seem like there was no upgrade steps for those 2 fields.

Might have been intentional as I can see it was reverted here - e009474

Answer 6 · 2021-05-24T01:26:04.000Z

Yeah, I got the same @aspark21
userid and courseid columns still allow NULLs because there was no update step to alter then, just e009474 that changes existing upgrade steps which are never called twice, so this didin't work for existing sites.
I'm happy to make a new PR for to fix remaning columns @jmcgettrick

Answer 7 · 2021-05-24T03:21:02.000Z

Hey @aspark21 @jmcgettrick could you please check if #585 would work for you?
I had to drop and recreate a unique index in these columns because moodle has a dependency check and throws column "plagiarism_turnitin_users->userid" cannot be modified. Dependency found with index "mdl_plagturnuser_use_uix (userid)"

Answer 8 · 2021-06-07T17:03:59.000Z

I'll have a look

Answer 9 · 2021-07-09T02:44:46.000Z

Hi @aspark21
Have you had a chance to have a look?

Answer 10 · 2021-10-17T23:38:56.000Z

Hey @aspark21 @jmcgettrick
Is there a chance you could have a look at #585 ?
I've rebased my pull branch.

Cheers,
Mikhail

Answer 11 · 2021-12-07T00:18:52.000Z

Hi @aspark21 @jmcgettrick

Please feel free to let me know if there anything I can help with this one.
The PR seems straightforward, but if there is anything you'd want to discuss please let me know.
I'm looking forward to seeing this integrated.

Kind regards,
Misha

Answer 12 · 2021-12-07T02:04:29.000Z

Ah yes sorry this dropped off my radar, I'll try and look at it this week.

Answer 13 · 2021-12-19T23:23:19.000Z

Thank you @aspark21

Answer 14 · 2022-01-14T05:07:58.000Z

Hi @aspark21

Sorry for chasing you, but we have an open ticket wich I can't close untill this is fixed. Is there any chance to have a look at the issue? If there anything that needs to be changed in the PR I'm happy to do so.

Kind regards,
Misha

Answer 15 · 2022-01-16T17:56:23.000Z

I've finally taken a look apologies for delay.

Starting point:
Moodle 3.9.11+ & plagiarism_turnitin 2021060801

-------------------------------------------------------------------------------
plagiarism_turnitin_users
 * column 'userid' should be NOT NULL (I)
-------------------------------------------------------------------------------
plagiarism_turnitin_courses
 * column 'courseid' should be NOT NULL (I)
 * column 'ownerid' is not expected (I)
 -------------------------------------------------------------------------------

Upgraded to latest plagiarism_turnitin origin master (2021091501)

-------------------------------------------------------------------------------
plagiarism_turnitin_users
 * column 'userid' should be NOT NULL (I)
-------------------------------------------------------------------------------
plagiarism_turnitin_courses
 * column 'courseid' should be NOT NULL (I)
-------------------------------------------------------------------------------

Upgraded to golenkovm/issue517_v2

No more warnings from
php admin/cli/check_database_schema.php

Unfortunately John has left Turnitin so it looks like @dominicgunn or @stvleung as the current owners of the Turnitin Github will have to review instead

Answer 16 · 2022-01-16T18:03:08.000Z

Can always try @dwinn but is a long shot, he might also have left.

Answer 17 · 2022-01-25T04:00:58.000Z

Thanks @aspark21 for having a look

Answer 18 · 2022-01-25T04:01:52.000Z

Hey @dominicgunn @stvleung @dwinn

Can somebody please merge this PR?

Kind regards,
Misha

Answer 19 · 2022-03-27T23:11:26.000Z

Hi @dominicgunn @stvleung @dwinn

Is there a chance you could have a look at this PR? Thanks in advance. Appreciate this.

Kind regards,
Misha

Answer 20 · 2022-05-03T01:08:04.000Z

Hi @dominicgunn @stvleung @dwinn

Please feel free to let me know if I can do anything to help you with this PR. Appreciate this.

Kind regards,
Misha

Answer 21 · 2022-07-18T05:20:03.000Z

Please, anyone @dominicgunn @stvleung @dwinn

Answer 22 · 2022-07-18T06:28:16.000Z

#585 has been rebased.
I also created #627 for develop branch.

Answer 23 · 2022-07-18T09:04:10.000Z

Hi @golenkovm ,
Thank you for reporting the issue and the pull request, we will try to get this released as soon as we can.

Answer 24 · 2024-07-24T18:07:06.000Z

Thank you for reporting this. Because the latest version of the plagiarism plugin is supported for versions of Moodle 4.1 and higher, I am closing this ticket. However, if you find this issue is occurring with the latest version of the plugin in any of the supported Moodle versions, please create a new ticket and we will address it.