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

Fix report form isTableSelected to treat relative date filters as filters #11882

Merged
merged 1 commit into from
Mar 28, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 27, 2018

Overview

The function isTableSelected is used to determine whether a table is being referenced in field, filter, order by parameters etc. Fix it to detect relative date filters

Before

Despite selection of a param like ['receive_date_relative'] => 'this.quarter' isTableSelected will return FALSE

After

isTableSelected correctly returns TRUE

Technical Details

isTableSelected looks to see if the filter has been passed in. Without this it looks in ->_params[{}_value] &
for {}_op to be null or 'not null' but misses relative date filters.

Comments

This is pretty hard to replicate as it is a helper function, but, I think it's fairly easy to proof by reading the code. I also moved || to the start of the lines

…ters.

isTableSelected looks to see if the filter has been passed in. Without this it looks in ->_params[{}_value] &
for {}_op to be null or 'not null' but misses relative date filters.
@eileenmcnaughton
Copy link
Contributor Author

@yashodha ties in with the report fix ups we have been doing

@jitendrapurohit
Copy link
Contributor

reviewing this now.

Copy link
Contributor

@jitendrapurohit jitendrapurohit left a comment

Choose a reason for hiding this comment

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

Tested this by debugging the value of $this->filteredTables and confirmed that it includes correct table name on selecting a value for date filters.

@eileenmcnaughton
Copy link
Contributor Author

Thanks @jitendrapurohit

@eileenmcnaughton eileenmcnaughton merged commit 7aed41f into civicrm:master Mar 28, 2018
@eileenmcnaughton eileenmcnaughton deleted the report_filter branch March 28, 2018 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants