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

feature dockerize & deployment plugin & attachment button #69

Merged
merged 45 commits into from
Oct 23, 2023

Conversation

KuhnMn
Copy link
Contributor

@KuhnMn KuhnMn commented Jun 22, 2023

This PR includes the extraction of the deployment functionality into its own plugin, an extended Dockerfile, as well as an initial button and window for attaching deployment artifacts to service tasks.

wiomoc and others added 12 commits June 12, 2023 10:30
* Add Issue Templates for Features

Add feature issue templates similar to the ones of the qunicorn repository but adjusted to the style of the workflow modeler.
* add xml viewer

* remove ace

* add border to resize editor

* enable live update on diagram change

* fix tests

* increase initial size of xml viewer & limit max height

* change size of modeler when xml viewer is enabled

* remove jump after first resize

* remove button from bottom left, add button to toolbar

* add icon
* Add deployment model visualization

Ref: #9

* Add deployment model visualization

Ref: #9

* Orientate at top-node

* refactor stroke style

* add test

* add test

* add test

* fix top level node detection behavior and error handling

* Fix crash on hide

---------

Co-authored-by: Maximilian Kuhn <maximilian.kuhn@ymail.com>
@LaviniaStiliadou
Copy link
Contributor

LaviniaStiliadou commented Jul 4, 2023

There are some issues regarding this PR:

  • I get the following error message when clicking on the "Artifact Wizard" Button multiple times:
    Warning: You are calling ReactDOMClient.createRoot() on a container that has already been passed to createRoot() before. Instead, call root.render() on the existing root instead if you want to update it.

    Possible solution:

    • remove wizardDiv from the QuantumWorkflowModeler
    • create the wizardDiv inside ArtifactUpload and check if the wizardDiv exist before the createRoot. If yes -> remove, else create.
  • Also rename the wizardDiv to something meaningful. Imagine adding new modals to the properties panel then no one would understand what wizardDiv is

  • Add a hover effect for the Artifact Wizard Button

  • Change the layout for Artifact Wizard Button so it looks similar to the other buttons. For example the border is too thick, the current font size is 15px but we use 13px in the properties panel

  • Rename Artifact Wizard to Upload Artifact

  • Rename OpenTosca to OpenTOSCA

  • Rename Show deployment to Show Deployment

  • Rename Hide deployment to Hide Deployment

  • The icon for OpenTOSCA is misleading. I suggest using the following icon

  • The icons for the first subbuttons of the OpenTOSCA Plugin are identical. I suggest using the following icons: show, hide

  • Also, I am missing some docs. Regarding the new functionality show/hide deployment, the new icon ...

@LaviniaStiliadou
Copy link
Contributor

  • Remove wizard also from the code (ArtifactWizardModal -> ArtifactWizard)
  • QuantumWorkflowModeler: modal-container is not really a meaningful name, imagine having multiple modals
  • separate the styling of the OpenTOSCA Button and the Service Deployment Icon, only the plugin should have the new icon as suggested in my previous comment
  • screenshot is not up-to-date
  • add a title to the upload artifact button to be consistent with the other buttons
  • take a look at your code documentation, in the ArtifactWizardModal you have the comments of the ConfigModal

# Conflicts:
#	components/bpmn-q/public/index.html
@wiomoc
Copy link
Contributor

wiomoc commented Jul 13, 2023

QuantumWorkflowModeler: modal-container is not really a meaningful name, imagine having multiple modals

Using the createRoot technic, they all can use the same container

wiomoc and others added 5 commits July 13, 2023 16:00
Also fixed some typos in the code comments
* Implement ArtifactType Dropdown

* Implement artifact upload

* Add Creation of Service Template

Add functionality which creates the service template, which includes a node of a fitting node type for the selected artifact.

* Auto Update SCAR

* Add loading notification and change CSAR naming

* fix artifact upload

---------

Co-authored-by: Maximilian Kuhn <maximilian.kuhn@ymail.com>
Co-authored-by: Christoph Walcher <christoph-wa@gmx.de>
@LaviniaStiliadou
Copy link
Contributor

  • Remove ArtifactWizardModal
  • I get the following error message:
    Warning: Each child in a list should have a unique "key" prop. Check the render method of `ArtifactUploadModal`.
  • When updating the wineryEndpoint, I have to switch the selected element so the wineryEndpoint is also updated for the ArtifactUpload, fix this
  • Rename your OpenTosca js files to OpenTOSCA to stay consistent with the other plugins
  • customPropertiesProvider in the index.js file is not a meaningful name
  • Why do we still have wizard definitions? Take a look at your artifact-modal.css file
  • Updates in the topology model are not visible when triggering show
  • restrict the movement of boundary events so they do not overlap with the new icon
  • Currently, it is not possible to update the ServiceTemplate when uploading a new artifact
  • Do we really need the !== 'false' check in the PluginHandler?

wiomoc and others added 6 commits July 24, 2023 10:40
Signed-off-by: Christoph Walcher <st180462@stud.uni-stuttgart.de>
Signed-off-by: Maximilian Kuhn <maximilian.kuhn@ymail.com>
Signed-off-by: Furkan Lokman <furkan.lokman@hotmail.com>
Signed-off-by: Maximilian Kuhn <maximilian.kuhn@ymail.com>
# Conflicts:
#	components/bpmn-q/modeler-component/editor/plugin/PluginHandler.js
#	components/bpmn-q/modeler-component/extensions/opentosca/configTabs/OpenTOSCATab.js
#	components/bpmn-q/modeler-component/extensions/quantme/QuantMEPlugin.js
#	components/bpmn-q/modeler-component/extensions/quantme/framework-config/config.js
#	components/bpmn-q/package-lock.json
#	components/bpmn-q/test/tests/editor/plugin.spec.js
#	components/bpmn-q/webpack.config.js
Copy link
Contributor

@LaviniaStiliadou LaviniaStiliadou left a comment

Choose a reason for hiding this comment

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

Summary:

  • Change the license year to 2023
  • Look again at the naming of OpenTOSCA
  • Add the option to update the service template when uploading a new file
  • Fix test

.vscode/settings.json Outdated Show resolved Hide resolved
.github/workflows/docker-publish.yaml Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
README.md Show resolved Hide resolved
components/bpmn-q/webpack.config.js Show resolved Hide resolved
Copy link
Contributor

@LaviniaStiliadou LaviniaStiliadou left a comment

Choose a reason for hiding this comment

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

  • Upload von WAR-Java17 and WAR-Java8 always say that the service template creation failed. The service template exists but looks wrong
  • Update of service template is not working correctly when doing this: Upload first PythonArchiveArtifact then WAR --> two top node, same for WAR then PythonArchiveArtifact

@wederbn wederbn merged commit 28dedbc into PlanQK:master Oct 23, 2023
1 check passed
@wederbn wederbn deleted the dev-main branch October 23, 2023 08:42
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.

5 participants