Add plugins menu and client plugins #1143

philippfromme · 2019-01-25T10:54:43Z

add plugins menu
add client plugins (style and script)
add integration test
verify menu properties
expose plug-in path helpers in a reasonable manner (Add plugins menu and client plugins #1143 (comment))

Closes #1083

nikku

A few pointers before we close this one out:

ES6 migration -> due to the fact that you migrated a lot of stuff in the app as part of this PR it is super hard to understand what it is about
Ensure backwards compatibility: How is this reated to the feature you're implementing? What is it about? I have no idea looking at both your comment + linked commit and the code

Is it possible to split up the actual feature from the ES 6 refactoring and other (possibly unrelated) changes?

app/lib/plugins.js

philippfromme · 2019-01-25T13:47:01Z

I could seperate the ES6 refactoring, yes.

As for the backwards compatibility, we've renamed/removed a couple of properties from the state that is passed to the backend. Any menu plugins that were built using a property that isn't there anymore would break (e.g. dmnRuleEditing). Actually, I just noticed that while merging my changes with yours some of my changes regarding that got lost. I'll add them.

nikku · 2019-01-28T13:03:02Z

Some plug-ins we should test for support:

Utilities we need to support (plug-ins build upon these):

Camunda Modeler Plugin Helpers, to be supported / updated to latest Modeler version

Regarding ensuring compatibility:

Make sure we restore it + add test coverage for it in an isolated, single commit
Create test plug-in that uses all extension points (and fails hard, if they are not present)

Error Handling:

How do we handle the load failure of a plug-in (bundle error, ...)? Premise: Keep app running regardless and give user a notice.

barmac · 2019-01-28T14:29:46Z

Reduced Palette does not work, but it seems to fail calling bpmn-js internal stuff.

Camunda Modeler Plugin - UserTask Generated Form Preview and Embedded Form Generator has some css issue, but anyway it works.

Token Simulation for the Camunda Modeler stopped to work. We get the following exception:

EventBus.js?cd8f:376 unhandled error in event listener
EventBus._invokeListener @ EventBus.js?cd8f:376
EventBus._invokeListeners @ EventBus.js?cd8f:348
EventBus.fire @ EventBus.js?cd8f:310
./node_modules/bpmn-js-token-simulation/lib/features/toggle-mode/modeler/ToggleMode.js.ToggleMode.toggleMode @ ToggleMode.js:63
EventBus.js?cd8f:377 Error: A DOM element reference is required
    at new ClassList (index.esm.js:75)
    at classes (index.esm.js:63)
    at HideModelerElements.js:27
    at invokeFunction (EventBus.js?cd8f:509)
    at EventBus._invokeListener (EventBus.js?cd8f:362)
    at EventBus._invokeListeners (EventBus.js?cd8f:348)
    at EventBus.fire (EventBus.js?cd8f:310)
    at ToggleMode../node_modules/bpmn-js-token-simulation/lib/features/toggle-mode/modeler/ToggleMode.js.ToggleMode.toggleMode (ToggleMode.js:63)
EventBus._invokeListener @ EventBus.js?cd8f:377
EventBus._invokeListeners @ EventBus.js?cd8f:348
EventBus.fire @ EventBus.js?cd8f:310
./node_modules/bpmn-js-token-simulation/lib/features/toggle-mode/modeler/ToggleMode.js.ToggleMode.toggleMode @ ToggleMode.js:63
EventBus.js?cd8f:379 Uncaught Error: A DOM element reference is required
    at new ClassList (index.esm.js:75)
    at classes (index.esm.js:63)
    at HideModelerElements.js:27
    at invokeFunction (EventBus.js?cd8f:509)
    at EventBus._invokeListener (EventBus.js?cd8f:362)
    at EventBus._invokeListeners (EventBus.js?cd8f:348)
    at EventBus.fire (EventBus.js?cd8f:310)
    at ToggleMode../node_modules/bpmn-js-token-simulation/lib/features/toggle-mode/modeler/ToggleMode.js.ToggleMode.toggleMode (ToggleMode.js:63)

barmac · 2019-01-28T15:14:33Z

Token Simulation for the Camunda Modeler is fixed in v0.9.0.

barmac · 2019-01-29T11:19:54Z

I just pushed a rewritten test suite for client side plugins implementation. Previously, we would mock the internal methods what made them uncovered during the tests.

client/src/remote/__tests__/PluginsSpec.js

barmac · 2019-01-29T13:03:18Z

These two helpers cause problems at the moment:
https://github.com/camunda/camunda-modeler-plugin-helpers/blob/master/index.js#L76
https://github.com/camunda/camunda-modeler-plugin-helpers/blob/master/index.js#L85

They are not included in any of the tested plugins. However, we documented them in the helpers repository. Thus, they are part of the public API and we can expect incompatibility problems if we remove them. I'm going to add window helpers which will use our plugin protocol instead of the real path.

nikku · 2019-01-30T09:29:22Z

Regarding your previous comment we should consider whether the current public API makes sense.

it was broken anyway for a majority of use-cases
it was only documented recently, targeted for the v3.0.0 release (https://github.com/camunda/camunda-modeler-plugin-helpers/releases/tag/v3.0.0)

Because of that we could consider it safe to remove it.

barmac · 2019-01-30T09:59:51Z

Cool. I agree with you. We should remember, though, to remove it also from the example plugin: https://github.com/camunda/camunda-modeler-plugin-example/blob/master/client/module.js#L11

barmac · 2019-01-30T10:20:41Z

It appeared that unnamed moddle extensions cause a failure of BpmnEditor during construction. I pushed a fix for this.

nikku · 2019-01-30T11:43:24Z

Let's sync tomorrow and wrap up this topic.

barmac · 2019-01-30T11:59:18Z

We also might want to handle somehow errors thrown in menu actions. Now it's just the Electron displaying uncaught exception in the main process. I pushed a WIP commit with partial fix to that.

barmac · 2019-01-30T14:10:16Z

I created a plugin to test the extension points: https://github.com/barmac/camunda-modeler-test-plugin. Tomorrow we can also add it to dev mode of Camunda Modeler.

They will return and use arrays and objects for easier traversing within tests.

Previously, it had to be a function even for menu items which are intended to be always enabled.

Previously, an unnamed moddle extension caused the component to fail during construction.

barmac · 2019-01-31T15:58:59Z

Just found a bug that we sometimes add error label twice. I'm gonna fix it today.

barmac · 2019-01-31T16:10:36Z

@nikku I think it's ready to merge.

Window#getModelerDirectory * removed without replacement Window#getPluginsDirectory * deprecated; users should use app-plugins:// prefixed urls instead

barmac · 2019-02-01T08:34:41Z

I'll fix the Windows path problem.

Sanitize asset paths to prevent directory traversal.

ghost assigned philippfromme Jan 25, 2019

ghost added the needs review Review pending label Jan 25, 2019

philippfromme changed the title ~~Add plugins menu and client plugins~~ WIP Add plugins menu and client plugins Jan 25, 2019

nikku reviewed Jan 25, 2019

View reviewed changes

philippfromme changed the title ~~WIP Add plugins menu and client plugins~~ Add plugins menu and client plugins Jan 25, 2019

nikku reviewed Jan 25, 2019

View reviewed changes

app/lib/plugins.js Show resolved Hide resolved

ghost assigned barmac Jan 25, 2019

philippfromme force-pushed the 1083-plugins branch from 00fa6ec to c60cd66 Compare January 25, 2019 15:41

nikku reviewed Jan 29, 2019

View reviewed changes

client/src/remote/__tests__/PluginsSpec.js Outdated Show resolved Hide resolved

barmac force-pushed the 1083-plugins branch 2 times, most recently from 8078d85 to e5c086f Compare January 29, 2019 14:31

ghost assigned nikku Jan 30, 2019

barmac force-pushed the 1083-plugins branch from 0ea2400 to ead65d9 Compare January 30, 2019 10:19

barmac changed the title ~~Add plugins menu and client plugins~~ WIP Add plugins menu and client plugins Jan 30, 2019

philippfromme added 4 commits January 31, 2019 14:05

chore(app): replace var with let/const

d9e97d5

chore(app): refactor plugins to ES6

3c7afa4

feat(app): add plugins menu

dcde484

feat(client): add client plugins

24ea6d3

barmac and others added 8 commits January 31, 2019 14:05

test(plugins): improve test coverage

1bf5799

test(plugins): ensure backwards compatibility

0ef2518

test(plugins): test all public plugins methods

cf6bac3

feat(plugins): restore global helpers to access plugins directory

53f5e51

test(menu): simplify electron menu mocks

7278847

They will return and use arrays and objects for easier traversing within tests.

feat(plugins): accept non-callable as value for menuItem#enabled

7dfbd73

Previously, it had to be a function even for menu items which are intended to be always enabled.

chore(client/remote): simplify Plugins

39f96c5

fix(bpmn-editor): catch errors for invalid moddle extensions

c3584a2

Previously, an unnamed moddle extension caused the component to fail during construction.

barmac force-pushed the 1083-plugins branch 2 times, most recently from beea666 to 9fdc702 Compare January 31, 2019 14:22

barmac changed the title ~~WIP Add plugins menu and client plugins~~ Add plugins menu and client plugins Jan 31, 2019

barmac added 4 commits January 31, 2019 16:50

test(menu): make sure it builds menu regardless plugin error

7936b33

fix(menu): handle errors thrown in plugin menu actions

46f99bc

test(dmn): reverse mock modeler breaking change

1abda29

test(plugins): cover ghost paths case

774c23a

barmac force-pushed the 1083-plugins branch from 1978e5a to 774c23a Compare January 31, 2019 15:51

fix(menu): properly label plugin with error

91fab75

chore(plugins): indicate global helper deprecation state

e06a588

Window#getModelerDirectory * removed without replacement Window#getPluginsDirectory * deprecated; users should use app-plugins:// prefixed urls instead

barmac force-pushed the 1083-plugins branch 3 times, most recently from e8895e8 to b426e64 Compare February 1, 2019 10:03

chore(plugins): disable directory traversal

3152fdc

Sanitize asset paths to prevent directory traversal.

barmac force-pushed the 1083-plugins branch from b426e64 to 3152fdc Compare February 1, 2019 10:13

barmac merged commit ff57aa7 into master Feb 1, 2019

delete-merged-branch bot deleted the 1083-plugins branch February 1, 2019 11:02

ghost removed the needs review Review pending label Feb 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add plugins menu and client plugins #1143

Add plugins menu and client plugins #1143

philippfromme commented Jan 25, 2019 •

edited by barmac

Loading

nikku left a comment

philippfromme commented Jan 25, 2019

nikku commented Jan 28, 2019 •

edited

Loading

barmac commented Jan 28, 2019

barmac commented Jan 28, 2019

barmac commented Jan 29, 2019

barmac commented Jan 29, 2019

nikku commented Jan 30, 2019

barmac commented Jan 30, 2019

barmac commented Jan 30, 2019 •

edited

Loading

nikku commented Jan 30, 2019

barmac commented Jan 30, 2019

barmac commented Jan 30, 2019 •

edited

Loading

barmac commented Jan 31, 2019

barmac commented Jan 31, 2019

barmac commented Feb 1, 2019

Add plugins menu and client plugins #1143

Add plugins menu and client plugins #1143

Conversation

philippfromme commented Jan 25, 2019 • edited by barmac Loading

nikku left a comment

Choose a reason for hiding this comment

philippfromme commented Jan 25, 2019

nikku commented Jan 28, 2019 • edited Loading

barmac commented Jan 28, 2019

barmac commented Jan 28, 2019

barmac commented Jan 29, 2019

barmac commented Jan 29, 2019

nikku commented Jan 30, 2019

barmac commented Jan 30, 2019

barmac commented Jan 30, 2019 • edited Loading

nikku commented Jan 30, 2019

barmac commented Jan 30, 2019

barmac commented Jan 30, 2019 • edited Loading

barmac commented Jan 31, 2019

barmac commented Jan 31, 2019

barmac commented Feb 1, 2019

philippfromme commented Jan 25, 2019 •

edited by barmac

Loading

nikku commented Jan 28, 2019 •

edited

Loading

barmac commented Jan 30, 2019 •

edited

Loading

barmac commented Jan 30, 2019 •

edited

Loading