dev/core/#/233 Expose information about where a contact has been merged to #12489

eileenmcnaughton · 2018-07-17T05:16:12Z

Overview

Exposes information about what contact a contact deleted by merge has been merged from. Via the api it exposes this and also which contacts have been merged into a given contact.

Before

Poor visibility on where a contact has been deleted as part of a merge

After

Merged nature is much clearer.

In addition the calculus for the 'merged to' & 'merged from' is exposed via api

Technical Details

We have a fairly opaque data structure for linking merged contacts to the contacts they have been merged to. There have been various discussions about changing it so I feel like the ownership of exposing this relationship should sit in core.

Examples of where this could be used by an extension are

extended reports offers an address history report (which can be a contact summary tab) - it should provide this history across multiple contacts
various privacy extensions should find and delete data for contacts deleted by merge as well as the current contact.

The apis used here (open to any form of bikeshedding on action name / params) are:

$result = civicrm_api3('Contact', 'getmergedto', ['contact_id' => $contactID]);

$result = [
  'values' => [5 => ['id' => 5],
  'count'.....
];

$result = civicrm_api3('Contact', 'getmergedfrom', ['contact_id' => $contactID]);

$result = [
  'values' => [5 => ['id' => 5], 6 => ['id' => 6]]
  'count'.....
];

In each case result is an array of one (getmergedto) or n (getmergedfrom) contact records, with only 'id' populated.

Comments

https://lab.civicrm.org/dev/core/issues/233

@totten @colemanw @JKingsnorth

civibot · 2018-07-17T05:16:14Z

(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).

…stination contact and source contacts for merged contacts. This 'data structure' is 'owned' by core & somewhat subject to future change so establishing a tested methodology for retrieving contact history in core can support extensions such as - extended reports - which exposes an address_history tab - privacy extensions - which need to recover and potentially delete contacts that were merged into the current contact

…act deleted by merge

eileenmcnaughton · 2018-07-18T00:27:37Z

test fail is unrelated

totten · 2018-07-18T00:53:43Z

(CiviCRM Review Template WORD-1.1)

(r-explain) Pass
(r-test) Undecided: There's a failure in api_v3_ReportTemplateTest.testActivitySummary. I don't recall this as a common false-negative, but if you do recall it as a common false-negative, then I'll believe you. ;)
(r-code) Pass: Code style is generally consistent.
(r-doc) Soft Pass: The UX change is subtle enough that it doesn't need user docs, IMHO. It's up to whether the DX needs docs.
(r-maint) Pass: Adds coverage for new functions.
(r-run) Undecided: Not assessed
(r-user) Pass: The UI updates in the screenshot look sensible.
(r-tech) Pass: Mostly new code. Hard to see how it would break anything else.

eileenmcnaughton · 2018-07-18T01:00:39Z

@totten yes test is a known bug (from a merge yesterday although hopefully fix is in) - what is holding this up now?

eileenmcnaughton · 2018-07-18T04:56:09Z

test this please

eileenmcnaughton · 2018-07-18T04:56:36Z

(there is space in test queue to re-running as it's harder to get people to look when there is a red flag)

stesi561 · 2018-07-18T09:09:33Z

In addition to the above.

Run it (r-run) Pass after merging a contact it shows as advertised. Tried merging the contact that a contact had been merged into - it correctly points to the final contact. Had a look at the api, works as expected.

eileenmcnaughton · 2018-07-18T09:36:57Z

@stesi561 you are my new favourite person - really appreciate this.

Adding merge-on-pass

eileenmcnaughton · 2018-07-18T10:36:28Z

unrelated fail

JKingsnorth · 2018-07-18T11:08:53Z

I'm a bit late to this party, but it looks OK to me.

I have some reservations about the display of the (This contact has been merged into...). (It might have been nicer on the next line down after the name, rather than inline). But not so strong as to block anything, and it doesn't impact the inline editing of the name because that's not possible on a deleted contact.

eileenmcnaughton · 2018-07-18T11:55:13Z

I'll happily merge a prettier version if you have an idea

mepps · 2018-07-18T17:59:08Z

CRM/Contact/Page/View.php

@@ -306,7 +306,7 @@ public static function checkUserPermission($page, $contactID = NULL) {
   */
  public static function setTitle($contactId, $isDeleted = FALSE) {
    static $contactDetails;
-    $displayName = $contactImage = NULL;


Why is the declaration of display name deleted?

because it was that funny grey in phpstorm (ie. it's immediately re-defined)

It would be grey because PHPStorm thinks it's being overwritten:

But really it's just confused. I think we should either leave it in, or remove the $contactImage declaration too, by the same logic =]

isn't the contactImage declaration removed?

Nope, just on the next line down =]

Yeah I agree with @JKingsnorth that we should remove both.

…ct and source contacts for merged contacts. Backport of civicrm#12489 This 'data structure' is 'owned' by core & somewhat subject to future change so establishing a tested methodology for retrieving contact history in core can support extensions such as - extended reports - which exposes an address_history tab - privacy extensions - which need to recover and potentially delete contacts that were merged into the current contact dev/core/#/233 Use merged data api to display navigation help on contact deleted by merge Bug: T199748 Change-Id: I299725a8d39cd45d205874b1f07641283cbb5c96

civicrm-builder added the master label Jul 17, 2018

eileenmcnaughton force-pushed the mergees branch from 393d274 to 7089a03 Compare July 17, 2018 05:19

eileenmcnaughton changed the title ~~Mergees~~ dev/core/#/233 Expose information about where a contact has been merged to Jul 17, 2018

eileenmcnaughton added 2 commits July 17, 2018 19:33

dev/core/#/233 Use merged data api to display navigation help on cont…

93ddaac

…act deleted by merge

eileenmcnaughton force-pushed the mergees branch from 7089a03 to 93ddaac Compare July 17, 2018 07:33

eileenmcnaughton added the merge on pass label Jul 18, 2018

eileenmcnaughton closed this Jul 18, 2018

eileenmcnaughton reopened this Jul 18, 2018

eileenmcnaughton merged commit b6bba74 into civicrm:master Jul 18, 2018

eileenmcnaughton deleted the mergees branch July 18, 2018 10:36

mepps reviewed Jul 18, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev/core/#/233 Expose information about where a contact has been merged to #12489

dev/core/#/233 Expose information about where a contact has been merged to #12489

eileenmcnaughton commented Jul 17, 2018 •

edited

Loading

civibot bot commented Jul 17, 2018

eileenmcnaughton commented Jul 18, 2018

totten commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

stesi561 commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

JKingsnorth commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

mepps Jul 18, 2018

eileenmcnaughton Jul 18, 2018

JKingsnorth Jul 19, 2018

eileenmcnaughton Jul 19, 2018

JKingsnorth Jul 19, 2018

mepps Jul 19, 2018

dev/core/#/233 Expose information about where a contact has been merged to #12489

dev/core/#/233 Expose information about where a contact has been merged to #12489

Conversation

eileenmcnaughton commented Jul 17, 2018 • edited Loading

Overview

Before

After

Technical Details

Comments

civibot bot commented Jul 17, 2018

eileenmcnaughton commented Jul 18, 2018

totten commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

stesi561 commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

JKingsnorth commented Jul 18, 2018

eileenmcnaughton commented Jul 18, 2018

mepps Jul 18, 2018

Choose a reason for hiding this comment

eileenmcnaughton Jul 18, 2018

Choose a reason for hiding this comment

JKingsnorth Jul 19, 2018

Choose a reason for hiding this comment

eileenmcnaughton Jul 19, 2018

Choose a reason for hiding this comment

JKingsnorth Jul 19, 2018

Choose a reason for hiding this comment

mepps Jul 19, 2018

Choose a reason for hiding this comment

eileenmcnaughton commented Jul 17, 2018 •

edited

Loading