Skip to content

Commit f42e5db

Browse files
MichaelBuessemeyerMichaelBuessemeyer
committed
fix left over broken tests
1 parent 93e86a4 commit f42e5db

File tree

7 files changed

+169
-28
lines changed

7 files changed

+169
-28
lines changed

‎frontend/javascripts/test/reducers/update_action_application/skeleton.spec.ts‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ const actionNamesHelper: Record<ApplicableSkeletonUpdateAction["name"], true> =
9494
updateActiveNode: true,
9595
updateTreeVisibility: true,
9696
updateTreeGroupVisibility: true,
97+
updateActiveTree: true,
9798
};
9899
const actionNamesList = Object.keys(actionNamesHelper);
99100

@@ -152,6 +153,11 @@ describe("Update Action Application for SkeletonTracing", () => {
152153
makeBasicGroupObject(3, "group 3"),
153154
makeBasicGroupObject(7, "group 7"),
154155
]),
156+
SkeletonTracingActions.setActiveNodeAction(11),
157+
SkeletonTracingActions.setTreeGroupAction(3, 1),
158+
SkeletonTracingActions.setTreeGroupAction(3, 2),
159+
// Toggle on and off to ensure compaction compacts actions to an updateTreeGroupVisibility action.
160+
SkeletonTracingActions.toggleTreeGroupAction(3),
155161
SkeletonTracingActions.toggleTreeGroupAction(3),
156162
SkeletonTracingActions.setTreeGroupAction(7, 2),
157163
SkeletonTracingActions.setTreeEdgeVisibilityAction(2, false),
@@ -187,22 +193,20 @@ describe("Update Action Application for SkeletonTracing", () => {
187193
userActions.slice(0, beforeVersionIndex),
188194
);
189195

190-
const state2WithoutActiveState = applyActions(state2WithActiveTree, [
191-
SkeletonTracingActions.setActiveNodeAction(null),
196+
const state2WithoutActiveBoundingBox = applyActions(state2WithActiveTree, [
192197
setActiveUserBoundingBoxId(null),
193198
]);
194199

195200
const actionsToApply = userActions.slice(beforeVersionIndex, afterVersionIndex + 1);
196201
const state3 = applyActions(
197202
state2WithActiveTree,
198-
actionsToApply.concat([
199-
SkeletonTracingActions.setActiveNodeAction(null),
200-
setActiveUserBoundingBoxId(null),
201-
]),
203+
actionsToApply.concat([setActiveUserBoundingBoxId(null)]),
202204
);
203-
expect(state2WithoutActiveState !== state3).toBeTruthy();
205+
expect(state2WithoutActiveBoundingBox !== state3).toBeTruthy();
204206

205-
const skeletonTracing2 = enforceSkeletonTracing(state2WithoutActiveState.annotation);
207+
const skeletonTracing2 = enforceSkeletonTracing(
208+
state2WithoutActiveBoundingBox.annotation,
209+
);
206210
const skeletonTracing3 = enforceSkeletonTracing(state3.annotation);
207211

208212
const updateActionsBeforeCompaction = Array.from(
@@ -220,12 +224,13 @@ describe("Update Action Application for SkeletonTracing", () => {
220224
seenActionTypes.add(action.name);
221225
}
222226

223-
const reappliedNewState = transformStateAsReadOnly(state2WithoutActiveState, (state) =>
224-
applyActions(state, [
225-
SkeletonTracingActions.applySkeletonUpdateActionsFromServerAction(updateActions),
226-
SkeletonTracingActions.setActiveNodeAction(null),
227-
setActiveUserBoundingBoxId(null),
228-
]),
227+
const reappliedNewState = transformStateAsReadOnly(
228+
state2WithoutActiveBoundingBox,
229+
(state) =>
230+
applyActions(state, [
231+
SkeletonTracingActions.applySkeletonUpdateActionsFromServerAction(updateActions),
232+
setActiveUserBoundingBoxId(null),
233+
]),
229234
);
230235

231236
expect(reappliedNewState).toEqual(state3);

‎frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -695,13 +695,21 @@ describe("SkeletonTracingSaga", () => {
695695
},
696696
});
697697
expect(simplifiedSecondBatch[5]).toMatchObject({
698+
name: "updateActiveTree",
699+
value: {
700+
actionTracingId: "skeletonTracingId",
701+
activeNode: 6,
702+
activeTree: 3,
703+
},
704+
});
705+
expect(simplifiedSecondBatch[6]).toMatchObject({
698706
name: "updateActiveNode",
699707
value: {
700708
actionTracingId: "skeletonTracingId",
701709
activeNode: 6,
702710
},
703711
});
704-
expect(simplifiedSecondBatch.length).toBe(6);
712+
expect(simplifiedSecondBatch.length).toBe(7);
705713
});
706714

707715
it("compactUpdateActions should detect a tree merge (3/3)", () => {
@@ -774,13 +782,21 @@ describe("SkeletonTracingSaga", () => {
774782
},
775783
});
776784
expect(simplifiedFirstBatch[3]).toMatchObject({
785+
name: "updateActiveTree",
786+
value: {
787+
actionTracingId: "skeletonTracingId",
788+
activeNode: 1,
789+
activeTree: 1,
790+
},
791+
});
792+
expect(simplifiedFirstBatch[4]).toMatchObject({
777793
name: "updateActiveNode",
778794
value: {
779795
actionTracingId: "skeletonTracingId",
780796
activeNode: 1,
781797
},
782798
});
783-
expect(simplifiedFirstBatch.length).toBe(4);
799+
expect(simplifiedFirstBatch.length).toBe(5);
784800

785801
// the creation of another tree, two nodes and one edge (b)
786802
const simplifiedSecondBatch = simplifiedUpdateActions[1].actions;
@@ -789,13 +805,21 @@ describe("SkeletonTracingSaga", () => {
789805
expect(simplifiedSecondBatch[2].name).toBe("createNode");
790806
expect(simplifiedSecondBatch[3].name).toBe("createEdge");
791807
expect(simplifiedSecondBatch[4]).toMatchObject({
808+
name: "updateActiveTree",
809+
value: {
810+
actionTracingId: "skeletonTracingId",
811+
activeNode: 6,
812+
activeTree: 2,
813+
},
814+
});
815+
expect(simplifiedSecondBatch[5]).toMatchObject({
792816
name: "updateActiveNode",
793817
value: {
794818
actionTracingId: "skeletonTracingId",
795819
activeNode: 6,
796820
},
797821
});
798-
expect(simplifiedSecondBatch.length).toBe(5);
822+
expect(simplifiedSecondBatch.length).toBe(6);
799823

800824
// a second merge (c)
801825
const simplifiedThirdBatch = simplifiedUpdateActions[2].actions;
@@ -826,13 +850,21 @@ describe("SkeletonTracingSaga", () => {
826850
});
827851

828852
expect(simplifiedThirdBatch[3]).toMatchObject({
853+
name: "updateActiveTree",
854+
value: {
855+
actionTracingId: "skeletonTracingId",
856+
activeNode: 1,
857+
activeTree: 1,
858+
},
859+
});
860+
expect(simplifiedThirdBatch[4]).toMatchObject({
829861
name: "updateActiveNode",
830862
value: {
831863
actionTracingId: "skeletonTracingId",
832864
activeNode: 1,
833865
},
834866
});
835-
expect(simplifiedThirdBatch.length).toBe(4);
867+
expect(simplifiedThirdBatch.length).toBe(5);
836868
});
837869

838870
it("compactUpdateActions should detect a tree split (1/3)", () => {
@@ -1085,13 +1117,21 @@ describe("SkeletonTracingSaga", () => {
10851117
expect(simplifiedSecondBatch[3].name).toBe("deleteEdge");
10861118
expect(simplifiedSecondBatch[4].name).toBe("deleteEdge");
10871119
expect(simplifiedSecondBatch[5]).toMatchObject({
1120+
name: "updateActiveTree",
1121+
value: {
1122+
actionTracingId: "skeletonTracingId",
1123+
activeNode: 3,
1124+
activeTree: 2,
1125+
},
1126+
});
1127+
expect(simplifiedSecondBatch[6]).toMatchObject({
10881128
name: "updateActiveNode",
10891129
value: {
10901130
actionTracingId: "skeletonTracingId",
10911131
activeNode: 3,
10921132
},
10931133
});
1094-
expect(simplifiedSecondBatch.length).toBe(6);
1134+
expect(simplifiedSecondBatch.length).toBe(7);
10951135
expect(simplifiedUpdateActions2.length).toBe(1);
10961136
});
10971137

@@ -1159,13 +1199,21 @@ describe("SkeletonTracingSaga", () => {
11591199
},
11601200
});
11611201
expect(simplifiedFirstBatch[1]).toMatchObject({
1202+
name: "updateActiveTree",
1203+
value: {
1204+
actionTracingId: "skeletonTracingId",
1205+
activeTree: 1,
1206+
activeNode: null,
1207+
},
1208+
});
1209+
expect(simplifiedFirstBatch[2]).toMatchObject({
11621210
name: "updateActiveNode",
11631211
value: {
11641212
actionTracingId: "skeletonTracingId",
11651213
activeNode: null,
11661214
},
11671215
});
1168-
expect(simplifiedFirstBatch.length).toBe(2);
1216+
expect(simplifiedFirstBatch.length).toBe(3);
11691217
});
11701218

11711219
it("compactUpdateActions should not detect a deleted tree if there is no deleted tree", () => {

‎frontend/javascripts/viewer/model/reducers/skeletontracing_reducer_helpers.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -768,9 +768,11 @@ export function deleteComment(
768768
export function toggleAllTreesReducer(
769769
state: WebknossosState,
770770
skeletonTracing: SkeletonTracing,
771+
shouldBecomeVisible?: boolean,
771772
): WebknossosState {
772773
// Let's make all trees visible if there is one invisible tree
773-
const shouldBecomeVisible = skeletonTracing.trees.values().some((tree) => !tree.isVisible);
774+
shouldBecomeVisible =
775+
shouldBecomeVisible ?? skeletonTracing.trees.values().some((tree) => !tree.isVisible);
774776

775777
const newTrees = skeletonTracing.trees.clone();
776778
for (const [treeId, tree] of skeletonTracing.trees.entries()) {

‎frontend/javascripts/viewer/model/reducers/update_action_application/skeleton.ts‎

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ import update from "immutability-helper";
22
import DiffableMap from "libs/diffable_map";
33
import {
44
enforceSkeletonTracing,
5+
findTreeByNodeId,
56
getTree,
67
getTreeGroupsMap,
78
} from "viewer/model/accessors/skeletontracing_accessor";
89
import { changeUserBoundingBoxAction } from "viewer/model/actions/annotation_actions";
910
import {
10-
setActiveNodeAction,
1111
setTreeEdgeVisibilityAction,
1212
setTreeGroupsAction,
1313
setTreeVisibilityAction,
@@ -20,6 +20,7 @@ import { updateUserBoundingBox } from "../annotation_reducer";
2020
import {
2121
getMaximumNodeId,
2222
setExpandedTreeGroups,
23+
toggleAllTreesReducer,
2324
toggleTreeGroupReducer,
2425
} from "../skeletontracing_reducer_helpers";
2526
import {
@@ -377,11 +378,57 @@ function applySingleAction(
377378
}
378379
// User specific actions
379380
case "updateActiveNode": {
380-
if (ua.value.activeNode != null) {
381-
return SkeletonTracingReducer(state, setActiveNodeAction(ua.value.activeNode));
382-
} else {
383-
return state;
381+
if (ua.value.activeNode == null) {
382+
return update(state, {
383+
annotation: {
384+
skeleton: {
385+
activeNodeId: {
386+
$set: null,
387+
},
388+
},
389+
},
390+
});
384391
}
392+
const tree = findTreeByNodeId(
393+
enforceSkeletonTracing(state.annotation).trees,
394+
ua.value.activeNode,
395+
);
396+
if (tree) {
397+
return update(state, {
398+
annotation: {
399+
skeleton: {
400+
activeNodeId: {
401+
$set: ua.value.activeNode,
402+
},
403+
activeTreeId: {
404+
$set: tree.treeId,
405+
},
406+
},
407+
},
408+
});
409+
}
410+
return state;
411+
}
412+
case "updateActiveTree": {
413+
const skeletonTracing = enforceSkeletonTracing(state.annotation);
414+
if (ua.value.activeTree) {
415+
const tree = getTree(skeletonTracing, ua.value.activeTree);
416+
if (!tree) {
417+
return state;
418+
}
419+
}
420+
return update(state, {
421+
annotation: {
422+
skeleton: {
423+
activeTreeId: {
424+
$set: ua.value.activeTree,
425+
},
426+
activeNodeId: {
427+
$set: ua.value.activeNode,
428+
},
429+
},
430+
},
431+
});
385432
}
386433
case "updateTreeVisibility": {
387434
return SkeletonTracingReducer(
@@ -398,7 +445,11 @@ function applySingleAction(
398445
ua.value.isVisible,
399446
);
400447
} else {
401-
return state;
448+
return toggleAllTreesReducer(
449+
state,
450+
enforceSkeletonTracing(state.annotation),
451+
ua.value.isVisible,
452+
);
402453
}
403454
}
404455
default: {

‎frontend/javascripts/viewer/model/sagas/saving/save_queue_draining.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ export function* sendSaveRequestToServer(): Saga<number> {
127127
*/
128128

129129
const fullSaveQueue = yield* select((state) => state.save.queue);
130-
const saveQueue = sliceAppropriateBatchCount(fullSaveQueue);
130+
const withoutFEOnlyActions = filterOutFrontendOnlySupportedActions(fullSaveQueue);
131+
const saveQueue = sliceAppropriateBatchCount(withoutFEOnlyActions);
131132
let compactedSaveQueue = compactSaveQueue(saveQueue);
132133
const version = yield* select((state) => state.annotation.version);
133134
const annotationId = yield* select((state) => state.annotation.annotationId);
@@ -301,6 +302,16 @@ function sliceAppropriateBatchCount(batches: Array<SaveQueueEntry>): Array<SaveQ
301302
return slicedBatches;
302303
}
303304

305+
function filterOutFrontendOnlySupportedActions(
306+
updateActionsBatches: Array<SaveQueueEntry>,
307+
): Array<SaveQueueEntry> {
308+
const batchesWithoutFrontendOnlyActions = updateActionsBatches.map((batch) => ({
309+
...batch,
310+
actions: batch.actions.filter((a) => !("isFrontendOnly" in a.value && a.value.isFrontendOnly)),
311+
}));
312+
return batchesWithoutFrontendOnlyActions;
313+
}
314+
304315
export function addVersionNumbers(
305316
updateActionsBatches: Array<SaveQueueEntry>,
306317
lastVersion: number,

‎frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ import {
7272
deleteNode,
7373
deleteTree,
7474
updateActiveNode,
75+
updateActiveTree,
7576
updateNode,
7677
updateTree,
7778
updateTreeEdgesVisibility,
@@ -669,6 +670,10 @@ export function* diffSkeletonTracing(
669670
);
670671
}
671672

673+
if (prevSkeletonTracing.activeTreeId !== skeletonTracing.activeTreeId) {
674+
yield updateActiveTree(skeletonTracing);
675+
}
676+
// Active node id should always have precedence over the tree id, thus set it last.
672677
if (prevSkeletonTracing.activeNodeId !== skeletonTracing.activeNodeId) {
673678
yield updateActiveNode(skeletonTracing);
674679
}

0 commit comments

Comments
 (0)