Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/main/8929 migration issues #8970

Closed

Conversation

jonasraoni
Copy link
Contributor

No description provided.

@jonasraoni jonasraoni force-pushed the bugfix/main/8929-migration-issues branch 2 times, most recently from fd2c8f1 to 7e76dd6 Compare May 10, 2023 09:37
@jonasraoni jonasraoni force-pushed the bugfix/main/8929-migration-issues branch from 7e76dd6 to c303da4 Compare May 18, 2023 06:13
@jonasraoni jonasraoni linked an issue May 18, 2023 that may be closed by this pull request
@jonasraoni jonasraoni force-pushed the bugfix/main/8929-migration-issues branch 4 times, most recently from ecb9751 to 9917c92 Compare May 24, 2023 12:16
@jonasraoni
Copy link
Contributor Author

@asmecher, I'm not sure if the tests will pass, but I'm not going to do extra changes after the last commit, only fixes.

I'll try to explain briefly what I did.

  1. First I executed a generic query to find missing FKs against OJS 3.4 and created all of them.
  2. I didn't want to validate manually the existing cleanup code (too error prone), so using this model/ready database, I wrote a query to generate the PHP code to cleanup the foreign keys (if the field is NOT NULL > delete records, otherwise it depends), which resulted in extra cleanups.
  3. Then I compared this code against the previous cleanup, mostly to collect exceptions:
  • submission.current_publication_id is physically nullable, but conceptually required
  • Special values, such as library_files.submission_id = 0
  • ...
  1. On top of that I've added other minor fixes (handle a missing issueId at the publication_settings, attempt to grab a missing submission.locale from the publications table, etc.)
  2. Sorting the cleanup operations manually didn't give me confidence that things will work. So I wrote another query to collect the foreign key relationships, and used this to process the operations considering the "side-effects". It might re-run some cleanups, but it's better safe than sorry.

About the extra foreign keys, I've added another migration to handle them at the end, I searched fast and found at least one migration making use of the special value 0.

@jonasraoni jonasraoni force-pushed the bugfix/main/8929-migration-issues branch 4 times, most recently from 4096b4a to 97cc242 Compare May 25, 2023 14:24
@jonasraoni
Copy link
Contributor Author

@asmecher As the tests passed, I've created and assigned the remaining PRs to the issue:
image

@jonasraoni
Copy link
Contributor Author

jonasraoni commented May 25, 2023

I just did a migration for Coalition Publica and it worked fine, after fixing the bug below (the field should be connected with the authors table instead of users), I'll include a fix for it as well:
https://github.com/pkp/ojs/blob/89363cae4a766851e2670db1a2cc18c2624bfdcb/classes/migration/upgrade/v3_4_0/I6093_AddForeignKeys.php#L91

I think it's worth to:

  • Double check the foreign key setup on the migrations (only the ones responsible to create entities), to ensure everything is well wired
  • Upgrade an installation from OJS 3.1 and compare their database structure, they should be identical.

@jonasraoni jonasraoni force-pushed the bugfix/main/8929-migration-issues branch 3 times, most recently from 80ef387 to f9901b4 Compare June 6, 2023 08:06
@jonasraoni jonasraoni force-pushed the bugfix/main/8929-migration-issues branch from f9901b4 to aace490 Compare June 6, 2023 10:57
@jonasraoni
Copy link
Contributor Author

jonasraoni commented Jun 6, 2023

Superseded by #9071

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors while upgrading from OJS-3.3.0-14 to OJS-3.4.0rc2
2 participants