Reports fail for non-admins with related fields #3155
Conversation
If a report is made by a normal user within a group it will fail if there are related fields in the report. This is due to the table alias not being passed along to the logic to include the SecurityGroups queries.
|
Jason, if I'm not mistaken, you're supposed to target the I hope you can find an option to just change the base branch of this PR, without having to re-do it... |
modules/AOR_Reports/AOR_Report.php
Outdated
| @@ -1208,7 +1208,7 @@ function build_report_query_join($name, $alias, $parentAlias, SugarBean $module, | |||
| $join = $module->$name->getJoin($params, true); | |||
| $query['join'][$alias] = $join['join']; | |||
| if($rel_module != null) { | |||
| $query['join'][$alias] .= $this->build_report_access_query($rel_module, $name); | |||
| $query['join'][$alias] .= $this->build_report_access_query($rel_module, "`$alias`"); | |||
There was a problem hiding this comment.
I think backticks are only supported in MySQL
There was a problem hiding this comment.
I followed the same practice already used in that file elsewhere.
There was a problem hiding this comment.
Matt, Jason, See #3165 for correct way to let the db manager handle adding backticks depending on if it's using to a database which supports them or not.
There was a problem hiding this comment.
See also PR #1086 very similar to this one.
|
@pgorod Sorry, I'll remember that next time. Anything that I need to do for this specific PR? |
|
@eggsurplus I see that mattlorimer has already changed the base branch on this PR, so now it's correct. |
|
@eggsurplus Is it possible to update your PR to reflect removing the backtick (didn't want to take your commit/fix away from you) :) We have updated areas outside DBManager to remove backticks directly into dynamic queries. |
|
Yes. Are you saying that the DBManager logic will no longer format with ticks going forward? |
|
Apologies @eggsurplus I should've been more clear. The DBManager deals with the backticks by a function.
https://github.com/salesagility/SuiteCRM/pull/3165/files#diff-aede7afff7f65db47e05554c029a1ceeR1381 |
|
I take some responsibility as well. Too much context switching! I made the change as requested. |
|
Right. Microsoft SQL Server and PostgreSQL don't support ticks. MySQL allows ticks yet doesn't require them. Standard SQL says use double quotes! |
|
Thanks for that @chris001. So at the moment outside of the MySQL DBManager there shouldn't be any backticks implemented into queries. The function:
provides the appropriate encapsulation. MSSQL = [ ] and MySQl is the backticks. Consideration to standardise this to double quotes is noted. However just looking it up there could be a consideration on the SQL Server for QUOTED_IDENTIFIER = OFF. |
|
This should be delved into deeper and determine what other database abstraction layers handle quoting ticks etc, e.g. Doctrine DB Abstraction Layer, it's sort of the state of the art. |
If a report is made by a normal user within a group it will fail if there are related fields in the report. This is due to the table alias not being passed along to the logic to include the SecurityGroups queries.