fix(report): correct the save of a revision into an existing series

extensions/cornerstone-dicom-seg/src/commandsModule.ts

@@ -321,9 +321,13 @@ const commandsModule = ({
         throw new Error('No segmentation found');
       }
 
-      const { label, predecessorImageId } = segmentation;
+      const { label, predecessorImageId, generatedLabel } = segmentation;
+      // The dialog offers the name that the user chose first, ahead of the
+      // remembered descriptions.  A generated name such as `Segmentation 3` is
+      // not such a name, so a generated name goes last instead.
+      const chosenLabel = label && label !== generatedLabel ? label : '';
       const defaultSeriesDescription =
-        label || (modality === 'RTSTRUCT' ? 'Contours' : 'Segmentation');
+        (!chosenLabel && label) || (modality === 'RTSTRUCT' ? 'Contours' : 'Segmentation');
 
       const {
         value: reportName,
@@ -337,6 +341,7 @@ const commandsModule = ({
         predecessorImageId,
         title: modality === 'RTSTRUCT' ? 'Save Contours' : 'Save Segmentation',
         modality,
+        itemName: chosenLabel,
         defaultSeriesDescription,
         enableDownload: true,
       });
@@ -363,7 +368,7 @@ const commandsModule = ({
           options: {
             // Resolve store overrides against the data source we are storing into.
             dataSource: dataSourceName,
-            SeriesDescription: series ? undefined : reportName || defaultSeriesDescription,
+            SeriesDescription: series ? undefined : reportName || label || defaultSeriesDescription,
             SeriesNumber: series ? undefined : seriesNumber,
             predecessorImageId: series,
           },

extensions/cornerstone/src/services/SegmentationService/SegmentationService.test.ts

@@ -1263,6 +1263,54 @@ describe('SegmentationService', () => {
       expect(retrievedSegmentationId).toEqual(expect.any(String));
     });
 
+    describe('generatedLabel', () => {
+      // `storeSegmentation` offers the label of a segmentation as the first name
+      // for a new series, but only when the user chose that name.  The service
+      // records the name that the service invents, so the save can tell the two
+      // apart.
+      const displaySet = {
+        imageIds: ['imageId'],
+        isDynamicVolume: false,
+        SeriesNumber: 1,
+        SeriesDescription: 'Series Description',
+        Modality: 'SEG',
+      } as unknown as AppTypes.DisplaySet;
+
+      const createWith = async (options?: Record<string, unknown>) => {
+        const stored = { segmentationId: 'created' } as cstTypes.Segmentation;
+
+        jest
+          .spyOn(imageLoader, 'createAndCacheDerivedLabelmapImages')
+          .mockReturnValue([{ imageId: 'imageId' }] as csTypes.IImage[]);
+        jest
+          .spyOn(cstSegmentation.state, 'getSegmentations')
+          .mockReturnValue([{ segmentationId: 'segmentationId' }] as cstTypes.Segmentation[]);
+        jest.spyOn(cstSegmentation.state, 'getSegmentation').mockReturnValue(stored);
+        jest.spyOn(service, 'addOrUpdateSegmentation').mockReturnValue(undefined);
+
+        await service.createLabelmapForDisplaySet(displaySet, options);
+        return stored;
+      };
+
+      it('records a label that the service invents', async () => {
+        const stored = await createWith();
+
+        expect(stored.generatedLabel).toBe('Segmentation 2');
+      });
+
+      it('records nothing for a label that the caller gives', async () => {
+        const stored = await createWith({ label: 'Liver' });
+
+        expect(stored.generatedLabel).toBeUndefined();
+      });
+
+      it('records a label that the caller reports as generated', async () => {
+        const stored = await createWith({ label: 'Segmentation 7', labelIsGenerated: true });
+
+        expect(stored.generatedLabel).toBe('Segmentation 7');
+      });
+    });
+
     it('should create a labelmap for a dynamic volume display set', async () => {
       const segmentationId = 'segmentationId';
       const displaySet = {

extensions/cornerstone/src/services/SegmentationService/SegmentationService.ts

@@ -428,6 +428,8 @@ class SegmentationService extends PubSubService implements ISegmentationServiceI
       segments?: { [segmentIndex: number]: Partial<cstTypes.Segment> };
       FrameOfReferenceUID?: string;
       label?: string;
+      /** The caller invented the label, so the user has not chosen a name. */
+      labelIsGenerated?: boolean;
     }
   ): Promise<string> {
     return this._createSegmentationForDisplaySet(displaySet, LABELMAP, options);
@@ -440,6 +442,8 @@ class SegmentationService extends PubSubService implements ISegmentationServiceI
       segments?: { [segmentIndex: number]: Partial<cstTypes.Segment> };
       FrameOfReferenceUID?: string;
       label?: string;
+      /** The caller invented the label, so the user has not chosen a name. */
+      labelIsGenerated?: boolean;
     }
   ): Promise<string> {
     return this._createSegmentationForDisplaySet(displaySet, CONTOUR, options);
@@ -461,6 +465,8 @@ class SegmentationService extends PubSubService implements ISegmentationServiceI
       segments?: { [segmentIndex: number]: Partial<cstTypes.Segment> };
       FrameOfReferenceUID?: string;
       label?: string;
+      /** The caller invented the label, so the user has not chosen a name. */
+      labelIsGenerated?: boolean;
     }
   ): Promise<string> {
     // Todo: random does not makes sense, make this better, like
@@ -526,6 +532,17 @@ class SegmentationService extends PubSubService implements ISegmentationServiceI
     }
 
     this.addOrUpdateSegmentation(segmentationPublicInput);
+
+    // `storeSegmentation` compares the label with this name, to tell a name that
+    // the user chose from a generated one such as `Segmentation 3`.
+    if (options?.labelIsGenerated || !options?.label) {
+      const segmentation = this.getSegmentation(segmentationId);
+
+      if (segmentation) {
+        segmentation.generatedLabel = label;
+      }
+    }
+
     return segmentationId;
   }
 

extensions/cornerstone/src/utils/createSegmentationForViewport.ts

@@ -64,6 +64,7 @@ export async function createSegmentationForViewport(
   const segmentationCreationOptions = {
     label,
     segmentationId,
+    labelIsGenerated: !options.label,
     segments: _createDefaultSegments(options.createInitialSegment),
   };
 

extensions/default/src/DicomWebDataSource/retrieveStudyMetadata.js

@@ -82,10 +82,31 @@ export function retrieveStudyMetadata(
  * Delete the cached study metadata retrieval promise to ensure that the browser will
  * re-retrieve the study metadata when it is next requested.
  *
+ * Promises are cached under `<data source name>:<StudyInstanceUID>` (see above),
+ * but every caller knows only the study — a data source exports this function
+ * directly, unbound, and the callers that matter (storing a derived artifact, and
+ * the microscopy save) hold a study UID and nothing else. Looking the bare UID up
+ * as a key therefore never matched, and this function had never removed anything:
+ * any re-retrieve after a store returned the promise resolved *before* it. Match
+ * on the study instead, across whichever data sources have cached it.
+ *
  * @param {String} StudyInstanceUID The UID of the Study to be removed from cache
  */
 export function deleteStudyMetadataPromise(StudyInstanceUID) {
-  if (StudyMetaDataPromises.has(StudyInstanceUID)) {
-    StudyMetaDataPromises.delete(StudyInstanceUID);
+  if (!StudyInstanceUID) {
+    return;
   }
+
+  const suffix = `:${StudyInstanceUID}`;
+
+  for (const promiseId of [...StudyMetaDataPromises.keys()]) {
+    if (promiseId === StudyInstanceUID || promiseId.endsWith(suffix)) {
+      StudyMetaDataPromises.delete(promiseId);
+    }
+  }
+}
+
+/** Test seam: the cached promises, so a test can assert what invalidation removed. */
+export function _getStudyMetadataPromiseCache() {
+  return StudyMetaDataPromises;
 }

extensions/default/src/DicomWebDataSource/retrieveStudyMetadata.test.js

@@ -0,0 +1,75 @@
+import {
+  deleteStudyMetadataPromise,
+  _getStudyMetadataPromiseCache,
+} from './retrieveStudyMetadata.js';
+
+const STUDY = '1.2.840.113619.2.55.3.1234';
+const OTHER_STUDY = '9.9.9';
+
+describe('deleteStudyMetadataPromise', () => {
+  beforeEach(() => {
+    _getStudyMetadataPromiseCache().clear();
+  });
+
+  // Promises are cached under `<data source name>:<StudyInstanceUID>`, but every
+  // caller holds only the study UID. Looking the bare UID up as a key matched
+  // nothing, so storing a derived artifact never invalidated anything and a
+  // re-retrieve returned the pre-save promise.
+  it('removes the promise cached under the data source qualified key', () => {
+    const cache = _getStudyMetadataPromiseCache();
+    cache.set(`dicomweb:${STUDY}`, 'stale');
+
+    deleteStudyMetadataPromise(STUDY);
+
+    expect(cache.has(`dicomweb:${STUDY}`)).toBe(false);
+  });
+
+  it('removes the study from every data source that cached it', () => {
+    const cache = _getStudyMetadataPromiseCache();
+    cache.set(`dicomweb:${STUDY}`, 'stale');
+    cache.set(`dicomwebproxy:${STUDY}`, 'stale');
+
+    deleteStudyMetadataPromise(STUDY);
+
+    expect(cache.size).toBe(0);
+  });
+
+  it('leaves other studies cached', () => {
+    const cache = _getStudyMetadataPromiseCache();
+    cache.set(`dicomweb:${STUDY}`, 'stale');
+    cache.set(`dicomweb:${OTHER_STUDY}`, 'keep');
+
+    deleteStudyMetadataPromise(STUDY);
+
+    expect(cache.has(`dicomweb:${OTHER_STUDY}`)).toBe(true);
+  });
+
+  // A study whose UID is a suffix of another must not be caught by the match.
+  it('does not remove a study whose UID merely ends with the same digits', () => {
+    const cache = _getStudyMetadataPromiseCache();
+    cache.set(`dicomweb:${STUDY}`, 'stale');
+    cache.set(`dicomweb:77${STUDY}`, 'keep');
+
+    deleteStudyMetadataPromise(STUDY);
+
+    expect(cache.has(`dicomweb:77${STUDY}`)).toBe(true);
+  });
+
+  it('still removes an unqualified key, for any caller that cached one', () => {
+    const cache = _getStudyMetadataPromiseCache();
+    cache.set(STUDY, 'stale');
+
+    deleteStudyMetadataPromise(STUDY);
+
+    expect(cache.has(STUDY)).toBe(false);
+  });
+
+  it('does nothing without a study', () => {
+    const cache = _getStudyMetadataPromiseCache();
+    cache.set(`dicomweb:${STUDY}`, 'keep');
+
+    deleteStudyMetadataPromise(undefined);
+
+    expect(cache.size).toBe(1);
+  });
+});

extensions/default/src/Panels/createReportDialogPrompt.tsx

@@ -11,9 +11,11 @@ import PROMPT_RESPONSES from '../utils/_shared/PROMPT_RESPONSES';
  *     from.  That is the series the dialog offers to extend, and it defaults to
  *     extending it instead of creating a new series.  Without one, the dialog
  *     only offers to create a new series.
- *   - `defaultSeriesDescription` is the series description offered when a new
- *     series is being created, typically the name of the thing being saved such
- *     as the segmentation name or 'Contours'.
+ *   - `itemName` is the name that the user chose for the item, such as the
+ *     segmentation name.  A new series offers this name first.  A generated name
+ *     is not such a name, and belongs in `defaultSeriesDescription`.
+ *   - `defaultSeriesDescription` is the name for an item that has no other name,
+ *     such as 'Contours' or 'Measurements'.  A new series offers this name last.
  *   - `itemType` is the type of item being stored, used as the key that the
  *     series descriptions used before are remembered under.  Defaults to the
  *     modality, so that segmentations, contours and reports are remembered
@@ -22,14 +24,12 @@ import PROMPT_RESPONSES from '../utils/_shared/PROMPT_RESPONSES';
  *     descriptions to remember and offer for this type of item, 0 to remember
  *     none of them.
  *
- * The dialog offers exactly two destinations, and says which one is in effect:
- *   - `New Series` creates a new series, with an editable series number
- *     (defaulting to one past the existing series of this modality) and series
- *     description.  The description defaults to the one last used for this type
- *     of item, or to `defaultSeriesDescription` when there isn't one, and both
- *     are offered as completions of what gets typed.
- *   - `Extend Existing` stores into the series the data was loaded from, which
- *     keeps its own series number and description, so neither is editable.
+ * The dialog offers three destinations - `Save to current`, `Save as new` and
+ * `Replace existing` - and says which one is in effect.  Each destination stores
+ * all of the current data as one object, and the dialog merges nothing.  The
+ * behaviour doc describes the destinations, the series that the dialog offers,
+ * and the names for a new series:
+ * `platform/docs/docs/behaviours/report-dialog-save-destinations.md`.
  *
  * The response is:
  *   - `value`, the series description of the object/series being created.  When
@@ -52,6 +52,7 @@ export default function CreateReportDialogPrompt({
   modality = 'SR',
   minSeriesNumber = 0,
   predecessorImageId,
+  itemName = '',
   defaultSeriesDescription = '',
   itemType,
   rememberedDescriptionCount = 5,
@@ -90,6 +91,7 @@ export default function CreateReportDialogPrompt({
         dataSources: allowMultipleDataSources ? dataSources : undefined,
         predecessorImageId,
         minSeriesNumber,
+        itemName,
         defaultSeriesDescription,
         itemType,
         rememberedDescriptionCount,