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.

Sign up for GitHub

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

Jump to bottom

[BAPL-1336] PIM UI usability and stability #1869

Merged
merged 19 commits into from Aug 16, 2019
Merged

[BAPL-1336] PIM UI usability and stability #1869

merged 19 commits into from Aug 16, 2019

Conversation

ruromero
Copy link
Member

@ruromero ruromero commented Jul 18, 2019 •
edited

Fix https://issues.jboss.org/browse/JBPM-8474
Fix https://issues.jboss.org/browse/JBPM-8542
Fix https://issues.jboss.org/browse/JBPM-8508
Fix https://issues.jboss.org/browse/JBPM-8473
Fix https://issues.jboss.org/browse/JBPM-8663
Fix https://issues.jboss.org/browse/JBPM-8471
Fix https://issues.jboss.org/browse/JBPM-8472

Summary of changes:

JBPM-8474 process nodes mapping screen issues

  • Mappings are shown as tags using the name instead of the UID
  • Mappings can be removed using the (x) icon in the tag
  • Nodes can be only mapped once and must be of the same type

JBPM-8542 Migration Plan import does not work

  • Fixed migration plan import
  • Errors are shown to the user in case the import fails
  • Import button is not enabled until a valid JSON is added
  • Plans are refreshed after the import is successful

JBPM-8508 node mapping not loaded when editing Migration Plan

  • Definitions are loaded when the process selection step is displayed and the loaded map has a valid source/target process definition selected

JBPM-8473 improve container and process selection

  • Available containers and processes are loaded from the server and presented as a dependent dropdown
  • Definitions are loaded upon process selection to avoid the "Load definition" button
  • Validation messages have been added to ensure source/target are different
  • Errors are displayed as notifications if the definitions can't be loaded

JBPM-8663 Validation following PF3 style

  • Plan name must not be null
  • CallbackURL / scheduledStartTime must be valid but are optional
  • Required fields have a red asterisk after the label
  • Validation messages are displayed below the input field
  • Next button is disabled until the current step is valid

JBPM-8471 only basic css theme

  • Added Patternfly3 header
  • Set page title
  • Added some minimal css styles
  • Fixed migration filter validation to allow retrieve all
  • Align plans and migrations tables styles

JBPM-8472 horizontal scrollbar in basic view

  • Modified CSS

Other fixes

  • Refactor REST client to simplify communication with the server
  • Replace HTML with React components in many places (not all)
  • RunningInstances.status is converted into a literal
  • Move from sourceProcessId and sourceContainerId to source.processId and source.containerId
  • Enhanced development experience by mocking the JS server
  • Clean up npm dependencies and imported resources (PF fonts/css)
  • Add PropTypes for component validation in many places (not all)

@ruromero ruromero changed the title Import [BAPL-1336] PIM UI usability and stability Jul 18, 2019
@ruromero ruromero mentioned this pull request Jul 18, 2019
Closed
@jstastny-cz
Copy link

jstastny-cz commented Jul 25, 2019 •
edited

@jstastny-cz
Copy link

JBPM-8663 - not allowing localhost as callback url, is that intended?

jstastny-cz
jstastny-cz requested changes Jul 25, 2019
View reviewed changes
Copy link

@jstastny-cz jstastny-cz left a comment

Choose a reason for hiding this comment

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

A few comments inline.

Java classes probably do not follow our code style standards - indent width should be 4 spaces.

process-migration-service/frontend/src/component/tabMigration/MigrationDefinitions.js Outdated Show resolved Hide resolved
process-migration-service/frontend/src/component/tabMigrationPlan/MigrationPlansBase.js Outdated Show resolved Hide resolved
process-migration-service/frontend/src/component/tabMigrationPlan/MigrationPlansTable.js Outdated Show resolved Hide resolved
...ice/frontend/src/component/tabMigrationPlan/wizardExecuteMigration/WizardExecuteMigration.js Outdated Show resolved Hide resolved
@ruromero
Copy link
Member Author

ruromero commented Aug 6, 2019

JBPM-8663 - not allowing localhost as callback url, is that intended?

Fixed

@ruromero
Copy link
Member Author

ruromero commented Aug 6, 2019

Added link to https://issues.jboss.org/browse/JBPM-8472

MarianMacik
MarianMacik reviewed Aug 6, 2019
View reviewed changes
Copy link
Member

@MarianMacik MarianMacik left a comment

Choose a reason for hiding this comment

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

Mostly minor comments. Otherwise looks good. Very good refactoring, I like how clean the frontend REST clients are now. And BTW thanks for creating multiple JS tests inside one JS test classes, I think this is the way to go when writing JS tests.

process-migration-service/frontend/mock_data/migration.json Outdated Show resolved Hide resolved
process-migration-service/frontend/src/__tests__/tabMigrationPlan/PagePlanName.test.js Outdated Show resolved Hide resolved
process-migration-service/src/test/resources/META-INF/persistence.xml Outdated Show resolved Hide resolved
@ruromero ruromero force-pushed the import branch 3 times, most recently from e4ca62f to dca85cf Compare August 9, 2019 10:54
@ruromero
[JBPM-8501] Allow reloading KIE Server clients
ed60238 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[JBPM-8492] Allow to map any type of task
8aada6c 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[JBPM-8473] Use dropdown for Container/Process selection
99ea42c 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
the cleaning lady got crazy
6b42af1 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
Introduce ProcessRef and format all java code
8a593db 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Enhance usability and stability
9891e1c 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Fix unit tests
d979a2a 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[JBPM-8471] Add header and some minimal css
215768e 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Fix formatting
1170b53 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Fix deletePlan and allow localhost
7c32239 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Use server object when edit migration
31712a0 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Use processInstanceId
cb65703 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] small fixes
16896c5 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] missing updated snapshots
bfb2a29 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Use JTA with filtered resources
d4ed2da 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Revert to HtmlWebpackPluginConfig
54ef4a2 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Clean up
f47e198 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Don't drop db after tests
707a980 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
@ruromero
[BAPL-1336] Exclude transitive dep and remove unused code in tests
b2551d0 
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
MarianMacik
MarianMacik approved these changes Aug 14, 2019
View reviewed changes
process-migration-service/pom.xml
<exclusions>
<exclusion>
<artifactId>narayana-jts-idlj</artifactId>
<groupId>org.jboss.narayana.jts</groupId>
Copy link
Member

Choose a reason for hiding this comment

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

I checked with guys from Thorntail. This is not needed at all, so the exclusion is fine.

@MarianMacik
Copy link
Member

jenkins retest this

@MarianMacik MarianMacik merged commit cf5a12d into kiegroup:master Aug 16, 2019
@ruromero ruromero deleted the import branch August 16, 2019 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarianMacik MarianMacik MarianMacik approved these changes

@jstastny-cz jstastny-cz jstastny-cz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@ruromero @jstastny-cz @MarianMacik