Allow for the deselection of sorting filters

[reecebenson]

Overview

Unable to remove sorting filters on reports, "removing" them and refreshing the results would retain the sorting filters regardless.

Steps to Reproduce

1. Go to CiviCRM dmaster
2. Navigate Reports -> Contact Reports
3. Select "Constituent Summary"
4. Open the "Sorting" tab
5. Add several Columns for sorting
6. Click "Refresh Results"
7. Open the "Sorting" tab
8. Delete several added Columns.
9. Click "Refresh Results"
10. Open the "Sorting" tab again, columns reappear.

Before

The IDs of the Column and Order elements did not reflect the same element IDs used to reset the values.

After

The IDs of the Column and Order elements now reflect the same element IDs used to reset the values. Successfully able to remove sorting filters.

Technical Details

Fixes incorrect element IDs used in JavaScript.

[civicrm-builder]

Can one of the admins verify this patch?

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

add to whitelist

[eileenmcnaughton]

@reecebenson that's for the PR - we'll see if we can get someone to give it a quick r-run & then it will be mergeable - I don't see any technical issue with the approach / your conclusions

Is this your first PR to CiviCRM? If so we'll need a second PR from you .... - adding you to our contributors file - https://github.com/civicrm/civicrm-core/blob/master/contributor-key.yml

[reecebenson]

@reecebenson that's for the PR - we'll see if we can get someone to give it a quick r-run & then it will be mergeable - I don't see any technical issue with the approach / your conclusions

Is this your first PR to CiviCRM? If so we'll need a second PR from you .... - adding you to our contributors file - https://github.com/civicrm/civicrm-core/blob/master/contributor-key.yml

It is indeed! I've submitted a previous PR but it was closed and someone else came up with an alternative (better) solution. I've done a second PR to add myself to the contributors here.

[eileenmcnaughton]

Ah you should really have been added as a contributor for the closed PR too as you did contribute even if an alternate was merged - but we have to pick that sort of thing up manually

[demeritcowboy]

I did a run. Of course I didn't look at the screenshot at first, so just noting for other people like me that this is referring to clicking on the triangle to the left of the dropdown item, as opposed to changing the dropdown item to -none-, which works both before and after. What happens though is that when you use the triangle you get this warning. And it's only when you use the triangle.

Notice: Undefined index: column in CRM_Report_Form->preProcessOrderBy() (line 4228 of /.../web/sites/all/modules/civicrm/CRM/Report/Form.php).

So I think the PR is partly fixing the problem but not fully, and at least now it's showing a separate problem which was hidden before, i.e. that the hideRow function is incomplete in some way.

I haven't tested but just looking at the dropdown I think instead of .val('') it needs to be .val('-');

[reecebenson]

I did a run. Of course I didn't look at the screenshot at first, so just noting for other people like me that this is referring to clicking on the triangle to the left of the dropdown item, as opposed to changing the dropdown item to -none-, which works both before and after. What happens though is that when you use the triangle you get this warning. And it's only when you use the triangle.

Notice: Undefined index: column in CRM_Report_Form->preProcessOrderBy() (line 4228 of /.../web/sites/all/modules/civicrm/CRM/Report/Form.php).

So I think the PR is partly fixing the problem but not fully, and at least now it's showing a separate problem which was hidden before, i.e. that the hideRow function is incomplete in some way.

I haven't tested but just looking at the dropdown I think instead of .val('') it needs to be .val('-');

Hi @demeritcowboy,

I was able to reproduce the same, but believe this disappears once using the value of "-" as you've suggested. I've updated the PR to reflect, would you mind testing again?

Cheers,
Reece

[demeritcowboy]

Looks good. Thanks @reecebenson

[eileenmcnaughton]

Merging based on @demeritcowboy review