mike-goodwin / owasp-threat-dragon-core

OWASP Threat Dragon core files
Apache License 2.0
11 stars 16 forks source link

Create boundary name #83

Closed andk123 closed 4 years ago

andk123 commented 4 years ago

78

Boundaries are now created with a label to be able to identify different boundaries similarly to a flow. It should not affect previous boundaries created (without a name) and they can still be saved without a name, or a name can be added if needed.

andk123 commented 4 years ago

Will solve code refactoring.

jgadsden commented 4 years ago

Tested with the both old and new json model files, and also tested existing app with both old and new json model files, to ensure compatibility between releases and data model. All good!

Sorry @andk123 - somehow commented and also by mistake closed this request at the same time. Oops, finger trouble, have reopened it.

andk123 commented 4 years ago
  1. Okay I see. I was working mostly on the web app. The desktop app seems to behave a bit differently since the settings button does not appear (See picture taken in the web app). image
  2. Now that I think of it, that is an excellent idea.

I will fix/implement both changes and create a new pull request @jgadsden. Thanks!

andk123 commented 4 years ago

@jgadsden The two issues should be solved now. Let me know what you think. Arnold

andk123 commented 4 years ago

Everything is working now.

jgadsden commented 4 years ago

I can merge this pull request, but as an aside the code climate issue could be solved (if I understand correctly) by substituting :

            if (newElement.attributes.type == "tm.Flow") {    
                newElement.attributes.source = {'x' : 30, 'y' : 20};
                newElement.attributes.target = {'x' : 110, 'y' : 100};
                newElement.attributes.labels[0].attrs.text.text = label;
                delete newElement.attributes.vertices;
            }
            else if (newElement.attributes.type == "tm.Boundary") {    
                newElement.attributes.source = {'x' : 30, 'y' : 20};
                newElement.attributes.target = {'x' : 110, 'y' : 100};
                if (label != ""){
                    newElement.attributes.labels[0].attrs.text.text = label;
                }
                delete newElement.attributes.vertices;
            }

with

            if (newElement.attributes.type == "tm.Flow" || newElement.attributes.type == "tm.Boundary") {    
                newElement.attributes.source = {'x' : 30, 'y' : 20};
                newElement.attributes.target = {'x' : 110, 'y' : 100};
                if (label != ""){
                    newElement.attributes.labels[0].attrs.text.text = label;
                }
                delete newElement.attributes.vertices;
            }

as label is not empty for Flow. This brings the function lines below 25. Shall I merge this PR as is?

andk123 commented 4 years ago

I can merge this pull request, but as an aside the code climate issue could be solved (if I understand correctly) by substituting :

            if (newElement.attributes.type == "tm.Flow") {    
                newElement.attributes.source = {'x' : 30, 'y' : 20};
                newElement.attributes.target = {'x' : 110, 'y' : 100};
                newElement.attributes.labels[0].attrs.text.text = label;
                delete newElement.attributes.vertices;
            }
            else if (newElement.attributes.type == "tm.Boundary") {    
                newElement.attributes.source = {'x' : 30, 'y' : 20};
                newElement.attributes.target = {'x' : 110, 'y' : 100};
                if (label != ""){
                    newElement.attributes.labels[0].attrs.text.text = label;
                }
                delete newElement.attributes.vertices;
            }

with

            if (newElement.attributes.type == "tm.Flow" || newElement.attributes.type == "tm.Boundary") {    
                newElement.attributes.source = {'x' : 30, 'y' : 20};
                newElement.attributes.target = {'x' : 110, 'y' : 100};
                if (label != ""){
                    newElement.attributes.labels[0].attrs.text.text = label;
                }
                delete newElement.attributes.vertices;
            }

as label is not empty for Flow. This brings the function lines below 25. Shall I merge this PR as is?

The 2nd method is good. Initially, I thought I would need to modify the boundary attributes, but I ended up keeping them as is. Code updated!