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.

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

Batch search in contributions is buggy when extension is enabled #4

Open
MegaphoneJon opened this issue May 29, 2019 · 1 comment
Open

Comments

@MegaphoneJon
Copy link
Contributor

Hi all,

I've been trying to track down the cause of bug core#707 and why it persists even when I thought I patched it. I eventually figured out that this extension redirects AJAX calls to itself when searching the grants.

My question is: What is lost by NOT redirecting? I inserted return CRM_Financial_Page_AJAX::getFinancialTransactionsList(); to the top of CRM_Grantfinancialsupport_Util::getFinancialTransactionsList() and it worked well - but we haven't started using grant financials yet (but will be imminently).

Based on the relevant git blame, it seems like this change was made for "AO-86" and mostly handles returning a different set of links for grant batches. Could this be resolved via hook_civicrm_links instead?

@monishdeb monishdeb mentioned this issue May 31, 2019
@monishdeb
Copy link
Member

monishdeb commented May 31, 2019

@MegaphoneJon, unfortunately, its not tied to links only but also depends on a CRM_Grantfinancialsupport_Util ::getBatchFinancialItems() which is responsible to fetch grant OR contribution ID if a financial trxn is tied to it. Now firstly thank you for your suggestion about using hook_civicrm_links but as I recall I didn't choose to go on that direction because then it involves firing small query inside this link_hook for each payment links which is not good in terms of performance.

But then we don't have any hook where I can extend the core SQL query that fetch payments for c a batch. I hope Pradeep's civicrm/civicrm-core#13556 got merged so I can redo my fix.

Anyways as per core#707 I understand the gravity why it is so important to remove the core override of that AJAX function reason why I have submitted a new PR #5 about calling the link hook and get rid of that override. Can you please review this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants