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#1846 Regression (sorta) Hacks to avoid failure on upgrade due to new field not yet being in the DB #17722

Merged
merged 5 commits into from Jul 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions CRM/Core/BAO/CustomField.php
Expand Up @@ -278,6 +278,7 @@ public static function buildOptions($fieldName, $context = NULL, $props = []) {
*
* @return array
* an array of active custom fields.
* @throws \CRM_Core_Exception
*/
public static function &getFields(
$customDataType = 'Individual',
Expand Down Expand Up @@ -392,6 +393,10 @@ public static function &getFields(
if ($onlyParent) {
$extends .= " AND $cgTable.extends_entity_column_value IS NULL AND $cgTable.extends_entity_column_id IS NULL ";
}
// Temporary hack - in 5.27 a new field is added to civicrm_custom_field. There is a high
// risk this function is called before the upgrade page can be reached and if
// so it will potentially result in fatal error.
$serializeField = CRM_Core_BAO_Domain::isDBVersionAtLeast('5.27.alpha1') ? "$cfTable.serialize," : '';

$query = "SELECT $cfTable.id, $cfTable.label,
$cgTable.title,
Expand All @@ -410,7 +415,7 @@ public static function &getFields(
$cfTable.date_format,
$cfTable.time_format,
$cgTable.is_multiple,
$cfTable.serialize,
$serializeField
$cgTable.table_name,
og.name as option_group_name
FROM $cfTable
Expand Down Expand Up @@ -493,7 +498,7 @@ public static function &getFields(
$fields[$dao->id]['is_required'] = $dao->is_required;
$fields[$dao->id]['table_name'] = $dao->table_name;
$fields[$dao->id]['column_name'] = $dao->column_name;
$fields[$dao->id]['serialize'] = $dao->serialize;
$fields[$dao->id]['serialize'] = $serializeField ? $dao->serialize : (int) self::isSerialized($dao);
$fields[$dao->id]['where'] = $dao->table_name . '.' . $dao->column_name;
// Probably we should use a different fn to get the extends tables but this is a refactor so not changing that now.
$fields[$dao->id]['extends_table'] = array_key_exists($dao->extends, CRM_Core_BAO_CustomQuery::$extendsMap) ? CRM_Core_BAO_CustomQuery::$extendsMap[$dao->extends] : '';
Expand Down Expand Up @@ -2543,7 +2548,7 @@ public static function isSerialized($field) {
if ($html_type === 'CheckBox' || strpos($html_type, 'Multi') !== FALSE) {
return TRUE;
}
// Otherwise this is the new standard as of 5.26
// Otherwise this is the new standard as of 5.27
return is_object($field) ? !empty($field->serialize) : !empty($field['serialize']);
}

Expand Down
23 changes: 23 additions & 0 deletions CRM/Core/BAO/Domain.php
Expand Up @@ -82,6 +82,29 @@ public static function version($skipUsingCache = FALSE) {
);
}

/**
* Is a database update required to apply latest schema changes.
*
* @return bool
*
* @throws \CRM_Core_Exception
*/
public static function isDBUpdateRequired() {
$dbVersion = CRM_Core_BAO_Domain::version();
$codeVersion = CRM_Utils_System::version();
return version_compare($dbVersion, $codeVersion) < 0;
}

/**
* Checks that the current DB schema is at least $min version
*
* @param string|number $min
* @return bool
*/
public static function isDBVersionAtLeast($min) {
return version_compare(self::version(), $min, '>=');
}

/**
* Get the location values of a domain.
*
Expand Down
6 changes: 2 additions & 4 deletions CRM/Utils/Check/Component/Env.php
Expand Up @@ -739,10 +739,8 @@ public function checkDbVersion() {
);
}
else {
$codeVersion = CRM_Utils_System::version();

// if db.ver < code.ver, time to upgrade
if (version_compare($dbVersion, $codeVersion) < 0) {
if (CRM_Core_BAO_Domain::isDBUpdateRequired()) {
$messages[] = new CRM_Utils_Check_Message(
__FUNCTION__,
ts('New codebase version detected. You must visit <a href=\'%1\'>upgrade screen</a> to upgrade the database.', [1 => $upgradeUrl]),
Expand All @@ -753,7 +751,7 @@ public function checkDbVersion() {
}

// if db.ver > code.ver, sth really wrong
if (version_compare($dbVersion, $codeVersion) > 0) {
if (version_compare($dbVersion, CRM_Utils_System::version()) > 0) {
$messages[] = new CRM_Utils_Check_Message(
__FUNCTION__,
ts('Your database is marked with an unexpected version number: %1. The v%2 codebase may not be compatible with your database state.
Expand Down
4 changes: 4 additions & 0 deletions CRM/Utils/Check/Component/Schema.php
Expand Up @@ -94,6 +94,10 @@ public function checkMissingLogTables() {
* @return array
*/
public function checkSmartGroupCustomFieldCriteria() {
if (CRM_Core_BAO_Domain::isDBUpdateRequired()) {
// Do not run this check when the db has not been updated as it might fail on non-updated schema issues.
return [];
}
$messages = $problematicSG = [];
$customFieldIds = array_keys(CRM_Core_BAO_CustomField::getFields('ANY', FALSE, FALSE, NULL, NULL, FALSE, FALSE, FALSE));
try {
Expand Down
2 changes: 1 addition & 1 deletion api/v3/CustomField.php
Expand Up @@ -122,7 +122,7 @@ function civicrm_api3_custom_field_delete($params) {
* @return array
*/
function civicrm_api3_custom_field_get($params) {
if (($params['legacy_html_type'] ?? TRUE) && !empty($params['return'])) {
if (CRM_Core_BAO_Domain::isDBVersionAtLeast('5.27.alpha1') && ($params['legacy_html_type'] ?? TRUE) && !empty($params['return'])) {
if (is_array($params['return'])) {
$params['return'][] = 'serialize';
}
Expand Down
2 changes: 1 addition & 1 deletion sql/test_data_second_domain.mysql
Expand Up @@ -963,4 +963,4 @@ INSERT INTO civicrm_navigation
VALUES
( @domainID, CONCAT('civicrm/report/instance/', @instanceID,'&reset=1'), 'Mailing Detail Report', 'Mailing Detail Report', 'administer CiviMail', 'OR', @reportlastID, '1', NULL, @instanceID+2 );
UPDATE civicrm_report_instance SET navigation_id = LAST_INSERT_ID() WHERE id = @instanceID;
UPDATE civicrm_domain SET version = '4.6.alpha1';
UPDATE civicrm_domain SET version = '5.27.alpha1';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seamuslee001 I tackled this in e37554a

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seamuslee001 and @totten tackled it in #17734

2 changes: 1 addition & 1 deletion xml/schema/Core/CustomField.xml
Expand Up @@ -290,7 +290,7 @@
<pseudoconstant>
<callback>CRM_Core_SelectValues::fieldSerialization</callback>
</pseudoconstant>
<add>5.26</add>
<add>5.27</add>
</field>
<field>
<name>filter</name>
Expand Down