Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FilterableDataTable] Fix issue with GROUPCONCAT and multiselect #7453

Merged

Conversation

pierre-p-s
Copy link
Contributor

@pierre-p-s pierre-p-s commented May 17, 2021

Brief summary of changes

When using a multiselect on a group concat set of parameters, the filters gave only perfect matches. For example, if a candidate has 2 subproject AD and MCI, setting the filter to AD will not return the candidate although it should

Issue caught after release on CCNA of v23.0.3

Also thanks @HenriRabalais for the help

jsx/DataTable.js Outdated Show resolved Hide resolved
Co-authored-by: CamilleBeau <[email protected]>
jsx/Filter.js Outdated Show resolved Hide resolved
@CamilleBeau CamilleBeau added the Passed manual tests PR has been successfully tested by at least one peer label May 19, 2021
Copy link
Collaborator

@HenriRabalais HenriRabalais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is definitely functionality worth investigating since it seems like it has a big CCNA use case. I think the issue here though is that this PR changes the functionality of the multiselect filter from operating using an equals paradigm, to an includes paradigm. In my opinion, a multiselect should function as multiple selects. Meaning, exact matching between all the selected filter fields, and all the data fields. From what I understand, the proposed PR's changes this to an includes or "multisearch" paradigm, analogous to a search bar that has predetermined options to choose from, which still allow the data to pass the filter even if it is a partial match.

Ideally, I would eventually like to see a drop down beside every filter that allows the user to select the filtering style as equals (e.g. =), includes (e.g. ⊂ or ≈), excludes (e.g. ! or ⊄), or any other relevant filtering types. In the meantime, I personally think this functionality could go into a filter type of it's own, so as not to replace the current multiselect functionality.

I think a discussion of this at a Loris meeting could be beneficial, but I know @laemtl and @ridz1208 had opinions on this as well.

@pierre-p-s
Copy link
Contributor Author

pierre-p-s commented Jun 3, 2021

After discussion during loris meeting, this fix should go through and we should look into making a toggle for the filter that Henri proposed in his above comment. Issue created for new feature: #7458

@driusan
Copy link
Collaborator

driusan commented Jun 3, 2021

@HenriRabalais can you update your review

@HenriRabalais HenriRabalais dismissed their stale review June 4, 2021 17:33

Issue discussed at the loris meeting

@pierre-p-s pierre-p-s requested a review from HenriRabalais June 14, 2021 18:35
@driusan driusan merged commit 36d1d0a into aces:23.0-release Jun 16, 2021
@ridz1208 ridz1208 added this to the 23.0.4 milestone Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed manual tests PR has been successfully tested by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants