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

[REF] Depends on civicrm-core#25414 Output pre-upgrade message when u… #663

Open
wants to merge 1 commit into
base: 7.x-master
Choose a base branch
from

Conversation

seamuslee001
Copy link
Contributor

…pgrading civicrm db using drush

ping @demeritcowboy @totten

@civibot
Copy link

civibot bot commented Jan 25, 2023

(Standard links)

@totten
Copy link
Member

totten commented Feb 1, 2023

I ran this in combination with the core PR, with an upgrade from 5.51=>master[5.59ish]. Here's how the output looked:

[bknix-min:~/bknix/build/dmaster/web/sites/all/modules/civicrm] drush cvupdb
Pre Upgrade Message:
<br />The default copies of the message templates listed below will be updated to handle 
new features or correct a problem. Your installation has customized versions of these 
message templates, and you will need to apply the updates manually after running this upgrade. 
<a href='https://docs.civicrm.org/user/en/latest/email/message-templates/#modifying-system-workflow
-message-templates' style='color:white; text-decoration:underline; font-weight:bold;' target='_blank'>
Click here</a> for detailed instructions. <ul><li><em>Contributions - Receipt (off-line)</em> - Update
 to new smarty variables for line items, tax</li></ul><p>The handling of invalid smarty template code
 in mailings, reminders and other automated messages has changed. For details, see <a href=
"https://docs.civicrm.org/sysadmin/en/latest/upgrade/version-specific/#civicrm-559" target="_blank">
upgrade notes</a>.</p>

Upgrade outputs:
WARNING: CiviCRM has changed the meaning of the date format specifier %A
when using CRM_Utils_Date::customFormat or {crmDate}. It was
identical to %P and the change will help compatibility with php 8.1.

Your Date Format [1] settings have been automatically updated.


Links:
------
[1] http://dmaster.bknix:8001/civicrm/admin/setting/date?reset=1

This is a bit incongruous in that:

  • (Small nit) "Pre-Upgrade Message" and "Upgrade Output" don't quite match. Maybe "Pre-Upgrade Messages" and "Post-Upgrade Messages"?
  • (Bigger issue) The "Pre-Upgrade Message" displays raw HTML on the console. The post-upgrade message displays more readable text (per htmlToText()).

If you look at a bit closer, the getPreUpgradeMessage() returns a string (HTML), and the run() returns an array with two formulations (['message'=>HTML, 'text'=>TEXT]).

The run() contract has been around longer, so maybe the contract for getPreUpgradeMessage() should a similar array? Then the client (drush, wp-cli, etc) can decide whether to use the HTML or TEXT version?

@totten
Copy link
Member

totten commented Feb 1, 2023

If you use civicrm/civicrm-core#25488 and the following patch, then the output will be a bit more consistent.

diff --git a/drush/civicrm.drush.inc b/drush/civicrm.drush.inc
index a63976a..145ab99 100644
--- a/drush/civicrm.drush.inc
+++ b/drush/civicrm.drush.inc
@@ -741,10 +741,10 @@ function drush_civicrm_upgrade_db() {
     return TRUE;
   }
   $upgradeHeadless = new CRM_Upgrade_Headless();
-  drush_print("Pre Upgrade Message:\n" . $upgradeHeadless->getPreUpgradeMessage());
+  drush_print("Pre-Upgrade Messages:\n" . $upgradeHeadless->getPreUpgradeMessage()['text']);
   // FIXME Exception handling?
   $result = $upgradeHeadless->run();
-  drush_print("Upgrade outputs:\n" . $result['text']);
+  drush_print("Post-Upgrade Messages:\n" . $result['text']);
 }
 
 /**

@seamuslee001
Copy link
Contributor Author

yeh ok @totten that makes sense I'll add this in tomorrow my time and we can merge this and i'll do similar for wp-cli

@totten
Copy link
Member

totten commented Feb 1, 2023

OK, sounds good

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

Successfully merging this pull request may close these issues.

2 participants