Skip to content

Commit

Permalink
Merge pull request #10626 from owncloud/make-full-text-search-default
Browse files Browse the repository at this point in the history
feat: make full text search default
  • Loading branch information
JammingBen authored Mar 19, 2024
2 parents f5bab81 + bc052dd commit fee4b99
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 72 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/enhancement-full-text-search-default
Original file line number Diff line number Diff line change
@@ -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
71 changes: 29 additions & 42 deletions packages/web-app-files/src/components/Search/List.vue
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@

<item-filter-toggle
v-if="fullTextSearchEnabled"
:filter-label="$gettext('Search only in content')"
filter-name="fullText"
class="files-search-filter-full-text oc-mr-s"
:filter-label="$gettext('Title only')"
filter-name="titleOnly"
class="files-search-filter-title-only oc-mr-s"
/>
</div>
<app-loading-spinner v-if="loading" />
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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<InstanceType<typeof ItemFilter>>) => {
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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',
Expand All @@ -479,7 +466,7 @@ export default defineComponent({
configOptions,
loadAvailableTagsTask,
fileListHeaderY,
fullTextSearchEnabled: computed(() => capabilityStore.searchContent?.enabled),
fullTextSearchEnabled,
getMatchingSpace,
availableTags,
tagFilter,
Expand Down
28 changes: 16 additions & 12 deletions packages/web-app-files/tests/unit/components/Search/List.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}

Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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]}")`
)
})
})
Expand Down Expand Up @@ -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}*"`)
})
})
})
Expand All @@ -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)

Expand Down
19 changes: 16 additions & 3 deletions packages/web-app-search/src/portals/SearchBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ import {
createLocationCommon,
isLocationCommonActive,
isLocationSpacesActive,
queryItemAsString,
useAuthStore,
useCapabilityStore,
useResourcesStore
} from '@ownclouders/web-pkg'
import Mark from 'mark.js'
Expand All @@ -123,6 +125,7 @@ export default defineComponent({
components: { SearchBarFilter },
setup() {
const router = useRouter()
const capabilityStore = useCapabilityStore()
const showCancelButton = ref(false)
const isMobileWidth = inject<Ref<boolean>>('isMobileWidth')
const scopeQueryValue = useRouteQuery('scope')
Expand All @@ -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)
)
Expand Down Expand Up @@ -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}`)
Expand Down
2 changes: 1 addition & 1 deletion packages/web-pkg/src/router/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const buildRoutes = (components: RouteComponents): RouteRecordRaw[] => [
'provider',
'q_tags',
'q_lastModified',
'q_fullText',
'q_titleOnly',
'q_mediaType',
'scope',
'useScope'
Expand Down
3 changes: 1 addition & 2 deletions tests/e2e/cucumber/features/search/fullTextSearch.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
3 changes: 3 additions & 0 deletions tests/e2e/cucumber/features/search/search.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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 |

Expand Down Expand Up @@ -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 |
Expand Down Expand Up @@ -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 |
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/cucumber/steps/ui/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
const { page } = this.actorsEnvironment.getActor({ key: stepUser })
const searchObject = new objects.applicationFiles.Search({ page })
await searchObject.toggleSearchInFileContent({ enableOrDisable })
await searchObject.toggleSearchTitleOnly({ enableOrDisable })
}
)
When(
Expand Down
14 changes: 6 additions & 8 deletions tests/e2e/support/objects/app-files/search/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> => {
return page.locator(searchResultMessageSelector).innerText()
Expand Down Expand Up @@ -80,16 +80,14 @@ export const clearFilter = async ({
await page.locator(util.format(clearFilterSelector, filter)).click()
}

export const toggleSearchInFileContent = async ({
export const toggleSearchTitleOnly = async ({
enableOrDisable,
page
}: {
enableOrDisable: string
page: Page
}): Promise<void> => {
const selector =
enableOrDisable === 'enable'
? enableSearchInFileContentSelector
: disableSearchInFileContentSelector
enableOrDisable === 'enable' ? enableSearchTitleOnlySelector : disableSearchTitleOnlySelector
await page.locator(selector).click()
}
4 changes: 2 additions & 2 deletions tests/e2e/support/objects/app-files/search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class Search {
await po.clearFilter({ page: this.#page, filter: string })
}

async toggleSearchInFileContent({ enableOrDisable: string }): Promise<void> {
await po.toggleSearchInFileContent({ enableOrDisable: string, page: this.#page })
async toggleSearchTitleOnly({ enableOrDisable: string }): Promise<void> {
await po.toggleSearchTitleOnly({ enableOrDisable: string, page: this.#page })
}
}

0 comments on commit fee4b99

Please sign in to comment.