From 49f927120ccb2b8bc95ddeb6adfc95eb8e22d3da Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Wed, 20 Feb 2019 15:29:28 +1300
Subject: [PATCH] Code cleanup - remove extraneous permissions clase

As the test demonstrates this permission is applied without this line. It is applied
even when the skipPermission is TRUE but that has been code-commented for now rather
than resolved
---
 CRM/Contact/BAO/Query.php                     | 1 +
 CRM/Contribute/BAO/Query.php                  | 3 ---
 CRM/Financial/BAO/FinancialType.php           | 1 +
 tests/phpunit/api/v3/FinancialTypeACLTest.php | 8 +++++---
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index 7c4d39a275..837d3dbe24 100644
--- a/CRM/Contact/BAO/Query.php
+++ b/CRM/Contact/BAO/Query.php
@@ -536,6 +536,7 @@ class CRM_Contact_BAO_Query {
       $component = 'membership';
     }
     if (isset($component)) {
+      // @todo should be if (isset($component && !$this->_skipPermission)
       CRM_Financial_BAO_FinancialType::buildPermissionedClause($this->_whereClause, $component);
     }
 
diff --git a/CRM/Contribute/BAO/Query.php b/CRM/Contribute/BAO/Query.php
index 6b2ff0e93c..51350bc924 100644
--- a/CRM/Contribute/BAO/Query.php
+++ b/CRM/Contribute/BAO/Query.php
@@ -237,9 +237,6 @@ class CRM_Contribute_BAO_Query extends CRM_Core_BAO_Query {
         return;
 
       case 'financial_type_id':
-        // @todo we need to make this resemble a hook approach.
-        CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($financialTypes);
-        $query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause("civicrm_contribution.$name", 'IN', array_keys($financialTypes), 'String');
       case 'invoice_id':
       case 'invoice_number':
       case 'payment_instrument_id':
diff --git a/CRM/Financial/BAO/FinancialType.php b/CRM/Financial/BAO/FinancialType.php
index 8ce7ad05fb..445475e295 100644
--- a/CRM/Financial/BAO/FinancialType.php
+++ b/CRM/Financial/BAO/FinancialType.php
@@ -377,6 +377,7 @@ class CRM_Financial_BAO_FinancialType extends CRM_Financial_DAO_FinancialType {
    *
    */
   public static function buildPermissionedClause(&$whereClauses, $component = NULL, $alias = NULL) {
+    // @todo the relevant addSelectWhere clause should be called.
     if (!self::isACLFinancialTypeStatus()) {
       return FALSE;
     }
diff --git a/tests/phpunit/api/v3/FinancialTypeACLTest.php b/tests/phpunit/api/v3/FinancialTypeACLTest.php
index b64abb1cbc..b275df4e06 100644
--- a/tests/phpunit/api/v3/FinancialTypeACLTest.php
+++ b/tests/phpunit/api/v3/FinancialTypeACLTest.php
@@ -188,6 +188,7 @@ class api_v3_FinancialTypeACLTest extends CiviUnitTestCase {
       'add contributions of type Donation',
     ]);
     $contribution = $this->callAPISuccess('Contribution', 'create', $this->_params);
+    $this->callAPISuccess('Contribution', 'create', array_merge($this->_params, ['financial_type_id' => 'Member Dues']));
 
     $params = array(
       'id' => $contribution['id'],
@@ -197,9 +198,10 @@ class api_v3_FinancialTypeACLTest extends CiviUnitTestCase {
     $this->assertEquals($contribution['count'], 0);
 
     $this->addFinancialAclPermissions([['view', 'Donation']]);
-    $contribution = $this->callAPISuccess('contribution', 'get', $params);
-
-    $this->assertEquals($contribution['count'], 1);
+    $this->callAPISuccessGetSingle('contribution', $params);
+    $this->callAPISuccessGetCount('contribution', ['financial_type_id' => 'Member Dues', 'check_permissions' => 1], 0);
+    $this->markTestIncomplete('check_permissions = 0 should be respected but is not - I have added a todo at the right place but not changed it as yet');
+    $this->callAPISuccessGetCount('contribution', ['financial_type_id' => 'Member Dues'], 1);
   }
 
   /**
-- 
2.25.1