FIX Allow `convertSearchToArray` to handle repeating elements

[mikenuguid]

Description

Manual testing steps

Issues

• https://github.com/silverstripe/silverstripe-admin/issues/1632

Pull request checklist

• [x] The target branch is correct
  • See picking the right version
• [x] All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
  • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
• [x] The commit messages follow our commit message guidelines
• [x] The PR follows our contribution guidelines
• [x] Code changes follow our coding conventions
• [x] This change is covered with tests (or tests aren't necessary for this change)
• [ ] Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
• [x] CI is green

[GuySartorelli]

Hi,

Thank you for the contribution.

I see that you've deleted the PR templte. I've added it back in. Can you please fill in the template, and check all of the boxes that apply? This really helps us get a feel for what state your contribution is in, what it does, and gives us a good starting point to review it.

[mikenuguid]

@GuySartorelli any feedback on this? There's a merge conflict which needs to be resolved too.

[GuySartorelli]

Sorry, I hadn't noticed you had updated the PR description. Can you please add some manual testing steps? It's not clear how I can test this piece of work. Please reply when you've updated it so I know it's ready for me to look at it again.

[mikenuguid]

@GuySartorelli you could use content review report.

• Once added, open one of the existing reports e.g. pages due for review report
• Update paremeterFields() method with a ListboxField e.g.

  $filtersList->push(
  ListboxField::create('testlistbox', 'listbox', ['1', '2', '3', '4'])
  );

• Open the updated report on the CMS and select at least 2 options from the listbox
• The URL should include both options on the filter e.g.

  admin/reports/show/SilverStripe-ContentReview-Reports-PagesDueForReviewReport?filters%5Btestlistbox%5D%5B%5D=0%2C1 // admin/reports/show/SilverStripe-ContentReview-Reports-PagesDueForReviewReport?filters[testlistbox][]=0,1

• Previously it only displays a single value, now it should display multiple values

[GuySartorelli]

Following those instructions, both before and after the PR I get ?filters%5Btestlistbox%5D%5B%5D=0&filters%5Btestlistbox%5D%5B%5D=1 (i.e. ?filters[testlistbox][]=0&filters[testlistbox][]=1) as the query string - which is correct.

It seems like this is making no change for me. Tested with both firefox and chromium.

Are there any more steps you can provide me to reproduce this from a fresh installation (with or without silverstripe/contentreview added)?

[mikenuguid]

That's different from what I'm getting. Also, the output of that js method (convertSearchToArray) should not contain duplicate keys so the GET params should be filters[testlistbox][]=0,1 (if fix is applied) and filters[testlistbox][]=1 (if not).

[GuySartorelli]

That's different from what I'm getting.

Okay, well... if you can give me sufficiently detailed steps to reproduce both the original problem and the behaviour you're seeing with your PR I'll be happy to review further - but if I can't reproduce the bug nor the results you're seeing with the PR I can't merge this.