diff --git a/packages/frontend-2/lib/viewer/composables/commentManagement.ts b/packages/frontend-2/lib/viewer/composables/commentManagement.ts index 0562dbb1f..983e47101 100644 --- a/packages/frontend-2/lib/viewer/composables/commentManagement.ts +++ b/packages/frontend-2/lib/viewer/composables/commentManagement.ts @@ -129,7 +129,7 @@ export function useSubmitComment() { projectId: projectId.value, resourceIdString: resourceIdString.value, content, - viewerState: serialize(), + viewerState: serialize({ concreteResourceIdString: true }), screenshot } } diff --git a/packages/frontend-2/lib/viewer/composables/serialization.ts b/packages/frontend-2/lib/viewer/composables/serialization.ts index 1060a497e..8d0d2a4ae 100644 --- a/packages/frontend-2/lib/viewer/composables/serialization.ts +++ b/packages/frontend-2/lib/viewer/composables/serialization.ts @@ -14,7 +14,37 @@ type SerializedViewerState = SpeckleViewer.ViewerState.SerializedViewerState export function useStateSerialization() { const state = useInjectedViewerState() - const serialize = (): SerializedViewerState => { + /** + * We don't want to save a comment w/ implicit identifiers like ones that only have a model ID or a folder prefix, because + * those can resolve to completely different versions/objects as time goes on + */ + const buildConcreteResourceIdString = () => { + const resources = state.resources.response.resourceItems + const builder = SpeckleViewer.ViewerRoute.resourceBuilder() + + for (const resource of resources.value) { + if (resource.modelId && resource.versionId) { + builder.addModel(resource.modelId, resource.versionId) + } else { + builder.addObject(resource.objectId) + } + } + + const finalString = builder.toString() + return finalString || state.resources.request.resourceIdString.value + } + + const serialize = ( + options?: Partial<{ + /** + * Instead of saving the current resourceIdString value, build a more concrete one that specifies exact version & object ids, so that the + * string doesn't resolve to different objects in the future. Useful when serializing state for posterity (e.g. for new comment threads) + */ + concreteResourceIdString: boolean + }> + ): SerializedViewerState => { + const { concreteResourceIdString } = options || {} + const camControls = state.viewer.instance.cameraHandler.activeCam.controls const box = state.viewer.instance.getCurrentSectionBox() @@ -33,7 +63,9 @@ export function useStateSerialization() { }, resources: { request: { - resourceIdString: state.resources.request.resourceIdString.value, + resourceIdString: concreteResourceIdString + ? buildConcreteResourceIdString() + : state.resources.request.resourceIdString.value, threadFilters: { ...state.resources.request.threadFilters.value } } }, diff --git a/packages/server/modules/core/services/commit/viewerResources.ts b/packages/server/modules/core/services/commit/viewerResources.ts index c3edc503f..c46d3597d 100644 --- a/packages/server/modules/core/services/commit/viewerResources.ts +++ b/packages/server/modules/core/services/commit/viewerResources.ts @@ -404,8 +404,8 @@ export function doViewerResourcesFit( export function viewerResourcesToString(resources: ViewerResourceItem[]): string { const builder = SpeckleViewer.ViewerRoute.resourceBuilder() for (const resource of resources) { - if (resource.modelId) { - builder.addModel(resource.modelId, resource.versionId || undefined) + if (resource.modelId && resource.versionId) { + builder.addModel(resource.modelId, resource.versionId) } else { builder.addObject(resource.objectId) }