fix(timeline): Filter links with SQL and only show filter-compatible results #1346
Conversation
|
Changes Unknown when pulling 8f00705 on Mardak:gh1302-loading into * on mozilla:master*. |
Mardak
force-pushed
the
gh1302-loading
branch
from
8f00705 to
f77db2f
Compare
| // Match each token in either the title or url | ||
| let tokens = filter.trim().toLowerCase().split(/\s+/); | ||
| tokens.forEach((token, i) => { | ||
| filterClause = `${filterClause} AND ( |
There was a problem hiding this comment.
Question: how come we're using the blank string from filterClause here?
There was a problem hiding this comment.
The other Clauses start with "AND ..." because they're chaining to the WHERE. This is the same except there's a dynamic number of chains based on how many tokens are being used, e.g., " AND token1 ... AND token2 ... AND token3 ..." The empty string is the initial value so that the subsequent token clauses can be concatenated.
| function RequestMoreRecentLinks(beforeDate, filter = "") { | ||
| return RequestRecentLinks({ | ||
| data: {beforeDate}, | ||
| data: {beforeDate, filter}, |
There was a problem hiding this comment.
just to make sure, this is for infinite scrolling yes?
There was a problem hiding this comment.
Yup. It's to include the query for fetching more when infinitely scrolling.
| Filter, | ||
| History | ||
| History: Object.assign({}, History, { | ||
| // Only include rows that are less filtered than the current filter |
There was a problem hiding this comment.
the comment is kind of unclear - could you explain?
There was a problem hiding this comment.
A query of "query" is more filtered/specific than a query of "qu" which is more specific than "". If the current query is "que" it would include rows for "" and "qu" but not "query" because "query" is more specific. This is desired because the user might be searching for "queso".
|
looks good, R+! |
Fix #1302 by passing the filter to
PlacesProviderallowing it to do the multi-token matching on title/url. These SQL-filtered results are tagged with the filter they were matched with, so that the selector only shows the results that are relevant to the current filter. This is faster because the 20 results the timeline gets are already most likely going to match the filter instead of going through many "pages" of non-matches.This also fixes #1261 at the same time as it doesn't try showing many pages of results when clearing search. By including the tagged filter, more specific searches can be quickly ignored when deleting in addition to there being fewer fetched rows in the first place.
r?@sarracini