CRM-19770 - Add is_star column to civicrm_activity #9629
Conversation
colemanw
commented
Jan 4, 2017
•
- CRM-19770: Favorite (starred) activities
Fixes misplaced upgrade steps that should be in 4.7.16 instead of 4.7.15.
| @@ -280,14 +280,8 @@ public function upgrade_4_7_14($rev) { | |||
| * @param string $rev | |||
| */ | |||
| public function upgrade_4_7_15($rev) { | |||
| $this->addTask('CRM-19723 - Add icon column to civicrm_option_value', 'addColumn', | |||
| 'civicrm_option_value', 'icon', "varchar(255) COMMENT 'crm-i icon class' DEFAULT NULL"); | |||
| $this->addTask('CRM-19626 - Add min_amount column to civicrm_price_set', 'addColumn', | |||
There was a problem hiding this comment.
For posterity: We discussed this one on Mattermost. This step is in an odd position where the 4.7.15 uses a SQL-backed upgrade and 4.7.16 uses a newer PHP-backed upgrade. However, testing shows that they end up in the same place, and they still appear mutually compatible, and the 4.7.16 code is better.
| $this->addTask('CRM-19723 - Add icon column to civicrm_option_value', 'addColumn', | ||
| 'civicrm_option_value', 'icon', "varchar(255) COMMENT 'crm-i icon class' DEFAULT NULL"); | ||
| $this->addTask('CRM-19769 - Add color column to civicrm_tag', 'addColumn', | ||
| 'civicrm_tag', 'color', "varchar(255) COMMENT 'Hex color value e.g. #ffffff' DEFAULT NULL"); |
There was a problem hiding this comment.
Belated nitpick... and this isn't going to block #9629... but varchar(255) seems much longer than a color needs. varchar(8) is probably enough, and varchar(32) would also be pretty comfortable.
There was a problem hiding this comment.
@totten I actually obsessed about that a little before looking it up and discovering (lo and behold)... it doesn't matter!
http://stackoverflow.com/questions/8295131/best-practices-for-sql-varchar-column-length
There was a problem hiding this comment.
@colemanw interesting, my mental model of varchar(nnn) was a bit wrong!
FWIW... one of those answers cites http://stackoverflow.com/a/1151833/4195300, but it seems to give a more complicated view -- i.e. the length shouldn't affect disk usage but may affect query performance.
|
Confirmed that the revised upgrade works -- for both the new field and the refactored upgrade steps. |