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

Centralize version loading #10746

Merged
merged 11 commits into from
Apr 9, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Bugfix: Versions loaded multiple times when opening sidebar

We've fixed a bug, where the versions endpoint was fetched multiple times when the sidebar was opened
and therefore added unnecessary load to the server.

https://github.com/owncloud/web/pull/10746
https://github.com/owncloud/web/issues/10381
https://github.com/owncloud/web/issues/10619
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ import {
useUserStore,
useCapabilityStore,
useConfigStore,
useClientService,
useResourcesStore,
formatDateFromJSDate
} from '@ownclouders/web-pkg'
Expand Down Expand Up @@ -174,18 +173,12 @@ export default defineComponent({
type: Boolean,
required: false,
default: true
},
versionsEnabled: {
type: Boolean,
required: false,
default: true
}
},
setup(props) {
const configStore = useConfigStore()
const userStore = useUserStore()
const capabilityStore = useCapabilityStore()
const clientService = useClientService()
const { getMatchingSpace } = useGetMatchingSpace()

const language = useGettext()
Expand All @@ -196,6 +189,7 @@ export default defineComponent({
const { user } = storeToRefs(userStore)

const resource = inject<Ref<Resource>>('resource')
const versions = inject<Ref<Resource[]>>('versions')
const space = inject<Ref<SpaceResource>>('space')

const previewService = usePreviewService()
Expand All @@ -204,15 +198,6 @@ export default defineComponent({
const authStore = useAuthStore()
const { publicLinkContextReady } = storeToRefs(authStore)

const versions = ref<Resource[]>([])
const loadVersions = async (fileId: Resource['fileId']) => {
try {
versions.value = await clientService.webdav.listFileVersions(fileId)
} catch (e) {
console.error(e)
}
}

const isPreviewEnabled = computed(() => {
if (unref(resource).isFolder) {
return false
Expand Down Expand Up @@ -261,23 +246,6 @@ export default defineComponent({
return formatRelativeDateFromJSDate(new Date(date), language.current)
}

watch(
resource,
() => {
if (unref(resource)) {
loadPreviewTask.perform(unref(resource))
if (
props.versionsEnabled &&
!unref(resource).isFolder &&
!unref(publicLinkContextReady)
) {
loadVersions(unref(resource).fileId)
}
}
},
{ immediate: true }
)

const contextualHelper = {
isEnabled: configStore.options.contextHelpers,
data: tagsHelper({ configStore })
Expand All @@ -296,6 +264,16 @@ export default defineComponent({
return upperFirst(displayDate)
})

watch(
resource,
() => {
if (unref(resource)) {
loadPreviewTask.perform(unref(resource))
}
},
{ immediate: true }
)

return {
user,
preview,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<template>
<div id="oc-file-versions-sidebar" class="-oc-mt-s">
<oc-loader v-if="areVersionsLoading" />
<ul v-else-if="versions.length" class="oc-m-rm oc-position-relative">
<ul v-if="versions.length" class="oc-m-rm oc-position-relative">
<li class="spacer oc-pb-l" aria-hidden="true"></li>
<li
v-for="(item, index) in versions"
Expand Down Expand Up @@ -63,9 +62,8 @@ import {
useDownloadFile,
useResourcesStore
} from '@ownclouders/web-pkg'
import { computed, defineComponent, inject, Ref, ref, unref, watch } from 'vue'
import { computed, defineComponent, inject, Ref, unref } from 'vue'
import { isShareSpaceResource, Resource, SpaceResource } from '@ownclouders/web-client/src/helpers'
import { useTask } from 'vue-concurrency'
import { useGettext } from 'vue3-gettext'

export default defineComponent({
Expand All @@ -85,30 +83,7 @@ export default defineComponent({

const space = inject<Ref<SpaceResource>>('space')
const resource = inject<Ref<Resource>>('resource')

const versions = ref<Resource[]>([])
const fetchVersionsTask = useTask(function* () {
try {
versions.value = yield clientService.webdav.listFileVersions(unref(resource).fileId)
} catch (e) {
console.error(e)
}
})
const areVersionsLoading = computed(() => {
return !fetchVersionsTask.last || fetchVersionsTask.isRunning
})
watch(
[() => unref(resource)?.id, () => unref(resource)?.etag],
([id, etag]) => {
if (!id || !etag) {
return
}
fetchVersionsTask.perform()
},
{
immediate: true
}
)
const versions = inject<Ref<Resource[]>>('versions')

const isRevertible = computed(() => {
if (props.isReadOnly) {
Expand Down Expand Up @@ -138,8 +113,6 @@ export default defineComponent({
})
}
}

fetchVersionsTask.perform()
}
const downloadVersion = (version: Resource) => {
return downloadFile(unref(space), unref(resource), version.name)
Expand All @@ -158,16 +131,12 @@ export default defineComponent({
space,
resource,
versions,
areVersionsLoading,
isRevertible,
revertToVersion,
downloadVersion,
formatVersionDateRelative,
formatVersionDate,
formatVersionFileSize,

// HACK: exported for unit tests
fetchVersionsTask
formatVersionFileSize
}
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
useCapabilityStore
} from '@ownclouders/web-pkg'
import {
isIncomingShareResource,
isProjectSpaceResource,
isShareResource,
isShareSpaceResource,
Expand Down Expand Up @@ -227,7 +228,10 @@ export const useSideBarPanels = () => {
}
if (
isLocationTrashActive(router, 'files-trash-generic') ||
isLocationPublicActive(router, 'files-public-link')
isLocationPublicActive(router, 'files-public-link') ||
isLocationSharesActive(router, 'files-shares-with-others') ||
AlexAndBear marked this conversation as resolved.
Show resolved Hide resolved
isLocationSharesActive(router, 'files-shares-via-link') ||
isIncomingShareResource(items[0])
) {
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ describe('ResourceDetails component', () => {
})
})
}
mocks.$clientService.webdav.listFileVersions.mockResolvedValue([])

const file = {
id: '0',
Expand Down Expand Up @@ -76,7 +75,8 @@ describe('ResourceDetails component', () => {
provide: {
...mocks,
space,
resource: file
resource: file,
versions: []
}
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,14 @@ function createWrapper({
const publicLocation = createLocationPublic('files-public-link')
const currentRoute = isPublicLinkContext ? publicLocation : spacesLocation
const mocks = defaultComponentMocks({ currentRoute: mock<RouteLocation>(currentRoute as any) })
mocks.$clientService.webdav.listFileVersions.mockResolvedValue(versions)
const capabilities = { files: { tags: tagsEnabled } }
return {
wrapper: mount(FileDetails, {
global: {
stubs: { 'router-link': true, 'resource-icon': true },
provide: {
...mocks,
versions,
resource,
space: mockDeep<SpaceResource>()
},
Expand Down
Loading