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#382 Ensure that no db errors are generated when trying to up… #12785

Merged
merged 1 commit into from
Sep 12, 2018

Conversation

seamuslee001
Copy link
Contributor

…date a civicrm group linked to an organization when the group id doesn't match the id of the relevant row in civicrm_group_organization

Overview

When updating a group linked to an organization. If the record id in the civicrm_group_organization table doesn't match that of the groupID there can be a db error generated because of the unique key on the civicrm_group_organization table

Before

DB error generated

After

No DB error

ping @mattwire @eileenmcnaughton @nganivet @monishdeb This is especially an issue in Multisite configurations

@@ -450,6 +450,10 @@ public static function create(&$params) {
if (!empty($params['organization_id'])) {
$groupOrg = $params;
$groupOrg['group_id'] = $group->id;
// dev/core#382 Keeping the id here can cause db errors as it tries to update the wrong record in the Organization table
if (isset($groupOrg['id'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@seamuslee001 I think it makes more sense to change the whole part inside the if to

  CRM_Contact_BAO_GroupOrganization::add(['group_id' => $group->id, 'organization_id' => $params['organization_id']]);
  • currently we are passing $params to CRM_Contact_BAO_GroupOrganization::add purely for the 'convenience' of them having one key in common.

Note that change also requires a change to stop accepting by reference in GroupOrganization::add but that seems generally good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eileenmcnaughton for the moment i think i'll just not worry about fixing the passing by reference but have made an update to cover what your after

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 OK - sure squash these & merge-on-pass

…date a civicrm group linked to an organization when the group id doesn't match the id of the relevant row in civicrm_group_organization

Only Pass necessary params to GroupOrganization::add function
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton done

@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

@eileenmcnaughton
Copy link
Contributor

test fail unrelated

@eileenmcnaughton eileenmcnaughton merged commit f3a84c7 into civicrm:master Sep 12, 2018
@eileenmcnaughton eileenmcnaughton deleted the lab_core_382 branch September 12, 2018 21:30
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