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

SavedSearch: add UI_name index to upgrade script #18811

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

colemanw
Copy link
Member

Overview

PR #18809 added 2 fields and an index to the schema. It also added an upgrader for the 2 fields but forgot the index. This adds the index to the upgrader.

Before

Missing index from upgrade.

After

Index added on upgrade.

Technical Details

Extends the CRM_Core_BAO_SchemaHandler::createIndexes() function to add the keyword "UNIQUE" when the prefix is "UI" (by convention we use "UI" to stand for "Unique Index").

@civibot
Copy link

civibot bot commented Oct 20, 2020

(Standard links)

@civibot civibot bot added the master label Oct 20, 2020
@colemanw
Copy link
Member Author

@eileenmcnaughton what do you think of this solution?

@eileenmcnaughton
Copy link
Contributor

hmm - I'd like @totten to weigh in - he was involved in that scoping

@totten
Copy link
Member

totten commented Oct 21, 2020

Patch seems reasonable. I'll give it an r-run.

@totten
Copy link
Member

totten commented Oct 21, 2020

OK, so I took schema snapshots from four situations

  1. Install 5.29
  2. Install 5.29 and upgrade to master@head
  3. Install 5.29 and upgrade to master+this PR
  4. Install master@head

I can confirm this fixes the upgrade -- adding the same UNIQUE KEY UI_name (name) that appears in master@head. So we can merge.

Tangentially, there are still a couple other discrepancies comparing 3 and 4:

--- table civicrm_acl_cache (5.29 upgraded to master)
+++ table civicrm_acl_cache (new master)
-KEY `FK_civicrm_acl_cache_contact_id` (`contact_id`),
+KEY `index_contact_id` (`contact_id`),

--- table civicrm_acl_contact_cache (5.29 upgraded to master)
+++ table civicrm_acl_contact_cache (new master)
-KEY `FK_civicrm_acl_contact_cache_contact_id` (`contact_id`)

--- table civicrm_event (5.29 upgraded to master)
+++ table civicrm_event (new master)
-`selfcancelxfer_time` int(11) DEFAULT NULL,
+`selfcancelxfer_time` int(11) DEFAULT '0' COMMENT 'Number of hours prior to event start date to allow self-service cancellation or transfer.',

--- table civicrm_mail_settings (5.29 upgraded to master)
+++ table civicrm_mail_settings (new master)
-  `is_non_case_email_skipped` tinyint(4) NOT NULL DEFAULT '0' COMMENT 'Skip emails which do not have a Case ID or Case hash',
-  `is_contact_creation_disabled_if_no_match` tinyint(4) NOT NULL DEFAULT '0' COMMENT 'If this option is enabled, CiviCRM will not create new contacts when filing emails',
+  `is_non_case_email_skipped` tinyint(4) DEFAULT '0' COMMENT 'Enabling this option will have CiviCRM skip any emails that do not have the Case ID or Case Hash so that the system will only process emails that can be placed on case records. Any emails that are not processed will be moved to the ignored folder.',
+  `is_contact_creation_disabled_if_no_match` tinyint(4) DEFAULT '0',

@totten totten merged commit f5fcfe4 into civicrm:master Oct 21, 2020
@eileenmcnaughton eileenmcnaughton deleted the savedSearchIndex branch October 21, 2020 01:34
@colemanw
Copy link
Member Author

@totten what to do about the other anomalies?

@colemanw
Copy link
Member Author

@totten I feel like if we started digging we'll find tons of discrepancies between the installer and the upgraders. An auto-reconcile script would be nice.

@eileenmcnaughton
Copy link
Contributor

@totten @colemanw it might be a good thing for another day if the differences are not material (e.g defaults on fields)

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.

3 participants