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

report#47 Report Bookkeeping add time field for date filter. #18268

Merged
merged 2 commits into from
Aug 28, 2020

Conversation

sunilpawar
Copy link
Contributor

@sunilpawar sunilpawar commented Aug 27, 2020

Same issue : https://lab.civicrm.org/dev/report/-/issues/7 ,with some solution : #13571

But not its not working ..., Reason Parent Class CRM_Utils_Type::T_DATE + CRM_Utils_Type::T_TIME Time handling changed.

Add Time field and Add Sort on Contribution Receive Date and Transaction Date.
Screenshot 2020-08-27 at 10 31 17 PM


https://lab.civicrm.org/dev/report/-/issues/47

@civibot
Copy link

civibot bot commented Aug 27, 2020

(Standard links)

@civibot civibot bot added the master label Aug 27, 2020
@@ -300,8 +300,7 @@ public function __construct() {
'operatorType' => CRM_Report_Form::OP_INT,
'type' => CRM_Utils_Type::T_INT,
],
'receive_date' => ['operatorType' => CRM_Report_Form::OP_DATE],
'receipt_date' => ['operatorType' => CRM_Report_Form::OP_DATE],
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we removing receipt_date?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

re-added receipt_date

@@ -536,8 +537,10 @@ public function where() {
$relative = $this->_params["{$fieldName}_relative"] ?? NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

@sunilpawar everything above this line looks fine. But on the where clause - could we just stop overriding it?

We'd need to add an implementation of whereClause() to add the handling of
'credit_accounting_code',
'credit_name',
'credit_contact_id',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eileenmcnaughton added these field in whereClause clause .

Existing functionality does work by giving date range with time but it always from 20200827000000 to 20200827235959 , there was no control on time field to specify it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@sunilpawar thanks for confirming - your final fix will fix more than just this report I think

@eileenmcnaughton
Copy link
Contributor

I pulled this down & was able to

  • order by receive date
  • filter by receive date - including specifying the time.

The changes improve the code - Merge-on-pass

@eileenmcnaughton
Copy link
Contributor

test this please

@seamuslee001
Copy link
Contributor

Test fail is unrelated

@seamuslee001 seamuslee001 merged commit 638f9c7 into civicrm:master Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants