From dd5fdba51cfd321ebcb782fc1d608116d7ec714b Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sat, 9 Jul 2022 17:51:15 +0200 Subject: [PATCH 1/5] build(gui): add lodash for object equality check --- api-editor/gui/package-lock.json | 7 +++---- api-editor/gui/package.json | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api-editor/gui/package-lock.json b/api-editor/gui/package-lock.json index 0370de2ef..d16169819 100644 --- a/api-editor/gui/package-lock.json +++ b/api-editor/gui/package-lock.json @@ -21,6 +21,7 @@ "framer-motion": "^6.3.16", "idb-keyval": "^6.2.0", "katex": "^0.16.0", + "lodash": "^4.17.21", "react": "^18.2.0", "react-chartjs-2": "^4.2.0", "react-dom": "^18.2.0", @@ -8154,8 +8155,7 @@ "node_modules/lodash": { "version": "4.17.21", "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", - "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", - "dev": true + "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" }, "node_modules/lodash.memoize": { "version": "4.1.2", @@ -18045,8 +18045,7 @@ "lodash": { "version": "4.17.21", "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", - "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", - "dev": true + "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" }, "lodash.memoize": { "version": "4.1.2", diff --git a/api-editor/gui/package.json b/api-editor/gui/package.json index 2586fd303..a8ecd1a43 100644 --- a/api-editor/gui/package.json +++ b/api-editor/gui/package.json @@ -25,6 +25,7 @@ "framer-motion": "^6.3.16", "idb-keyval": "^6.2.0", "katex": "^0.16.0", + "lodash": "^4.17.21", "react": "^18.2.0", "react-chartjs-2": "^4.2.0", "react-dom": "^18.2.0", From ef4029383fadde7d9a94f1e8305a2bca8d61e4b5 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sat, 9 Jul 2022 18:22:53 +0200 Subject: [PATCH 2/5] feat(gui): only add author when annotation was actually changed --- .../features/annotations/annotationSlice.ts | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/api-editor/gui/src/features/annotations/annotationSlice.ts b/api-editor/gui/src/features/annotations/annotationSlice.ts index 871fea183..31bbd1194 100644 --- a/api-editor/gui/src/features/annotations/annotationSlice.ts +++ b/api-editor/gui/src/features/annotations/annotationSlice.ts @@ -23,6 +23,7 @@ import { ValueAnnotation, } from './versioning/AnnotationStoreV2'; import { migrateAnnotationStoreToCurrentVersion } from './versioning/migrations'; +import { isEqual } from 'lodash/fp'; export const EXPECTED_ANNOTATION_SLICE_SCHEMA_VERSION = 1; @@ -585,6 +586,18 @@ const annotationsSlice = createSlice({ upsertValueAnnotation(state, action: PayloadAction) { updateCreationOrChangedCount(state, state.annotations.valueAnnotations[action.payload.target]); + if (action.payload.variant === 'required' || action.payload.variant === 'omitted') { + // @ts-ignore + delete action.payload.defaultValue; + // @ts-ignore + delete action.payload.defaultValueType; + } + + if (action.payload.defaultValueType === 'number' && typeof action.payload.defaultValue === 'string') { + // @ts-ignore + action.payload.defaultValue = parseFloat(action.payload.defaultValue); + } + state.annotations.valueAnnotations[action.payload.target] = withAuthorAndReviewers( state.annotations.valueAnnotations[action.payload.target], action.payload, @@ -597,6 +610,18 @@ const annotationsSlice = createSlice({ action.payload.forEach((annotation) => { updateCreationOrChangedCount(state, state.annotations.valueAnnotations[annotation.target]); + if (annotation.variant === 'required' || annotation.variant === 'omitted') { + // @ts-ignore + delete annotation.defaultValue; + // @ts-ignore + delete annotation.defaultValueType; + } + + if (annotation.defaultValueType === 'number' && typeof annotation.defaultValue === 'string') { + // @ts-ignore + annotation.defaultValue = parseFloat(annotation.defaultValue); + } + state.annotations.valueAnnotations[annotation.target] = withAuthorAndReviewers( state.annotations.valueAnnotations[annotation.target], annotation, @@ -678,7 +703,9 @@ const withAuthorAndReviewers = function ( let authors = oldAnnotation?.authors ?? []; const reviewers = oldAnnotation?.reviewers ?? []; - authors = [...authors.filter((it) => it !== author), author]; + if (shouldUpdateAuthor(oldAnnotation, newAnnotation)) { + authors = [...authors.filter((it) => it !== author), author]; + } return { ...newAnnotation, @@ -687,6 +714,28 @@ const withAuthorAndReviewers = function ( }; }; +const shouldUpdateAuthor = function (oldAnnotation: T | void, newAnnotation: T): boolean { + // A new annotation was created + if (!oldAnnotation) { + return true; + } + + // Unify the metadata, so we only compare the actual annotation data + const oldAnnotationWithoutMetadata = annotationWithoutMetadata(oldAnnotation); + const newAnnotationWithoutMetadata = annotationWithoutMetadata(newAnnotation); + return !isEqual(oldAnnotationWithoutMetadata, newAnnotationWithoutMetadata); +}; + +const annotationWithoutMetadata = function (annotation: T): T { + return { + ...annotation, + authors: undefined, + reviewers: undefined, + reviewResult: undefined, + comment: undefined, + }; +}; + const withToggledReviewer = function ( state: AnnotationSlice, oldAnnotation: T, From a7d356cddc379d3520910d1b3bdcbbaa2cb3bf97 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sat, 9 Jul 2022 18:28:57 +0200 Subject: [PATCH 3/5] fix(gui): only increase counter for changed annotations if annotation was changed --- .../features/annotations/annotationSlice.ts | 82 ++++++++++++++----- 1 file changed, 62 insertions(+), 20 deletions(-) diff --git a/api-editor/gui/src/features/annotations/annotationSlice.ts b/api-editor/gui/src/features/annotations/annotationSlice.ts index 31bbd1194..9c91a61ec 100644 --- a/api-editor/gui/src/features/annotations/annotationSlice.ts +++ b/api-editor/gui/src/features/annotations/annotationSlice.ts @@ -168,7 +168,11 @@ const annotationsSlice = createSlice({ updateQueue(state); }, upsertBoundaryAnnotation(state, action: PayloadAction) { - updateCreationOrChangedCount(state, state.annotations.boundaryAnnotations[action.payload.target]); + updateCreationOrChangedCount( + state, + state.annotations.boundaryAnnotations[action.payload.target], + action.payload, + ); state.annotations.boundaryAnnotations[action.payload.target] = withAuthorAndReviewers( state.annotations.boundaryAnnotations[action.payload.target], @@ -205,7 +209,7 @@ const annotationsSlice = createSlice({ state.annotations.calledAfterAnnotations[action.payload.annotation.target] = {}; } - updateCreationOrChangedCount(state, oldAnnotation); + updateCreationOrChangedCount(state, oldAnnotation, action.payload.annotation); state.annotations.calledAfterAnnotations[action.payload.annotation.target][ action.payload.annotation.calledAfterName @@ -283,7 +287,11 @@ const annotationsSlice = createSlice({ }, // Cannot review complete annotations upsertDescriptionAnnotation(state, action: PayloadAction) { - updateCreationOrChangedCount(state, state.annotations.descriptionAnnotations[action.payload.target]); + updateCreationOrChangedCount( + state, + state.annotations.descriptionAnnotations[action.payload.target], + action.payload, + ); state.annotations.descriptionAnnotations[action.payload.target] = withAuthorAndReviewers( state.annotations.descriptionAnnotations[action.payload.target], @@ -308,7 +316,11 @@ const annotationsSlice = createSlice({ updateQueue(state); }, upsertEnumAnnotation(state, action: PayloadAction) { - updateCreationOrChangedCount(state, state.annotations.enumAnnotations[action.payload.target]); + updateCreationOrChangedCount( + state, + state.annotations.enumAnnotations[action.payload.target], + action.payload, + ); state.annotations.enumAnnotations[action.payload.target] = withAuthorAndReviewers( state.annotations.enumAnnotations[action.payload.target], @@ -380,7 +392,7 @@ const annotationsSlice = createSlice({ } } - updateCreationOrChangedCount(state, oldAnnotation); + updateCreationOrChangedCount(state, oldAnnotation, action.payload.annotation); state.annotations.groupAnnotations[action.payload.annotation.target][action.payload.annotation.groupName] = withAuthorAndReviewers(oldAnnotation, action.payload.annotation, state.username); @@ -420,7 +432,11 @@ const annotationsSlice = createSlice({ updateQueue(state); }, upsertMoveAnnotation(state, action: PayloadAction) { - updateCreationOrChangedCount(state, state.annotations.moveAnnotations[action.payload.target]); + updateCreationOrChangedCount( + state, + state.annotations.moveAnnotations[action.payload.target], + action.payload, + ); state.annotations.moveAnnotations[action.payload.target] = withAuthorAndReviewers( state.annotations.moveAnnotations[action.payload.target], @@ -432,7 +448,7 @@ const annotationsSlice = createSlice({ }, upsertMoveAnnotations(state, action: PayloadAction) { action.payload.forEach((annotation) => { - updateCreationOrChangedCount(state, state.annotations.moveAnnotations[annotation.target]); + updateCreationOrChangedCount(state, state.annotations.moveAnnotations[annotation.target], annotation); state.annotations.moveAnnotations[annotation.target] = withAuthorAndReviewers( state.annotations.moveAnnotations[annotation.target], @@ -458,7 +474,11 @@ const annotationsSlice = createSlice({ updateQueue(state); }, upsertPureAnnotation(state, action: PayloadAction) { - updateCreationOrChangedCount(state, state.annotations.pureAnnotations[action.payload.target]); + updateCreationOrChangedCount( + state, + state.annotations.pureAnnotations[action.payload.target], + action.payload, + ); state.annotations.pureAnnotations[action.payload.target] = withAuthorAndReviewers( state.annotations.pureAnnotations[action.payload.target], @@ -483,7 +503,11 @@ const annotationsSlice = createSlice({ updateQueue(state); }, upsertRemoveAnnotation(state, action: PayloadAction) { - updateCreationOrChangedCount(state, state.annotations.removeAnnotations[action.payload.target]); + updateCreationOrChangedCount( + state, + state.annotations.removeAnnotations[action.payload.target], + action.payload, + ); state.annotations.removeAnnotations[action.payload.target] = withAuthorAndReviewers( state.annotations.removeAnnotations[action.payload.target], @@ -495,7 +519,7 @@ const annotationsSlice = createSlice({ }, upsertRemoveAnnotations(state, action: PayloadAction) { action.payload.forEach((annotation) => { - updateCreationOrChangedCount(state, state.annotations.removeAnnotations[annotation.target]); + updateCreationOrChangedCount(state, state.annotations.removeAnnotations[annotation.target], annotation); state.annotations.removeAnnotations[annotation.target] = withAuthorAndReviewers( state.annotations.removeAnnotations[annotation.target], @@ -521,7 +545,11 @@ const annotationsSlice = createSlice({ updateQueue(state); }, upsertRenameAnnotation(state, action: PayloadAction) { - updateCreationOrChangedCount(state, state.annotations.renameAnnotations[action.payload.target]); + updateCreationOrChangedCount( + state, + state.annotations.renameAnnotations[action.payload.target], + action.payload, + ); state.annotations.renameAnnotations[action.payload.target] = withAuthorAndReviewers( state.annotations.renameAnnotations[action.payload.target], @@ -533,7 +561,7 @@ const annotationsSlice = createSlice({ }, upsertRenameAnnotations(state, action: PayloadAction) { action.payload.forEach((annotation) => { - updateCreationOrChangedCount(state, state.annotations.renameAnnotations[annotation.target]); + updateCreationOrChangedCount(state, state.annotations.renameAnnotations[annotation.target], annotation); state.annotations.renameAnnotations[annotation.target] = withAuthorAndReviewers( state.annotations.renameAnnotations[annotation.target], @@ -559,7 +587,11 @@ const annotationsSlice = createSlice({ updateQueue(state); }, upsertTodoAnnotation(state, action: PayloadAction) { - updateCreationOrChangedCount(state, state.annotations.todoAnnotations[action.payload.target]); + updateCreationOrChangedCount( + state, + state.annotations.todoAnnotations[action.payload.target], + action.payload, + ); state.annotations.todoAnnotations[action.payload.target] = withAuthorAndReviewers( state.annotations.todoAnnotations[action.payload.target], @@ -584,7 +616,11 @@ const annotationsSlice = createSlice({ updateQueue(state); }, upsertValueAnnotation(state, action: PayloadAction) { - updateCreationOrChangedCount(state, state.annotations.valueAnnotations[action.payload.target]); + updateCreationOrChangedCount( + state, + state.annotations.valueAnnotations[action.payload.target], + action.payload, + ); if (action.payload.variant === 'required' || action.payload.variant === 'omitted') { // @ts-ignore @@ -608,7 +644,7 @@ const annotationsSlice = createSlice({ }, upsertValueAnnotations(state, action: PayloadAction) { action.payload.forEach((annotation) => { - updateCreationOrChangedCount(state, state.annotations.valueAnnotations[annotation.target]); + updateCreationOrChangedCount(state, state.annotations.valueAnnotations[annotation.target], annotation); if (annotation.variant === 'required' || annotation.variant === 'omitted') { // @ts-ignore @@ -687,9 +723,15 @@ const updateQueue = function (state: AnnotationSlice) { state.queueIndex = state.queueIndex + 1; }; -const updateCreationOrChangedCount = function (state: AnnotationSlice, annotationOrNull: Annotation | null) { - if (annotationOrNull) { - state.numberOfAnnotationsChanged++; +const updateCreationOrChangedCount = function ( + state: AnnotationSlice, + oldAnnotation: Annotation | null, + newAnnotation: Annotation, +) { + if (oldAnnotation) { + if (annotationWasChanged(oldAnnotation, newAnnotation)) { + state.numberOfAnnotationsChanged++; + } } else { state.numberOfAnnotationsCreated++; } @@ -703,7 +745,7 @@ const withAuthorAndReviewers = function ( let authors = oldAnnotation?.authors ?? []; const reviewers = oldAnnotation?.reviewers ?? []; - if (shouldUpdateAuthor(oldAnnotation, newAnnotation)) { + if (annotationWasChanged(oldAnnotation, newAnnotation)) { authors = [...authors.filter((it) => it !== author), author]; } @@ -714,7 +756,7 @@ const withAuthorAndReviewers = function ( }; }; -const shouldUpdateAuthor = function (oldAnnotation: T | void, newAnnotation: T): boolean { +const annotationWasChanged = function (oldAnnotation: T | void, newAnnotation: T): boolean { // A new annotation was created if (!oldAnnotation) { return true; From a9af7c59b54c4351a6a6b59465d13a9af29c79d3 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sat, 9 Jul 2022 18:31:44 +0200 Subject: [PATCH 4/5] refactor(gui): function now does what its name suggests --- .../gui/src/features/annotations/annotationSlice.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/api-editor/gui/src/features/annotations/annotationSlice.ts b/api-editor/gui/src/features/annotations/annotationSlice.ts index 9c91a61ec..1de6572f3 100644 --- a/api-editor/gui/src/features/annotations/annotationSlice.ts +++ b/api-editor/gui/src/features/annotations/annotationSlice.ts @@ -745,7 +745,7 @@ const withAuthorAndReviewers = function ( let authors = oldAnnotation?.authors ?? []; const reviewers = oldAnnotation?.reviewers ?? []; - if (annotationWasChanged(oldAnnotation, newAnnotation)) { + if (!oldAnnotation || annotationWasChanged(oldAnnotation, newAnnotation)) { authors = [...authors.filter((it) => it !== author), author]; } @@ -756,15 +756,12 @@ const withAuthorAndReviewers = function ( }; }; -const annotationWasChanged = function (oldAnnotation: T | void, newAnnotation: T): boolean { - // A new annotation was created - if (!oldAnnotation) { - return true; - } +const annotationWasChanged = function (oldAnnotation: T, newAnnotation: T): boolean { // Unify the metadata, so we only compare the actual annotation data const oldAnnotationWithoutMetadata = annotationWithoutMetadata(oldAnnotation); const newAnnotationWithoutMetadata = annotationWithoutMetadata(newAnnotation); + return !isEqual(oldAnnotationWithoutMetadata, newAnnotationWithoutMetadata); }; From a437bed3a4bee7ed608ad8e462f963b63ac95856 Mon Sep 17 00:00:00 2001 From: lars-reimann Date: Sat, 9 Jul 2022 16:36:06 +0000 Subject: [PATCH 5/5] style: apply automatic fixes of linters --- api-editor/gui/src/features/annotations/annotationSlice.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/api-editor/gui/src/features/annotations/annotationSlice.ts b/api-editor/gui/src/features/annotations/annotationSlice.ts index 1de6572f3..1e887ea1e 100644 --- a/api-editor/gui/src/features/annotations/annotationSlice.ts +++ b/api-editor/gui/src/features/annotations/annotationSlice.ts @@ -757,7 +757,6 @@ const withAuthorAndReviewers = function ( }; const annotationWasChanged = function (oldAnnotation: T, newAnnotation: T): boolean { - // Unify the metadata, so we only compare the actual annotation data const oldAnnotationWithoutMetadata = annotationWithoutMetadata(oldAnnotation); const newAnnotationWithoutMetadata = annotationWithoutMetadata(newAnnotation);