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

dev/core#1714 Fix mis-filtering on activity type #17215

Merged
merged 1 commit into from
May 1, 2020
Merged

dev/core#1714 Fix mis-filtering on activity type #17215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 5 additions & 36 deletions CRM/Activity/BAO/Query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,19 +180,16 @@ public static function whereClauseSingle(&$values, &$query) {

$fields = CRM_Activity_BAO_Activity::exportableFields();
$fieldSpec = $query->getFieldSpec($name);

$query->_tables['civicrm_activity'] = $query->_whereTables['civicrm_activity'] = 1;
if ($query->_mode & CRM_Contact_BAO_Query::MODE_ACTIVITY) {
$query->_skipDeleteClause = TRUE;
}
// @todo we want to do this in a more metadata driven way, and also in contribute.
// But for the rc...
$namesToConvert = [
'activity_status' => 'activity_status_id',
];
$name = $namesToConvert[$name] ?? $name;

switch ($name) {
case 'activity_type':
case 'activity_type_id':
case 'activity_status':
case 'activity_status_id':
case 'activity_engagement_level':
case 'activity_id':
Expand All @@ -201,42 +198,14 @@ public static function whereClauseSingle(&$values, &$query) {
// We no longer expect "subject" as a specific criteria (as of CRM-19447),
// but we still use activity_subject in Activity.Get API
case 'activity_subject':

$qillName = $name;
if (in_array($name, ['activity_engagement_level', 'activity_id'])) {
$name = $qillName = str_replace('activity_', '', $name);
}
if (in_array($name, [
'activity_subject',
'activity_priority_id',
])) {
$name = str_replace('activity_', '', $name);
$qillName = str_replace('_id', '', $qillName);
}
if ($name == 'activity_campaign_id') {
$name = 'campaign_id';
}

$dataType = !empty($fields[$qillName]['type']) ? CRM_Utils_Type::typeToString($fields[$qillName]['type']) : 'String';

$where = $fieldSpec['where'];
if (!$where) {
$where = 'civicrm_activity.' . $name;
}
$query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause($where, $op, $value, $dataType);
list($op, $value) = CRM_Contact_BAO_Query::buildQillForFieldValue('CRM_Activity_DAO_Activity', $name, $value, $op);
$query->_qill[$grouping][] = ts('%1 %2 %3', [
1 => $fields[$qillName]['title'],
2 => $op,
3 => $value,
]);
$query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause($fieldSpec['where'], $op, $value, CRM_Utils_Type::typeToString($fieldSpec['type']));
$query->_qill[$grouping][] = $query->getQillForField($fieldSpec['is_pseudofield_for'] ?? $fieldSpec['name'], $value, $op, $fieldSpec);
break;

case 'activity_text':
self::whereClauseSingleActivityText($values, $query);
break;

case 'activity_type':
case 'activity_priority':
$query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause("$name.label", $op, $value, 'String');
list($op, $value) = CRM_Contact_BAO_Query::buildQillForFieldValue('CRM_Activity_DAO_Activity', $name, $value, $op);
Expand Down
5 changes: 3 additions & 2 deletions CRM/Contact/BAO/Query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,8 +516,9 @@ public function __construct(
$this->_fields = array_merge($this->_fields, $fields);

// add activity fields
$fields = CRM_Activity_BAO_Activity::exportableFields();
$this->_fields = array_merge($this->_fields, $fields);
$this->_fields = array_merge($this->_fields, CRM_Activity_BAO_Activity::exportableFields());
// Add hack as no unique name is defined for the field but the search form is in denial.
$this->_fields['activity_priority_id'] = $this->_fields['priority_id'];

// add any fields provided by hook implementers
$extFields = CRM_Contact_BAO_Query_Hook::singleton()->getFields();
Expand Down
45 changes: 45 additions & 0 deletions tests/phpunit/CRM/Contact/BAO/QueryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,11 +437,56 @@ public function getSearchProfileData() {
];
}

/**
* Test similarly handled activity fields qill and where clauses.
*
* @throws \CRM_Core_Exception
*/
public function testSearchBuilderActivityType() {
$queryObj = new CRM_Contact_BAO_Query([['activity_type', '=', '3', 1, 0]]);
$this->assertContains('WHERE ( ( civicrm_activity.activity_type_id = 3 )', $queryObj->getSearchSQL());
$this->assertEquals('Activity Type = Email', $queryObj->_qill[1][0]);

$queryObj = new CRM_Contact_BAO_Query([['activity_type_id', '=', '3', 1, 0]]);
$this->assertContains('WHERE ( ( civicrm_activity.activity_type_id = 3 )', $queryObj->getSearchSQL());
$this->assertEquals('Activity Type ID = Email', $queryObj->_qill[1][0]);

$queryObj = new CRM_Contact_BAO_Query([['activity_status', '=', '3', 1, 0]]);
$this->assertContains('WHERE ( ( civicrm_activity.status_id = 3 )', $queryObj->getSearchSQL());
$this->assertEquals('Activity Status = Cancelled', $queryObj->_qill[1][0]);

$queryObj = new CRM_Contact_BAO_Query([['activity_status_id', '=', '3', 1, 0]]);
$this->assertContains('WHERE ( ( civicrm_activity.status_id = 3 )', $queryObj->getSearchSQL());
$this->assertEquals('Activity Status = Cancelled', $queryObj->_qill[1][0]);

$queryObj = new CRM_Contact_BAO_Query([['activity_engagement_level', '=', '3', 1, 0]]);
$this->assertContains('WHERE ( ( civicrm_activity.engagement_level = 3 )', $queryObj->getSearchSQL());
$this->assertEquals('Engagement Index = 3', $queryObj->_qill[1][0]);

$queryObj = new CRM_Contact_BAO_Query([['activity_id', '=', '3', 1, 0]]);
$this->assertContains('WHERE ( ( civicrm_activity.id = 3 )', $queryObj->getSearchSQL());
$this->assertEquals('Activity ID = 3', $queryObj->_qill[1][0]);

$queryObj = new CRM_Contact_BAO_Query([['activity_campaign_id', '=', '3', 1, 0]]);
$this->assertContains('WHERE ( ( civicrm_activity.campaign_id = 3 )', $queryObj->getSearchSQL());
$this->assertEquals('Campaign = 3', $queryObj->_qill[1][0]);

$queryObj = new CRM_Contact_BAO_Query([['activity_priority_id', '=', '3', 1, 0]]);
$this->assertContains('WHERE ( ( civicrm_activity.priority_id = 3 )', $queryObj->getSearchSQL());
$this->assertEquals('Priority = Low', $queryObj->_qill[1][0]);

$queryObj = new CRM_Contact_BAO_Query([['activity_subject', '=', '3', 1, 0]]);
$this->assertContains("WHERE ( ( civicrm_activity.subject = '3' )", $queryObj->getSearchSQL());
$this->assertEquals("Subject = '3'", $queryObj->_qill[1][0]);
}

/**
* Test set up to test calling the query object per GroupContactCache BAO usage.
*
* CRM-17254 ensure that if only the contact_id is required other fields should
* not be appended.
*
* @throws \CRM_Core_Exception
*/
public function testGroupContactCacheAddSearch() {
$returnProperties = ['contact_id'];
Expand Down
15 changes: 12 additions & 3 deletions tests/phpunit/CRM/Contact/Form/Search/SearchContactTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ class CRM_Contact_Form_Search_SearchContactTest extends CiviUnitTestCase {

/**
* Test contact sub type search.
*
* @throws \CRM_Core_Exception
*/
public function testContactSubtype() {
foreach (['Contact_sub_type', 'Contact2__sub__type'] as $contactSubType) {
$subType = $this->callAPISuccess('ContactType', 'create', [
'name' => $contactSubType,
'label' => $contactSubType,
'is_active' => 1,
'parent_id' => "Individual",
'parent_id' => 'Individual',
]);
// Contact Type api munge name in create mode
// Therefore updating the name in update mode
Expand All @@ -39,6 +41,11 @@ public function testContactSubtype() {
$this->searchContacts('Contact2__sub__type');
}

/**
* @param string $contactSubType
*
* @throws \CRM_Core_Exception
*/
protected function searchContacts($contactSubType) {
// create contact
$params = [
Expand Down Expand Up @@ -78,11 +85,13 @@ protected function searchContacts($contactSubType) {
/**
* Test to search based on Group type.
* https://lab.civicrm.org/dev/core/issues/726
*
* @throws \CRM_Core_Exception
*/
public function testContactSearchOnGroupType() {
$groupTypes = $this->callAPISuccess('OptionValue', 'get', [
'return' => ["id", "name"],
'option_group_id' => "group_type",
'return' => ['id', 'name'],
'option_group_id' => 'group_type',
])['values'];
$groupTypes = array_column($groupTypes, 'id', 'name');

Expand Down