(dev/core#2979) remove the limit of 15 max values for multiple values… #22214
Conversation
|
(Standard links)
|
|
Any idea why it's 15? I don't see anything in this history that explains it. Is there a reason not to just remove the limit completely? |
|
Jenkins re test this please |
CRM/Report/Utils/Get.php
Outdated
| if (!preg_match('/^(\d+)(,\d+){0,14}$/', $value)) { | ||
|
|
||
| //change the max value to 20, ideally remove condition | ||
| if (!preg_match('/^(\d+)(,\d+){0,20}$/', $value)) { | ||
| // extra check. Also put a limit of 15 max values. |
There was a problem hiding this comment.
@yashodha should remove this comment as well then
|
@demeritcowboy Yes, ideally no need to keep a max limit. We might be better off removing this condition. This will just result in not respecting the criteria as the value is set to null when the limit is exceeded. |
|
Maybe there should be some limit to prevent some potential denial of service problem. Anyway I don't have any objections, was just wondering if there was some reason for it being "15". Do you want to remove the comment at line 176? |
b390cc1 to
7e875c8
Compare
|
test this please |
|
@demeritcowboy Thanks for merging this! |
… can also be retrieved from url in reports
Overview
We send the parameters as string so that multiple values can also be retrieved from url in reports
for e.g url like - "memtype_in=in&memtype_value=1,2,3
However, there is a restriction for 15 values esp when you search for activity types which can be numerous. In this this case, if the values exceeds 15 the criteria are not respected.
Proposal to remove the limit of 15 max values.
https://github.com/civicrm/civicrm-core/blob/master/CRM/Report/Utils/Get.php#L173
Before
params not respected if values exceed 15
After
values till 20 are respected.
Comments
Let me know if the max value is agreeable. It's especially for activity types as we already have a number of activity types that CiviCRM ships with.