CRM-20733 - Relationship report improvements #10679
Conversation
yashodha
commented
Jul 17, 2017
•
yashodha
commented
- CRM-20733: Relationship report improvements
| if (empty($whereClauses)) { | ||
| $this->_where = 'WHERE ( 1 ) '; | ||
| $this->_having = ''; | ||
| $this->_where = "WHERE ( {$this->_aliases['civicrm_contact']}.is_deleted = 0 AND {$this->_aliases['civicrm_contact_b']}.is_deleted = 0 ) "; |
There was a problem hiding this comment.
@Edzelopez @colemanw what do you think of this. I took a look at a few other reports & they weren't filtering out deleted contacts. I kind of feel like if we want them filtered out the generated ACL clause should be doing that for all reports?
Overall PR looks good, just trying to think it through WRT report ACL work that has been happening
There was a problem hiding this comment.
Why does this remove the conditional?
There was a problem hiding this comment.
Always empty at that point I think
There was a problem hiding this comment.
I just ran into the is_deleted issue (trashed contacts were displaying in the report) and was going to file a PR, but see this already addresses it. The assumption throughout Civi is that trashed contacts are excluded unless options are explicitly provided to display them. I think the same should hold true here. Either add a filter option to include/not include trashed contacts, or hard code their exclusion.
Personally, I think that although a filter option wouldn't hurt, it isn't necessary unless there's specific demand.
|
I feel like this has had an amount of review appropriate to the nature of the change & @lcdservices makes a good case on the is_deleted. Merging |
