Skip to content

Conversation

@janniclas
Copy link
Contributor

This pull request contains the functionality to drag and drop edges from existing components to reconnect them to another component.
Also multiple bug fixes and stability improvements of the graph module have been implemented

closes: #108 and #85

@ghost ghost assigned janniclas Feb 14, 2019
@ghost ghost added the review label Feb 14, 2019
@codecov-io
Copy link

codecov-io commented Feb 14, 2019

Codecov Report

Merging #115 into develop will decrease coverage by 0.17%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop    #115      +/-   ##
==========================================
- Coverage     4.52%   4.34%   -0.18%     
==========================================
  Files           12      12              
  Lines          221     230       +9     
  Branches        17      16       -1     
==========================================
  Hits            10      10              
- Misses         211     220       +9
Impacted Files Coverage Δ
app/controllers/ApiRouter.scala 0% <0%> (ø) ⬆️
app/controllers/InstanceRegistryController.scala 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 628f83e...51e94cc. Read the comment docs.

Copy link
Contributor

@johannesduesing johannesduesing left a comment

Choose a reason for hiding this comment

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

As requested i did a functional test on this code, which already worked pretty well when i tested it with a rather big setup on the deployment server on Thursday. This time i used a smaller setup with 2 APIs and one WebApp.
It's working fine for me, re-assigning, coloring and hiding components and edges is working as expected. The only issue i can see is that you are currently able to re-assign a component to the same instance that it is already using, which is actually triggering a re-assignment call to the registry. I would expect to either hide the currently used dependency or just give the user a message that re-assignment is not needed because the desired dependency is already established.
Since i remember talking with @janniclas about this i'm not sure if that's intended behaviour for this PR, so i will still approve it in the current state.

@janniclas janniclas merged commit 5b3db88 into develop Feb 18, 2019
@ghost ghost removed the review label Feb 18, 2019
@janniclas janniclas deleted the feature/dragAndDropEdges branch February 18, 2019 07:56
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.

4 participants