Skip to content
This repository was archived by the owner on Jan 19, 2025. It is now read-only.

Commit a32ce13

Browse files
lars-reimannlars-reimann
authored
feat(gui): only add author when annotation was actually changed (#977)
* build(gui): add lodash for object equality check * feat(gui): only add author when annotation was actually changed * fix(gui): only increase counter for changed annotations if annotation was changed * refactor(gui): function now does what its name suggests * style: apply automatic fixes of linters Co-authored-by: lars-reimann <[email protected]>
1 parent 24dc499 commit a32ce13

File tree

3 files changed

+110
-23
lines changed

3 files changed

+110
-23
lines changed

‎api-editor/gui/package-lock.json‎

Lines changed: 3 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api-editor/gui/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"framer-motion": "^6.3.16",
2626
"idb-keyval": "^6.2.0",
2727
"katex": "^0.16.0",
28+
"lodash": "^4.17.21",
2829
"react": "^18.2.0",
2930
"react-chartjs-2": "^4.2.0",
3031
"react-dom": "^18.2.0",

‎api-editor/gui/src/features/annotations/annotationSlice.ts‎

Lines changed: 106 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
ValueAnnotation,
2424
} from './versioning/AnnotationStoreV2';
2525
import { migrateAnnotationStoreToCurrentVersion } from './versioning/migrations';
26+
import { isEqual } from 'lodash/fp';
2627

2728
export const EXPECTED_ANNOTATION_SLICE_SCHEMA_VERSION = 1;
2829

@@ -167,7 +168,11 @@ const annotationsSlice = createSlice({
167168
updateQueue(state);
168169
},
169170
upsertBoundaryAnnotation(state, action: PayloadAction<BoundaryAnnotation>) {
170-
updateCreationOrChangedCount(state, state.annotations.boundaryAnnotations[action.payload.target]);
171+
updateCreationOrChangedCount(
172+
state,
173+
state.annotations.boundaryAnnotations[action.payload.target],
174+
action.payload,
175+
);
171176

172177
state.annotations.boundaryAnnotations[action.payload.target] = withAuthorAndReviewers(
173178
state.annotations.boundaryAnnotations[action.payload.target],
@@ -204,7 +209,7 @@ const annotationsSlice = createSlice({
204209
state.annotations.calledAfterAnnotations[action.payload.annotation.target] = {};
205210
}
206211

207-
updateCreationOrChangedCount(state, oldAnnotation);
212+
updateCreationOrChangedCount(state, oldAnnotation, action.payload.annotation);
208213

209214
state.annotations.calledAfterAnnotations[action.payload.annotation.target][
210215
action.payload.annotation.calledAfterName
@@ -282,7 +287,11 @@ const annotationsSlice = createSlice({
282287
},
283288
// Cannot review complete annotations
284289
upsertDescriptionAnnotation(state, action: PayloadAction<DescriptionAnnotation>) {
285-
updateCreationOrChangedCount(state, state.annotations.descriptionAnnotations[action.payload.target]);
290+
updateCreationOrChangedCount(
291+
state,
292+
state.annotations.descriptionAnnotations[action.payload.target],
293+
action.payload,
294+
);
286295

287296
state.annotations.descriptionAnnotations[action.payload.target] = withAuthorAndReviewers(
288297
state.annotations.descriptionAnnotations[action.payload.target],
@@ -307,7 +316,11 @@ const annotationsSlice = createSlice({
307316
updateQueue(state);
308317
},
309318
upsertEnumAnnotation(state, action: PayloadAction<EnumAnnotation>) {
310-
updateCreationOrChangedCount(state, state.annotations.enumAnnotations[action.payload.target]);
319+
updateCreationOrChangedCount(
320+
state,
321+
state.annotations.enumAnnotations[action.payload.target],
322+
action.payload,
323+
);
311324

312325
state.annotations.enumAnnotations[action.payload.target] = withAuthorAndReviewers(
313326
state.annotations.enumAnnotations[action.payload.target],
@@ -379,7 +392,7 @@ const annotationsSlice = createSlice({
379392
}
380393
}
381394

382-
updateCreationOrChangedCount(state, oldAnnotation);
395+
updateCreationOrChangedCount(state, oldAnnotation, action.payload.annotation);
383396

384397
state.annotations.groupAnnotations[action.payload.annotation.target][action.payload.annotation.groupName] =
385398
withAuthorAndReviewers(oldAnnotation, action.payload.annotation, state.username);
@@ -419,7 +432,11 @@ const annotationsSlice = createSlice({
419432
updateQueue(state);
420433
},
421434
upsertMoveAnnotation(state, action: PayloadAction<MoveAnnotation>) {
422-
updateCreationOrChangedCount(state, state.annotations.moveAnnotations[action.payload.target]);
435+
updateCreationOrChangedCount(
436+
state,
437+
state.annotations.moveAnnotations[action.payload.target],
438+
action.payload,
439+
);
423440

424441
state.annotations.moveAnnotations[action.payload.target] = withAuthorAndReviewers(
425442
state.annotations.moveAnnotations[action.payload.target],
@@ -431,7 +448,7 @@ const annotationsSlice = createSlice({
431448
},
432449
upsertMoveAnnotations(state, action: PayloadAction<MoveAnnotation[]>) {
433450
action.payload.forEach((annotation) => {
434-
updateCreationOrChangedCount(state, state.annotations.moveAnnotations[annotation.target]);
451+
updateCreationOrChangedCount(state, state.annotations.moveAnnotations[annotation.target], annotation);
435452

436453
state.annotations.moveAnnotations[annotation.target] = withAuthorAndReviewers(
437454
state.annotations.moveAnnotations[annotation.target],
@@ -457,7 +474,11 @@ const annotationsSlice = createSlice({
457474
updateQueue(state);
458475
},
459476
upsertPureAnnotation(state, action: PayloadAction<PureAnnotation>) {
460-
updateCreationOrChangedCount(state, state.annotations.pureAnnotations[action.payload.target]);
477+
updateCreationOrChangedCount(
478+
state,
479+
state.annotations.pureAnnotations[action.payload.target],
480+
action.payload,
481+
);
461482

462483
state.annotations.pureAnnotations[action.payload.target] = withAuthorAndReviewers(
463484
state.annotations.pureAnnotations[action.payload.target],
@@ -482,7 +503,11 @@ const annotationsSlice = createSlice({
482503
updateQueue(state);
483504
},
484505
upsertRemoveAnnotation(state, action: PayloadAction<RemoveAnnotation>) {
485-
updateCreationOrChangedCount(state, state.annotations.removeAnnotations[action.payload.target]);
506+
updateCreationOrChangedCount(
507+
state,
508+
state.annotations.removeAnnotations[action.payload.target],
509+
action.payload,
510+
);
486511

487512
state.annotations.removeAnnotations[action.payload.target] = withAuthorAndReviewers(
488513
state.annotations.removeAnnotations[action.payload.target],
@@ -494,7 +519,7 @@ const annotationsSlice = createSlice({
494519
},
495520
upsertRemoveAnnotations(state, action: PayloadAction<RemoveAnnotation[]>) {
496521
action.payload.forEach((annotation) => {
497-
updateCreationOrChangedCount(state, state.annotations.removeAnnotations[annotation.target]);
522+
updateCreationOrChangedCount(state, state.annotations.removeAnnotations[annotation.target], annotation);
498523

499524
state.annotations.removeAnnotations[annotation.target] = withAuthorAndReviewers(
500525
state.annotations.removeAnnotations[annotation.target],
@@ -520,7 +545,11 @@ const annotationsSlice = createSlice({
520545
updateQueue(state);
521546
},
522547
upsertRenameAnnotation(state, action: PayloadAction<RenameAnnotation>) {
523-
updateCreationOrChangedCount(state, state.annotations.renameAnnotations[action.payload.target]);
548+
updateCreationOrChangedCount(
549+
state,
550+
state.annotations.renameAnnotations[action.payload.target],
551+
action.payload,
552+
);
524553

525554
state.annotations.renameAnnotations[action.payload.target] = withAuthorAndReviewers(
526555
state.annotations.renameAnnotations[action.payload.target],
@@ -532,7 +561,7 @@ const annotationsSlice = createSlice({
532561
},
533562
upsertRenameAnnotations(state, action: PayloadAction<RenameAnnotation[]>) {
534563
action.payload.forEach((annotation) => {
535-
updateCreationOrChangedCount(state, state.annotations.renameAnnotations[annotation.target]);
564+
updateCreationOrChangedCount(state, state.annotations.renameAnnotations[annotation.target], annotation);
536565

537566
state.annotations.renameAnnotations[annotation.target] = withAuthorAndReviewers(
538567
state.annotations.renameAnnotations[annotation.target],
@@ -558,7 +587,11 @@ const annotationsSlice = createSlice({
558587
updateQueue(state);
559588
},
560589
upsertTodoAnnotation(state, action: PayloadAction<TodoAnnotation>) {
561-
updateCreationOrChangedCount(state, state.annotations.todoAnnotations[action.payload.target]);
590+
updateCreationOrChangedCount(
591+
state,
592+
state.annotations.todoAnnotations[action.payload.target],
593+
action.payload,
594+
);
562595

563596
state.annotations.todoAnnotations[action.payload.target] = withAuthorAndReviewers(
564597
state.annotations.todoAnnotations[action.payload.target],
@@ -583,7 +616,23 @@ const annotationsSlice = createSlice({
583616
updateQueue(state);
584617
},
585618
upsertValueAnnotation(state, action: PayloadAction<ValueAnnotation>) {
586-
updateCreationOrChangedCount(state, state.annotations.valueAnnotations[action.payload.target]);
619+
updateCreationOrChangedCount(
620+
state,
621+
state.annotations.valueAnnotations[action.payload.target],
622+
action.payload,
623+
);
624+
625+
if (action.payload.variant === 'required' || action.payload.variant === 'omitted') {
626+
// @ts-ignore
627+
delete action.payload.defaultValue;
628+
// @ts-ignore
629+
delete action.payload.defaultValueType;
630+
}
631+
632+
if (action.payload.defaultValueType === 'number' && typeof action.payload.defaultValue === 'string') {
633+
// @ts-ignore
634+
action.payload.defaultValue = parseFloat(action.payload.defaultValue);
635+
}
587636

588637
state.annotations.valueAnnotations[action.payload.target] = withAuthorAndReviewers(
589638
state.annotations.valueAnnotations[action.payload.target],
@@ -595,7 +644,19 @@ const annotationsSlice = createSlice({
595644
},
596645
upsertValueAnnotations(state, action: PayloadAction<ValueAnnotation[]>) {
597646
action.payload.forEach((annotation) => {
598-
updateCreationOrChangedCount(state, state.annotations.valueAnnotations[annotation.target]);
647+
updateCreationOrChangedCount(state, state.annotations.valueAnnotations[annotation.target], annotation);
648+
649+
if (annotation.variant === 'required' || annotation.variant === 'omitted') {
650+
// @ts-ignore
651+
delete annotation.defaultValue;
652+
// @ts-ignore
653+
delete annotation.defaultValueType;
654+
}
655+
656+
if (annotation.defaultValueType === 'number' && typeof annotation.defaultValue === 'string') {
657+
// @ts-ignore
658+
annotation.defaultValue = parseFloat(annotation.defaultValue);
659+
}
599660

600661
state.annotations.valueAnnotations[annotation.target] = withAuthorAndReviewers(
601662
state.annotations.valueAnnotations[annotation.target],
@@ -662,9 +723,15 @@ const updateQueue = function (state: AnnotationSlice) {
662723
state.queueIndex = state.queueIndex + 1;
663724
};
664725

665-
const updateCreationOrChangedCount = function (state: AnnotationSlice, annotationOrNull: Annotation | null) {
666-
if (annotationOrNull) {
667-
state.numberOfAnnotationsChanged++;
726+
const updateCreationOrChangedCount = function (
727+
state: AnnotationSlice,
728+
oldAnnotation: Annotation | null,
729+
newAnnotation: Annotation,
730+
) {
731+
if (oldAnnotation) {
732+
if (annotationWasChanged(oldAnnotation, newAnnotation)) {
733+
state.numberOfAnnotationsChanged++;
734+
}
668735
} else {
669736
state.numberOfAnnotationsCreated++;
670737
}
@@ -678,7 +745,9 @@ const withAuthorAndReviewers = function <T extends Annotation>(
678745
let authors = oldAnnotation?.authors ?? [];
679746
const reviewers = oldAnnotation?.reviewers ?? [];
680747

681-
authors = [...authors.filter((it) => it !== author), author];
748+
if (!oldAnnotation || annotationWasChanged(oldAnnotation, newAnnotation)) {
749+
authors = [...authors.filter((it) => it !== author), author];
750+
}
682751

683752
return {
684753
...newAnnotation,
@@ -687,6 +756,24 @@ const withAuthorAndReviewers = function <T extends Annotation>(
687756
};
688757
};
689758

759+
const annotationWasChanged = function <T extends Annotation>(oldAnnotation: T, newAnnotation: T): boolean {
760+
// Unify the metadata, so we only compare the actual annotation data
761+
const oldAnnotationWithoutMetadata = annotationWithoutMetadata(oldAnnotation);
762+
const newAnnotationWithoutMetadata = annotationWithoutMetadata(newAnnotation);
763+
764+
return !isEqual(oldAnnotationWithoutMetadata, newAnnotationWithoutMetadata);
765+
};
766+
767+
const annotationWithoutMetadata = function <T extends Annotation>(annotation: T): T {
768+
return {
769+
...annotation,
770+
authors: undefined,
771+
reviewers: undefined,
772+
reviewResult: undefined,
773+
comment: undefined,
774+
};
775+
};
776+
690777
const withToggledReviewer = function <T extends Annotation>(
691778
state: AnnotationSlice,
692779
oldAnnotation: T,

0 commit comments

Comments
 (0)