Add cancel_reason field in contribute query whereClauseSingle

[sluc23]

Overview

Contribution's Cancel Reason is not working when used in Search Builder using IN operator

Before

Query produced is wrong

SELECT 
  DISTINCT UPPER(LEFT(contact_a.sort_name, 1)) AS sort_name
FROM 
  civicrm_contact contact_a
LEFT JOIN 
  civicrm_contribution ON civicrm_contribution.contact_id = contact_a.id
WHERE 
  ((civicrm_contribution.financial_type_id IN ("3", "1",  "4","2")
        AND civicrm_contribution.cancel_reason IN ''))
  AND (contact_a.is_deleted = 0)
ORDER BY UPPER(LEFT(contact_a.sort_name, 1)) ASC 

[nativecode=1064 ** You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''' )  )  AND (contact_a.is_deleted = 0)    ORDER BY UPPER(LEFT(contact_a.sort_na' at line 1]

After

Technical Details

cancel_reason field is not treated as rest of contribution's fields in whereClauseSingle().
This PR just add this field to be treated as the rest of common contribution's fields

[jitendrapurohit]

Did a quick testing on search forms. It fixes the main issue and code changes looks fine to me.

[pradpnayak]

Question: Why do we need to add case for every contribution field of type varchar? Doesn't the default section responsible to handle this automatically?

[sluc23]

the default: section doesn't take care properly of string fields. If you see the result query, it adds where clause civicrm_contribution.financial_type_id IN ("3", "1", "4","2") when it's not necessary, and then the IN operator it's not parsed properly

Basically, I added cancel_reason in the same case block as other string field are parsed properly, like contribution_source

[pradpnayak]

But i still think we should fix the default section rather than adding additional case as it will break in future if anyone wants to add additional filter through hook.

I believe civicrm_contribution.financial_type_id IN ("3", "1", "4","2") is needed when ACL Financial type is enabled but shouldn't be on default section as it will keep adding same clause.

I am not sure why cancel reason is treated as IN operator rather 'LIKE' or '='

@eileenmcnaughton @monishdeb thoughts?

[eileenmcnaughton]

I agree the default section should work / be fixed. The switch statement mostly exists because of bad coding historically.

-We should really get a test on it too.

[sluc23]

ok, I agree as well that the proper fix is to correct default: code block. But for that fix I'm afraid I won't be able to take the lead to make a new PR
Refactoring that, might have a big impact on the whole Contribution Query class, and every piece of code / feature that's related with it, and I don't think I'm confident on how to fix it properly

[eileenmcnaughton]

@sluc23 can you please test #12775 - which achieves the same thing by fixing the metadata for the field (& using addField to add the field to the form - which is the metadata-based methodology)

[sluc23]

This PR #12775 fixes the issue. Closing!