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

Modern Search: Item selection not returning results on target web part #2577

Closed
IvanTheBearable opened this issue Nov 24, 2022 · 10 comments
Closed
Labels
API Search API related bug Something isn't working enhancement New feature or request v4 version 4

Comments

@IvanTheBearable
Copy link

IvanTheBearable commented Nov 24, 2022

Version used
Ex: 4.7.0

Describe the bug
I have two search results web parts set up with selection enabled on one and the second set up to filter based on that selection. This has been working perfectly for several months. Several weeks ago, it stopped working. Selecting an item in the first web part brought back zero results in the second. The reason is the structure of the RefinementFilters value in the post query sent by the target web part:

{
    "results": [
        "RefinableString113:101410"
    ]
}

This is supposed to be an FQL filter and, according to the documentation, the above is correct. However, it needs quotes around the value to work.

{
    "results": [
        "RefinableString113:\"101410\""
    ]
}

I have reproduced the problem with the Search Query Tool, and verified the fix. So, technically, this is a problem with the SharePoint back-end. Something has changed. But, it has been several weeks now and there is no sign if it being changed back. And I haven't worked out where I can go to report the problem to MS. But, I thought that maybe it would be a good safety measure for the Search Results web part to just put the quotes around the value anyway.

@wobba wobba added the API Search API related label Nov 24, 2022
@wobba
Copy link
Collaborator

wobba commented Nov 24, 2022

API related and I would probably use prop:string("101410") or prop:"101410". Strings are double quotes and not single quotes in FQL/KQL. Agree that single term with quotes should work and you are free to log this as an issue at the sp-dev-docs issue.

And you can easily use KQL in the query template instead of a refinement filter for your case.

@IvanTheBearable
Copy link
Author

IvanTheBearable commented Nov 27, 2022

Interesting. I hadn't thought of using manual mode. It sort-of solved my problem, but not quite. In default mode {{filters.selectedFilters}} told my template how many items were selected. That doesn't appear to work in manual mode. (Also, I think I've found an issue with {? ...} handling - but I'll save that for another ticket).

Anyway, I'm not sure I completely agree with writing this off as an API issue. Technically, the issue is with the API. But, the FQL syntax documentation does state

FQL does not always require a string to be enclosed in double quotation marks. ... However, we recommend that you use double quotation marks to avoid conflicts with reserved words.

So it seems like it might be good default behaviour, in line with the recommendation, to always quote strings. I don't have an environment set up to be able to test but it looks like it might be a relatively simple change in DataFilterHelper.ts > buildFqlRefinementString. This is adding the quotes:

// Enclose the expression with quotes if the value contains spaces
if (/\s/.test(value) && value.indexOf('range') === -1) {
    value = `"${value}"`;
}

Obviously it is not as simple as removing the test, and I don't know enough about the search API to know if this is missing something, but maybe some else-if's so it doesn't break the date and empty cases. Maybe something like:

if (moment(value, moment.ISO_8601, true).isValid()) {
  if (!startDate && (filterValue.operator === FilterComparisonOperator.Geq || filterValue.operator === FilterComparisonOperator.Gt)) {
      startDate = value;
  }

  if (!endDate && (filterValue.operator === FilterComparisonOperator.Lt || filterValue.operator === FilterComparisonOperator.Leq)) {
      endDate = value;
  }
} else if (isEmpty(value)) {
  // If the value is null or undefined, we replace it by the FQL expression string('')
  // Otherwise the query syntax won't be vaild resuting of to an HTTP 500 
  value = "string('')";
} else {
  // Enclose the expression with quotes
  value = `"${value}"`;
}

Cheers

Ivan

@wobba
Copy link
Collaborator

wobba commented Nov 28, 2022

The {? ...} issue is known. And you should either use double quotation marks (KQL and FQL) or no quotation marks. Never single quotation marks. That has never worked.

And I suggest you open up a PR if we should add quotes, as it should work fine without quotes (assuming there is no space).

Also, how does "range" come into play as your initial question has a simple number? And RefinableString would not support date ranges. Maybe add some more detail about exact setup, the values in refinablestring etc? Re-opened for now if you can provide repro steps and clarify your setup.

@wobba wobba reopened this Nov 28, 2022
@IvanTheBearable
Copy link
Author

Sorry, the single quotes thing was a typo in my initial post. I have fixed it. The issue is that no-quotes used to work and now it doesn't. The fault is 100% with the API, not with the Modern Search web part. I am just suggesting that it might be a good idea for the Modern Search web part to wrap with quotes, regardless of spaces, because:

  1. It solves this particular problem and we have no way of knowing if MS will fix the issue with the API (I have logged it with sp-dev-docs - thanks for the pointer)
  2. The FQL documentation does recommend using quotes all the time, to avoid conflicts with reserved words.

The particular property I am testing with is a string. It happens to contain a number but it is a text field in SharePoint, mapped to a string managed property in search.

"range" does not come into play. I was just quoting the point in the code where I think the quotes are being added. It is testing for the presence of a space and for the absence of range (value.indexOf('range') === -1). If both are true, it is adding the quotes. It may be that the range test needs to be put back into that final else.

I don't have approval to spend the time required to set up the development environment and do all the work required to do a proper PR. I've requested it but these things take time. So I'm just trying to get as much useful information on record as I can.

@wobba
Copy link
Collaborator

wobba commented Nov 28, 2022

Make sense to add the workaround, and I'll see if I can log the API issue internally at Microsoft.

@wobba
Copy link
Collaborator

wobba commented Nov 28, 2022

Tested in the query tool and seems prop:value works fine both with and without double quotes. Can you try in the search query tool as well?

Then again the query tool use single quotes in the JSON.

'results': ['RefinableString08:VirtualMachineSetup']

@IvanTheBearable
Copy link
Author

OK, I did a bit more testing with a different property on a different tenant. It turns out that the value being filtered on must be number to reproduce the issue. i.e. This returns results:

{'request': { 'Querytext':'test', 'EnableInterleaving': false, 'SelectProperties': {'results': ['RefinableString01'
      ]
    }, 'RefinementFilters': {'results': ['RefinableString01:Administration'
      ]
    }, 'SourceId':'8413cd39-2156-4e00-b54d-11efd9abdb89', 'ClientType':'AllResultsQuery'
  }
}

This also returns results:

{'request': { 'Querytext':'test', 'EnableInterleaving': false, 'SelectProperties': {'results': ['RefinableString01'
      ]
    }, 'RefinementFilters': {'results': ['RefinableString01:"12345"'
      ]
    }, 'SourceId':'8413cd39-2156-4e00-b54d-11efd9abdb89', 'ClientType':'AllResultsQuery'
  }
}

This returns no results:

{'request': { 'Querytext':'test', 'EnableInterleaving': false, 'SelectProperties': {'results': ['RefinableString01'
      ]
    }, 'RefinementFilters': {'results': ['RefinableString01:12345'
      ]
    }, 'SourceId':'8413cd39-2156-4e00-b54d-11efd9abdb89', 'ClientType':'AllResultsQuery'
  }
}

So, it looks like the API is doing some new data type processing on unquoted values and converting what looks like a number into a number. It might turn out that that was a deliberate change. On the one hand that reinforces my suggestion that all strings should be quoted to avoid confusion. But it raises a whole new question about how do we know the data type of the filter value.

@wobba
Copy link
Collaborator

wobba commented Dec 5, 2022

@IvanTheBearable can you repro the same with KQL in the query template? RefinableString01:12345 vs RefinableString01:"12345" or is it FQL only? And agree adding quotes seems smart. But if it's a number property we shouldn't. Add checks on the property name maybe.

@wobba wobba added bug Something isn't working enhancement New feature or request v4 version 4 labels Dec 5, 2022
@wobba
Copy link
Collaborator

wobba commented May 28, 2023

@IvanTheBearable What do you think about the change I proposed in #3039? It's a workaround. I tried to start testing with RefinableInt's and might be an API issue with those. So figured I'd take this one case only for now.

@wobba
Copy link
Collaborator

wobba commented Jun 4, 2023

Decided to merge it, and will then work for numbers in RefinableString filters.

@wobba wobba closed this as completed Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Search API related bug Something isn't working enhancement New feature or request v4 version 4
Projects
None yet
Development

No branches or pull requests

2 participants