Merge pull request #2342 from weaveworks/2341-bidrectional-bug

Fix for inaccurate bidrectional edges on edge hover

client/app/scripts/charts/__tests__/node-layout-test.js

@@ -1,5 +1,7 @@
 import { fromJS, Map } from 'immutable';
 
+import { constructEdgeId as edge } from '../../utils/layouter-utils';
+
 const makeMap = Map;
 
 describe('NodesLayout', () => {
@@ -30,9 +32,9 @@ describe('NodesLayout', () => {
         n4: {id: 'n4'}
       }),
       edges: fromJS({
-        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
-        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'},
-        'n2-n4': {id: 'n2-n4', source: 'n2', target: 'n4'}
+        [edge('n1', 'n3')]: {id: edge('n1', 'n3'), source: 'n1', target: 'n3'},
+        [edge('n1', 'n4')]: {id: edge('n1', 'n4'), source: 'n1', target: 'n4'},
+        [edge('n2', 'n4')]: {id: edge('n2', 'n4'), source: 'n2', target: 'n4'}
       })
     },
     rank4: {
@@ -43,9 +45,9 @@ describe('NodesLayout', () => {
         n4: {id: 'n4', rank: 'B'}
       }),
       edges: fromJS({
-        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
-        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'},
-        'n2-n4': {id: 'n2-n4', source: 'n2', target: 'n4'}
+        [edge('n1', 'n3')]: {id: edge('n1', 'n3'), source: 'n1', target: 'n3'},
+        [edge('n1', 'n4')]: {id: edge('n1', 'n4'), source: 'n1', target: 'n4'},
+        [edge('n2', 'n4')]: {id: edge('n2', 'n4'), source: 'n2', target: 'n4'}
       })
     },
     rank6: {
@@ -58,11 +60,11 @@ describe('NodesLayout', () => {
         n6: {id: 'n6', rank: 'B'},
       }),
       edges: fromJS({
-        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
-        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'},
-        'n1-n5': {id: 'n1-n5', source: 'n1', target: 'n5'},
-        'n2-n4': {id: 'n2-n4', source: 'n2', target: 'n4'},
-        'n2-n6': {id: 'n2-n6', source: 'n2', target: 'n6'},
+        [edge('n1', 'n3')]: {id: edge('n1', 'n3'), source: 'n1', target: 'n3'},
+        [edge('n1', 'n4')]: {id: edge('n1', 'n4'), source: 'n1', target: 'n4'},
+        [edge('n1', 'n5')]: {id: edge('n1', 'n5'), source: 'n1', target: 'n5'},
+        [edge('n2', 'n4')]: {id: edge('n2', 'n4'), source: 'n2', target: 'n4'},
+        [edge('n2', 'n6')]: {id: edge('n2', 'n6'), source: 'n2', target: 'n6'},
       })
     },
     removeEdge24: {
@@ -73,8 +75,8 @@ describe('NodesLayout', () => {
         n4: {id: 'n4'}
       }),
       edges: fromJS({
-        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
-        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+        [edge('n1', 'n3')]: {id: edge('n1', 'n3'), source: 'n1', target: 'n3'},
+        [edge('n1', 'n4')]: {id: edge('n1', 'n4'), source: 'n1', target: 'n4'}
       })
     },
     removeNode2: {
@@ -84,8 +86,8 @@ describe('NodesLayout', () => {
         n4: {id: 'n4'}
       }),
       edges: fromJS({
-        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
-        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+        [edge('n1', 'n3')]: {id: edge('n1', 'n3'), source: 'n1', target: 'n3'},
+        [edge('n1', 'n4')]: {id: edge('n1', 'n4'), source: 'n1', target: 'n4'}
       })
     },
     removeNode23: {
@@ -94,7 +96,7 @@ describe('NodesLayout', () => {
         n4: {id: 'n4'}
       }),
       edges: fromJS({
-        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+        [edge('n1', 'n4')]: {id: edge('n1', 'n4'), source: 'n1', target: 'n4'}
       })
     },
     single3: {
@@ -114,7 +116,7 @@ describe('NodesLayout', () => {
         n5: {id: 'n5'}
       }),
       edges: fromJS({
-        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+        [edge('n1', 'n4')]: {id: edge('n1', 'n4'), source: 'n1', target: 'n4'}
       })
     },
     singlePortrait6: {
@@ -127,7 +129,7 @@ describe('NodesLayout', () => {
         n6: {id: 'n6'}
       }),
       edges: fromJS({
-        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+        [edge('n1', 'n4')]: {id: edge('n1', 'n4'), source: 'n1', target: 'n4'}
       })
     }
   };

client/app/scripts/constants/naming.js

@@ -1,2 +1,2 @@
 
-export const EDGE_ID_SEPARATOR = '-';
+export const EDGE_ID_SEPARATOR = '---';

client/app/scripts/reducers/__tests__/root-test.js

@@ -1,5 +1,6 @@
 import {is, fromJS} from 'immutable';
 // Root reducer test suite using Jasmine matchers
+import { constructEdgeId } from '../../utils/layouter-utils';
 
 describe('RootReducer', () => {
   const ActionTypes = require('../../constants/action-types').default;
@@ -540,4 +541,15 @@ describe('RootReducer', () => {
     expect(nextState.get('selectedNodeId')).toBeFalsy();
     expect(nextState.getIn(['nodeDetails', 'n1'])).toBeFalsy();
   });
+  it('highlights bidirectional edges', () => {
+    const action = {
+      type: ActionTypes.ENTER_EDGE,
+      edgeId: constructEdgeId('abc123', 'def456')
+    };
+    const nextState = reducer(initialState, action);
+    expect(nextState.get('highlightedEdgeIds').toJS()).toEqual([
+      constructEdgeId('abc123', 'def456'),
+      constructEdgeId('def456', 'abc123')
+    ]);
+  });
 });

client/app/scripts/reducers/root.js

@@ -421,7 +421,10 @@ export function rootReducer(state = initialState, action) {
       // highlight edge
       state = state.update('highlightedEdgeIds', (highlightedEdgeIds) => {
         highlightedEdgeIds = highlightedEdgeIds.clear();
-        return highlightedEdgeIds.add(action.edgeId);
+        highlightedEdgeIds = highlightedEdgeIds.add(action.edgeId);
+        const opposite = action.edgeId.split(EDGE_ID_SEPARATOR).reverse().join(EDGE_ID_SEPARATOR);
+        highlightedEdgeIds = highlightedEdgeIds.add(opposite);
+        return highlightedEdgeIds;
       });
 
       return state;

client/app/scripts/utils/__tests__/layouter-utils-test.js

@@ -2,6 +2,7 @@ import { fromJS } from 'immutable';
 
 import {
   initEdgesFromNodes,
+  constructEdgeId as edge
 } from '../layouter-utils';
 
 
@@ -14,10 +15,10 @@ describe('LayouterUtils', () => {
         c: {}
       });
       expect(initEdgesFromNodes(input).toJS()).toEqual({
-        'a-b': { id: 'a-b', source: 'a', target: 'b', value: 1 },
-        'a-c': { id: 'a-c', source: 'a', target: 'c', value: 1 },
-        'b-a': { id: 'b-a', source: 'b', target: 'a', value: 1 },
-        'b-b': { id: 'b-b', source: 'b', target: 'b', value: 1 },
+        [edge('a', 'b')]: { id: edge('a', 'b'), source: 'a', target: 'b', value: 1 },
+        [edge('a', 'c')]: { id: edge('a', 'c'), source: 'a', target: 'c', value: 1 },
+        [edge('b', 'a')]: { id: edge('b', 'a'), source: 'b', target: 'a', value: 1 },
+        [edge('b', 'b')]: { id: edge('b', 'b'), source: 'b', target: 'b', value: 1 },
       });
     });
   });

client/app/scripts/utils/layouter-utils.js

@@ -3,7 +3,7 @@ import { Map as makeMap } from 'immutable';
 import { EDGE_ID_SEPARATOR } from '../constants/naming';
 
 
-function constructEdgeId(source, target) {
+export function constructEdgeId(source, target) {
   return [source, target].join(EDGE_ID_SEPARATOR);
 }