From 5a0d15e45189736384d07fd02dea349f7de5a58c Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Fri, 27 Oct 2023 17:18:38 +0200 Subject: [PATCH 01/11] web-app-external: open files in shares and public links in view mode by default --- packages/web-app-external/src/App.vue | 50 ++++++++++++++++--- .../components/AppTemplates/AppWrapper.vue | 1 + 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index d05935302e1..4c3c575acd0 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -28,14 +28,22 @@ import { PropType, computed, defineComponent, unref, nextTick, ref, watch, VNode import { useTask } from 'vue-concurrency' import { useGettext } from 'vue3-gettext' -import { Resource } from '@ownclouders/web-client/src' +import { Resource, SpaceResource } from '@ownclouders/web-client/src' import { urlJoin } from '@ownclouders/web-client/src/utils' -import { queryItemAsString, useRequest, useRouteQuery, useStore } from '@ownclouders/web-pkg' +import { + isSameResource, + queryItemAsString, + useRequest, + useRouteQuery, + useStore +} from '@ownclouders/web-pkg' import { configurationManager } from '@ownclouders/web-pkg' +import { isPublicSpaceResource, isShareSpaceResource } from '@ownclouders/web-client/src/helpers' export default defineComponent({ name: 'ExternalApp', props: { + space: { type: Object as PropType, required: true }, resource: { type: Object as PropType, required: true } }, emits: ['update:applicationName'], @@ -73,6 +81,29 @@ export default defineComponent({ }) } + const viewMode = ref('view') + const catchClickMicrosoftEdit = (event) => { + try { + if (JSON.parse(event.data)?.MessageId === 'UI_Edit') { + viewMode.value = 'write' + } + } catch (e) {} + } + watch( + applicationName, + (newAppName, oldAppName) => { + console.log('appNames', newAppName, oldAppName) + if (newAppName === 'Office365' && newAppName !== oldAppName) { + window.addEventListener('message', catchClickMicrosoftEdit) + } else { + window.removeEventListener('message', catchClickMicrosoftEdit) + } + }, + { + immediate: true + } + ) + const loadAppUrl = useTask(function* () { try { const fileId = props.resource.fileId @@ -84,7 +115,8 @@ export default defineComponent({ const query = stringify({ file_id: fileId, lang: language.current, - ...(unref(applicationName) && { app_name: unref(applicationName) }) + ...(unref(applicationName) && { app_name: unref(applicationName) }), + ...(unref(viewMode) && { view_mode: unref(viewMode) }) }) const url = `${baseUrl}?${query}` @@ -133,11 +165,17 @@ export default defineComponent({ }).restartable() watch( - props.resource, - () => { + [props.resource, viewMode], + ([newResource], [oldResource]) => { + if (!isSameResource(newResource, oldResource)) { + viewMode.value = + isShareSpaceResource(props.space) || isPublicSpaceResource(props.space) + ? 'view' + : 'write' + } loadAppUrl.perform() }, - { immediate: true } + { immediate: true, deep: true } ) return { diff --git a/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue b/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue index 917a17951a7..ac6d6d49aa7 100644 --- a/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue +++ b/packages/web-pkg/src/components/AppTemplates/AppWrapper.vue @@ -418,6 +418,7 @@ export default defineComponent({ const slotAttrs = computed(() => ({ url: unref(url), + space: unref(unref(currentFileContext).space), resource: unref(resource), isDirty: unref(isDirty), isReadOnly: unref(isReadOnly), From 1dcffe66221165ccd685196eec5c4f06dea1815c Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Fri, 27 Oct 2023 17:44:10 +0200 Subject: [PATCH 02/11] Simplify logic by not handling everything in a watcher --- packages/web-app-external/src/App.vue | 77 ++++++++++++++++----------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index 4c3c575acd0..c5de77a675b 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -44,7 +44,8 @@ export default defineComponent({ name: 'ExternalApp', props: { space: { type: Object as PropType, required: true }, - resource: { type: Object as PropType, required: true } + resource: { type: Object as PropType, required: true }, + isReadOnly: { type: Boolean, required: true } }, emits: ['update:applicationName'], setup(props, { emit }) { @@ -81,30 +82,7 @@ export default defineComponent({ }) } - const viewMode = ref('view') - const catchClickMicrosoftEdit = (event) => { - try { - if (JSON.parse(event.data)?.MessageId === 'UI_Edit') { - viewMode.value = 'write' - } - } catch (e) {} - } - watch( - applicationName, - (newAppName, oldAppName) => { - console.log('appNames', newAppName, oldAppName) - if (newAppName === 'Office365' && newAppName !== oldAppName) { - window.addEventListener('message', catchClickMicrosoftEdit) - } else { - window.removeEventListener('message', catchClickMicrosoftEdit) - } - }, - { - immediate: true - } - ) - - const loadAppUrl = useTask(function* () { + const loadAppUrl = useTask(function* (signal, viewMode: string) { try { const fileId = props.resource.fileId const baseUrl = urlJoin( @@ -116,7 +94,7 @@ export default defineComponent({ file_id: fileId, lang: language.current, ...(unref(applicationName) && { app_name: unref(applicationName) }), - ...(unref(viewMode) && { view_mode: unref(viewMode) }) + ...(viewMode && { view_mode: viewMode }) }) const url = `${baseUrl}?${query}` @@ -164,16 +142,51 @@ export default defineComponent({ } }).restartable() + // switch to write mode when edit is clicked + const catchClickMicrosoftEdit = (event) => { + try { + if (JSON.parse(event.data)?.MessageId === 'UI_Edit') { + if (props.isReadOnly) { + console.error('Cannot switch to write mode as file is read-only') + return + } + loadAppUrl.perform('write') + } + } catch (e) {} + } + watch( + applicationName, + (newAppName, oldAppName) => { + console.log('appNames', newAppName, oldAppName) + if (newAppName === 'Office365' && newAppName !== oldAppName) { + window.addEventListener('message', catchClickMicrosoftEdit) + } else { + window.removeEventListener('message', catchClickMicrosoftEdit) + } + }, + { + immediate: true + } + ) + watch( - [props.resource, viewMode], + [props.resource], ([newResource], [oldResource]) => { + let viewMode = 'write' + // FIXME: introduce config option (?) + const featureFlag = true if (!isSameResource(newResource, oldResource)) { - viewMode.value = - isShareSpaceResource(props.space) || isPublicSpaceResource(props.space) - ? 'view' - : 'write' + if (props.isReadOnly) { + viewMode = 'view' + } else if ( + featureFlag && + (isShareSpaceResource(props.space) || isPublicSpaceResource(props.space)) + ) { + viewMode = 'view' + } } - loadAppUrl.perform() + + loadAppUrl.perform(unref(viewMode)) }, { immediate: true, deep: true } ) From 5cb30699607e9b54dbe2600ad7a160ec31a4d38a Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Fri, 27 Oct 2023 17:53:57 +0200 Subject: [PATCH 03/11] Show error popup when trying to switch to edit mode for read-only files --- packages/web-app-external/src/App.vue | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index c5de77a675b..d962608d5f5 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -84,6 +84,13 @@ export default defineComponent({ const loadAppUrl = useTask(function* (signal, viewMode: string) { try { + if (props.isReadOnly && viewMode === 'write') { + store.dispatch('showErrorMessage', { + title: $gettext('Cannot open file in edit mode as it is read-only') + }) + return + } + const fileId = props.resource.fileId const baseUrl = urlJoin( configurationManager.serverUrl, @@ -146,10 +153,6 @@ export default defineComponent({ const catchClickMicrosoftEdit = (event) => { try { if (JSON.parse(event.data)?.MessageId === 'UI_Edit') { - if (props.isReadOnly) { - console.error('Cannot switch to write mode as file is read-only') - return - } loadAppUrl.perform('write') } } catch (e) {} From 3702a68427d3c568a741bb1c7014956ec40b02c5 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Fri, 27 Oct 2023 18:08:21 +0200 Subject: [PATCH 04/11] Simplify logic some more --- packages/web-app-external/src/App.vue | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index d962608d5f5..18bbe0e52bd 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -175,20 +175,19 @@ export default defineComponent({ watch( [props.resource], ([newResource], [oldResource]) => { - let viewMode = 'write' + if (isSameResource(newResource, oldResource)) { + return + } + + let viewMode = props.isReadOnly ? 'view' : 'write' // FIXME: introduce config option (?) const featureFlag = true - if (!isSameResource(newResource, oldResource)) { - if (props.isReadOnly) { - viewMode = 'view' - } else if ( - featureFlag && - (isShareSpaceResource(props.space) || isPublicSpaceResource(props.space)) - ) { - viewMode = 'view' - } + if ( + featureFlag && + (isShareSpaceResource(props.space) || isPublicSpaceResource(props.space)) + ) { + viewMode = 'view' } - loadAppUrl.perform(unref(viewMode)) }, { immediate: true, deep: true } From 0c04d8d92dc976f24fbd82e4f342695449c69cde Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Fri, 27 Oct 2023 18:09:05 +0200 Subject: [PATCH 05/11] Remove obsolete unref --- packages/web-app-external/src/App.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index 18bbe0e52bd..b948faa9376 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -188,7 +188,7 @@ export default defineComponent({ ) { viewMode = 'view' } - loadAppUrl.perform(unref(viewMode)) + loadAppUrl.perform(viewMode) }, { immediate: true, deep: true } ) From b65157a65e40bd9d39b5592ecb3a07871bb1efaf Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Wed, 8 Nov 2023 19:11:08 +0100 Subject: [PATCH 06/11] Remove console output --- packages/web-app-external/src/App.vue | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index b948faa9376..36f9f6ffe03 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -160,7 +160,6 @@ export default defineComponent({ watch( applicationName, (newAppName, oldAppName) => { - console.log('appNames', newAppName, oldAppName) if (newAppName === 'Office365' && newAppName !== oldAppName) { window.addEventListener('message', catchClickMicrosoftEdit) } else { From da2b90bcc6d5e451a482c11fbe4f249116f833c7 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Thu, 9 Nov 2023 11:44:30 +0100 Subject: [PATCH 07/11] Implement feature flag --- packages/web-app-external/src/App.vue | 17 +++++++++++++---- packages/web-pkg/src/configuration/manager.ts | 10 ++++++++++ packages/web-pkg/src/configuration/types.ts | 3 +++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index 36f9f6ffe03..5a4cdb18714 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -33,6 +33,7 @@ import { urlJoin } from '@ownclouders/web-client/src/utils' import { isSameResource, queryItemAsString, + useConfigurationManager, useRequest, useRouteQuery, useStore @@ -51,6 +52,7 @@ export default defineComponent({ setup(props, { emit }) { const language = useGettext() const store = useStore() + const configurationManager = useConfigurationManager() const { $gettext } = language const { makeRequest } = useRequest() @@ -149,6 +151,13 @@ export default defineComponent({ } }).restartable() + const determineOpenAsPreview = (appName: string) => { + const openAsPreview = configurationManager.options.editors.openAsPreview + return ( + openAsPreview === true || (Array.isArray(openAsPreview) && openAsPreview.includes(appName)) + ) + } + // switch to write mode when edit is clicked const catchClickMicrosoftEdit = (event) => { try { @@ -160,7 +169,7 @@ export default defineComponent({ watch( applicationName, (newAppName, oldAppName) => { - if (newAppName === 'Office365' && newAppName !== oldAppName) { + if (determineOpenAsPreview(newAppName) && newAppName !== oldAppName) { window.addEventListener('message', catchClickMicrosoftEdit) } else { window.removeEventListener('message', catchClickMicrosoftEdit) @@ -178,11 +187,11 @@ export default defineComponent({ return } + debugger + let viewMode = props.isReadOnly ? 'view' : 'write' - // FIXME: introduce config option (?) - const featureFlag = true if ( - featureFlag && + determineOpenAsPreview(unref(applicationName)) && (isShareSpaceResource(props.space) || isPublicSpaceResource(props.space)) ) { viewMode = 'view' diff --git a/packages/web-pkg/src/configuration/manager.ts b/packages/web-pkg/src/configuration/manager.ts index 0b610fa8858..42caa0650a2 100644 --- a/packages/web-pkg/src/configuration/manager.ts +++ b/packages/web-pkg/src/configuration/manager.ts @@ -101,6 +101,16 @@ export class ConfigurationManager { 'openLinksWithDefaultApp', get(options, 'openLinksWithDefaultApp', true) ) + + // when this setting is enabled, non-personal files i.e. files in shares, spaces or public links + // are opened in read only mode and the user needs another click to switch to edit mode + // it can be set to true/false or an array of wopi app names + set( + this.optionsConfiguration, + 'editors.openAsPreview', + get(options, 'editors.openAsPreview', false) + ) + set(this.optionsConfiguration, 'upload.companionUrl', get(options, 'upload.companionUrl', '')) set(this.optionsConfiguration, 'tokenStorageLocal', get(options, 'tokenStorageLocal', true)) set(this.optionsConfiguration, 'loginUrl', get(options, 'loginUrl', '')) diff --git a/packages/web-pkg/src/configuration/types.ts b/packages/web-pkg/src/configuration/types.ts index 99b07fa1ec1..6d809b9022c 100644 --- a/packages/web-pkg/src/configuration/types.ts +++ b/packages/web-pkg/src/configuration/types.ts @@ -29,6 +29,9 @@ export interface OptionsConfiguration { mode?: string isRunningOnEos?: boolean embedTarget?: string + editors?: { + openAsPreview?: boolean | string[] + } } export interface OAuth2Configuration { From f31fbad6671d1b53efde8c34315def06117f96d4 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Thu, 9 Nov 2023 12:49:49 +0100 Subject: [PATCH 08/11] run linter --- packages/web-app-external/src/App.vue | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index 5a4cdb18714..16d7c4ea714 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -38,7 +38,6 @@ import { useRouteQuery, useStore } from '@ownclouders/web-pkg' -import { configurationManager } from '@ownclouders/web-pkg' import { isPublicSpaceResource, isShareSpaceResource } from '@ownclouders/web-client/src/helpers' export default defineComponent({ From 05ed300860af16eefb18c68fe1e18999c76f2bab Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Thu, 9 Nov 2023 13:55:28 +0100 Subject: [PATCH 09/11] test: fix unit tests --- .../web-app-external/tests/unit/app.spec.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/web-app-external/tests/unit/app.spec.ts b/packages/web-app-external/tests/unit/app.spec.ts index 624dd21fbcc..d1f025a69ce 100644 --- a/packages/web-app-external/tests/unit/app.spec.ts +++ b/packages/web-app-external/tests/unit/app.spec.ts @@ -1,11 +1,11 @@ -import { mock } from 'jest-mock-extended' +import { mock, mockDeep } from 'jest-mock-extended' import { createStore, defaultPlugins, defaultStoreMockOptions, shallowMount } from 'web-test-helpers' -import { useRequest, useRouteQuery } from '@ownclouders/web-pkg' +import { ConfigurationManager, useRequest, useRouteQuery } from '@ownclouders/web-pkg' import { ref } from 'vue' import { Resource } from '@ownclouders/web-client' @@ -14,7 +14,15 @@ import App from '../../src/App.vue' jest.mock('@ownclouders/web-pkg', () => ({ ...jest.requireActual('@ownclouders/web-pkg'), useRequest: jest.fn(), - useRouteQuery: jest.fn() + useRouteQuery: jest.fn(), + useConfigurationManager: () => + mockDeep({ + options: { + editors: { + openAsPreview: false + } + } + }) })) const appUrl = 'https://example.test/d12ab86/loe009157-MzBw' @@ -38,10 +46,6 @@ describe('The app provider extension', () => { jest.spyOn(console, 'error').mockImplementation(() => undefined) }) - afterEach(() => { - jest.clearAllMocks() - }) - it('should fail for unauthenticated users', async () => { const makeRequest = jest.fn().mockResolvedValue({ ok: true, From 0cd57cce3a77596c36573102225d790e40181bf1 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Fri, 10 Nov 2023 10:30:56 +0100 Subject: [PATCH 10/11] add config to dev docs --- docs/getting-started.md | 1 + packages/web-app-external/src/App.vue | 12 +++++++++--- packages/web-pkg/src/configuration/manager.ts | 8 ++++---- packages/web-pkg/src/configuration/types.ts | 2 +- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/docs/getting-started.md b/docs/getting-started.md index 31c1c0d17f8..3a89d8ea58e 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -73,6 +73,7 @@ Depending on the backend you are using, there are sample config files provided i - `options.upload.xhr.timeout` Specifies the timeout for XHR uploads in milliseconds. - `options.editor.autosaveEnabled` Specifies if the autosave for the editor apps is enabled. - `options.editor.autosaveInterval` Specifies the time interval for the autosave of editor apps in seconds. +- `options.editor.openAsPreview` Specifies if non-personal files i.e. files in shares, spaces or public links are being opened in read only mode so the user needs to manually switch to edit mode. Can be set to `true`, `false` or an array of web app/editor names. - `options.contextHelpersReadMore` Specifies whether the "Read more" link should be displayed or not. - `options.openLinksWithDefaultApp` Specifies whether single file link shares should be opened with default app or not. - `options.tokenStorageLocal` Specifies whether the access token will be stored in the local storage when set to `true` or in the session storage when set to `false`. If stored in the local storage, login state will be persisted across multiple browser tabs, means no additional logins are required. Defaults to `true`. diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index 16d7c4ea714..12a9fe35cf6 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -38,7 +38,11 @@ import { useRouteQuery, useStore } from '@ownclouders/web-pkg' -import { isPublicSpaceResource, isShareSpaceResource } from '@ownclouders/web-client/src/helpers' +import { + isProjectSpaceResource, + isPublicSpaceResource, + isShareSpaceResource +} from '@ownclouders/web-client/src/helpers' export default defineComponent({ name: 'ExternalApp', @@ -151,7 +155,7 @@ export default defineComponent({ }).restartable() const determineOpenAsPreview = (appName: string) => { - const openAsPreview = configurationManager.options.editors.openAsPreview + const openAsPreview = configurationManager.options.editor.openAsPreview return ( openAsPreview === true || (Array.isArray(openAsPreview) && openAsPreview.includes(appName)) ) @@ -191,7 +195,9 @@ export default defineComponent({ let viewMode = props.isReadOnly ? 'view' : 'write' if ( determineOpenAsPreview(unref(applicationName)) && - (isShareSpaceResource(props.space) || isPublicSpaceResource(props.space)) + (isShareSpaceResource(props.space) || + isPublicSpaceResource(props.space) || + isProjectSpaceResource(props.space)) ) { viewMode = 'view' } diff --git a/packages/web-pkg/src/configuration/manager.ts b/packages/web-pkg/src/configuration/manager.ts index 42caa0650a2..e852ce3144b 100644 --- a/packages/web-pkg/src/configuration/manager.ts +++ b/packages/web-pkg/src/configuration/manager.ts @@ -103,12 +103,12 @@ export class ConfigurationManager { ) // when this setting is enabled, non-personal files i.e. files in shares, spaces or public links - // are opened in read only mode and the user needs another click to switch to edit mode - // it can be set to true/false or an array of wopi app names + // are opened in read only mode and the user needs another click to switch to edit mode. + // it can be set to true/false or an array of web app/editor names. set( this.optionsConfiguration, - 'editors.openAsPreview', - get(options, 'editors.openAsPreview', false) + 'editor.openAsPreview', + get(options, 'editor.openAsPreview', false) ) set(this.optionsConfiguration, 'upload.companionUrl', get(options, 'upload.companionUrl', '')) diff --git a/packages/web-pkg/src/configuration/types.ts b/packages/web-pkg/src/configuration/types.ts index 6d809b9022c..9ade3b87160 100644 --- a/packages/web-pkg/src/configuration/types.ts +++ b/packages/web-pkg/src/configuration/types.ts @@ -29,7 +29,7 @@ export interface OptionsConfiguration { mode?: string isRunningOnEos?: boolean embedTarget?: string - editors?: { + editor?: { openAsPreview?: boolean | string[] } } From 47800c1bc8a748a72b5bdfb724df5b6efd071daa Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Fri, 10 Nov 2023 10:37:16 +0100 Subject: [PATCH 11/11] fixup! add config to dev docs --- packages/web-app-external/tests/unit/app.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web-app-external/tests/unit/app.spec.ts b/packages/web-app-external/tests/unit/app.spec.ts index d1f025a69ce..76190a39f1b 100644 --- a/packages/web-app-external/tests/unit/app.spec.ts +++ b/packages/web-app-external/tests/unit/app.spec.ts @@ -18,7 +18,7 @@ jest.mock('@ownclouders/web-pkg', () => ({ useConfigurationManager: () => mockDeep({ options: { - editors: { + editor: { openAsPreview: false } }