Fix edge directionality in graph layout

[fbarl]

Fixes #2267.

The problem was that we were not making a difference between source and target nodes when constructing edges for the layout engine (for the purpose of not rendering the same edge twice).

Note: The bidirectional edges are now represented as two directed edges, both in the layout engine and when rendering. So they are actually rendered as two overlapping edges, although that's currently not visible. I intentionally left it like that so that we could make them non-overlapping if we want or have an easier time drawing the arrows later on.

[rade]

The bidirectional edges are now represented as two directed edges, both in the layout engine and when rendering. So they are actually rendered as two overlapping edges, although that's currently not visible. I intentionally left it like that so that we could make them non-overlapping if we want or have an easier time drawing the arrows later on.

Hmm. Not sure about that. I would expect bidirectional edges to be rendered as one edge with two arrowheads.

If it's relatively easy to have one edge for bidirectional edges, let's do that. If not, then what you have is ok.

[rade]

btw, was this always broken, or is it a regression?

[fbarl]

@rade Alright, I modified the code so that it doesn't render both edges, but I'm still sending them both to the layout engine so that the rank assignment would be more accurate. I cleaned up that code and added some comments and unit tests.

I think I broke edge directions in one of my optimizations stories, so I don't think it was there for too long. However, I think we were always ignoring one direction in case of bidirectional edges, which is something this story also fixes. There is a chance this fix will improve stability and accuracy of some rendered graphs slightly.

[rade]

I think I broke edge directions in one of my optimizations stories,

Did that breakage make it into 1.2? If so, we may want to release a 1.2.1 with a fix. cc @2opremio

[fbarl]

@rade Fortunately the breakage didn't make it into any of the releases as it was introduced on master only 2 days ago when merging #2221 (line 19 in nodes-chart-layout.js).

[jpellizzari]

LGTM. Nice to have tests for this!