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

[full-ci] Space root not updated #7546

Merged
merged 3 commits into from
Oct 4, 2022
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-reactivity-on-update
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Spaces reactivity on update

We've fixed a bug where updated data for a space would not show up in the UI before reloading.

https://github.com/owncloud/web/issues/7521
https://github.com/owncloud/web/pull/7546
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ import { SideBarEventTopics } from '../../../composables/sideBar'
export default defineComponent({
name: 'SpaceDetails',
components: { SpaceQuota },
inject: ['displayedItem'],
setup() {
const store = useStore()
const accessToken = useAccessToken({ store })
Expand Down Expand Up @@ -121,11 +120,11 @@ export default defineComponent({
return { loadImageTask, spaceImage }
},
computed: {
...mapGetters('Files', ['currentFileOutgoingLinks']),
...mapGetters('Files', ['currentFileOutgoingLinks', 'highlightedFile']),
...mapGetters('runtime/spaces', ['spaceMembers']),
...mapGetters(['user']),
space() {
return this.displayedItem.value
return this.highlightedFile
},
hasShares() {
return this.hasMemberShares || this.hasLinkShares
Expand Down
84 changes: 30 additions & 54 deletions packages/web-app-files/src/components/SideBar/SideBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ import {
isLocationCommonActive,
isLocationPublicActive,
isLocationSharesActive,
isLocationSpacesActive,
isLocationTrashActive
isLocationSpacesActive
} from '../../router'
import { computed, defineComponent, PropType } from '@vue/composition-api'
import {
Expand Down Expand Up @@ -143,7 +142,7 @@ export default defineComponent({
},

computed: {
...mapGetters('Files', ['highlightedFile', 'selectedFiles', 'currentFolder']),
...mapGetters('Files', ['highlightedFile', 'selectedFiles']),
...mapGetters(['fileSideBars', 'capabilities']),
...mapGetters('runtime/spaces', ['spaces']),
...mapState(['user']),
Expand Down Expand Up @@ -202,79 +201,56 @@ export default defineComponent({
}
return !pathSegments.length
},
highlightedFileThumbnail() {
return this.highlightedFile?.thumbnail
},
highlightedFileFavorite() {
return this.highlightedFile?.starred
},
highlightedFileIsSpace() {
return this.highlightedFile?.type === 'space'
}
},
watch: {
highlightedFile(newFile, oldFile) {
if (!this.isSingleResource) {
return
}

if (oldFile && isEqual(newFile, oldFile)) {
return
}

const loadShares =
!!oldFile ||
this.isSpacesProjectsLocation ||
this.isSharedWithMeLocation ||
this.isSharedWithOthersLocation ||
this.isSharedViaLinkLocation ||
this.isSearchLocation ||
this.isFavoritesLocation
this.fetchFileInfo(loadShares)
},

highlightedFileThumbnail(thumbnail) {
this.$set(this.selectedFile, 'thumbnail', thumbnail)
},
highlightedFile: {
handler(newFile, oldFile) {
const noChanges = oldFile && isEqual(newFile, oldFile)
if (!this.isSingleResource || !this.highlightedFile || noChanges) {
return
}

highlightedFileFavorite(starred) {
this.$set(this.selectedFile, 'starred', starred)
}
},
async created() {
if (!this.areMultipleSelected) {
await this.fetchFileInfo(false)
const loadShares =
!!oldFile ||
this.isSpacesProjectsLocation ||
this.isSharedWithMeLocation ||
this.isSharedWithOthersLocation ||
this.isSharedViaLinkLocation ||
this.isSearchLocation ||
this.isFavoritesLocation
this.fetchFileInfo(loadShares)
},
deep: true
}
},
methods: {
...mapActions('Files', ['loadSharesTree']),

async fetchFileInfo(loadShares) {
if (!this.highlightedFile) {
this.selectedFile = {}
return
this.loading = true
const highlightedFileIsShare =
this.isSharedWithMeLocation ||
this.isSharedWithOthersLocation ||
this.isSharedViaLinkLocation

if (loadShares) {
this.loadShares()
}

if (
isLocationTrashActive(this.$router, 'files-trash-generic') ||
this.highlightedFileIsSpace
) {
if (loadShares) {
this.loadShares()
}
if (!highlightedFileIsShare) {
this.selectedFile = { ...this.highlightedFile }
this.loading = false
return
}

this.loading = true
// shared resources look different, hence we need to fetch the actual resource here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always wondered why we have highlightedFile AND selectedFile. Turns out, it's needed because share resources look different than "normal" resources. Some information like mdate is missing in a built share. Therefore we need to fetch the actual resource in such cases.

Maybe the OCS share API will be extended by the missing properties in the future. Then we could skip this whole fetching-stuff here.

try {
this.selectedFile = await (this.webdav as WebDAV).getFileInfo(this.space, {
path: this.highlightedFile.path
})
this.$set(this.selectedFile, 'thumbnail', this.highlightedFile.thumbnail || null)
if (loadShares) {
this.loadShares()
}
} catch (error) {
this.selectedFile = { ...this.highlightedFile }
console.error(error)
Expand Down
7 changes: 5 additions & 2 deletions packages/web-app-files/src/components/SideBar/SpaceInfo.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@
</template>

<script>
import { mapGetters } from 'vuex'

export default {
name: 'SpaceInfo',
inject: ['displayedItem'],
computed: {
...mapGetters('Files', ['highlightedFile']),

space() {
return this.displayedItem.value
return this.highlightedFile
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions packages/web-app-files/src/components/Spaces/QuotaModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export default defineComponent({
},
methods: {
...mapActions(['showMessage']),
...mapMutations('Files', ['UPDATE_RESOURCE_FIELD']),
...mapMutations('runtime/spaces', ['UPDATE_SPACE_FIELD']),

changeSelectedQuotaOption(option) {
Expand All @@ -80,6 +81,11 @@ export default defineComponent({
field: 'spaceQuota',
value: data.quota
})
this.UPDATE_RESOURCE_FIELD({
id: this.space.id,
field: 'spaceQuota',
value: data.quota
})
this.showMessage({
title: this.$gettext('Space quota was changed successfully')
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ import Vuex from 'vuex'
import GetTextPlugin from 'vue-gettext'
import fileSideBars from '@files/src/fileSideBars'
import stubs from '@/tests/unit/stubs'
import Files from '@/__fixtures__/files'
import merge from 'lodash-es/merge'
import { buildResource, renameResource } from '@files/src/helpers/resources'
import { buildResource } from '@files/src/helpers/resources'
import { clientService } from 'web-pkg/src/services'
import { createLocationPublic, createLocationSpaces } from '../../../../src/router'

Expand All @@ -22,84 +21,11 @@ jest.mock('@files/src/helpers/resources', () => {
}
})

const simpleOwnFolder = {
type: 'folder',
ownerId: 'marie',
ownerDisplayName: 'Marie',
mdate: 'Wed, 21 Oct 2015 07:28:00 GMT',
size: '740'
}

const selectors = {
noSelectionInfoPanel: 'noselection-stub'
}

describe('SideBar', () => {
describe('file info', () => {
beforeEach(() => {
buildResource.mockImplementation((item) => item)
})
afterEach(() => {
jest.clearAllMocks()
})
it('fetches file info if 1 item is selected', () => {
const mockFileInfo = jest.fn()
mockFileInfo.mockReturnValueOnce(Files['/'][1])

createWrapper({
item: simpleOwnFolder,
selectedItems: [simpleOwnFolder],
fileFetchMethod: mockFileInfo
})

expect(mockFileInfo).toHaveBeenCalledTimes(1)
})

it('fetches file info if the selected item changes', async () => {
const spyOnFetchFileInfo = jest
.spyOn(SideBar.methods, 'fetchFileInfo')
.mockImplementation(jest.fn)

const wrapper = createWrapper({
item: simpleOwnFolder,
selectedItems: [simpleOwnFolder]
})

// fetchFileInfo is called once in created()
expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(1)

// it should be called again when a different file is loaded
const resource = Files['/'][4]
resource.path = `${resource.name}`
wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', resource)
await wrapper.vm.$nextTick()
expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(2)

// and again if the file is renamed
const renamedResource = renameResource(
{ webDavPath: '' },
Object.assign({}, resource),
'foobar.png',
''
)
wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', Object.assign(renamedResource))
await wrapper.vm.$nextTick()
expect(spyOnFetchFileInfo).toHaveBeenCalledTimes(3)
})

it('does not fetch file info if multiple items are selected', () => {
const mockFileInfo = jest.fn().mockReturnValue({})

createWrapper({
item: simpleOwnFolder,
selectedItems: [simpleOwnFolder, simpleOwnFolder],
mocks: { $client: { files: { fileInfo: mockFileInfo } } }
})

expect(mockFileInfo).not.toHaveBeenCalled()
})
})

describe('no selection info panel', () => {
afterEach(() => {
jest.clearAllMocks()
Expand Down Expand Up @@ -221,8 +147,7 @@ function createWrapper({
},
getters: {
highlightedFile: (state) => state.highlightedFile,
selectedFiles: () => selectedItems,
currentFolder: () => simpleOwnFolder
selectedFiles: () => selectedItems
},
mutations: {
SET_HIGHLIGHTED_FILE(state, file) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import GetTextPlugin from 'vue-gettext'
import stubs from '@/tests/unit/stubs'

import SpaceInfo from '@files/src/components/SideBar/SpaceInfo.vue'
import Vuex from 'vuex'

const spaceMock = {
type: 'space',
Expand All @@ -16,6 +17,7 @@ const spaceMock = {
}

const localVue = createLocalVue()
localVue.use(Vuex)
localVue.use(GetTextPlugin, {
translations: 'does-not-matter.json',
silent: true
Expand Down Expand Up @@ -48,6 +50,21 @@ describe('SpaceInfo', () => {

function createWrapper(spaceResource) {
return shallowMount(SpaceInfo, {
store: new Vuex.Store({
modules: {
Files: {
namespaced: true,
state: {
highlightedFile: spaceResource
},
getters: {
highlightedFile: (state) => {
return state.highlightedFile
}
}
}
}
}),
localVue,
stubs: {
...stubs,
Expand All @@ -61,11 +78,6 @@ function createWrapper(spaceResource) {
formatRelativeDateFromRFC
}
}
],
provide: {
displayedItem: {
value: spaceResource
}
}
]
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ function getWrapper() {
modules: {
Files: {
namespaced: true,
mutations: {
UPDATE_RESOURCE_FIELD: jest.fn()
},
state: {
currentFolder: {
id: '1fe58d8b-aa69-4c22-baf7-97dd57479f22',
Expand Down