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

dev/core#2961 fix upgrade bug on civicrm_note + logging #22150

Closed
wants to merge 1 commit into from

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Nov 28, 2021

Overview

dev/core#2961 fix upgrade bug on civicrm_note + logging

Before

Reported issues with upgrade failure on 5.37 upgrade - these appear to be because we create a new field and then attempt to populate it before taking any action that would cause the corresponding log table field to be created

After

Create the new field in the log table straight after the field is created via the script

Technical Details

Normally we reconcile logging tables are the end - however in this case we try to populate it straight away ....

I did think we turned off logging before trying to upgrade?

ALTER TABLE civicrm_note ADD COLUMN `note_date` timestamp NULL  DEFAULT CURRENT_TIMESTAMP COMMENT 'Date attached to the note';
ALTER TABLE civicrm_note ADD COLUMN `created_date` timestamp NULL  DEFAULT CURRENT_TIMESTAMP COMMENT 'When the note was created';
UPDATE civicrm_note SET note_date = modified_date, created_date = modified_date, modified_date = modified_date;

Comments

https://lab.civicrm.org/dev/core/-/issues/2961

@civibot
Copy link

civibot bot commented Nov 28, 2021

(Standard links)

@colemanw
Copy link
Member

This looks like the right fix, but probably should be against the RC branch?

@eileenmcnaughton
Copy link
Contributor Author

@colemanw yeah - it's not a very recent regression though

@eileenmcnaughton
Copy link
Contributor Author

test this please

@totten
Copy link
Member

totten commented Dec 2, 2021

@seamuslee001 and I both agreed the concept here is good. However, the test failures do seem relevant:

    CivicrmUpgradeTest.4_6_0-setupsh_sql_bz2
    CivicrmUpgradeTest.4_6_11-setupsh_sql_bz2
    CivicrmUpgradeTest.4_6_36-setupsh_sql_bz2
    CivicrmUpgradeTest.4_7_0-setupsh_sql_bz2

I think if we can fix this early during RC cycle, then we can switch it to RC.

@eileenmcnaughton
Copy link
Contributor Author

closing for now - needs work

@J0WI
Copy link
Contributor

J0WI commented Mar 17, 2022

Thanks, this fixed the upgrade to 5.37 for me.

@eileenmcnaughton eileenmcnaughton deleted the log_up branch March 17, 2022 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants