Remove "Copy Case custom data" code (circa 2013)

[jusfreeman]

Overview

Unable to merge Contacts and Cases with Case custom data, raises an error - DB Error: already exists (applies to CiviCRM 5.16.0 - CiviCRM 5.18.alpha1). See https://lab.civicrm.org/dev/core/issues/1183 for more details.

Before

Unable to merge Contacts and Cases with Case custom data, raises an error - DB Error: already exists.

Steps to reproduce:

1. Create Contact A and Contact B
2. Create custom fields for the Case Type
3. Create a Case for Contact A
4. Select the Case Type with the custom fields. Populate the custom fields.
5. Create a Case for Contact B
6. Select the Case Type with the custom fields. Populate the custom fields.
7. Search for Contact A/B
8. Merge Contact A/B
9. Observe the DB Error: already exists and merge fails to complete

After

Can merge Contacts and Cases with Case custom data without error.

Technical Details

Removes code introduced with #1204 - CRM-11662 Add missing logic to copy case custom fields in CRM_Case_BAO_C... #1204

Comments

Happy to say goodbye to old code.

Agileware Ref: CIVICRM-1290

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[jusfreeman]

Credit to @eileenmcnaughton for finding where to fix this!

[yashodha]

@jusfreeman tested this, works fine.

[demeritcowboy]

• General standards
  • [r-explain] PASS
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
    • And it also solves the reassign case issue from Migrate custom file attachments to new case on reassignment. #14213
• Developer standards
  • [r-tech] PASS
  • [r-code] PASS
  • [r-maint] N/A
    • Would it make sense to bring back the test from Migrate custom file attachments to new case on reassignment. #14213 ?
  • [r-test] PASS

[jusfreeman]

I'll update this PR to include the unit test from #14213

[eileenmcnaughton]

@jusfreeman can you please also target the 5.17 branch rather than master as this is a regression

[eileenmcnaughton]

Hmm that wouldn't apply cleanly on master - I switched to 5.17 & still

[GitScan\Exception\ProcessErrorException]
Process failed:
[[ COMMAND: git apply --check --ignore-whitespace ]]
[[ CWD: /home/jenkins/bknix-dfl/build/core-15051-6faa5/web/sites/all/modules/civicrm ]]
[[ EXIT CODE: 1 ]]
[[ STDOUT ]]
[[ STDERR ]]
error: patch failed: tests/phpunit/CRM/Case/BAO/CaseTest.php:170
error: tests/phpunit/CRM/Case/BAO/CaseTest.php: patch does not apply

Can you do ```git pull --rebase origin/5.17``

[jusfreeman]

@eileenmcnaughton that OK now?

[eileenmcnaughton]

It's running - assuming it passes we will backport to 5.16 & drop a 5.16 release early next week

[jusfreeman]

Great, back to my Weetibix then... nom nom nom. 🥄

[eileenmcnaughton]

@jusfreeman failure relates :-(

[demeritcowboy]

The problem is the file doesn't actually exist in the filesystem, so when copyGeneric (indirectly) calls CRM_Core_BAO_File::path() it returns nulls. Assuming the test system allows creating files, change the lines where you set fileA and fileB to something like:

    $path1 = Civi::paths()->getPath('[civicrm.files]/custom/test_file_uri');
    $path2 = Civi::paths()->getPath('[civicrm.files]/custom/test_file_uri_2');
    system('touch ' . $path1);
    system('touch ' . $path2);
    $fileA = $this->callAPISuccess('File', 'create', array(
      'uri' => $path1,
    ));
    $fileB = $this->callAPISuccess('File', 'create', array(
      'uri' => $path2,
    ));

Works for me.

[demeritcowboy]

Just realized my C background took over for a second. Of course file_put_contents() would be a php way to create a file instead of system().

[eileenmcnaughton]

@jusfreeman are you on this test - will try to get 5.16.x patch release out in next few days if this is merged

[jusfreeman]

Yes, will update PR to get that pesky unit test passing.

[yashodha]

@jusfreeman there seems to be a test failing. Also, can you squash all your commits?

[demeritcowboy]

Cool I can see from the test log it's passing now. Guess the parent path is what it didn't like and wants it to be in civicrm/custom. Nice @jusfreeman!

https://test.civicrm.org/job/CiviCRM-Core-PR/28426/consoleText:
...
ok 75 - CRM_Case_BAO_CaseTest::testCaseReassignForCustomFiles

[eileenmcnaughton]

@jusfreeman might be worth squashing the commits now if it has got past the sticking per per above

[eileenmcnaughton]

erm - no - that didn't do it

[eileenmcnaughton]

oh - I think you rebased in master commits.... this is against 5.17

[jusfreeman]

@eileenmcnaughton @demeritcowboy @yashodha @seamuslee001 thanks for all your support and encouragement along the way. It's been an interesting journey and I think this is done now.

[eileenmcnaughton]

May you live in interesting times :-)