From be7d7d117a07033e21bfd1fbac175a538669ed68 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 29 Dec 2021 00:03:09 +0100 Subject: [PATCH 1/6] Implement `processSearchQuery()` function --- app/utils/search.js | 34 ++++++++++++++++++++++++++++++++++ tests/utils/search-test.js | 22 ++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 app/utils/search.js create mode 100644 tests/utils/search-test.js diff --git a/app/utils/search.js b/app/utils/search.js new file mode 100644 index 00000000000..75b8ac7718c --- /dev/null +++ b/app/utils/search.js @@ -0,0 +1,34 @@ +const KEYWORDS_PREFIX = 'keywords:'; + +/** + * Process a search query string and extract filters like `keywords:`. + * + * @param {string} query + * @return {{ q: string, keyword?: string, all_keywords?: string }} + */ +export function processSearchQuery(query) { + let tokens = query.trim().split(/\s+/); + + let queries = []; + let keywords = []; + for (let token of tokens) { + if (token.startsWith(KEYWORDS_PREFIX)) { + keywords = token + .slice(KEYWORDS_PREFIX.length) + .split(',') + .map(it => it.trim()) + .filter(Boolean); + } else { + queries.push(token); + } + } + + let result = { q: queries.join(' ') }; + if (keywords.length === 1) { + result.keyword = keywords[0]; + } else if (keywords.length !== 0) { + result.all_keywords = keywords.join(' '); + } + + return result; +} diff --git a/tests/utils/search-test.js b/tests/utils/search-test.js new file mode 100644 index 00000000000..bc86a02efdc --- /dev/null +++ b/tests/utils/search-test.js @@ -0,0 +1,22 @@ +import { module, test } from 'qunit'; + +import { processSearchQuery } from '../../utils/search'; + +module('processSearchQuery()', function () { + const TESTS = [ + ['foo', { q: 'foo' }], + [' foo bar ', { q: 'foo bar' }], + ['foo keywords:bar', { q: 'foo', keyword: 'bar' }], + ['foo keywords:', { q: 'foo' }], + ['keywords:bar foo', { q: 'foo', keyword: 'bar' }], + ['foo \t keywords:bar baz', { q: 'foo baz', keyword: 'bar' }], + ['foo keywords:bar,baz', { q: 'foo', all_keywords: 'bar baz' }], + ['foo keywords:bar keywords:baz', { q: 'foo', keyword: 'baz' }], + ]; + + for (let [input, expectation] of TESTS) { + test(input, function (assert) { + assert.deepEqual(processSearchQuery(input), expectation); + }); + } +}); From 9d503303bb6fe13281548cdd0f9d43b35705bd7a Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 29 Dec 2021 00:05:07 +0100 Subject: [PATCH 2/6] controllers/search: Add support for search query filter processing --- app/controllers/search.js | 7 +++++- tests/acceptance/search-test.js | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/app/controllers/search.js b/app/controllers/search.js index b666880eba3..45005de5bf9 100644 --- a/app/controllers/search.js +++ b/app/controllers/search.js @@ -7,6 +7,7 @@ import { restartableTask } from 'ember-concurrency'; import { bool, reads } from 'macro-decorators'; import { pagination } from '../utils/pagination'; +import { processSearchQuery } from '../utils/search'; export default class SearchController extends Controller { @service store; @@ -61,6 +62,10 @@ export default class SearchController extends Controller { q = q.trim(); } - return yield this.store.query('crate', { all_keywords, page, per_page, q, sort }); + let searchOptions = all_keywords + ? { page, per_page, sort, q, all_keywords } + : { page, per_page, sort, ...processSearchQuery(q) }; + + return yield this.store.query('crate', searchOptions); } } diff --git a/tests/acceptance/search-test.js b/tests/acceptance/search-test.js index 5ab206ac6b2..8998bad7c41 100644 --- a/tests/acceptance/search-test.js +++ b/tests/acceptance/search-test.js @@ -191,4 +191,42 @@ module('Acceptance | search', function (hooks) { await visit('/search?q=rust&page=3&per_page=15&sort=new&all_keywords=fire ball'); assert.verifySteps(['/api/v1/crates']); }); + + test('supports `keyword:bla` filters', async function (assert) { + this.server.get('/api/v1/crates', function (schema, request) { + assert.step('/api/v1/crates'); + + assert.deepEqual(request.queryParams, { + all_keywords: 'fire ball', + page: '3', + per_page: '15', + q: 'rust', + sort: 'new', + }); + + return { crates: [], meta: { total: 0 } }; + }); + + await visit('/search?q=rust keywords:fire,ball&page=3&per_page=15&sort=new'); + assert.verifySteps(['/api/v1/crates']); + }); + + test('`all_keywords` query parameter takes precedence over `keyword` filters', async function (assert) { + this.server.get('/api/v1/crates', function (schema, request) { + assert.step('/api/v1/crates'); + + assert.deepEqual(request.queryParams, { + all_keywords: 'fire ball', + page: '3', + per_page: '15', + q: 'rust keywords:foo', + sort: 'new', + }); + + return { crates: [], meta: { total: 0 } }; + }); + + await visit('/search?q=rust keywords:foo&page=3&per_page=15&sort=new&all_keywords=fire ball'); + assert.verifySteps(['/api/v1/crates']); + }); }); From 0ddea11b83e0782c27f1faefc8b2c62eb207bf16 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 29 Dec 2021 00:07:30 +0100 Subject: [PATCH 3/6] processSearchQuery: Add support for `keyword:` filter --- app/utils/search.js | 8 +++++++- tests/utils/search-test.js | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/app/utils/search.js b/app/utils/search.js index 75b8ac7718c..a6a1abb189d 100644 --- a/app/utils/search.js +++ b/app/utils/search.js @@ -1,3 +1,4 @@ +const KEYWORD_PREFIX = 'keyword:'; const KEYWORDS_PREFIX = 'keywords:'; /** @@ -12,7 +13,12 @@ export function processSearchQuery(query) { let queries = []; let keywords = []; for (let token of tokens) { - if (token.startsWith(KEYWORDS_PREFIX)) { + if (token.startsWith(KEYWORD_PREFIX)) { + let value = token.slice(KEYWORD_PREFIX.length).trim(); + if (value) { + keywords.push(value); + } + } else if (token.startsWith(KEYWORDS_PREFIX)) { keywords = token .slice(KEYWORDS_PREFIX.length) .split(',') diff --git a/tests/utils/search-test.js b/tests/utils/search-test.js index bc86a02efdc..7c38f9d4f31 100644 --- a/tests/utils/search-test.js +++ b/tests/utils/search-test.js @@ -12,6 +12,7 @@ module('processSearchQuery()', function () { ['foo \t keywords:bar baz', { q: 'foo baz', keyword: 'bar' }], ['foo keywords:bar,baz', { q: 'foo', all_keywords: 'bar baz' }], ['foo keywords:bar keywords:baz', { q: 'foo', keyword: 'baz' }], + ['foo keyword:bar keyword:baz', { q: 'foo', all_keywords: 'bar baz' }], ]; for (let [input, expectation] of TESTS) { From de64ff6eeed73bbab5dec0a7debb757b435fff2c Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 29 Dec 2021 00:12:56 +0100 Subject: [PATCH 4/6] processSearchQuery: Add support for `category:` filter --- app/utils/search.js | 16 ++++++++++++++-- tests/utils/search-test.js | 3 +++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/utils/search.js b/app/utils/search.js index a6a1abb189d..52a2b39d8e2 100644 --- a/app/utils/search.js +++ b/app/utils/search.js @@ -1,3 +1,4 @@ +const CATEGORY_PREFIX = 'category:'; const KEYWORD_PREFIX = 'keyword:'; const KEYWORDS_PREFIX = 'keywords:'; @@ -5,15 +6,21 @@ const KEYWORDS_PREFIX = 'keywords:'; * Process a search query string and extract filters like `keywords:`. * * @param {string} query - * @return {{ q: string, keyword?: string, all_keywords?: string }} + * @return {{ q: string, keyword?: string, all_keywords?: string, category?: string }} */ export function processSearchQuery(query) { let tokens = query.trim().split(/\s+/); let queries = []; let keywords = []; + let category = null; for (let token of tokens) { - if (token.startsWith(KEYWORD_PREFIX)) { + if (token.startsWith(CATEGORY_PREFIX)) { + let value = token.slice(CATEGORY_PREFIX.length).trim(); + if (value) { + category = value; + } + } else if (token.startsWith(KEYWORD_PREFIX)) { let value = token.slice(KEYWORD_PREFIX.length).trim(); if (value) { keywords.push(value); @@ -30,11 +37,16 @@ export function processSearchQuery(query) { } let result = { q: queries.join(' ') }; + if (keywords.length === 1) { result.keyword = keywords[0]; } else if (keywords.length !== 0) { result.all_keywords = keywords.join(' '); } + if (category) { + result.category = category; + } + return result; } diff --git a/tests/utils/search-test.js b/tests/utils/search-test.js index 7c38f9d4f31..2209e658b6c 100644 --- a/tests/utils/search-test.js +++ b/tests/utils/search-test.js @@ -13,6 +13,9 @@ module('processSearchQuery()', function () { ['foo keywords:bar,baz', { q: 'foo', all_keywords: 'bar baz' }], ['foo keywords:bar keywords:baz', { q: 'foo', keyword: 'baz' }], ['foo keyword:bar keyword:baz', { q: 'foo', all_keywords: 'bar baz' }], + ['foo category:', { q: 'foo' }], + ['foo category:no-std', { q: 'foo', category: 'no-std' }], + ['foo category:no-std keywords:bar,baz', { q: 'foo', all_keywords: 'bar baz', category: 'no-std' }], ]; for (let [input, expectation] of TESTS) { From f8e87223b0dc98cbb8fdce4e23d04853329bf787 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Fri, 7 Jan 2022 19:22:13 +0100 Subject: [PATCH 5/6] processSearchQuery: Remove support for `keywords:` filter as discussed in the team meeting --- app/utils/search.js | 9 +-------- tests/acceptance/search-test.js | 2 +- tests/utils/search-test.js | 12 +++++------- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/app/utils/search.js b/app/utils/search.js index 52a2b39d8e2..f7b04897e23 100644 --- a/app/utils/search.js +++ b/app/utils/search.js @@ -1,9 +1,8 @@ const CATEGORY_PREFIX = 'category:'; const KEYWORD_PREFIX = 'keyword:'; -const KEYWORDS_PREFIX = 'keywords:'; /** - * Process a search query string and extract filters like `keywords:`. + * Process a search query string and extract filters like `keyword:`. * * @param {string} query * @return {{ q: string, keyword?: string, all_keywords?: string, category?: string }} @@ -25,12 +24,6 @@ export function processSearchQuery(query) { if (value) { keywords.push(value); } - } else if (token.startsWith(KEYWORDS_PREFIX)) { - keywords = token - .slice(KEYWORDS_PREFIX.length) - .split(',') - .map(it => it.trim()) - .filter(Boolean); } else { queries.push(token); } diff --git a/tests/acceptance/search-test.js b/tests/acceptance/search-test.js index 8998bad7c41..99fd9ed9041 100644 --- a/tests/acceptance/search-test.js +++ b/tests/acceptance/search-test.js @@ -207,7 +207,7 @@ module('Acceptance | search', function (hooks) { return { crates: [], meta: { total: 0 } }; }); - await visit('/search?q=rust keywords:fire,ball&page=3&per_page=15&sort=new'); + await visit('/search?q=rust keyword:fire keyword:ball&page=3&per_page=15&sort=new'); assert.verifySteps(['/api/v1/crates']); }); diff --git a/tests/utils/search-test.js b/tests/utils/search-test.js index 2209e658b6c..ba59b68ac1d 100644 --- a/tests/utils/search-test.js +++ b/tests/utils/search-test.js @@ -6,16 +6,14 @@ module('processSearchQuery()', function () { const TESTS = [ ['foo', { q: 'foo' }], [' foo bar ', { q: 'foo bar' }], - ['foo keywords:bar', { q: 'foo', keyword: 'bar' }], - ['foo keywords:', { q: 'foo' }], - ['keywords:bar foo', { q: 'foo', keyword: 'bar' }], - ['foo \t keywords:bar baz', { q: 'foo baz', keyword: 'bar' }], - ['foo keywords:bar,baz', { q: 'foo', all_keywords: 'bar baz' }], - ['foo keywords:bar keywords:baz', { q: 'foo', keyword: 'baz' }], + ['foo keyword:bar', { q: 'foo', keyword: 'bar' }], + ['foo keyword:', { q: 'foo' }], + ['keyword:bar foo', { q: 'foo', keyword: 'bar' }], + ['foo \t keyword:bar baz', { q: 'foo baz', keyword: 'bar' }], ['foo keyword:bar keyword:baz', { q: 'foo', all_keywords: 'bar baz' }], ['foo category:', { q: 'foo' }], ['foo category:no-std', { q: 'foo', category: 'no-std' }], - ['foo category:no-std keywords:bar,baz', { q: 'foo', all_keywords: 'bar baz', category: 'no-std' }], + ['foo category:no-std keyword:bar keyword:baz', { q: 'foo', all_keywords: 'bar baz', category: 'no-std' }], ]; for (let [input, expectation] of TESTS) { From 2144fcac601f996d32d3593479326ec10cdb9d53 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 20 Jan 2022 21:08:08 +0100 Subject: [PATCH 6/6] controllers/search: Show warning when using multiple `category:` filters --- app/controllers/search.js | 7 ++++++- app/styles/application.module.css | 12 ++++++++++++ app/styles/search.module.css | 9 +++++++++ app/templates/search.hbs | 6 ++++++ app/utils/search.js | 2 +- 5 files changed, 34 insertions(+), 2 deletions(-) diff --git a/app/controllers/search.js b/app/controllers/search.js index 45005de5bf9..7558f097661 100644 --- a/app/controllers/search.js +++ b/app/controllers/search.js @@ -7,7 +7,7 @@ import { restartableTask } from 'ember-concurrency'; import { bool, reads } from 'macro-decorators'; import { pagination } from '../utils/pagination'; -import { processSearchQuery } from '../utils/search'; +import { CATEGORY_PREFIX, processSearchQuery } from '../utils/search'; export default class SearchController extends Controller { @service store; @@ -49,6 +49,11 @@ export default class SearchController extends Controller { @bool('totalItems') hasItems; + get hasMultiCategoryFilter() { + let tokens = this.q.trim().split(/\s+/); + return tokens.filter(token => token.startsWith(CATEGORY_PREFIX)).length > 1; + } + @action fetchData() { this.dataTask.perform().catch(() => { // we ignore errors here because they are handled in the template already diff --git a/app/styles/application.module.css b/app/styles/application.module.css index cb85369db02..86a5acf81ae 100644 --- a/app/styles/application.module.css +++ b/app/styles/application.module.css @@ -6,6 +6,18 @@ --grey200: hsl(200, 17%, 96%); --green800: hsl(115, 31%, 31%); --green900: hsl(115, 31%, 21%); + + --orange-50: #fff7ed; + --orange-100: #ffedd5; + --orange-200: #fed7aa; + --orange-300: #fdba74; + --orange-400: #fb923c; + --orange-500: #f97316; + --orange-600: #ea580c; + --orange-700: #c2410c; + --orange-800: #9a3412; + --orange-900: #7c2d12; + --yellow500: hsl(44, 100%, 60%); --yellow700: hsl(44, 67%, 50%); diff --git a/app/styles/search.module.css b/app/styles/search.module.css index 84c5d76e6be..2c3df955803 100644 --- a/app/styles/search.module.css +++ b/app/styles/search.module.css @@ -5,6 +5,15 @@ margin-bottom: 25px; } +.warning { + margin: 0 0 16px; + padding: 8px; + color: var(--orange-700); + background: var(--orange-100); + border-left: solid var(--orange-400) 4px; + border-radius: 2px; +} + .sort-by-label { composes: small from './shared/typography.module.css'; } diff --git a/app/templates/search.hbs b/app/templates/search.hbs index 349d52764a1..c371a7311ae 100644 --- a/app/templates/search.hbs +++ b/app/templates/search.hbs @@ -8,6 +8,12 @@ data-test-header /> +{{#if this.hasMultiCategoryFilter}} +
+ Support for using multiple category: filters is not yet implemented. +
+{{/if}} + {{#if this.firstResultPending}}

Loading search results...

{{else if this.dataTask.lastComplete.error}} diff --git a/app/utils/search.js b/app/utils/search.js index f7b04897e23..610406cc4ec 100644 --- a/app/utils/search.js +++ b/app/utils/search.js @@ -1,4 +1,4 @@ -const CATEGORY_PREFIX = 'category:'; +export const CATEGORY_PREFIX = 'category:'; const KEYWORD_PREFIX = 'keyword:'; /**