CRM-20193 show financial_trxn currency on batch form. #9942
Conversation
To replicate this go to civicrm/financial/batch?reset=1&action=add & add a new batch arriving at civicrm/batchtransaction?reset=1&bid=2 The table at the bottom of the page is affected. This is a portion of the commit by pradpnayak which I understand, was able to test and can approve the code for. There is still a portion on the ticket I have not yet understood enough to approve
|
@JoeMurray I can merge this - or wait for an explanation of that last line I don't understand in the other PR |
| if (!$notPresent) { | ||
| $where = " ( civicrm_entity_batch.batch_id = {$entityID} | ||
| AND civicrm_entity_batch.entity_table = 'civicrm_financial_trxn' |
There was a problem hiding this comment.
This criteria of entity_table = ft seems to replicate https://github.com/civicrm/civicrm-core/pull/9942/files#diff-4edf3d9f312c7543c00ef7d38bf21044L652, which is still present at https://github.com/civicrm/civicrm-core/pull/9942/files#diff-4edf3d9f312c7543c00ef7d38bf21044R654. I haven't followed through logic of $searchValue and $notPresent, but looking at the diff on a computer screen instead of my phone it looks better.
There was a problem hiding this comment.
Thanks alot, @eileenmcnaughton !! Much appreciated. Looking over the PR on a big screen I'm more confident there is no problem with it. I've suggested a change to avoid replicating a logical part of query. Do my comments make sense to you? FWIW, I had the sense yesterday that there might be two versions of the query produced for different cases, and that the work of Pradeep only handled one, and that the problematic line from earlier version might have been a problem on the use case/query version that I did not provide to him.
| AND civicrm_entity_batch.entity_table = 'civicrm_financial_trxn' | ||
| AND civicrm_entity_financial_trxn.entity_table = 'civicrm_contribution') "; | ||
| $where = " civicrm_entity_batch.batch_id = {$entityID} | ||
| AND civicrm_entity_batch.entity_table = 'civicrm_financial_trxn'"; |
There was a problem hiding this comment.
Please remove 732 as replicating https://github.com/civicrm/civicrm-core/pull/9942/files#diff-4edf3d9f312c7543c00ef7d38bf21044R654
To replicate this go to civicrm/financial/batch?reset=1&action=add & add a new batch arriving at
civicrm/batchtransaction?reset=1&bid=2
The table at the bottom of the page is affected.
This is a portion of the commit by pradpnayak which I understand, was able to test and can approve the code for.
There is still a portion on the ticket I have not yet understood enough to approve
@JoeMurray I've spend 50 mins making sense of #9930 & can definitely approve this part #9930 - which is what the ticket says. I'm still getting my head around the changes to the joins in the query