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

Incorrect Node Argument Order in addEdge Function #68

Open
etesami opened this issue Aug 25, 2024 · 1 comment
Open

Incorrect Node Argument Order in addEdge Function #68

etesami opened this issue Aug 25, 2024 · 1 comment

Comments

@etesami
Copy link
Contributor

etesami commented Aug 25, 2024

In the getGraphDataFunction, the addEdge function is called with the node arguments in the wrong order. Typically, an edge is drawn from a claim to its composition and composite resources. This means the source should be the claim, and the target should be the composition or composite resources. However, the function is currently being called with the composition and composite resources as the source and the claim as the target, which is incorrect.

function graphDataFromClaim(claim: ClaimExtended, navigate: NavigateFunction): GraphData {
const graphData = new GraphData()
const claimId = graphData.addNode(NodeTypes.Claim, claim, true, navigate)
const compId = graphData.addNode(NodeTypes.Composition, claim.composition, false, navigate);
graphData.addEdge(compId, claimId)
const xrId = graphData.addNode(NodeTypes.CompositeResource, claim.compositeResource, false, navigate);
graphData.addEdge(xrId, claimId)
// TODO: check that composite resource points to the same composition and draw line between them
claim.managedResources?.map(res => {
const resId = graphData.addNode(NodeTypes.ManagedResource, res, false, navigate);
graphData.addEdge(resId, xrId)
})
return graphData;
}

Impact: This incorrect ordering has caused confusion in other parts of the code. For example, it has led to settings being configured incorrectly, such as using markerStart instead of markerEnd here:

edge.markerStart = marker

Additionally, this error results in calling getLayoutElement with RL arguments, even though the graph is supposed to be displayed with LR (left-to-right) settings:

const {nodes: layoutedNodes, edges: layoutedEdges} = getLayoutedElements(
initialNodes,
initialEdges,
"RL"
);

I have a proposed solution for this issue. Please let me know if this understanding is correct and desirable.

@undera
Copy link
Collaborator

undera commented Aug 26, 2024

It's a bit hard to understand without illustration. Can you post here a screenshot of diagram that you believe is incorrect?

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

No branches or pull requests

2 participants