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

MAE-360: Add batch column and filter to constituent and contribution detail reports #220

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ahed-compucorp
Copy link
Contributor

@ahed-compucorp ahed-compucorp commented Jan 4, 2021

Overview

We need to show DD batch name in the following reports :

  • Contact Detail Report.
  • Contribute Detail Report.

For Contact Detail Report we added Instruction Batch column and field.
For Contribute Detail Report we added Payment Batch column.

Please note that civicrm_alterReportVar hook will be fired 3 times:

  • manualdirectdebit_civicrm_alterReportVar('columns', $var, $reportForm)
  • manualdirectdebit_civicrm_alterReportVar('sql', $var, $reportForm)
  • manualdirectdebit_civicrm_alterReportVar('rows', $var, $reportForm)

And we have a corresponding method handler:

  • updateColumns
  • updateSql
  • updateRows

Contact Detail Report Before

Screenshot from 2021-01-04 14-18-30
Filters tab
Screenshot from 2021-01-04 14-18-57

Contact Detail Report After

Screenshot from 2021-01-04 14-16-29
Filters tab
Screenshot from 2021-01-04 14-17-08

Contribute Detail Report Before

Screenshot from 2021-01-04 14-20-21

Contribute Detail Report After

Screenshot from 2021-01-04 14-19-46

Comments

I could only add payment batch column to contribute detail report using a hacky way and couldn't add a filter for that because of a bug in the core.

The civicrm_alterReportVar is not working for contribute detail report when the user select the new column or choose the new filter, and it will show an error message DB Error: no such field

Here at Detail.php#L956 our changes to the _form attribute are lost and the error will be fired here Detail.php#L576

Unknown column 'some_table.some_column' in 'where clause'

I've tried to overcome this issue by doing (so hacky) :

  1. Update the columns ContributeDetailReport.php#L85 so it will show in the report columns section.
  2. When the user select the column and submit the form we unset our new column ContributeDetailReport.php#L41
  3. Instead of accessing $reportForm object we access the request directly ContributeDetailReport.php#L69
  4. We use _columnHeaders attribute in the sql hook ContributeDetailReport.php#L102
  5. Because of 2 the column will not appear in the columns section but the result will have the new column.

* @param CRM_Report_Form_Contact_Detail|array $var
* @param CRM_Report_Form_Contact_Detail $reportForm
*/
public function handle($varType, &$var, &$reportForm) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this class and in CRM/ManualDirectDebit/Hook/Alter/ContributeDetailReport.php

can you change all the other methods to be private and just keep this as public

$batchTypeIds = CRM_Core_OptionGroup::values('batch_type', FALSE, FALSE, FALSE, $condition, 'value');

$params = [
'type_id' => ['IN' => $batchTypeIds],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

civicrm API will retreuve only 25 records by default, so you need to pass this :

$params = [
'type_id' => ['IN' => $batchTypeIds],
'options' => ['limit' => 0],
..etc
]

to remove this restriction

$batchTypeIds = CRM_Core_OptionGroup::values('batch_type', FALSE, FALSE, FALSE, $condition, 'value');

$params = [
'type_id' => ['IN' => $batchTypeIds],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here about 'options' => ['limit' => 0],

@omarabuhussein omarabuhussein changed the title MAE-360: Add batch column and filter to contact to both constituent detail and contribute detail reports MAE-360: Add batch column and filter to contact to both contact and contribution detail reports Jan 4, 2021
@omarabuhussein omarabuhussein changed the title MAE-360: Add batch column and filter to contact to both contact and contribution detail reports MAE-360: Add batch column and filter to contact to contact and contribution detail reports Jan 4, 2021
@omarabuhussein omarabuhussein changed the title MAE-360: Add batch column and filter to contact to contact and contribution detail reports MAE-360: Add batch column and filter to contact to constituent and contribution detail reports Jan 4, 2021
@ahed-compucorp ahed-compucorp changed the title MAE-360: Add batch column and filter to contact to constituent and contribution detail reports MAE-360: Add batch column and filter to constituent and contribution detail reports Jan 4, 2021
This refactor will help checking filters properly
and to prevent handling any filter with missing or improper value
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

Successfully merging this pull request may close these issues.

2 participants