feat(settings): add admin & user setting iframe #4373

codewithvk · 2025-01-07T16:59:25Z

Initial cool admin setting iframe setup
WIP auth
Temp: Auth handling stuff
WIP: wopi setting upload
Created an AppData-based directory for system settings and user settings
Create a Settings controller API endpoint for handling AppData-based directories
Temporary Commit: Created a temporary UI to validate the functionality of user and system settings.
Change the admin settings iframe URL to adminIntegratorSettings.
WOPI: Update the wopi setting upload route to accept a file and store it to system-settings dir
add delete button and create wopi/setting route to handle wopi file request
Manage setting configs files with dynamic routes
Code cleanup: Remove POC helper functions
Send WOPI setting base URL to integrator
fix: fetch config url
wopi: add delete setting file route
refactor: token generation for iframe
feat(user-settings): introduce iframe for user settings
fix: linting issue for CI
fix: generate token for user shared config url
fix: accept document and setting url token for file upload
fix: set proper WOPI response and remove unnecessary WOPI callback from iframe
fix(settings): remove iframe title and section
fix(settings): fetch directory from root folder
fix(wopi): share SharedSettings to wopi checkfileInfo
fix(wopi): Generating setting token for guest users
fix(settings): generate user config per userId & handle guest users
fix: composer psalm error via pointing correct folder
fix: code cleanups
fix: move admin setting iframe below server-config
refactor: some code cleanups
config: support hasSettingIframeSupport capability
fix: Avoid warning on file id explode
feat(wopi): pass WOPI settings if iframe capability is allowed
fix: admin iframe load issue

Some todos that next Prs can fix

Todo: Extract common url similar functions (eg, getWopiUrlForFile, getWopiUrlForTemplate, generateSettings) (Pointer: https://github.com/nextcloud/richdocuments/pull/4373/files#diff-6a541540d5e2e26d471aebd8ef43b32967e5064da4774146a8a177cf32ecf88cR992)
Todo: Can do in next PR: fetch end point from discovery end point (https://github.com/nextcloud/richdocuments/pull/4373/files#diff-ce81efd19e432a3cd0658d52cccd7c8fbd4b01f4ada9b174f949578fe9af8578R93)
Todo: Ideally adminIntegrator iframe URL that can be obtained from the discovery endpoint of Collabora
Todo: Improve/implement uri caching for url(feat(settings): add admin & user setting iframe #4373 (comment))

Resolves: #
Target version: main

juliusknorr

Some early feedback, general approach seems good 👍

lib/Controller/DocumentController.php

lib/TokenManager.php

lib/Db/WopiMapper.php

lib/Controller/DocumentController.php

lib/Service/SettingsService.php

codewithvk · 2025-01-11T19:00:20Z

Related serverside PR : nextcloud/server#50145

pedropintosilva · 2025-01-15T09:45:28Z

@codewithvk @juliusknorr thanks. Probably worth to create a separated PR for the vue conversion, right?

elzody · 2025-01-22T18:17:21Z

In order to resolve the failing tests, you should do the following:

Make sure all the new files (and existing files, even) have the SPDX reuse header at the top
Run composer run cs:fix to address formatting and styling issues, etc.
Run composer run psalm -- --no-cache which will inform you of errors which should be fixed

codewithvk · 2025-01-23T07:24:05Z

In order to resolve the failing tests, you should do the following:

Make sure all the new files (and existing files, even) have the SPDX reuse header at the top

Run composer run cs:fix to address formatting and styling issues, etc.

Run composer run psalm -- --no-cache which will inform you of errors which should be fixed

@elzody We could have one psalm error because we need to merge PR :nextcloud/server#50145 before.. I fixed other errors...Also could you please run CI - as I don't have access to run CI.

src/components/AdminSettings.vue

juliusknorr · 2025-01-23T10:48:56Z

src/components/PersonalSettings.vue

@@ -34,6 +34,17 @@
 			</em>
 		</p>

+		<!-- user settings iframe  -->
+		<div id="user-cool-frame-section" class="section">
+			<h2>{{ t('richdocuments', 'Collabora User Settings') }}</h2>


We should call this differently as we want to remove the duplicate word "settings" from all seections we have (ref #4351)

How it looks now:

I have discussed with Vivek that we should also remove Collabora since it's part of Office settings section.

If we also remove settings (even thought I do see "Personal settings" in the #4351 you have referenced) then we either replace it with something such as "preferences" or - and I think better - we simply remove that h2 completely and let the headings in the iframe being h2 (so, autotext and workbook will become upper level which probably makes more sense)

@pedropintosilva Thanks for adding a screenshot :)

juliusknorr · 2025-01-23T10:49:31Z

@codewithvk Can you also add some screenshots of how this looks in the settings page for review of @marcoambrosini as designer?

codewithvk · 2025-01-24T06:35:24Z

@juliusknorr @elzody I'm not able to figure out why wopi integration for db test broken, any idea?

codewithvk · 2025-01-24T07:36:09Z

Modified bit of UI of iframe

cc: @marcoambrosini @juliusknorr @pedropintosilva

pedropintosilva · 2025-01-24T07:46:29Z

Modified bit of UI of iframe

cc: @marcoambrosini @juliusknorr @pedropintosilva

Awesome, much better, thanks!

I think We can still make it more clear in terms of the elements and their relation. What I mean is that right now the "Upload AutoText/Wordbook" buttons seem that they are referring to the list of files (which is not the case, those files were already uploaded). Best to clarify the main action by renaming it to "Add new AutoText" / "Add new Wordbook" but that seems a bit too long (specially when considering translations) -> so, what about "New AutoText" and "New Wordbook". I have noticed that in https://nextcloud-vue-components.netlify.app/#/Components/App%20containers/NcAppNavigation?id=ncappnavigationnewitem that sometimes they use the additional plus icon as a prefix (Maybe we could use it? @jancborchardt , @marcoambrosini )

juliusknorr · 2025-01-24T14:17:10Z

lib/Service/SettingsService.php

+		try {
+			$categories = [];
+			$folder = $this->appData->getFolder($type);
+			$directories = $folder->getFullDirectoryListing();


Sorry I missed that the server change was also required, but we cannot get that in for 31 anymore due to API freeze.

However you can directly access it by injecting IRootFolder and then fetching the path directly from it

$instanceId = $this->config->getSystemValue('instanceid', null); if ($instanceId === null) { } return $this->rootFolder->get('appdata_' . $instanceId . '/richdocuments/your-path');

@juliusknorr Is it safe to access the rootFolder from a security perspective?

May be for now we can do it as it seems we don't have other options.

Yes, that is fine, the server API that you have would do similar calls

@juliusknorr I have added this commit - could you please checkout and restart CI ? 3e6964b

codewithvk · 2025-01-28T13:02:38Z

lib/Controller/WopiController.php

+		$uri = $nextcloudUrl . '/index.php/apps/richdocuments/wopi/settings' . '?type=' . $type . '&access_token=' . $this->generateSettingToken($userId) . '&fileId=' . '-1';
+		return [
+			'uri' => $uri,
+			'stamp' => time()


@juliusknorr As of now temporary I'm sending stamp as time but we need to pass here Category folders etag/stamp - May be we can create method inside settingsService but still we need fetch stamp from Folder/SimpleFolder or even Node. I try to test with Node etag it seems not updating is something updated in child.

As a simple approach you could try the using getMTime which is probably enough, but I'm not entirely sure this propagates as well

If that doesn't we may need to manually update the etag whenever a file is added/updated/removed through richdocuments

The following code should work for that then:

$folder->getStorage()->getCache()->update($folder->getId(), [ 'etag' => uniqid(), ]);

Just for future reference, the reason the etag is not propagating directly is because we skip it in https://github.com/nextcloud/server/blob/41c53648ad18e3a94b6ba18ad0e6c7c09a520bd9/lib/private/Files/Storage/Common.php#L346 as in other cases we do not make use of that for anything in appdata or we have a separate mountpoint like for collectives where we can apply our own storage wrapper nextcloud/collectives@b54e6be but that won't work for the case here.

Will do in next PR.

Signed-off-by: codewithvk <[email protected]>

…om iframe Signed-off-by: codewithvk <[email protected]>

Signed-off-by: codewithvk <[email protected]>

Signed-off-by: Julius Knorr <[email protected]>

juliusknorr · 2025-02-05T12:44:38Z

I have no clue yet why tests are failing, but this must be somewhat related to the branch, all recent prs against main succeed with cypress: https://github.com/nextcloud/richdocuments/pulls?q=sort%3Aupdated-desc+is%3Apr+is%3Aclosed+base%3Amain

I'll continue digging

juliusknorr · 2025-02-05T12:47:44Z

lib/Controller/WopiController.php

+			$wopi = $this->wopiMapper->getWopiForToken($access_token);
+
+			$userId = $wopi->getEditorUid();
+			// TODO: auth - for admin??


What would be left here?

Nothing, Older comment ig. Will remove it

juliusknorr · 2025-02-05T12:53:01Z

lib/Service/SettingsService.php

+		}
+	}
+
+	// TODO: Handle installDefaultSystemFiles setting


Please track todos as checklist items in the pr description rather then in the code so we can file them in follow up issues on merge

codewithvk · 2025-02-05T12:53:44Z

I have no clue yet why tests are failing, but this must be somewhat related to the branch, all recent prs against main succeed with cypress: https://github.com/nextcloud/richdocuments/pulls?q=sort%3Aupdated-desc+is%3Apr+is%3Aclosed+base%3Amain

I'll continue digging

I have found that a few tests are failing locally, and they also fail on the main branch (based on my local testing). So, perhaps I'll try to create a separate PR to fix those Cypress tests.

juliusknorr · 2025-02-05T13:25:03Z

Pushed two prs:

One with a minimal change to see if cypress passes generally fix: Skip early if font file path is not existing #4445
One your your branch pushed to the nextcloud repo origin, just to see if maybe the github fork does something weird to the ci run private/juliushaertl/cool settings iframe 2 #4446

juliusknorr · 2025-02-05T15:57:38Z

Seems to proof that main is running fine, just this branch, still no clue yet

codewithvk · 2025-02-05T16:12:36Z

Seems to proof that main is running fine, just this branch, still no clue yet

For me - Main branch not works :)

Signed-off-by: codewithvk <[email protected]>

codewithvk requested review from elzody and juliusknorr as code owners January 7, 2025 16:59

codewithvk force-pushed the private/codewithvk/cool_setting_iframe branch from 62c98ce to 104c780 Compare January 7, 2025 17:02

codewithvk marked this pull request as draft January 7, 2025 17:04

juliusknorr reviewed Jan 7, 2025

View reviewed changes

codewithvk commented Jan 10, 2025

View reviewed changes

lib/Service/SettingsService.php Outdated Show resolved Hide resolved

codewithvk force-pushed the private/codewithvk/cool_setting_iframe branch from 905e1e2 to cc91ef8 Compare January 11, 2025 18:22

codewithvk requested a review from juliusknorr January 20, 2025 07:55

codewithvk force-pushed the private/codewithvk/cool_setting_iframe branch from c042023 to 5e11402 Compare January 20, 2025 10:56

codewithvk marked this pull request as ready for review January 21, 2025 12:34

This comment was marked as spam.

Sign in to view

github-actions bot added the feedback-requested label Jan 22, 2025

codewithvk force-pushed the private/codewithvk/cool_setting_iframe branch from 5e11402 to bf5a8bf Compare January 23, 2025 05:54

codewithvk changed the title ~~WIP: Admin setting iframe~~ Admin & User setting iframe Jan 23, 2025

codewithvk changed the title ~~Admin & User setting iframe~~ feat(settings): add admin & user setting iframe Jan 23, 2025

juliusknorr reviewed Jan 23, 2025

View reviewed changes

src/components/AdminSettings.vue Outdated Show resolved Hide resolved

juliusknorr reviewed Jan 23, 2025

View reviewed changes

juliusknorr reviewed Jan 24, 2025

View reviewed changes

codewithvk force-pushed the private/codewithvk/cool_setting_iframe branch 2 times, most recently from 3e6964b to 5302259 Compare January 27, 2025 12:42

codewithvk commented Jan 28, 2025

View reviewed changes

codewithvk force-pushed the private/codewithvk/cool_setting_iframe branch from 9e7c050 to 5124cff Compare January 29, 2025 09:58

codewithvk and others added 16 commits February 5, 2025 13:15

feat(user-settings): introduce iframe for user settings

fc0915a

Signed-off-by: codewithvk <[email protected]>

fix: linting issue for CI

bdfa8b9

Signed-off-by: codewithvk <[email protected]>

fix: generate token for user shared config url

6686315

Signed-off-by: codewithvk <[email protected]>

fix: accept document and setting url token for file upload

dbf9d3a

Signed-off-by: codewithvk <[email protected]>

fix: set proper WOPI response and remove unnecessary WOPI callback fr…

82bb7a8

…om iframe Signed-off-by: codewithvk <[email protected]>

fix(settings): remove iframe title and section

0e79c20

Signed-off-by: codewithvk <[email protected]>

fix(settings): fetch directory from root folder

59a2192

Signed-off-by: codewithvk <[email protected]>

fix(wopi): share SharedSettings to wopi checkfileInfo

747c528

Signed-off-by: codewithvk <[email protected]>

fix(wopi): Generating setting token for guest users

5defde2

Signed-off-by: codewithvk <[email protected]>

fix(settings): generate user config per userId & handle guest users

969a4f9

Signed-off-by: codewithvk <[email protected]>

fix: composer psalm error via pointing correct folder

495a02c

Signed-off-by: codewithvk <[email protected]>

fix: code cleanups

9554d4f

Signed-off-by: codewithvk <[email protected]>

fix: move admin setting iframe below server-config

7c7c779

Signed-off-by: codewithvk <[email protected]>

refactor: some code cleanups

bf69966

Signed-off-by: codewithvk <[email protected]>

config: support hasSettingIframeSupport capability

149e43f

Signed-off-by: codewithvk <[email protected]>

fix: Avoid warning on file id explode

e170b53

Signed-off-by: Julius Knorr <[email protected]>

juliusknorr force-pushed the private/codewithvk/cool_setting_iframe branch from 56a8c72 to e170b53 Compare February 5, 2025 12:15

juliusknorr reviewed Feb 5, 2025

View reviewed changes

codewithvk added 3 commits February 5, 2025 23:21

feat(wopi): pass WOPI settings if iframe capability is allowed

3fbe5e2

Signed-off-by: codewithvk <[email protected]>

fix: admin iframe load issue

85483d6

Signed-off-by: codewithvk <[email protected]>

fix(cosmetic): remove unnecessary comments

fdb0200

Signed-off-by: codewithvk <[email protected]>

codewithvk requested a review from juliusknorr February 5, 2025 19:30

juliusknorr approved these changes Feb 5, 2025

View reviewed changes

juliusknorr merged commit 452fb40 into nextcloud:main Feb 5, 2025
72 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(settings): add admin & user setting iframe #4373

feat(settings): add admin & user setting iframe #4373

codewithvk commented Jan 7, 2025 •

edited

Loading

juliusknorr left a comment

codewithvk commented Jan 11, 2025

pedropintosilva commented Jan 15, 2025

This comment was marked as spam.

elzody commented Jan 22, 2025

codewithvk commented Jan 23, 2025 •

edited

Loading

juliusknorr Jan 23, 2025

pedropintosilva Jan 23, 2025

codewithvk Jan 23, 2025

juliusknorr commented Jan 23, 2025

codewithvk commented Jan 24, 2025

codewithvk commented Jan 24, 2025

pedropintosilva commented Jan 24, 2025 •

edited

Loading

juliusknorr Jan 24, 2025

codewithvk Jan 24, 2025

juliusknorr Jan 24, 2025

codewithvk Jan 27, 2025

codewithvk Jan 28, 2025

juliusknorr Jan 28, 2025

juliusknorr Jan 28, 2025 •

edited

Loading

codewithvk Feb 4, 2025

juliusknorr commented Feb 5, 2025

juliusknorr Feb 5, 2025

codewithvk Feb 5, 2025

juliusknorr Feb 5, 2025

codewithvk commented Feb 5, 2025 •

edited

Loading

juliusknorr commented Feb 5, 2025 •

edited

Loading

juliusknorr commented Feb 5, 2025

codewithvk commented Feb 5, 2025

feat(settings): add admin & user setting iframe #4373

feat(settings): add admin & user setting iframe #4373

Conversation

codewithvk commented Jan 7, 2025 • edited Loading

juliusknorr left a comment

Choose a reason for hiding this comment

codewithvk commented Jan 11, 2025

pedropintosilva commented Jan 15, 2025

This comment was marked as spam.

elzody commented Jan 22, 2025

codewithvk commented Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliusknorr commented Jan 23, 2025

codewithvk commented Jan 24, 2025

codewithvk commented Jan 24, 2025

pedropintosilva commented Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliusknorr Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliusknorr commented Feb 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codewithvk commented Feb 5, 2025 • edited Loading

juliusknorr commented Feb 5, 2025 • edited Loading

juliusknorr commented Feb 5, 2025

codewithvk commented Feb 5, 2025

codewithvk commented Jan 7, 2025 •

edited

Loading

codewithvk commented Jan 23, 2025 •

edited

Loading

pedropintosilva commented Jan 24, 2025 •

edited

Loading

juliusknorr Jan 28, 2025 •

edited

Loading

codewithvk commented Feb 5, 2025 •

edited

Loading

juliusknorr commented Feb 5, 2025 •

edited

Loading