From bc052dddb42a9d806eddd942d9e02ed4c592d1f3 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Mon, 18 Mar 2024 09:52:38 +0100 Subject: [PATCH] feat: make full text search default Enables full text search as default if the server supports it. --- .../enhancement-full-text-search-default | 6 ++ .../src/components/Search/List.vue | 71 ++++++++----------- .../tests/unit/components/Search/List.spec.ts | 28 ++++---- .../web-app-search/src/portals/SearchBar.vue | 19 ++++- packages/web-pkg/src/router/common.ts | 2 +- .../features/search/fullTextSearch.feature | 3 +- .../cucumber/features/search/search.feature | 3 + tests/e2e/cucumber/steps/ui/search.ts | 4 +- .../objects/app-files/search/actions.ts | 14 ++-- .../support/objects/app-files/search/index.ts | 4 +- 10 files changed, 82 insertions(+), 72 deletions(-) create mode 100644 changelog/unreleased/enhancement-full-text-search-default diff --git a/changelog/unreleased/enhancement-full-text-search-default b/changelog/unreleased/enhancement-full-text-search-default new file mode 100644 index 00000000000..408a8f80eb3 --- /dev/null +++ b/changelog/unreleased/enhancement-full-text-search-default @@ -0,0 +1,6 @@ +Enhancement: Full text search default + +Full text search is now enabled by default when searching for files if the server supports it. + +https://github.com/owncloud/web/issues/10534 +https://github.com/owncloud/web/pull/10626 diff --git a/packages/web-app-files/src/components/Search/List.vue b/packages/web-app-files/src/components/Search/List.vue index 9d29b86e139..395ff0398b8 100644 --- a/packages/web-app-files/src/components/Search/List.vue +++ b/packages/web-app-files/src/components/Search/List.vue @@ -71,9 +71,9 @@ @@ -272,11 +272,13 @@ export default defineComponent({ const tagParam = useRouteQuery('q_tags') const lastModifiedParam = useRouteQuery('q_lastModified') const mediaTypeParam = useRouteQuery('q_mediaType') - const fullTextParam = useRouteQuery('q_fullText') + const titleOnlyParam = useRouteQuery('q_titleOnly') + + const fullTextSearchEnabled = computed(() => capabilityStore.searchContent?.enabled) const displayFilter = computed(() => { return ( - capabilityStore.searchContent?.enabled || + unref(fullTextSearchEnabled) || unref(availableTags).length || capabilityStore.searchLastMofifiedDate?.enabled ) @@ -339,21 +341,26 @@ export default defineComponent({ } const buildSearchTerm = (manuallyUpdateFilterChip = false) => { - const query = {} + const query: string[] = [] const humanSearchTerm = unref(searchTerm) - const isContentOnlySearch = queryItemAsString(unref(fullTextParam)) == 'true' + const isTitleOnlySearch = queryItemAsString(unref(titleOnlyParam)) == 'true' + const useFullTextSearch = unref(fullTextSearchEnabled) && !isTitleOnlySearch + + if (!!humanSearchTerm) { + let nameQuery = `name:"*${humanSearchTerm}*"` + + if (useFullTextSearch) { + nameQuery = `(name:"*${humanSearchTerm}*" OR content:"${humanSearchTerm}")` + } - if (isContentOnlySearch && !!humanSearchTerm) { - query['content'] = `"${humanSearchTerm}"` - } else if (!!humanSearchTerm) { - query['name'] = `"*${humanSearchTerm}*"` + query.push(nameQuery) } const humanScopeQuery = unref(scopeQuery) const isScopedSearch = unref(doUseScope) === 'true' if (isScopedSearch && humanScopeQuery) { - query['scope'] = `${humanScopeQuery}` + query.push(`scope:${humanScopeQuery}`) } const updateFilter = (v: Ref>) => { @@ -370,46 +377,26 @@ export default defineComponent({ const humanTagsParams = queryItemAsString(unref(tagParam)) if (humanTagsParams) { - query['tag'] = humanTagsParams.split('+').map((t) => `"${t}"`) + const tags = humanTagsParams.split('+').map((t) => `"${t}"`) + query.push(`tag:(${tags.join(' OR ')})`) updateFilter(tagFilter) } const lastModifiedParams = queryItemAsString(unref(lastModifiedParam)) if (lastModifiedParams) { - query['mtime'] = `"${lastModifiedParams}"` + query.push(`mtime:${lastModifiedParams}`) updateFilter(lastModifiedFilter) } const mediaTypeParams = queryItemAsString(unref(mediaTypeParam)) if (mediaTypeParams) { - query['mediatype'] = mediaTypeParams.split('+').map((t) => `"${t}"`) + const mediatypes = mediaTypeParams.split('+').map((t) => `"${t}"`) + query.push(`mediatype:(${mediatypes.join(' OR ')})`) updateFilter(mediaTypeFilter) } - return ( - // By definition (KQL spec) OR, AND or (GROUP) is implicit for simple cases where - // different or identical keys are part of the query. - // - // We list these operators for the following reasons nevertheless explicit: - // * request readability - // * code readability - // * complex cases readability - Object.keys(query) - .reduce((acc, prop) => { - const isArrayValue = Array.isArray(query[prop]) - - if (!isArrayValue) { - acc.push(`${prop}:${query[prop]}`) - } - - if (isArrayValue) { - acc.push(`${prop}:(${query[prop].join(' OR ')})`) - } - - return acc - }, []) - .sort((a, b) => a.startsWith('scope:') - b.startsWith('scope:')) - .join(' AND ') - ) + return query + .sort((a, b) => Number(a.startsWith('scope:')) - Number(b.startsWith('scope:'))) + .join(' AND ') } const breadcrumbs = computed(() => { @@ -457,7 +444,7 @@ export default defineComponent({ { const isSameTerm = newVal?.term === oldVal?.term const isSameFilter = [ - 'q_fullText', + 'q_titleOnly', 'q_tags', 'q_lastModified', 'q_mediaType', @@ -479,7 +466,7 @@ export default defineComponent({ configOptions, loadAvailableTagsTask, fileListHeaderY, - fullTextSearchEnabled: computed(() => capabilityStore.searchContent?.enabled), + fullTextSearchEnabled, getMatchingSpace, availableTags, tagFilter, diff --git a/packages/web-app-files/tests/unit/components/Search/List.spec.ts b/packages/web-app-files/tests/unit/components/Search/List.spec.ts index 70266f96755..c14d54d0be2 100644 --- a/packages/web-app-files/tests/unit/components/Search/List.spec.ts +++ b/packages/web-app-files/tests/unit/components/Search/List.spec.ts @@ -25,7 +25,7 @@ const selectors = { resourceTableStub: 'resource-table-stub', tagFilter: '.files-search-filter-tags', lastModifiedFilter: '.files-search-filter-last-modified', - fullTextFilter: '.files-search-filter-full-text', + titleOnlyFilter: '.files-search-filter-title-only', filter: '.files-search-result-filter' } @@ -66,7 +66,7 @@ describe('List component', () => { name: 'files-common-search', query: merge( { - q_fullText: 'q_fullText', + q_titleOnly: 'q_titleOnly', q_tags: 'q_tags', q_lastModified: 'q_lastModified', useScope: 'useScope' @@ -123,7 +123,7 @@ describe('List component', () => { }) await wrapper.vm.loadAvailableTagsTask.last expect(wrapper.emitted('search')[0][0]).toEqual( - `name:"*${searchTerm}*" AND tag:("${availableTags[0]}" OR "${availableTags[1]}")` + `(name:"*${searchTerm}*" OR content:"${searchTerm}") AND tag:("${availableTags[0]}" OR "${availableTags[1]}")` ) }) }) @@ -176,25 +176,29 @@ describe('List component', () => { }) await wrapper.vm.loadAvailableTagsTask.last expect(wrapper.emitted('search')[0][0]).toEqual( - `name:"*${searchTerm}*" AND mtime:"${lastModifiedFilterQuery}"` + `(name:"*${searchTerm}*" OR content:"${searchTerm}") AND mtime:${lastModifiedFilterQuery}` ) }) }) - describe('fullText', () => { + describe('titleOnly', () => { it('should render filter if enabled via capabilities', () => { const { wrapper } = getWrapper({ fullTextSearchEnabled: true }) - expect(wrapper.find(selectors.fullTextFilter).exists()).toBeTruthy() + expect(wrapper.find(selectors.titleOnlyFilter).exists()).toBeTruthy() }) - it('should set initial filter when fullText is set active via query param', async () => { + it('should not render filter if not enabled via capabilities', () => { + const { wrapper } = getWrapper({ fullTextSearchEnabled: false }) + expect(wrapper.find(selectors.titleOnlyFilter).exists()).toBeFalsy() + }) + it('should set initial filter when titleOnly is set active via query param', async () => { const searchTerm = 'term' const { wrapper } = getWrapper({ searchTerm, - fullTextFilterQuery: 'true', + titleOnlyFilterQuery: 'true', fullTextSearchEnabled: true }) await wrapper.vm.loadAvailableTagsTask.last - expect(wrapper.emitted('search')[0][0]).toEqual(`content:"${searchTerm}"`) + expect(wrapper.emitted('search')[0][0]).toEqual(`name:"*${searchTerm}*"`) }) }) }) @@ -205,14 +209,14 @@ function getWrapper({ resources = [], searchTerm = '', tagFilterQuery = null, - fullTextFilterQuery = null, - fullTextSearchEnabled = false, + titleOnlyFilterQuery = null, + fullTextSearchEnabled = true, availableLastModifiedValues = {}, lastModifiedFilterQuery = null, mocks = {} } = {}) { vi.mocked(queryItemAsString).mockImplementationOnce(() => searchTerm) - vi.mocked(queryItemAsString).mockImplementationOnce(() => fullTextFilterQuery) + vi.mocked(queryItemAsString).mockImplementationOnce(() => titleOnlyFilterQuery) vi.mocked(queryItemAsString).mockImplementationOnce(() => tagFilterQuery) vi.mocked(queryItemAsString).mockImplementationOnce(() => lastModifiedFilterQuery) diff --git a/packages/web-app-search/src/portals/SearchBar.vue b/packages/web-app-search/src/portals/SearchBar.vue index 4bcabb0452d..9be0a2e3ede 100644 --- a/packages/web-app-search/src/portals/SearchBar.vue +++ b/packages/web-app-search/src/portals/SearchBar.vue @@ -105,7 +105,9 @@ import { createLocationCommon, isLocationCommonActive, isLocationSpacesActive, + queryItemAsString, useAuthStore, + useCapabilityStore, useResourcesStore } from '@ownclouders/web-pkg' import Mark from 'mark.js' @@ -123,6 +125,7 @@ export default defineComponent({ components: { SearchBarFilter }, setup() { const router = useRouter() + const capabilityStore = useCapabilityStore() const showCancelButton = ref(false) const isMobileWidth = inject>('isMobileWidth') const scopeQueryValue = useRouteQuery('scope') @@ -143,6 +146,8 @@ export default defineComponent({ const loading = ref(false) const currentFolderAvailable = ref(false) + const fullTextSearchEnabled = computed(() => capabilityStore.searchContent?.enabled) + const listProviderAvailable = computed(() => unref(availableProviders).some((p) => !!p.listSearch) ) @@ -179,18 +184,26 @@ export default defineComponent({ if (!unref(term)) { return } - const terms = [`name:"*${unref(term)}*"`] + + const terms: string[] = [] + + let nameQuery = `name:"*${unref(term)}*"` + if (unref(fullTextSearchEnabled)) { + nameQuery = `(name:"*${unref(term)}*" OR content:"${unref(term)}")` + } + + terms.push(nameQuery) if ( unref(currentFolderAvailable) && unref(locationFilterId) === SearchLocationFilterConstants.currentFolder ) { - let scope + let scope: string if (unref(currentFolder)?.fileId) { scope = unref(currentFolder)?.fileId } else { - scope = unref(scopeQueryValue) + scope = queryItemAsString(unref(scopeQueryValue)) } terms.push(`scope:${scope}`) diff --git a/packages/web-pkg/src/router/common.ts b/packages/web-pkg/src/router/common.ts index d37dfe5033e..b9d37f9edea 100644 --- a/packages/web-pkg/src/router/common.ts +++ b/packages/web-pkg/src/router/common.ts @@ -32,7 +32,7 @@ export const buildRoutes = (components: RouteComponents): RouteRecordRaw[] => [ 'provider', 'q_tags', 'q_lastModified', - 'q_fullText', + 'q_titleOnly', 'q_mediaType', 'scope', 'useScope' diff --git a/tests/e2e/cucumber/features/search/fullTextSearch.feature b/tests/e2e/cucumber/features/search/fullTextSearch.feature index 400e0ac7233..4d10488cf39 100644 --- a/tests/e2e/cucumber/features/search/fullTextSearch.feature +++ b/tests/e2e/cucumber/features/search/fullTextSearch.feature @@ -77,8 +77,7 @@ Feature: Search | fileToShare.txt | | spaceFolder/spaceTextfile.txt | - When "Brian" enables the option to search in file content - And "Brian" searches "Cheers" using the global search and the "all files" filter and presses enter + When "Brian" searches "Cheers" using the global search and the "all files" filter and presses enter Then following resources should be displayed in the files list for user "Brian" | resource | | textfile.txt | diff --git a/tests/e2e/cucumber/features/search/search.feature b/tests/e2e/cucumber/features/search/search.feature index 4b666c70980..78d34f06b15 100644 --- a/tests/e2e/cucumber/features/search/search.feature +++ b/tests/e2e/cucumber/features/search/search.feature @@ -104,6 +104,7 @@ Feature: Search # search difficult names When "Alice" searches "strängéनेपालीName" using the global search and the "all files" filter and presses enter + And "Alice" enables the option to search title only Then following resources should be displayed in the files list for user "Alice" | strängéनेपालीName | @@ -171,6 +172,7 @@ Feature: Search | mediaTest.zip | I'm a Archive | When "Alice" opens the "files" app And "Alice" searches "mediaTest" using the global search and the "all files" filter and presses enter + And "Alice" enables the option to search title only And "Alice" selects mediaType "Document" from the search result filter chip Then following resources should be displayed in the files list for user "Alice" | resource | @@ -223,6 +225,7 @@ Feature: Search And "Alice" opens the "files" app When "Alice" opens folder "mainFolder" And "Alice" searches "mediaTest" using the global search and the "current folder" filter and presses enter + And "Alice" enables the option to search title only And "Alice" selects lastModified "last 30 days" from the search result filter chip Then following resources should be displayed in the files list for user "Alice" | resource | diff --git a/tests/e2e/cucumber/steps/ui/search.ts b/tests/e2e/cucumber/steps/ui/search.ts index 24ad4216718..6666dbc2232 100644 --- a/tests/e2e/cucumber/steps/ui/search.ts +++ b/tests/e2e/cucumber/steps/ui/search.ts @@ -23,11 +23,11 @@ When( ) When( - /^"([^"]*)" (enable|disable)s the option to search in file content?$/, + /^"([^"]*)" (enable|disable)s the option to search title only?$/, async function (this: World, stepUser: string, enableOrDisable: string): Promise { const { page } = this.actorsEnvironment.getActor({ key: stepUser }) const searchObject = new objects.applicationFiles.Search({ page }) - await searchObject.toggleSearchInFileContent({ enableOrDisable }) + await searchObject.toggleSearchTitleOnly({ enableOrDisable }) } ) When( diff --git a/tests/e2e/support/objects/app-files/search/actions.ts b/tests/e2e/support/objects/app-files/search/actions.ts index c96abf8562c..bea8631d189 100644 --- a/tests/e2e/support/objects/app-files/search/actions.ts +++ b/tests/e2e/support/objects/app-files/search/actions.ts @@ -11,10 +11,10 @@ const mediaTypeOutside = '.files-search-result-filter' const clearFilterSelector = '.item-filter-%s .oc-filter-chip-clear' const lastModifiedFilterSelector = '.item-filter-lastModified' const lastModifiedFilterItem = '[data-test-value="%s"]' -const enableSearchInFileContentSelector = - '//div[contains(@class,"files-search-filter-full-text")]//button[contains(@class,"oc-filter-chip-button")]' -const disableSearchInFileContentSelector = - '//div[contains(@class,"files-search-filter-full-text")]//button[contains(@class,"oc-filter-chip-clear")]' +const enableSearchTitleOnlySelector = + '//div[contains(@class,"files-search-filter-title-only")]//button[contains(@class,"oc-filter-chip-button")]' +const disableSearchTitleOnlySelector = + '//div[contains(@class,"files-search-filter-title-only")]//button[contains(@class,"oc-filter-chip-clear")]' export const getSearchResultMessage = ({ page }: { page: Page }): Promise => { return page.locator(searchResultMessageSelector).innerText() @@ -80,7 +80,7 @@ export const clearFilter = async ({ await page.locator(util.format(clearFilterSelector, filter)).click() } -export const toggleSearchInFileContent = async ({ +export const toggleSearchTitleOnly = async ({ enableOrDisable, page }: { @@ -88,8 +88,6 @@ export const toggleSearchInFileContent = async ({ page: Page }): Promise => { const selector = - enableOrDisable === 'enable' - ? enableSearchInFileContentSelector - : disableSearchInFileContentSelector + enableOrDisable === 'enable' ? enableSearchTitleOnlySelector : disableSearchTitleOnlySelector await page.locator(selector).click() } diff --git a/tests/e2e/support/objects/app-files/search/index.ts b/tests/e2e/support/objects/app-files/search/index.ts index 7b2f9870509..e82b90ae4dd 100644 --- a/tests/e2e/support/objects/app-files/search/index.ts +++ b/tests/e2e/support/objects/app-files/search/index.ts @@ -28,7 +28,7 @@ export class Search { await po.clearFilter({ page: this.#page, filter: string }) } - async toggleSearchInFileContent({ enableOrDisable: string }): Promise { - await po.toggleSearchInFileContent({ enableOrDisable: string, page: this.#page }) + async toggleSearchTitleOnly({ enableOrDisable: string }): Promise { + await po.toggleSearchTitleOnly({ enableOrDisable: string, page: this.#page }) } }