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

CRM-20927 Migrate Upgrade code from mysql.tpl to PHP for saftey and s… #10858

Merged
merged 2 commits into from Aug 16, 2017

Conversation

seamuslee001
Copy link
Contributor

…tability

Overview

This just migrates the adding of the module_data

Before

At present the Column is added in the SQL function which can have a hard failure if the column already exists in the databsae

After

Column is now added through php using the addColumn helper function which checks to see if the column is present already

@totten does this makes sense to you give you wrote the original upgrade script

* @param string $rev
*/
public function upgrade_4_7_25($rev) {
$this->addTask('CRM-20927 - Add Generic Column for other data', 'addColumn',
Copy link
Member

Choose a reason for hiding this comment

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

@seamuslee001 Thanks, this is a good idea/safer code-convention.

Nitpick: from the perspective of an admin watching the upgrade UI, this would display a message "CRM-20927 - Add Generic Column for other data", which seems meaningless at a glance. The capitalization also looks pretty arbitrary.

Maybe something like "CRM-20927 - Add column to 'civicrm_menu' for additional metadata"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks tim, makes more sense agreed , have updated message now

@totten
Copy link
Member

totten commented Aug 16, 2017

Retested on my local and confirmed that the new column is still created. Good to go.

@totten totten merged commit c0f36b4 into civicrm:master Aug 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants