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

Advanced search queries #2934

Merged
merged 12 commits into from
Apr 13, 2021
Merged

Advanced search queries #2934

merged 12 commits into from
Apr 13, 2021

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Apr 1, 2021

Based on #2933

  • Implement advanced search queries (see detailed description below)
  • Proper pagination in unified search providers
  • Implement card comment search in unified search
  • Add global search result UI to deck
  • Extend card details route in frontend to navigate to a specific sidebar tab directly
  • Some refactoring on the integration tests in order to writing the search tests and further ones more straight forward

Global search in Deck

image

A dedicated inline search input was added in order to properly indicate that a search filtering is still active even if the unified search is closed:

image

Search queries

The search will only return results that match all of the search tokens.

Example query: project tag:ToDo assigned:alice assigned:bob

Will return all cards where the card title or description contains project AND the tag ToDo is set AND the user alice is assigned AND the user bob is assigned

Filter Operators Query Tests
title : text token used for a case-insentitive search on the cards title x
description : text token used for a case-insentitive search on the cards description x
list : text token used for a case-insentitive search on the cards list name x
tag : text token used for a case-insentitive search on the assigned tags x
date : 'overdue', 'today', 'week', 'month', 'none' x
> < >= <= Compare the card due date to the passed date (see supported date formats) Card due dates are always considered UTC for comparison x
assigned : id or displayname of a user or group for a search on the assigned users or groups x

Other text tokens will be used to perform a case-insensitive search on the card title and description

Quotes can be used to pass a query with spaces, e.g. title:"My card"

return SearchResult::complete(
if (count($cardResults) < $query->getLimit()) {
return SearchResult::complete(
'Deck',
Copy link
Member

Choose a reason for hiding this comment

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

Translate?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess that is a different discussion, as we currently never translate the app name for deck.

);
}

return SearchResult::paginated(
'Deck',
Copy link
Member

Choose a reason for hiding this comment

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

same here

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

partial review

lib/Db/Card.php Outdated Show resolved Hide resolved
lib/Db/CardMapper.php Show resolved Hide resolved
lib/Db/CardMapper.php Outdated Show resolved Hide resolved
lib/Db/CardMapper.php Show resolved Hide resolved
lib/Db/CardMapper.php Outdated Show resolved Hide resolved
lib/Search/FilterStringParser.php Show resolved Hide resolved
lib/Service/SearchService.php Show resolved Hide resolved
src/components/search/GlobalSearchResults.vue Outdated Show resolved Hide resolved
src/components/search/GlobalSearchResults.vue Show resolved Hide resolved
<div class="card--placeholder">
<svg class="card-placeholder__gradient">
<defs>
<linearGradient id="card-placeholder__gradient">
Copy link
Member

Choose a reason for hiding this comment

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

please check the placeholder from Talk, I had to replace it as it was causing high cpu load due to dynamic SVG gradient not being GPU accelerated: nextcloud/spreed#4554

or raise a ticket to change this later

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do a follow up ticket for this and the unified search in the server where this was inspired from

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

second part of review

} else if (filter === 'tag') {
hasMatch = hasMatch && card.labels.findIndex((label) => label.title.toLowerCase().includes(filterOutQuotes(query).toLowerCase())) !== -1
} else if (filter === 'date') {
const datediffHour = ((new Date(card.duedate) - new Date()) / 3600 / 1000)
Copy link
Member

Choose a reason for hiding this comment

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

maybe use moment for diffing dates to avoid potential troubles when crossing DST offsets (rare, so probably not cricial)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for now using the same logic as the local board filters but I'll split this into a separate ticket for refining both both them.

Copy link
Member Author

Choose a reason for hiding this comment

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

src/store/card.js Show resolved Hide resolved
return false
}
const comparator = query[0] + (query[1] === '=' ? '=' : '')
const isValidComparator = ['<', '<=', '>', '>='].indexOf(comparator) !== -1
Copy link
Member

Choose a reason for hiding this comment

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

throw error in case of no valid comparator ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Until now invalid queries would just attempt to perform an equal comparison for whatever is passed after the :. I'd keep that for now and move the handling improvement of that into a ticket so we can discuss where and how to show possible errors especially with the unified search, where we'd need server improvements to return possible hints or errors to the user.

Copy link
Member Author

Choose a reason for hiding this comment

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

tests/integration/features/search.feature Show resolved Hide resolved
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

Code looks good now, thanks for the adjustments 👍

Base automatically changed from enh/fts-events to master April 13, 2021 12:12
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants