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.

Sign up for GitHub

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-21855 - Editing "A" side of relationship copies values to "B" side #11894

Closed
wants to merge 4 commits into from
Closed

CRM-21855 - Editing "A" side of relationship copies values to "B" side #11894

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
947da9b
Editing "A" side of relationship copies values to "B" side
alexander0205 Mar 28, 2018
102d16a
Revert "Editing "A" side of relationship copies values to "B" side"
alexander0205 Apr 3, 2018
bf71a99
Comentar contenido para reparar error de Relationship
alexander0205 Apr 3, 2018
4294dcc
Revert "Comentar contenido para reparar error de Relationship"
alexander0205 Apr 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions CRM/Contact/BAO/RelationshipType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,14 @@ public static function add(&$params, $ids = []) {

$relationshipType->copyValues($params);

// if label B to A is blank, insert the value label A to B for it
if (!strlen(trim($strName = CRM_Utils_Array::value('name_b_a', $params)))) {
$relationshipType->name_b_a = CRM_Utils_Array::value('name_a_b', $params);
}
if (!strlen(trim($strName = CRM_Utils_Array::value('label_b_a', $params)))) {
$relationshipType->label_b_a = CRM_Utils_Array::value('label_a_b', $params);
}
/* This generate error in editable */
Copy link
Contributor

Choose a reason for hiding this comment

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

Commenting code isn't a great idea, it leaves a lot of unused code lying around and makes files unnecessarily messy. We can recover any deleted code using git anyway.

Based on the comment I'd say you still have a little bit more to do in this PR:

  • [] Ensure label_a_b and label_b_a are required for creation in both API and UI
  • [] When updating an existing RelationshipType ensure the labels are not required

Also in the code you commented out below you changed something related to name_b_a. This might be outside of the scope of this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

@mickadoo if we make the label required in the api & people are already using it, only supplying one, we could break their code :-( We should probably try to sanitise UI behaviour & maintain api behaviour

Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton I'm fine with that. If it was me I would have just fixed the bug that I mentioned originally in the issue, and I'd probably just do that by changing this line:

if (!strlen(trim($strName = CRM_Utils_Array::value('label_b_a', $params)))) {
  $relationshipType->label_b_a = CRM_Utils_Array::value('label_a_b', $params);
}

to something like

$isEdit = !isset($params['id']);
$existingBALabel = NULL;
if ($isEdit) {
  $existingBALabel = CRM_Core_DAO::getFieldValue(
    CRM_Contact_DAO_RelationshipType::class,
    $params['id'],
    'label_b_a'
  );
}
$newBALabel = trim(CRM_Utils_Array::value('label_b_a', $params));

// If no new label_b_a in params and is empty in database use label_a_b as default
if (!$newBALabel && !$existingBALabel) {
  $relationshipType->label_b_a = CRM_Utils_Array::value('label_a_b', $params);
}

But I got the impression from the comments on this PR that more was required

// // if label B to A is blank, insert the value label A to B for it
// if (!strlen(trim($strName = CRM_Utils_Array::value('name_b_a', $params)))) {
// $relationshipType->name_b_a = CRM_Utils_Array::value('name_a_b', $params);
// }
// if (!strlen(trim($strName = CRM_Utils_Array::value('label_b_a', $params)))) {
// $relationshipType->label_b_a = CRM_Utils_Array::value('label_a_b', $params);
// }

$result = $relationshipType->save();

Expand Down