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

Fix spaces in search results #9026

Merged
merged 2 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/unreleased/bugfix-spaces-in-search-results
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Spaces in search results

Spaces in search results are no longer being displayed as folder resources, fixing wrong icons, parent folders and sidebar panels.

https://github.com/owncloud/web/issues/9022
https://github.com/owncloud/web/pull/9026
34 changes: 23 additions & 11 deletions packages/web-app-files/src/components/FilesList/ResourceTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
<oc-resource
:key="`${item.path}-${resourceDomSelector(item)}-${item.thumbnail}`"
:resource="item"
:is-path-displayed="getArePathsDisplayed(item)"
:is-path-displayed="arePathsDisplayed"
:parent-folder-name-default="getDefaultParentFolderName(item)"
:is-thumbnail-displayed="shouldDisplayThumbnails(item)"
:is-extension-displayed="areFileExtensionsShown"
Expand Down Expand Up @@ -192,7 +192,12 @@ import { mapGetters, mapActions, mapState } from 'vuex'
import { basename, dirname } from 'path'
import { useWindowSize } from '@vueuse/core'
import { Resource } from 'web-client'
import { buildShareSpaceResource, extractDomSelector, SpaceResource } from 'web-client/src/helpers'
import {
buildShareSpaceResource,
extractDomSelector,
isProjectSpaceResource,
SpaceResource
} from 'web-client/src/helpers'
import { ShareTypes } from 'web-client/src/helpers/share'

import {
Expand Down Expand Up @@ -220,7 +225,11 @@ import { ClipboardActions } from 'web-app-files/src/helpers/clipboardActions'
import { isResourceTxtFileAlmostEmpty } from 'web-app-files/src/helpers/resources'
import { determineSortFields } from 'web-app-files/src/helpers/ui/resourceTable'
import { useFileActionsRename } from 'web-app-files/src/composables/actions/files/useFileActionsRename'
import { createLocationShares, createLocationCommon } from 'web-app-files/src/router'
import {
createLocationShares,
createLocationCommon,
createLocationSpaces
} from 'web-app-files/src/router'
import { ref } from 'vue'

const TAGS_MINIMUM_SCREEN_WIDTH = 850
Expand Down Expand Up @@ -648,7 +657,7 @@ export default defineComponent({
openTagsSidebar() {
eventBus.publish(SideBarEventTopics.open)
},
openSharingSidebar(file) {
openSharingSidebar(file: Resource) {
let panelToOpen
if (file.type === 'space') {
panelToOpen = 'space-share'
Expand All @@ -659,17 +668,20 @@ export default defineComponent({
}
eventBus.publish(SideBarEventTopics.openWithPanel, panelToOpen)
},
folderLink(file) {
folderLink(file: Resource) {
return this.resourceRouteResolver.createFolderLink({
path: file.path,
fileId: file.fileId,
resource: file
})
},
parentFolderLink(file) {
parentFolderLink(file: Resource) {
if (file.shareId && file.path === '/') {
return createLocationShares('files-shares-with-me')
}
if (isProjectSpaceResource(file)) {
return createLocationSpaces('files-spaces-projects')
}

return this.resourceRouteResolver.createFolderLink({
path: dirname(file.path),
Expand Down Expand Up @@ -828,7 +840,7 @@ export default defineComponent({
linkCount
})
},
getOwnerAvatarDescription(resource) {
getOwnerAvatarDescription(resource: Resource) {
const translated = this.$gettext('This %{ resourceType } is owned by %{ ownerName }')
const resourceType =
resource.type === 'folder' ? this.$gettext('folder') : this.$gettext('file')
Expand All @@ -837,8 +849,11 @@ export default defineComponent({
ownerName: resource.owner[0].displayName
})
},
getDefaultParentFolderName(resource) {
getDefaultParentFolderName(resource: Resource) {
if (this.hasProjectSpaces) {
if (isProjectSpaceResource(resource)) {
return this.$gettext('Spaces')
}
const matchingSpace = this.resourceRouteResolver.getMatchingSpace(resource)
if (matchingSpace?.driveType === 'project') {
return matchingSpace.name
Expand All @@ -860,9 +875,6 @@ export default defineComponent({
}

return this.$gettext('Personal')
},
getArePathsDisplayed(resource) {
return this.arePathsDisplayed && resource.type !== 'space'
}
}
})
Expand Down
9 changes: 8 additions & 1 deletion packages/web-app-files/src/components/Search/Preview.vue
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { mapGetters } from 'vuex'
import { createLocationShares, createLocationSpaces } from '../../router'
import { basename, dirname } from 'path'
import { useCapabilityShareJailEnabled } from 'web-pkg/src/composables'
import { buildShareSpaceResource, Resource } from 'web-client/src/helpers'
import { buildShareSpaceResource, isProjectSpaceResource, Resource } from 'web-client/src/helpers'
import { configurationManager } from 'web-pkg/src/configuration'
import { eventBus } from 'web-pkg/src/services/eventBus'
import { createFileRouteOptions } from 'web-pkg/src/helpers/router'
Expand Down Expand Up @@ -115,6 +115,10 @@ export default defineComponent({
return this.$gettext('All files and folders')
}

if (isProjectSpaceResource(this.resource)) {
return this.$gettext('Spaces')
}

if (this.matchingSpace?.driveType === 'project') {
return this.matchingSpace.name
}
Expand All @@ -134,6 +138,9 @@ export default defineComponent({
if (this.resource.shareId && this.resource.path === '/') {
return createLocationShares('files-shares-with-me')
}
if (isProjectSpaceResource(this.resource)) {
return createLocationSpaces('files-spaces-projects')
}
return this.createFolderLink(dirname(this.resource.path), this.resource.parentFolderId)
}
},
Expand Down
17 changes: 13 additions & 4 deletions packages/web-app-files/src/search/sdk/list.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { SearchList, SearchResult } from 'web-app-search/src/types'
import ListComponent from '../../components/Search/List.vue'
import { ClientService } from 'web-pkg/src/services'
import { buildResource } from 'web-client/src/helpers'
import { Component } from 'vue'
import { DavProperties } from 'web-client/src/webdav/constants'
import { ProjectSpaceResource, buildResource, isProjectSpaceResource } from 'web-client/src/helpers'
import { Component, computed, Ref, unref } from 'vue'
import { DavProperties, DavProperty } from 'web-client/src/webdav/constants'
import { Store } from 'vuex'

export const searchLimit = 200
Expand All @@ -12,11 +12,19 @@ export default class List implements SearchList {
public readonly component: Component
private readonly store: Store<any>
private readonly clientService: ClientService
private readonly projectSpaces: Ref<ProjectSpaceResource[]>

constructor(store: Store<any>, clientService: ClientService) {
this.component = ListComponent
this.store = store
this.clientService = clientService
this.projectSpaces = computed(() =>
this.store.getters['runtime/spaces/spaces'].filter((s) => isProjectSpaceResource(s))
)
}

getMatchingSpace(id): ProjectSpaceResource {
return unref(this.projectSpaces).find((s) => s.id === id)
}

async search(term: string): Promise<SearchResult> {
Expand All @@ -36,7 +44,8 @@ export default class List implements SearchList {
return {
totalResults: range ? parseInt(range?.split('/')[1]) : null,
values: results.map((result) => {
const resource = buildResource(result)
const matchingSpace = this.getMatchingSpace(result.fileInfo[DavProperty.FileParent])
const resource = matchingSpace ? matchingSpace : buildResource(result)
// info: in oc10 we have no storageId in resources. All resources are mounted into the personal space.
if (!resource.storageId) {
resource.storageId = this.store.getters.user.id
Expand Down
17 changes: 13 additions & 4 deletions packages/web-app-files/src/search/sdk/preview.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { SearchPreview, SearchResult } from 'web-app-search/src/types'
import PreviewComponent from '../../components/Search/Preview.vue'
import { ClientService } from 'web-pkg/src/services'
import { buildResource } from 'web-client/src/helpers'
import { ProjectSpaceResource, buildResource, isProjectSpaceResource } from 'web-client/src/helpers'
import { Cache } from 'web-pkg/src/helpers/cache'
import { Component, unref } from 'vue'
import { Component, computed, Ref, unref } from 'vue'
import { Router } from 'vue-router'
import { DavProperties } from 'web-client/src/webdav/constants'
import { DavProperties, DavProperty } from 'web-client/src/webdav/constants'
import { Store } from 'vuex'

export const previewSearchLimit = 8
Expand All @@ -16,6 +16,7 @@ export default class Preview implements SearchPreview {
private readonly router: Router
private readonly store: Store<any>
private readonly clientService: ClientService
private readonly projectSpaces: Ref<ProjectSpaceResource[]>

constructor(store: Store<any>, router: Router, clientService: ClientService) {
this.component = PreviewComponent
Expand All @@ -24,6 +25,13 @@ export default class Preview implements SearchPreview {
// define how long the cache should be valid, maybe conf option?
this.cache = new Cache({ ttl: 10000, capacity: 100 })
this.clientService = clientService
this.projectSpaces = computed(() =>
this.store.getters['runtime/spaces/spaces'].filter((s) => isProjectSpaceResource(s))
)
}

getMatchingSpace(id): ProjectSpaceResource {
return unref(this.projectSpaces).find((s) => s.id === id)
}

public async search(term: string): Promise<SearchResult> {
Expand All @@ -45,7 +53,8 @@ export default class Preview implements SearchPreview {
DavProperties.Default
)
const resources = results.reduce((acc, result) => {
const resource = buildResource(result)
const matchingSpace = this.getMatchingSpace(result.fileInfo[DavProperty.FileParent])
const resource = matchingSpace ? matchingSpace : buildResource(result)
// info: in oc10 we have no storageId in resources. All resources are mounted into the personal space.
if (!resource.storageId) {
resource.storageId = this.store.getters.user.id
Expand Down
59 changes: 39 additions & 20 deletions packages/web-app-files/tests/unit/search/sdk.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,24 @@ import { Store } from 'vuex'
import { RouteLocation, Router } from 'vue-router'
import { mock, mockDeep } from 'jest-mock-extended'
import { ref } from 'vue'
import { createStore, defaultStoreMockOptions } from 'web-test-helpers/src'
import { ProjectSpaceResource } from 'web-client/src/helpers'
import { DavProperty } from 'web-client/src/webdav/constants'

const searchMock = jest.fn()
const clientService = mockDeep<ClientService>()
clientService.owncloudSdk.files.search.mockImplementation(searchMock)

jest.mock('web-client/src/helpers', () => ({
jest.mock('web-client/src/helpers/resource', () => ({
buildResource: (v) => v
}))

const store = mockDeep<Store<any>>({
getters: {
user: () => ({ id: 1 }),
capabilities: {
dav: {
reports: ['search-files']
}
}
}
})
const getStore = (spaces = []) => {
const storeOptions = defaultStoreMockOptions
storeOptions.getters.capabilities.mockReturnValue({ dav: { reports: ['search-files'] } })
storeOptions.modules.runtime.modules.spaces.getters.spaces.mockReturnValue(spaces)
return createStore(storeOptions)
}

const storeWithoutFileSearch = mockDeep<Store<any>>({
getters: { capabilities: { dav: { reports: [] } } }
Expand All @@ -42,7 +41,7 @@ describe('SDKProvider', () => {
{ route: 'bar', available: true }
].forEach((v) => {
const search = new SDKSearch(
store,
getStore(),
mock<Router>({
currentRoute: ref(mock<RouteLocation>({ name: v.route }))
}),
Expand All @@ -54,11 +53,11 @@ describe('SDKProvider', () => {
})

it('can search', async () => {
const search = new SDKSearch(store, mock<Router>(), clientService)
const search = new SDKSearch(getStore(), mock<Router>(), clientService)
const files = [
{ id: 'foo', name: 'foo' },
{ id: 'bar', name: 'bar' },
{ id: 'baz', name: 'baz' }
{ id: 'foo', name: 'foo', fileInfo: {} },
{ id: 'bar', name: 'bar', fileInfo: {} },
{ id: 'baz', name: 'baz', fileInfo: {} }
]

const noTerm = await search.previewSearch.search('')
Expand All @@ -71,19 +70,39 @@ describe('SDKProvider', () => {
const withTermCached = await search.previewSearch.search('foo')
expect(withTermCached.values.map((r) => r.data)).toMatchObject(files)
})
it('properly returns space resources', async () => {
const spaceId = '1'
const space = mock<ProjectSpaceResource>({ id: spaceId, name: 'foo', driveType: 'project' })
const search = new SDKSearch(getStore([space]), mock<Router>(), clientService)
const files = [{ id: 'foo', name: 'foo', fileInfo: { [DavProperty.FileParent]: space.id } }]

searchMock.mockReturnValueOnce({ results: files })
const withTerm = (await search.previewSearch.search('foo')) as any
expect(withTerm.values.map((r) => r.data)[0].id).toEqual(spaceId)
})
})
describe('SDKProvider listSearch', () => {
it('can search', async () => {
const search = new SDKSearch(store, mock<Router>(), clientService)
const search = new SDKSearch(getStore(), mock<Router>(), clientService)
const files = [
{ id: 'foo', name: 'foo' },
{ id: 'bar', name: 'bar' },
{ id: 'baz', name: 'baz' }
{ id: 'foo', name: 'foo', fileInfo: {} },
{ id: 'bar', name: 'bar', fileInfo: {} },
{ id: 'baz', name: 'baz', fileInfo: {} }
]

searchMock.mockReturnValueOnce({ results: files })
const withTerm = (await search.listSearch.search('foo')) as any
expect(withTerm.values.map((r) => r.data)).toMatchObject(files)
})
it('properly returns space resources', async () => {
const spaceId = '1'
const space = mock<ProjectSpaceResource>({ id: spaceId, driveType: 'project' })
const search = new SDKSearch(getStore([space]), mock<Router>(), clientService)
const files = [{ id: 'foo', name: 'foo', fileInfo: { [DavProperty.FileParent]: space.id } }]

searchMock.mockReturnValueOnce({ results: files })
const withTerm = (await search.listSearch.search('foo')) as any
expect(withTerm.values.map((r) => r.data)[0].id).toEqual(spaceId)
})
})
})
1 change: 1 addition & 0 deletions packages/web-client/src/helpers/space/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export function buildSpace(data): SpaceResource {
mdate: data.lastModifiedDateTime,
size: data.quota?.used,
indicators: [],
tags: [],
permissions: '',
starred: false,
etag: '',
Expand Down