removed REGEXP_IN_MATCH regex to make it flexible to use with plugins (PR #10476)

GitHub

Can you provide more context for this change? It’s not clear why we should be doing this.

Can you provide more context for this change? It’s not clear why we should be doing this.

I was working with discourse-assign plugin, in that we needed to add 2 settings in advanced-search-settings which are in:assigned and in:not_assigned. When I tried doing that I get the issue when I select value from in dropdown then it doesn’t shows in header due to this REGEXP.

So, I conveyed to @davidtaylorhq regarding this and added this PR

@davidtaylorhq assigning you since you have the background here. It does seem like this change would affect the current behaviour but maybe I’m missing something.

@Ahmedgagan I checked this out locally, and it does seem to introduce a behaviour change. When I tick one of the ‘special’ in: checkboxes, the dropdown value changes:

Screenshot 2020-08-28 at 16 23 53

It also allows any value to appear in the dropdown, even if it doesn’t work:

Screenshot 2020-08-28 at 16 26 07

Previously, this would not happen:

Screenshot 2020-08-28 at 16 24 59

So I think we need a different solution here. Maybe we could build the regex at runtime, based on the values in inOptionsForUsers and inOptionsForAll?

@Ahmedgagan I checked this out locally, and it does seem to introduce a behaviour change. When I tick one of the ‘special’ in: checkboxes, the dropdown value changes:

Screenshot 2020-08-28 at 16 23 53

It also allows any value to appear in the dropdown, even if it doesn’t work:

Screenshot 2020-08-28 at 16 26 07

Previously, this would not happen:

Screenshot 2020-08-28 at 16 24 59

So I think we need a different solution here. Maybe we could build the regex at runtime, based on the values in inOptionsForUsers and inOptionsForAll?

Sure, I’ll take a look at this issue😅

BTW @davidtaylorhq this also happens in status dropdown, so can we the behaviour same as status dropdown or we need to change it here?

If we can fix the status dropdown as well, that would be great!

Look good to me :+1:

Any chances you could add an acceptance test to ensure this work properly and doesn’t regress in the future?

Look good to me :+1:

Any chances you could add an acceptance test to ensure this work properly and doesn’t regress in the future?

Sure, I’ll add the acceptance test for this😀

    const statusSelector = selectKit(".search-advanced-options .select-kit#status");
    assert.equal(statusSelector.header().label(), "any", 'has "any" populated');

What is this line testing? On line 302 you entered “status:none” but then never changed it.

Our issue was when we enter status:any_value then the header of dropdown was displaying any_value so to remove that we added this PR, so in this test, I filled status:none which is not a value in the dropdown, so it won’t change the header of dropdown but before it would display none in the dropdown header

Yup, and that’s what line 304 is testing. It’s testing that the value of the dropdown didn’t change when you entered an invalid value in the search query.

But this line (305) is checking that the search query is still “status:none”, which is always true since you entered that value on line 302 and then never changed it.

oooh got it, that’s right, ill remove it and add a commit in 5 mins