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

Improved search capabilities #1021

Merged
merged 10 commits into from
Jun 29, 2021
Merged

Improved search capabilities #1021

merged 10 commits into from
Jun 29, 2021

Conversation

grolu
Copy link
Contributor

@grolu grolu commented May 19, 2021

Added extended search capabilities

What this PR does / why we need it:
Adds more search capabilities (see below). This should solve several raised requirements (see linked issues).
Moreover, this PR introduces a search delay to prevent multiple searches to block the UI while typing.

Screenshot 2021-06-25 at 15 55 48

Which issue(s) this PR fixes:
Fixes #1004 Fixes #994 Fixes #960

Special notes for your reviewer:

Release note:

Added extended search capabilities to cluster search:
- Search params are now ANDed, allowing one to refine the search
- Use quotes for exact words or phrases
- Use minus sign to exclude words that you don't want

grolu added 2 commits May 18, 2021 09:55
Added extended search capabilities
@gardener-robot gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels May 19, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 19, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 19, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 19, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 19, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 19, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 19, 2021
@holgerkoser
Copy link
Contributor

I would propose to go with the gmail like advanced search query syntax and use something like search-query-parser.

const searchQueryParser = require("search-query-parser")

function parseSearchQuery(query) {
  const options = {
    tokenize: true,
    alwaysArray: true,
    offsets: false,
    keywords: ["name", "infra", "seed", "from", "purpose", "version", "ticket"],
    ranges: ["date"],
  }
  return searchQueryParser.parse(query, options)
}
const query =
  'from:foo@sap.com,bar@sap.com name:delete purpose:eval date:20200503-20210501 "free text" foo -hugo';

console.log(parseSearchQuery(query))
/*
{
  text: [ 'free text', 'foo' ],
  from: [ 'foo@sap.com', 'bar@sap.com' ],
  name: [ 'delete' ],
  purpose: [ 'eval' ],
  date: { from: '20200503', to: '20210501' },
  exclude: { text: 'hugo' }
}
*/

@grolu
Copy link
Contributor Author

grolu commented May 19, 2021

@holgerkoser I thought about that, too. I started with this as an attempt to keep it simple. Idk how parsers perform; we already face huge performance issues in larger lists. We can test it, though.

@gardener-robot
Copy link

@holgerkoser You have pull request review open invite, please check

@gardener-robot gardener-robot added lifecycle/stale Nobody worked on this for 6 months (will further age) lifecycle/rotten Nobody worked on this for 12 months (final aging stage) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels May 27, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 21, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 21, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 21, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 21, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 23, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 23, 2021
Comment on lines 154 to 155
const searchPattern = /(-?"[^"]+")*([\S]+)*/g
const termPattern = /^(-)?("?)([^"]+)("?)$/g
Copy link
Contributor

Choose a reason for hiding this comment

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

add tests for the regex so that we can be sure in the future that it still works as expected in case we need to adapt it

@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jun 25, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 25, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 25, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 25, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 25, 2021
Copy link
Contributor

@petersutter petersutter left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Jun 25, 2021
Copy link
Contributor

@holgerkoser holgerkoser left a comment

Choose a reason for hiding this comment

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

\lgtm

@grolu grolu merged commit 17bfd7e into master Jun 29, 2021
@grolu grolu deleted the enh/shoot_search_improvements branch July 20, 2021 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Nobody worked on this for 12 months (final aging stage) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/m Size of pull request is medium (see gardener-robot robot/bots/size.py)
Projects
None yet
7 participants