Skip to content

Commit 3f3a543

Browse files
artemmufazalovartemmufazalov
authored
fix(Versions): properly include nodes with defined roles in other group (#2327)
1 parent 9a13999 commit 3f3a543

File tree

2 files changed

+243
-6
lines changed

2 files changed

+243
-6
lines changed

‎src/containers/Versions/groupNodes.test.ts

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
import type {NodesPreparedEntity} from '../../store/reducers/nodes/types';
2+
3+
import {getGroupedStorageNodes, getGroupedTenantNodes, getOtherNodes} from './groupNodes';
4+
import {GroupByValue} from './types';
5+
6+
// Only needed for tests values are present
7+
const nodes: NodesPreparedEntity[] = [
8+
{
9+
// Storage node with many roles
10+
NodeId: 1,
11+
Version: '25-1-1',
12+
Roles: ['StateStorageBoard', 'SchemeBoard', 'Bootstrapper', 'StateStorage', 'Storage'],
13+
},
14+
{
15+
// Storage node
16+
NodeId: 2,
17+
Version: '25-1-2',
18+
Roles: ['Storage'],
19+
},
20+
{
21+
// Storage node
22+
NodeId: 3,
23+
Version: '25-1-3',
24+
Roles: ['Storage'],
25+
},
26+
{
27+
// Tenant node
28+
NodeId: 4,
29+
Version: '25-1-1',
30+
Roles: ['Tenant'],
31+
Tenants: ['/Root/db1'],
32+
},
33+
{
34+
// Tenant node
35+
NodeId: 5,
36+
Version: '25-1-2',
37+
Roles: ['Tenant'],
38+
Tenants: ['/Root/db2'],
39+
},
40+
{
41+
// Tenant node without Roles
42+
NodeId: 6,
43+
Version: '25-1-3',
44+
Tenants: ['/Root/db3'],
45+
},
46+
{
47+
// Node with some other role
48+
NodeId: 7,
49+
Version: '25-1-1',
50+
Roles: ['Bootstrapper'],
51+
},
52+
{
53+
// Node without roles
54+
NodeId: 8,
55+
Version: '25-1-1',
56+
Roles: [],
57+
},
58+
];
59+
60+
describe('getGroupedTenantNodes', () => {
61+
test('should return undefined when nodes is undefined', () => {
62+
const result = getGroupedTenantNodes(undefined, undefined, GroupByValue.VERSION);
63+
expect(result).toBeUndefined();
64+
});
65+
66+
test('should return undefined when nodes is empty', () => {
67+
const result = getGroupedTenantNodes([], undefined, GroupByValue.VERSION);
68+
expect(result).toBeUndefined();
69+
});
70+
71+
test('should group tenant nodes by version when groupByValue is VERSION', () => {
72+
const versionToColor = new Map([
73+
['25-1-1', 'red'],
74+
['25-1-2', 'blue'],
75+
['25-1-3', 'green'],
76+
]);
77+
78+
const result = getGroupedTenantNodes(nodes, versionToColor, GroupByValue.VERSION);
79+
80+
expect(result).toHaveLength(3);
81+
82+
// Check first version group (25-1-1)
83+
expect(result?.[0].title).toBe('25-1-1');
84+
expect(result?.[0].versionColor).toBe('red');
85+
expect(result?.[0].items).toHaveLength(1);
86+
expect(result?.[0].items?.[0].title).toBe('/Root/db1');
87+
expect(result?.[0].items?.[0].nodes).toHaveLength(1);
88+
expect(result?.[0].items?.[0].nodes?.[0].NodeId).toBe(4);
89+
90+
// Check second version group (25-1-2)
91+
expect(result?.[1].title).toBe('25-1-2');
92+
expect(result?.[1].versionColor).toBe('blue');
93+
expect(result?.[1].items).toHaveLength(1);
94+
expect(result?.[1].items?.[0].title).toBe('/Root/db2');
95+
expect(result?.[1].items?.[0].nodes).toHaveLength(1);
96+
expect(result?.[1].items?.[0].nodes?.[0].NodeId).toBe(5);
97+
98+
// Check third version group (25-1-3)
99+
expect(result?.[2].title).toBe('25-1-3');
100+
expect(result?.[2].versionColor).toBe('green');
101+
expect(result?.[2].items).toHaveLength(1);
102+
expect(result?.[2].items?.[0].title).toBe('/Root/db3');
103+
expect(result?.[2].items?.[0].nodes).toHaveLength(1);
104+
expect(result?.[2].items?.[0].nodes?.[0].NodeId).toBe(6);
105+
});
106+
107+
test('should group tenant nodes by tenant when groupByValue is TENANT', () => {
108+
const versionToColor = new Map([
109+
['25-1-1', 'red'],
110+
['25-1-2', 'blue'],
111+
['25-1-3', 'green'],
112+
]);
113+
114+
const result = getGroupedTenantNodes(nodes, versionToColor, GroupByValue.TENANT);
115+
116+
expect(result).toHaveLength(3);
117+
118+
// Check tenants are sorted alphabetically
119+
expect(result?.[0].title).toBe('/Root/db1');
120+
expect(result?.[1].title).toBe('/Root/db2');
121+
expect(result?.[2].title).toBe('/Root/db3');
122+
123+
// Check first tenant group (/Root/db1)
124+
expect(result?.[0].items).toHaveLength(1);
125+
expect(result?.[0].items?.[0].title).toBe('25-1-1');
126+
expect(result?.[0].items?.[0].versionColor).toBe('red');
127+
expect(result?.[0].items?.[0].nodes).toHaveLength(1);
128+
expect(result?.[0].items?.[0].nodes?.[0].NodeId).toBe(4);
129+
130+
// Check second tenant group (/Root/db2)
131+
expect(result?.[1].items).toHaveLength(1);
132+
expect(result?.[1].items?.[0].title).toBe('25-1-2');
133+
expect(result?.[1].items?.[0].versionColor).toBe('blue');
134+
expect(result?.[1].items?.[0].nodes).toHaveLength(1);
135+
expect(result?.[1].items?.[0].nodes?.[0].NodeId).toBe(5);
136+
137+
// Check third tenant group (/Root/db3)
138+
expect(result?.[2].items).toHaveLength(1);
139+
expect(result?.[2].items?.[0].title).toBe('25-1-3');
140+
expect(result?.[2].items?.[0].versionColor).toBe('green');
141+
expect(result?.[2].items?.[0].nodes).toHaveLength(1);
142+
expect(result?.[2].items?.[0].nodes?.[0].NodeId).toBe(6);
143+
});
144+
});
145+
146+
describe('getGroupedStorageNodes', () => {
147+
test('should return undefined when nodes is undefined', () => {
148+
const result = getGroupedStorageNodes(undefined, undefined);
149+
expect(result).toBeUndefined();
150+
});
151+
152+
test('should return undefined when nodes is empty', () => {
153+
const result = getGroupedStorageNodes([], undefined);
154+
expect(result).toBeUndefined();
155+
});
156+
157+
test('should group storage nodes by version', () => {
158+
const versionToColor = new Map([
159+
['25-1-1', 'red'],
160+
['25-1-2', 'blue'],
161+
['25-1-3', 'green'],
162+
]);
163+
164+
const result = getGroupedStorageNodes(nodes, versionToColor);
165+
166+
expect(result).toHaveLength(3);
167+
168+
// Check first version group (25-1-1)
169+
expect(result?.[0].title).toBe('25-1-1');
170+
expect(result?.[0].versionColor).toBe('red');
171+
expect(result?.[0].nodes).toHaveLength(1);
172+
expect(result?.[0].nodes?.[0].NodeId).toBe(1);
173+
174+
// Check second version group (25-1-2)
175+
expect(result?.[1].title).toBe('25-1-2');
176+
expect(result?.[1].versionColor).toBe('blue');
177+
expect(result?.[1].nodes).toHaveLength(1);
178+
expect(result?.[1].nodes?.[0].NodeId).toBe(2);
179+
180+
// Check third version group (25-1-3)
181+
expect(result?.[2].title).toBe('25-1-3');
182+
expect(result?.[2].versionColor).toBe('green');
183+
expect(result?.[2].nodes).toHaveLength(1);
184+
expect(result?.[2].nodes?.[0].NodeId).toBe(3);
185+
});
186+
});
187+
188+
describe('getOtherNodes', () => {
189+
test('should return undefined when nodes is undefined', () => {
190+
const result = getOtherNodes(undefined, undefined);
191+
expect(result).toBeUndefined();
192+
});
193+
194+
test('should return undefined when nodes is empty', () => {
195+
const result = getOtherNodes([], undefined);
196+
expect(result).toBeUndefined();
197+
});
198+
199+
test('should group other nodes by version', () => {
200+
const versionToColor = new Map([
201+
['25-1-1', 'red'],
202+
['25-1-2', 'blue'],
203+
['25-1-3', 'green'],
204+
]);
205+
206+
const result = getOtherNodes(nodes, versionToColor);
207+
208+
expect(result).toHaveLength(1);
209+
210+
// Check first version group (25-1-1)
211+
expect(result?.[0].title).toBe('25-1-1');
212+
expect(result?.[0].versionColor).toBe('red');
213+
expect(result?.[0].nodes).toHaveLength(2);
214+
215+
// The nodes with IDs 7 and 8 should be in the "other" category for version 25-1-1
216+
const nodeIds = result?.[0].nodes?.map((node) => node.NodeId);
217+
expect(nodeIds).toContain(7);
218+
expect(nodeIds).toContain(8);
219+
220+
// Check that there are no storage or tenant nodes in the result
221+
result?.forEach((group) => {
222+
group.nodes?.forEach((node) => {
223+
expect(node.Roles?.includes('Storage')).toBeFalsy();
224+
expect(node.Tenants).toBeUndefined();
225+
});
226+
});
227+
});
228+
});

‎src/containers/Versions/groupNodes.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@ export const getGroupedTenantNodes = (
2424

2525
return Object.keys(dividedByVersion)
2626
.map<GroupedNodesItem | null>((version) => {
27-
const filteredNodes = dividedByVersion[version].filter(({Tenants}) =>
28-
Boolean(Tenants),
29-
);
27+
const filteredNodes = dividedByVersion[version].filter(isTenantNode);
3028
const dividedByTenant = groupBy(filteredNodes, 'Tenants');
3129

3230
const items = Object.keys(dividedByTenant)
@@ -50,7 +48,7 @@ export const getGroupedTenantNodes = (
5048
})
5149
.filter((item): item is GroupedNodesItem => Boolean(item));
5250
} else {
53-
const filteredNodes = nodes.filter(({Tenants}) => Boolean(Tenants));
51+
const filteredNodes = nodes.filter(isTenantNode);
5452
const dividedByTenant = groupBy(filteredNodes, 'Tenants');
5553

5654
return Object.keys(dividedByTenant)
@@ -92,7 +90,7 @@ export const getGroupedStorageNodes = (
9290
return undefined;
9391
}
9492

95-
const storageNodes = nodes.filter(({Roles}) => Roles?.includes('Storage'));
93+
const storageNodes = nodes.filter(isStorageNode);
9694
const storageNodesDividedByVersion = groupBy(storageNodes, 'Version');
9795

9896
return Object.keys(storageNodesDividedByVersion).map((version) => {
@@ -112,7 +110,10 @@ export const getOtherNodes = (
112110
return undefined;
113111
}
114112

115-
const otherNodes = nodes.filter(({Roles, Version}) => !Roles && Version);
113+
// Nodes that are not included in other groups
114+
const otherNodes = nodes.filter(
115+
(node) => !isStorageNode(node) && !isTenantNode(node) && node.Version,
116+
);
116117
const otherNodesDividedByVersion = groupBy(otherNodes, 'Version');
117118

118119
return Object.keys(otherNodesDividedByVersion).map((version) => {
@@ -123,3 +124,11 @@ export const getOtherNodes = (
123124
};
124125
});
125126
};
127+
128+
function isStorageNode(node: NodesPreparedEntity) {
129+
return Boolean(node.Roles?.includes('Storage'));
130+
}
131+
132+
function isTenantNode(node: NodesPreparedEntity) {
133+
return Boolean(node.Tenants);
134+
}

0 commit comments

Comments
 (0)