[draw.io] Deployment config + .vsdx support #4337

VirtualEvan · 2020-11-18T12:23:05Z

Description

This PR allows to import .vsdx (Microsoft Visio files) directly from OC.
Once the file is open, if saved, a new <filename>_<timestamp>.drawio file is created with the modifications. (This avoids overwriting the original file with XML content). A message informing this should be shown ( [Bug] Messages not shown in fullscreen apps #4335 )
By default, the draw-io app now points to https://embed.diagrams.net (The official embed-specific draw.io server)

If specified, the configuration in config.json will be used for the connections to draw.io (server url, autosave and theme)

{
  ...
  "applications": [
    {
      "title": {
        "en": "draw-io"
      },
      "url": "https://custom.draw.io.url",
      "autosave": true,
      "theme": "atlas"
    }
  ]
}

Related Issue

Closes [Feature] draw.io vsdx support #4327
Closes [Feature] Add custom draw.io settings #4328

Motivation and Context

Privacy reasons
Flexibility
Support for more formats

How Has This Been Tested?

test environment:
Docker OC 10 + local Phoenix: following this docs
test case 1:
Use custom draw.io server, enable autosave, use the 'atlas' theme
test case 2:
Open a .vsdx file by clicking on it

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

Open tasks:

Import .vsdx files directly from OwnCloud
Custom draw.io server support
Autosave support
Different themes support
Specify draw.io settings in the global configuration
Use by default the embed-specific diagrams.net server
View mode for read-only diagrams

update-docs · 2020-11-18T12:23:09Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

CLAassistant · 2020-11-18T12:23:10Z

All committers have signed the CLA.

VirtualEvan · 2020-11-18T13:55:29Z

I have a couple of questions regarding the implementation of these functionalities.

I could not find a way to get the file permissions from inside an app, can that be achieved somehow? I am interest in knowing if the current user has read-only permissions on the open diagram.
The only place I found situable to put the draw.io configuration settings was the "applications" array in config.json as there are similar configurations for other apps. However, as it is an array, it is necessary to filter by the title and en (english name), in order to get the right app.
Is there some other place where this information could be set in a more optimal and reliable way? Or maybe change it from being an array to be a named object? Not sure how this change would affect other functionalities of Phoenix.

LukasHirt · 2020-11-24T12:52:01Z

I could not find a way to get the file permissions from inside an app, can that be achieved somehow? I am interest in knowing if the current user has read-only permissions on the open diagram.

You could do a call via our sdk - this.$client.files.fileInfo(filePath, ['{http://owncloud.org/ns}permissions'])

The only place I found situable to put the draw.io configuration settings was the "applications" array in config.json...

This will be possible after merging #4380. Applications array is only for displaying the app in the app switcher. The config shouldn't be used for settings outside of that. Otherwise, there's also settings service which enables you to create a settings bundle for extension https://owncloud.github.io/extensions/settings/bundles For this basic config it might be too much effort though.

LukasHirt · 2020-11-24T13:17:28Z

apps/draw-io/src/DrawIoEditor.vue

+          enabled: true
+        }
+      })
+      fetch(url, { headers })


Wouldn't this method work here? https://github.com/owncloud/owncloud-sdk/blob/c4b96e0b50bf989e792899913fe3a70fd1698b43/src/fileManagement.js#L76

I tried with getFileContent, but it looks like we cannot specify the responsetype. Drawio is able to handle the vsdx file as base64, so we need the file itself insted of its content. Also tried to re-create the file from its binary content, but I could not find a way to do so, the resultant file was always different.

VirtualEvan · 2020-12-07T10:51:17Z

Sorry for the delay, I've been quite busy and wanted to test as much as possible before comming back to you

You could do a call via our sdk - this.$client.files.fileInfo(filePath, ['{http://owncloud.org/ns}permissions'])

I am trying to show diagrams in chromeless mode when they are read-only.
The SDK allowed me to know if the file is read-only, but as it is asychronous I had to delay the render until the response is ready.

This will be possible after merging #4380. Applications array is only for displaying the app in the app switcher. The config shouldn't be used for settings outside of that...

By this I understand that the applications array should not be used to store configuration specific to applications, but the external_apps can be used for this, is it right? I added it under external_apps and was able to get it directly from the store.

LukasHirt · 2020-12-07T15:27:45Z

I am trying to show diagrams in chromeless mode when they are read-only.
The SDK allowed me to know if the file is read-only, but as it is asychronous I had to delay the render until the response is ready.

Is the delay okay or does it cause some issues? AFAICT the delay shouldn't take too long so from users POV it shouldn't be a big problem.

By this I understand that the applications array should not be used to store configuration specific to applications, but the external_apps can be used for this, is it right? I added it under external_apps and was able to get it directly from the store.

Correct. Nice!

LukasHirt · 2020-12-07T15:28:47Z

Can you try to rebase the PR? It might solve the failing tests.

As no locales are being used, is not necessary to load them This and the moment.js usage can be removed if the creation of the new .drawio file is managed with a modal instead of adding a timestamp to the filename

VirtualEvan · 2020-12-08T10:01:23Z

Rebasing fixed the issue with the tests, thanks!

Is the delay okay or does it cause some issues? AFAICT the delay shouldn't take too long so from users POV it shouldn't be a big problem.

It is not a problem in my local instance, but I was not sure how long could it take for other people with different/more complex setups. If you think it is OK then is perfect! :)

LukasHirt

Nice 🚀

LukasHirt · 2020-12-09T09:12:03Z

@VirtualEvan Let me pls know if there's something left you would like to commit here or if I can hit the merge button 😉

VirtualEvan · 2020-12-09T10:02:34Z

No, I think that's all. Please merge :)

Do you want me to add some documentation somewhere mentioning the options that can be configured for the app?

"external_apps": [
  {
    "id": "draw-io",
    "path": "this_should_exist_and_cannot_be_empty",
    "config": {
      "url": "https://drawio.web.cern.ch",
      "autosave": true,
      "theme": "atlas"
    }
  }
]

By the way, I noticed that for external apps, path is mandatory.
If it does not exist or is empty, then everything fails. Just putting a dummy value makes it work.
Not sure if it's a bug or if I missed something.

LukasHirt · 2020-12-09T10:08:19Z

Do you want me to add some documentation somewhere mentioning the options that can be configured for the app?

That would be awesome to have it documented. Maybe you could put it in the sample config.json files? If you want to add it here, I'll wait with the merging 😉 Thank you!

By the way, I noticed that for external apps, path is mandatory.

Yep, both id and path are mandatory. Without the path, the frontend doesn't know where to locate the app. I know that it's now creating confusion with the apps array where drawio is loaded. If you check the console, there should be actually a 404 error. That could be fixed by specifying the correct path and removing drawio from apps array to prevent having the app loaded twice.

VirtualEvan · 2020-12-09T10:49:19Z

OK, I think I got it.
Added default configuration to the oc10 and ocis sample config.json files

Let me know if there is something else I can add that could be helpful

LukasHirt · 2020-12-09T10:51:31Z

Thank you! I think that this is now in a perfect shape for merging! I'll hit the button when CI is green 😉

VirtualEvan force-pushed the vsdx-support branch from be3456c to a4a138f Compare November 18, 2020 12:42

LukasHirt reviewed Nov 24, 2020

View reviewed changes

VirtualEvan force-pushed the vsdx-support branch from 1ce8591 to 7b750a1 Compare December 7, 2020 10:50

VirtualEvan added 13 commits December 8, 2020 09:56

Add support for .vsdx files

7af495d

Enable autosave

9da7c1b

Use the embed-specific diagrams.net app

a7c1505

Disable unsaved changes warning after saving

b9f3db1

Set the right error message

a0ccf75

Add timestamp to the file name

d1b6bbf

Show message when importing a file

1f7d376

Use configurable settings for drawio

2f0e8e6

Do not load moment locales to reduce the bundle size

3671e2e

As no locales are being used, is not necessary to load them This and the moment.js usage can be removed if the creation of the new .drawio file is managed with a modal instead of adding a timestamp to the filename

Add changelog

4c7e6d4

Use chromeless mode if the file is read only

3748a0e

Get config from the right place: owncloud#4380

5c9d455

Load the application after checking the file permissions

8bc0e3b

VirtualEvan force-pushed the vsdx-support branch from 7b750a1 to 8bc0e3b Compare December 8, 2020 08:58

LukasHirt approved these changes Dec 8, 2020

View reviewed changes

Add config samples

b95c278

LukasHirt merged commit 190e0f8 into owncloud:master Dec 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[draw.io] Deployment config + .vsdx support #4337

[draw.io] Deployment config + .vsdx support #4337

VirtualEvan commented Nov 18, 2020 •

edited

update-docs bot commented Nov 18, 2020

CLAassistant commented Nov 18, 2020 •

edited

VirtualEvan commented Nov 18, 2020

LukasHirt commented Nov 24, 2020 •

edited

LukasHirt Nov 24, 2020 •

edited

VirtualEvan Dec 7, 2020

VirtualEvan commented Dec 7, 2020

LukasHirt commented Dec 7, 2020

LukasHirt commented Dec 7, 2020

VirtualEvan commented Dec 8, 2020 •

edited

LukasHirt left a comment

LukasHirt commented Dec 9, 2020

VirtualEvan commented Dec 9, 2020

LukasHirt commented Dec 9, 2020 •

edited

VirtualEvan commented Dec 9, 2020

LukasHirt commented Dec 9, 2020

[draw.io] Deployment config + .vsdx support #4337

[draw.io] Deployment config + .vsdx support #4337

Conversation

VirtualEvan commented Nov 18, 2020 • edited

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Open tasks:

update-docs bot commented Nov 18, 2020

CLAassistant commented Nov 18, 2020 • edited

VirtualEvan commented Nov 18, 2020

LukasHirt commented Nov 24, 2020 • edited

LukasHirt Nov 24, 2020 • edited

Choose a reason for hiding this comment

VirtualEvan Dec 7, 2020

Choose a reason for hiding this comment

VirtualEvan commented Dec 7, 2020

LukasHirt commented Dec 7, 2020

LukasHirt commented Dec 7, 2020

VirtualEvan commented Dec 8, 2020 • edited

LukasHirt left a comment

Choose a reason for hiding this comment

LukasHirt commented Dec 9, 2020

VirtualEvan commented Dec 9, 2020

LukasHirt commented Dec 9, 2020 • edited

VirtualEvan commented Dec 9, 2020

LukasHirt commented Dec 9, 2020

VirtualEvan commented Nov 18, 2020 •

edited

CLAassistant commented Nov 18, 2020 •

edited

LukasHirt commented Nov 24, 2020 •

edited

LukasHirt Nov 24, 2020 •

edited

VirtualEvan commented Dec 8, 2020 •

edited

LukasHirt commented Dec 9, 2020 •

edited