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-21421 Fix to allow updating an existing CaseContact record #11268

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Nov 10, 2017

Overview

Found during a webform submission that updates a case status, but easy to replicate using the Case.Create API.

The function CRM_Case_BAO_CaseContact::create requires that you either specify a CaseContact ID or it will try and add a new one. However, the database index UI_case_contact_id has recently been changed to be UNIQUE and was causing the function CRM_Case_BAO_CaseContact::create to crash. The change has been in CiviCRM since 4.7.20 but the change to the database would have been made at some point after CiviCRM started warning when indexes were inconsistent.

Before

It is not possible to update cases via the API once the UI_case_contact_id index has been updated to "UNIQUE".

After

It is possible to update cases via the API once the UI_case_contact_id index has been updated to "UNIQUE".

Technical Details

Adds an API call to CRM_Case_BAO_CaseContact which calls CaseContact.getsingle with contact_id and case_id as parameters. If an existing CaseContact.id is found that is used instead of trying to create a new one.

Comments

@colemanw I'm not sure if the way I've fixed this is the right way, ie. using the API, and I'm not sure why it appears that it was previously creating new CaseContact entries every time it was updating a case?
When you get a minute, would you mind reviewing this PR and advising what the correct fix should be?


@seamuslee001
Copy link
Contributor

@mattwire it would be nice to have a unit test for this but the change seems to make sense

@mattwire
Copy link
Contributor Author

@seamuslee001 Happy to create a unit test, I'd like to get @colemanw feedback first.

@colemanw
Copy link
Member

Something's not right here...

Aside from id, The civicrm_case_contact table only has two columns: contact_id and case_id. So trying to insert the same data again is bad input, and it should throw an error. So my first reaction is to close this PR unmerged and leave the system as-is, as it seems to be behaving correctly.

You mentioned that the problem you encountered is easy to reproduce with the api explorer. What are the steps?

@mattwire
Copy link
Contributor Author

Create a case:

$result = civicrm_api3('Case', 'create', array(
  'case_type_id' => "housing_support",
  'contact_id' => 203,
  'subject' => "test",
));

This will fail:

$result = civicrm_api3('CaseContact', 'create', array(
  'case_id' => 2,
  'contact_id' => 203,
));

With error:

{

    "error_code": "already exists",
    "sql": "INSERT INTO civicrm_case_contact (case_id , contact_id ) VALUES ( 2 ,  203 )  [nativecode=1062 ** Duplicate entry '2-203' for key 'UI_case_contact_id']",
    "tip": "add debug=1 to your API call to have more info about the error",
    "is_error": 1,
    "error_message": "DB Error: already exists",
    "debug_information": "INSERT INTO civicrm_case_contact (case_id , contact_id ) VALUES ( 2 ,  203 )  [nativecode=1062 ** Duplicate entry '2-203' for key 'UI_case_contact_id']"
}

This will also fail (it calls CRM_Case_BAO_CaseContact::create internally) (and this is how the issue was identified as this is what webform is doing):

$result = civicrm_api3('Case', 'create', array(
  'id' => 2,
  'status_id' => "Closed",
  'contact_id' => 203,
));

Here is the fatal error:

Nov 11 09:41:33  [info] $Fatal Error Details = Array
(
    [callback] => Array
        (
            [0] => CRM_Core_Error
            [1] => exceptionHandler
        )

    [code] => -5
    [message] => DB Error: already exists
    [mode] => 16
    [debug_info] => INSERT INTO civicrm_case_contact (case_id , contact_id ) VALUES ( 2 ,  203 )  [nativecode=1062 ** Duplicate entry '2-203' for key 'UI_case_contact_id']
    [type] => DB_Error
    [user_info] => INSERT INTO civicrm_case_contact (case_id , contact_id ) VALUES ( 2 ,  203 )  [nativecode=1062 ** Duplicate entry '2-203' for key 'UI_case_contact_id']
    [to_string] => [db_error: message="DB Error: already exists" code=-5 mode=callback callback=CRM_Core_Error::exceptionHandler prefix="" info="INSERT INTO civicrm_case_contact (case_id , contact_id ) VALUES ( 2 ,  203 )  [nativecode=1062 ** Duplicate entry '2-203' for key 'UI_case_contact_id']"]
)

This is reproduceable on a clean 4.7.27 and master, as long as the UI_case_contact_id index has been updated to UNIQUE (as it will be for all new installs and is specified in the XML since 4.7.20).

This can be reproduced using webform_civicrm by creating a simple webform:
Civi Fields:

  • existing contact
  • firstname
  • lastname

Enable the cases, set it to existing contact and update existing cases.

@colemanw
Copy link
Member

colemanw commented Nov 11, 2017

@mattwire I suggest #11270 as an alternative. Looking at the api code it was never intended to touch this field when updating the case. In fact you can see a few lines above it is designed to throw an error when you try to update the contact id of an existing case.

@mattwire mattwire force-pushed the CRM-21421_allow_updating_existing_casecontact branch 3 times, most recently from 2eab157 to 4c90ead Compare November 15, 2017 21:10
@colemanw
Copy link
Member

colemanw commented Nov 15, 2017

Hmm, why did this test pass even though #11270 was unmerged? Now that it is merged, this test should still pass but it worries me that it didn't fail before.

@colemanw
Copy link
Member

@civicrm-builder retest this please.

@mattwire mattwire force-pushed the CRM-21421_allow_updating_existing_casecontact branch from 4c90ead to be84446 Compare November 17, 2017 12:51
@mlutfy
Copy link
Member

mlutfy commented Nov 17, 2017

jenkins, test this please

@mattwire
Copy link
Contributor Author

@colemanw This should now fail before #11270 was applied, and pass afterwards.

@colemanw colemanw merged commit b8dd91f into civicrm:master Nov 17, 2017
sluc23 pushed a commit to ixiam/civicrm-core that referenced this pull request Jan 10, 2018
…ng_existing_casecontact

CRM-21421 Fix to allow updating an existing CaseContact record
@mattwire mattwire deleted the CRM-21421_allow_updating_existing_casecontact branch January 17, 2018 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants