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

CRM-21412 Do not give fatal error on report when no fields selected #11259

Merged
merged 1 commit into from
Nov 17, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Nov 8, 2017

Overview

This is a trivial fix to ensure that when a report that inherits the main 'select' function is rendered with no fields selected there is no hard error - by adding SELECT 1 instead of leaving the select clause missing.

Before

In some rare circumstances (especially when extension writers write reports) a report can be rendered with no fields selected for display. In this case a fatal db error occurs

After

Report shows empty rows but no hard fatal error

Technical Details

This is an upstream of a fix that has been in extended reports for some time.

Comments

I don't know of an obvious way to replicate it but I feel there is no risk of harm here (reports that do weird things manage their own selects) and it helps people writing custom reports.


@@ -2398,6 +2398,11 @@ public function select() {

Copy link
Member

@monishdeb monishdeb Nov 9, 2017

Choose a reason for hiding this comment

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

@eileenmcnaughton instead doing it in IF loop how about adding this:

     $select = empty($select) ? array('1') : $select;

Copy link
Member

Choose a reason for hiding this comment

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

@monishdeb Her example was also checking for just "select", which seems like a good idea?

Copy link
Contributor

Choose a reason for hiding this comment

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

@mlutfy wouldn't that only happen if $select is empty as per L2400 where $this->_select is constructed by adding "SELECT" onto the front of $select

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the revised change is what @monishdeb meant - although I have not squashed it onto one line because I wanted to keep the comment

@mlutfy mlutfy merged commit 87a5f5b into civicrm:master Nov 17, 2017
@eileenmcnaughton eileenmcnaughton deleted the report branch November 17, 2017 04:51
sluc23 pushed a commit to ixiam/civicrm-core that referenced this pull request Jan 10, 2018
CRM-21412 Do not give fatal error on report when no fields selected
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.

5 participants