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

Integrate st2flow #759

Merged
merged 87 commits into from
Jan 19, 2021
Merged

Integrate st2flow #759

merged 87 commits into from
Jan 19, 2021

Conversation

mickmcgrath13
Copy link
Contributor

@mickmcgrath13 mickmcgrath13 commented May 28, 2020

Closes #758

This just copies the relevant modules from st2flow to st2web and changes a few things to wire it up to work.

It adds a "Workflows" top nav item:
image

It also adds a new route for /action which has the standard ST2 UI Header:
image

It adds an "Edit" link in the details panel of actions that are orquesta or mistral workflows:
image

It adds a "new" link (plus button) to the bottom right of the actions list on the actions page:
image

how to test

git clone [email protected]:StackStorm/st2web.git
cd st2web
git checkout integrate-st2flow

add a file called config.local.js like:

angular.module('main')
  .constant('st2Config', {

    hosts: [
      {
        name: 'Verizon Training',
        url: 'https://st2instance.com/api',
        auth: 'https://st2instance.com/auth',
        stream: 'https://st2instance.com/stream',
      },
    ],

  });

and then run

docker-compose up

Known issues:

eslintrc

see notice/notice in .eslintrc.yml Resolved

Super expression error

(partially resolved.. see comments)

Rebase with master

Master has updated to clean up old and unused dependencies (among other things).

I've attempted this locally, and changes to the files (even with a docker-compose up --build and forcibly pruning docker images) didn't seem to have an effect on the built files.

Update: This is done

CSS

I didn't copy any of the css or modules/st2-style from st2flow to st2web

Some icons are missing. I didn't get a chance to test anything else

Remaining items

  • eslintrc.yml - notice/notice (as described above)
  • Rebase/merge with master
  • Clean up st2flow packages old / unused dependencies
  • CSS (as described above)
  • Testing (create a new workflow, edit a workflow, etc)
  • Migrate the relevant issues from st2flow

@mickmcgrath13
Copy link
Contributor Author

mickmcgrath13 commented May 28, 2020

re Super expression error:

trying to do this

class MetaEditor extends Editor {}

gets this error:

if (typeof superClass !== "function" && superClass !== null) {
  throw new TypeError("Super expression must either be null or a function");
}

superClass is $$typeof: Symbol(react.memo)

StackOverflow says (https://stackoverflow.com/questions/35777991/typeerror-super-expression-must-be-null-or-a-function-not-undefined-with-babel):

Your export doesn't match your import

I've tried both:

export class Editor extends Component
// with
import { Editor } from './editor';

and

export default class Editor extends Component
// with
import Editor from './editor';

(all 4 combinations of export/import, actually. with export class and import Editor or with export default class and import { Editor }, superClass is undefined)

Re redux possibility:

  • st2flow: "react-redux": "^6.0.1",
  • st2web: "react-redux": "7.0.2",

@mickmcgrath13
Copy link
Contributor Author

based on feedback from Bitovi internal, I changed this

// child
@connect(
  ({ ranges, notifications }) => ({ ranges, notifications })
)
export default class Editor extends Component<{
//parent
class MetaEditor extends Editor {}

to this:

// child
const editorConnect = ({ ranges, notifications }) => ({ ranges, notifications });
export { editorConnect };
export default class Editor extends Component<{
// parent
import { editorConnect } from '@stackstorm/st2flow-editor';
@connect(
  editorConnect
)
class MetaEditor extends Editor {}

for better or worse, this helped... will evaluate if this is a proper solution

@mickmcgrath13 mickmcgrath13 changed the title [WIP] initial attempt at integrating st2flow [WIP] Integrate st2flow May 28, 2020
@mickmcgrath13
Copy link
Contributor Author

re

will evaluate if this is a proper solution

It is a serviceable solution. See discussion on Bitovi's community slack: here

_( if you don't have access to Bitovi's community slack, sign up here: https://www.bitovi.com/community/slack) _

modules/st2flow-canvas/collapse-button.js Outdated Show resolved Hide resolved
@arm4b arm4b added this to the 3.3.0 milestone Jul 21, 2020
@punkrokk punkrokk requested a review from bgaeddert July 23, 2020 17:48
@m4dcoder
Copy link
Contributor

@mickmcgrath13, I came across the following issue when trying out this PR. I am not using docker compose to run it. I'm using make build and make install and then run st2web from nginx.

When I edit a workflow or open a blank canvas with the "plus" button, a new tab open but it is blank and I get the following error in the developer tools on Chrome.

main.js:1 Invariant Violation: Could not find "store" in the context of "Connect(Workflows)". Either wrap the root component in a <Provider>, or pass a custom React context provider to <Provider> and the corresponding React context consumer to Connect(Workflows) in connect options.
    at module.exports (https://172.168.50.28/js/main.js:1:2405836)
    at Connect.renderWrappedComponent (https://172.168.50.28/js/main.js:1:674543)
    at Connect.indirectRenderWrappedComponent (https://172.168.50.28/js/main.js:1:674421)
    at Tg (https://172.168.50.28/js/main.js:1:3055949)
    at bi (https://172.168.50.28/js/main.js:1:3077684)
    at ci (https://172.168.50.28/js/main.js:1:3078058)
    at Di (https://172.168.50.28/js/main.js:1:3084899)
    at Yh (https://172.168.50.28/js/main.js:1:3084286)
    at Xh (https://172.168.50.28/js/main.js:1:3083323)
    at qf (https://172.168.50.28/js/main.js:1:3082208)

Copy link
Contributor

@amanda11 amanda11 left a comment

Choose a reason for hiding this comment

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

I found a couple of problems that exists in this version of st2web that didn't exist in EWC (though that may be the same underlying problem). The scenarios are:

Scenario 1 - Loose Save indicator on Re-arrange workflow

  1. Add an action to a saved workflow - as there is unsaved changes the Save icon has a red outline to indicate there are unsaved changes
  2. Hit the re-arrange tasks button. The Save icon looses it's red outline - so you don't know there are unsaved changes.

In EWC then the button would have remained red.

Scenario 2- moving a task doesn't indicate Save is needed

  1. Edit an existing workflow and move the locaiton of a task. The Save indicator doesn't go red to indicate a change has been made. In EWC moving a task would have showed that a change had been made.

Copy link
Contributor

@amanda11 amanda11 left a comment

Choose a reason for hiding this comment

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

Workflow composer looks great, had a good play with editing and creating new workflows. Lot of testing on the Save indicator, and that all looks good.
Excellent work.

Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

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

Shital, This is an awesome job! Thank you for brining this effort to completion.

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Per initial @mickmcgrath13 message, PR added the Workflow tab nav item and I found that brilliant idea as previously the st2flow feature was pretty much hidden from users.

I couldn't find Workflows button in current implementation. Please include that too.

@amanda11
Copy link
Contributor

I believe the workflow tab caused some problems, and so was taken out of this PR a few rounds back. I think the intention was to get equivalent functionality to EWC in and then try and get the workflow tab in separately.

@amanda11
Copy link
Contributor

amanda11 commented Dec 21, 2020

Additional changes required. Fonts are off in the st2flow UI. Zoom icons in the st2flow UI are missing. The workflow icon in the main st2web UI upper nav menu does not open st2flow in new tab whereas the plus sign opens st2flow in a new tab. We will remove the workflow icon until we figure out better UX. The focus is to keep UX same as EWC. Blocking here so this won't be merged for v3.3 until this PR/feature is ready.

@armab This is the comment from a few months ago when the workflow tab was taken out. (I know when it was in there were other problems as well as what was mentioned above).

@arm4b
Copy link
Member

arm4b commented Dec 21, 2020

@amanda11 Thanks!

@m4dcoder Was the issue with the Workflows button in nav menu only about opening Workflow Designer in new window vs current window? If that's the only reason, would be great to keep that change, - doesn't matter new/existing window as it's highly helpful addition for the users.

@amanda11
Copy link
Contributor

amanda11 commented Jan 5, 2021

Had another re-test on latest version - looks good to me.

@blag
Copy link
Contributor

blag commented Jan 12, 2021

From the TSC: We need to update copyright headers in all repo files in this PR.

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Thanks everyone involved in this! 🎉

@m4dcoder m4dcoder merged commit 778cfe3 into master Jan 19, 2021
@m4dcoder m4dcoder deleted the integrate-st2flow branch January 19, 2021 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate st2flow (Workflow Designer) into st2web
5 participants