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

NO-ISSUE: DMN Editor: Unable to "unhide" a hidden Decision Service Node #2371

Merged
merged 26 commits into from
Jun 3, 2024

Conversation

kbowers-ibm
Copy link
Contributor

Temporarily disabling the ability to hide a decision service in the case where there isn't any other depiction of that Decision Service in another DRD.

@kbowers-ibm kbowers-ibm added the pr: DO NOT MERGE Draft PR, not ready for merging label May 24, 2024
@kbowers-ibm kbowers-ibm marked this pull request as ready for review May 28, 2024 10:03
@kbowers-ibm kbowers-ibm added pr: waiting-for-review Waiting for peer reviews and removed pr: DO NOT MERGE Draft PR, not ready for merging labels May 28, 2024
Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

I think this PR works, just not sure why it is needed only for the Decision Service node, to keep at least one Decision Service depiction visible, while for other node types we do not enforce this rule.

Also found one interesting scenario for the decision service node when we compare using single file vs included model.

Single model

  • Create new dmn file
  • add Decision Service node to the default DRD
  • add new DRD
  • add the Decision Service node also to the new DRD
  • So we have One Decision Service node depiction in two DRDs
  • Switch to the default DRD and hide, using X key, the decision service, we will have this state [1]

Included models

  • Create new dmn file
  • add Decision Service node to it
  • create another dmn file in the same workspace and include the file from the steps above
  • now in the default DRD add to the diagram the included decision service node
  • add new DRD to the diagram
  • add the included Decision Service node also to the new DRD
  • So we have one included Decision Service node depiction in two DRDs
  • Switch to the default DRD and hide, using X key, the decision service, we will have this state [2]

[1]
Screenshot 2024-05-28 170021

[2]
Screenshot 2024-05-28 170052

I think it is strange the states [1] and [2] are different, as the state of the models is basically the same. Or maybe it is only my inapprop[riate understanding?

@kbowers-ibm
Copy link
Contributor Author

@jomarko

just not sure why it is needed only for the Decision Service node, to keep at least one Decision Service depiction visible, while for other node types we do not enforce this rule.

This is a temporary workaround, as the logic currently isn't implemented to unhide a hidden decision service, when there's no other depiction of that decision service anywhere. This resulted in being able to hide a decision service, but not being able to unhide it, so this seemed to be the better temporary solution until all Decision Service logic is fully implemented

@kbowers-ibm
Copy link
Contributor Author

Not sure if I'm missing something about your scenario, but as far as I can tell that's the way all types of nodes work. The node will be present in the external nodes panel, and not in the drg elements panel

@tiagobento tiagobento changed the title NO-ISSUE: Unable to "unhide" a hidden Decision Service Node NO-ISSUE: DMN Editor: Unable to "unhide" a hidden Decision Service Node May 28, 2024
@ljmotta ljmotta added pr: DO NOT MERGE Draft PR, not ready for merging and removed pr: waiting-for-review Waiting for peer reviews labels May 29, 2024
@jomarko
Copy link
Contributor

jomarko commented May 29, 2024

I see new label, 'DO NOT MERGE', please let me know once this is ready for a review.

@ljmotta ljmotta added pr: waiting-for-review Waiting for peer reviews and removed pr: DO NOT MERGE Draft PR, not ready for merging labels May 31, 2024
@ljmotta ljmotta added the pr: DO NOT MERGE Draft PR, not ready for merging label Jun 3, 2024
@ljmotta
Copy link
Contributor

ljmotta commented Jun 3, 2024

Still missing a case where the external mode doesn't have a depict of a Decision Service. The Decision Service should be auto generated

@tiagobento tiagobento removed pr: DO NOT MERGE Draft PR, not ready for merging pr: waiting-for-review Waiting for peer reviews labels Jun 3, 2024
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.

7 participants