Repository: DigitalSlideArchive/HistomicsUI (language: python)

Issue:
Annotation import adds 'default' style
When importing a .json defining annotation styles the "default" style group is automatically added. The checkbox to overwrite existing styles has no effect on this.

Desired behavior - if the "default" style does not exist in the .json, don't create it.


Discussion / hints:
+1: Default style is not needed and confuses the pathologists. We cannot remove it.

Fix patch:
diff --git a/docs/config_options.rst b/docs/config_options.rst
new file mode 100644
index 00000000..775cb712
--- /dev/null
+++ b/docs/config_options.rst
@@ -0,0 +1,59 @@
+Configuration Options
+=====================
+
+YAML Configuration Files
+------------------------
+
+See the `large_image <https://github.com/girder/large_image/blob/master/docs/girder_config_options.rst>`_ documentation for general yaml configuration file details on specifying different values for different users and groups.
+
+.histomicsui_config.yaml
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+This is used to specify annotation groups available for marking annotations.
+
+::
+
+    ---
+    annotationGroups:
+      # If replaceGroups isn't present or false, groups will be merged with
+      # existing groups.  Those that have the same name will be updated.  If
+      # true, then all existing groups are removed.
+      replaceGroups: true
+      # The name of the default group.  If an existing annotation does not have
+      # a group, it will be given this group name.
+      defaultGroup: default
+      groups:
+        -
+          # The id is required.
+          id: default
+          # The fillColor and lineColor are of the form
+          # "rgb(<red>, <green>, <blue>)" or
+          # "rgba(<red>, <green>, <blue>, <alpha>)"
+          # where the colors are on a scale of 0 to 255 and the alpha is from
+          # 0 to 1.
+          fillColor: rgba(0,0,0,0)
+          lineColor: rgb(0,0,0)
+          # lineWidth is in pixels
+          lineWidth: 2
+        -
+          id: Sample
+          # Label is optional.
+          label: Sample Label
+          fillColor: rgba(0, 0, 0, 0.47)
+          lineColor: rgb(255, 80, 238)
+          lineWidth: 4
+        -
+          id: Red
+          fillColor: rgba(255, 0, 0, 0.25)
+          lineColor: rgb(255, 0, 0)
+          lineWidth: 2
+        -
+          id: Green
+          fillColor: rgba(0, 128, 0, 0.25)
+          lineColor: rgb(0, 128, 0)
+          lineWidth: 2
+        -
+          id: Blue
+          fillColor: rgba(0, 0, 255, 0.25)
+          lineColor: rgb(0, 0, 255)
+          lineWidth: 2
diff --git a/histomicsui/web_client/dialogs/editElement.js b/histomicsui/web_client/dialogs/editElement.js
index 9035bde2..4a6d9976 100644
--- a/histomicsui/web_client/dialogs/editElement.js
+++ b/histomicsui/web_client/dialogs/editElement.js
@@ -58,7 +58,7 @@ var EditElement = View.extend({
         var label = this.$('#h-element-label').val();
         data.label = label ? { value: label } : {};
         var group = this.$('#h-group-name').val();
-        data.group = group && group !== 'default' ? group : undefined;
+        data.group = group && group !== this._defaultGroup ? group : undefined;
 
         var lineWidth = this.$('#h-element-line-width').val();
         if (lineWidth) {
@@ -126,8 +126,9 @@ var dialog = new EditElement({
  * @param {ElementModel} annotationElement The element to edit
  * @returns {EditAnnotation} The dialog's view
  */
-function show(annotationElement) {
+function show(annotationElement, defaultGroup) {
     dialog.annotationElement = annotationElement;
+    dialog._defaultGroup = defaultGroup || 'default';
     dialog.setElement('#g-dialog-container').render();
     return dialog;
 }
diff --git a/histomicsui/web_client/dialogs/editStyleGroups.js b/histomicsui/web_client/dialogs/editStyleGroups.js
index f3de19ae..0c11cd78 100644
--- a/histomicsui/web_client/dialogs/editStyleGroups.js
+++ b/histomicsui/web_client/dialogs/editStyleGroups.js
@@ -68,7 +68,7 @@ const EditStyleGroups = View.extend({
         }
         data.label = label ? {value: label} : {};
         const group = data.id;
-        data.group = group && group !== 'default' ? group : undefined;
+        data.group = group && group !== this._defaultGroup ? group : undefined;
 
         const lineWidth = this.$('#h-element-line-width').val();
         if (lineWidth) {
@@ -155,8 +155,8 @@ const EditStyleGroups = View.extend({
             }
             this.collection.reset(styleModels);
             // make sure we have at least a default style
-            if (!this.collection.get('default')) {
-                this.collection.push(new StyleModel({id: 'default'}));
+            if (!this.collection.get(this._defaultGroup)) {
+                this.collection.push(new StyleModel({id: this._defaultGroup}));
             }
             this.model.set(this.collection.at(0).toJSON());
             if (oldid && this.collection.get(oldid)) {
@@ -238,8 +238,8 @@ const EditStyleGroups = View.extend({
                     this.collection.add(styleModels, {merge: true});
                 }
                 // make sure we have at least a default style
-                if (!this.collection.get('default')) {
-                    this.collection.push(new StyleModel({id: 'default'}));
+                if (!this.collection.get(this._defaultGroup)) {
+                    this.collection.push(new StyleModel({id: this._defaultGroup}));
                 }
                 this.model.set(this.collection.at(0).toJSON());
                 this.collection.each((model) => { model.save(); });
@@ -310,13 +310,14 @@ const EditStyleGroupsDialog = View.extend({
  * @param {StyleGroupCollection} collection
  * @returns {EditStyleGroup} The dialog's view
  */
-function show(style, groups) {
+function show(style, groups, defaultGroup) {
     const dialog = new EditStyleGroupsDialog({
         parentView: null,
         collection: groups,
         model: style,
         el: $('#g-dialog-container')
     });
+    dialog.form._defaultGroup = defaultGroup || 'default';
     return dialog.render();
 }
 
diff --git a/histomicsui/web_client/panels/AnnotationSelector.js b/histomicsui/web_client/panels/AnnotationSelector.js
index c7176e8e..3a34386a 100644
--- a/histomicsui/web_client/panels/AnnotationSelector.js
+++ b/histomicsui/web_client/panels/AnnotationSelector.js
@@ -548,7 +548,7 @@ var AnnotationSelector = Panel.extend({
             if (name !== 'Other') {
                 groupCount[name] = annotations.length;
             } else {
-                groupCount.default = annotations.length;
+                groupCount[this.parentView._defaultGroup || 'default'] = annotations.length;
             }
         });
         this.trigger('h:groupCount', groupCount);
diff --git a/histomicsui/web_client/panels/DrawWidget.js b/histomicsui/web_client/panels/DrawWidget.js
index 3cb1b0c7..be20f56a 100644
--- a/histomicsui/web_client/panels/DrawWidget.js
+++ b/histomicsui/web_client/panels/DrawWidget.js
@@ -61,15 +61,15 @@ var DrawWidget = Panel.extend({
 
         this._highlighted = {};
         this._groups = new StyleCollection();
-        this._style = new StyleModel({id: 'default'});
+        this._style = new StyleModel({id: this.parentView._defaultGroup});
         this.listenTo(this._groups, 'add change', this._handleStyleGroupsUpdate);
         this.listenTo(this._groups, 'remove', this.render);
         this.listenTo(this.collection, 'add remove reset', this._recalculateGroupAggregation);
         this.listenTo(this.collection, 'change update reset', this.render);
         this._groups.fetch().done(() => {
             // ensure the default style exists
-            if (this._groups.has('default')) {
-                this._style.set(this._groups.get('default').toJSON());
+            if (this._groups.has(this.parentView._defaultGroup)) {
+                this._style.set(this._groups.get(this.parentView._defaultGroup).toJSON());
             } else {
                 this._groups.add(this._style.toJSON());
                 this._groups.get(this._style.id).save();
@@ -218,14 +218,14 @@ var DrawWidget = Panel.extend({
                 if (label) {
                     newLabel = label;
                 } else if (['point', 'polyline', 'rectangle', 'ellipse', 'circle'].includes(elemType)) {
-                    newLabel = `${group || 'default'} ${elemType} ${parseInt(oldLabel[oldLabel.length - 1] || '')}`;
+                    newLabel = `${group || this.parentView._defaultGroup} ${elemType} ${parseInt(oldLabel[oldLabel.length - 1] || '')}`;
                 } else {
                     newLabel = oldLabel;
                 }
                 this.$(`.h-element[data-id="${id}"] .h-element-label`).text(newLabel).attr('title', label);
                 if (origGroup !== group && ['point', 'polyline', 'rectangle', 'ellipse', 'circle'].includes(elemType)) {
-                    this.updateCount(origGroup || 'default', -1);
-                    this.updateCount(group || 'default', 1);
+                    this.updateCount(origGroup || this.parentView._defaultGroup, -1);
+                    this.updateCount(group || this.parentView._defaultGroup, 1);
                 }
             }
             this._skipRenderHTML = true;
@@ -309,7 +309,7 @@ var DrawWidget = Panel.extend({
             id = this._getId(evt);
         }
         if (['point', 'polyline', 'rectangle', 'ellipse', 'circle'].includes(this.collection.get(id).attributes.type)) {
-            this.updateCount(this.collection.get(id).attributes.group || 'default', -1);
+            this.updateCount(this.collection.get(id).attributes.group || this.parentView._defaultGroup, -1);
         } else if (this.collection.get(id).attributes.type === 'pixelmap') {
             this.countPixelmap(this.collection.get(id), -1);
         }
@@ -331,6 +331,7 @@ var DrawWidget = Panel.extend({
             drawWidgetElement({
                 elements: elements,
                 style: this._style.id,
+                defaultGroup: this.parentView._defaultGroup,
                 highlighted: this._highlighted,
                 firstRender: false,
                 updateCount: this.updateCount,
@@ -797,7 +798,7 @@ var DrawWidget = Panel.extend({
     countPixelmap(pixelmap, operation) {
         let toChange = {};
         for (let ix = 0; ix < (pixelmap.get('boundaries') ? pixelmap.get('values').length / 2 : pixelmap.get('values').length); ix++) {
-            let groupName = (pixelmap.get('categories')[pixelmap.get('values')[ix]]).label || 'default';
+            let groupName = (pixelmap.get('categories')[pixelmap.get('values')[ix]]).label || this.parentView._defaultGroup;
             if (toChange[groupName]) {
                 toChange[groupName]++;
             } else {
@@ -819,9 +820,9 @@ var DrawWidget = Panel.extend({
      */
     _setStyleGroup(group) {
         this._style.set(group);
-        if (!this._style.get('group') && this._style.id !== 'default') {
+        if (!this._style.get('group') && this._style.id !== this.parentView._defaultGroup) {
             this._style.set('group', this._style.id);
-        } else if (this._style.get('group') && this._style.id === 'default') {
+        } else if (this._style.get('group') && this._style.id === this.parentView._defaultGroup) {
             this._style.unset('group');
         }
         this.$('.h-style-group').val(group.id);
@@ -966,7 +967,8 @@ var DrawWidget = Panel.extend({
     },
 
     _styleGroupEditor() {
-        var dlg = editStyleGroups(this._style, this._groups);
+        console.log(this);
+        var dlg = editStyleGroups(this._style, this._groups, this.parentView._defaultGroup);
         dlg.$el.on('hidden.bs.modal', () => {
             this.render();
             this.parentView.trigger('h:styleGroupsEdited', this._groups);
diff --git a/histomicsui/web_client/templates/panels/drawWidgetElement.pug b/histomicsui/web_client/templates/panels/drawWidgetElement.pug
index 4c20db38..cb554225 100644
--- a/histomicsui/web_client/templates/panels/drawWidgetElement.pug
+++ b/histomicsui/web_client/templates/panels/drawWidgetElement.pug
@@ -7,7 +7,7 @@ if elements.length
     -
       var classes = highlighted[element.id] ? ['h-highlight-element'] : [];
       element = element.toJSON();
-      var groupName = element['group'] ? element['group'] : 'default';
+      var groupName = element['group'] ? element['group'] : defaultGroup;
       var type = element.type == 'polyline' ? (element.closed ? 'polygon' : 'line') : element.type;
       var displayId = ++displayIdOffset + displayIdStart;
       var label = ((element.label || {}).value || type);
diff --git a/histomicsui/web_client/views/body/ImageView.js b/histomicsui/web_client/views/body/ImageView.js
index 0d9c0045..5652caa3 100644
--- a/histomicsui/web_client/views/body/ImageView.js
+++ b/histomicsui/web_client/views/body/ImageView.js
@@ -46,6 +46,7 @@ var ImageView = View.extend({
         'mousemove .geojs-map': '_trackMousePosition'
     },
     initialize(settings) {
+        this._defaultGroup = 'default';
         this.viewerWidget = null;
         this._mouseClickQueue = [];
         this._openId = null;
@@ -181,6 +182,7 @@ var ImageView = View.extend({
         this.pixelmapContextMenu.setElement(this.$('#h-pixelmap-context-menu')).render();
 
         if (this.model.id) {
+            this._getConfig(this.model.id);
             this._openId = this.model.id;
             if (this.viewerWidget) {
                 this.viewerWidget.destroy();
@@ -518,11 +520,11 @@ var ImageView = View.extend({
 
     _updatePixelmapElements(pixelmapElements, annotation) {
         const groups = new StyleCollection();
-        const defaultStyle = new StyleModel({ id: 'default' });
+        const defaultStyle = new StyleModel({ id: this._defaultGroup });
         groups.fetch().done(() => {
-            if (!groups.has('default')) {
+            if (!groups.has(this._defaultGroup)) {
                 groups.add(defaultStyle.toJSON());
-                groups.get('default').save();
+                groups.get(this._defaultGroup).save();
             }
             _.each(pixelmapElements, (pixelmap) => {
                 this._reconcilePixelmapCategories(pixelmap.get('id'), groups, annotation);
@@ -583,9 +585,9 @@ var ImageView = View.extend({
         });
 
         // move the default category to index 0 and adjust data array if needed
-        const originalDefaultIndex = _.findIndex(newCategories, { label: 'default' });
-        const updatedCategories = _.where(newCategories, { label: 'default' })
-            .concat(_.reject(newCategories, { label: 'default' }));
+        const originalDefaultIndex = _.findIndex(newCategories, { label: this._defaultGroup });
+        const updatedCategories = _.where(newCategories, { label: this._defaultGroup })
+            .concat(_.reject(newCategories, { label: this._defaultGroup }));
         pixelmap.set('categories', updatedCategories);
         if (originalDefaultIndex !== 0) {
             const originalData = pixelmap.get('values');
@@ -1328,7 +1330,7 @@ var ImageView = View.extend({
     _editElement(element) {
         const annotation = this.annotations.get(element.originalAnnotation);
         this._editAnnotation(annotation);
-        editElement(annotation.elements().get(element.id));
+        editElement(annotation.elements().get(element.id), this._defaultGroup);
     },
 
     _editElementShape(element, annotationId) {
@@ -1501,6 +1503,36 @@ var ImageView = View.extend({
             });
             return null;
         });
+    },
+    _getConfig(modelId) {
+        restRequest({
+            url: `folder/${this.model.get('folderId')}/yaml_config/.histomicsui_config.yaml`
+        }).done((val) => {
+            if (!val || this.model.id !== modelId) {
+                return;
+            }
+            if (val.annotationGroups) {
+                let groups = new StyleCollection();
+                groups.fetch().done(() => {
+                    if (!val || this.model.id !== modelId) {
+                        return;
+                    }
+                    this._defaultGroup = val.annotationGroups.defaultGroup || 'default';
+                    if ((val.annotationGroups.groups || []).length) {
+                        if (val.annotationGroups.replaceGroups) {
+                            while (groups.length) {
+                                groups.first().destroy();
+                            }
+                        }
+                        val.annotationGroups.groups.forEach((group) => {
+                            group.label = group.label ? {value: group.label} : undefined;
+                            groups.add(group);
+                        });
+                        groups.each((model) => { model.save(); });
+                    }
+                });
+            }
+        });
     }
 });
 export default ImageView;
diff --git a/histomicsui/web_client/views/popover/AnnotationContextMenu.js b/histomicsui/web_client/views/popover/AnnotationContextMenu.js
index 9111321f..8d5c86db 100644
--- a/histomicsui/web_client/views/popover/AnnotationContextMenu.js
+++ b/histomicsui/web_client/views/popover/AnnotationContextMenu.js
@@ -39,7 +39,7 @@ const AnnotationContextMenu = View.extend({
         this.collection.each((element) => {
             if (this.parentView.drawWidget && this.parentView.activeAnnotation.id === element.originalAnnotation.id) {
                 if (['point', 'polyline', 'rectangle', 'ellipse', 'circle'].includes(element.attributes.type)) {
-                    this.parentView.drawWidget.updateCount(element.attributes.group || 'default', -1);
+                    this.parentView.drawWidget.updateCount(element.attributes.group || this.parentView._defaultGroup, -1);
                 } else if (element.attributes.type === 'pixelmap') {
                     this.parentView.drawWidget.countPixelmap(element, -1);
                 }
@@ -60,14 +60,14 @@ const AnnotationContextMenu = View.extend({
         this.trigger('h:close');
     },
     _setStyleDefinition(group) {
-        const style = this.styles.get({ id: group }) || this.styles.get({ id: 'default' });
+        const style = this.styles.get({ id: group }) || this.styles.get({ id: this.parentView._defaultGroup });
         const styleAttrs = Object.assign({}, style.toJSON());
         delete styleAttrs.id;
         this.collection.each((element) => { /* eslint-disable backbone/no-silent */
             if (this.parentView.drawWidget && this.parentView.activeAnnotation.id === element.originalAnnotation.id &&
                 element.attributes.group !== group && ['point', 'polyline', 'rectangle', 'ellipse', 'circle'].includes(element.attributes.type)) {
-                this.parentView.drawWidget.updateCount(element.attributes.group || 'default', -1);
-                this.parentView.drawWidget.updateCount(group || 'default', 1);
+                this.parentView.drawWidget.updateCount(element.attributes.group || this.parentView._defaultGroup, -1);
+                this.parentView.drawWidget.updateCount(group || this.parentView._defaultGroup, 1);
             }
             if (group) {
                 styleAttrs.group = group;


Test patch:
diff --git a/tests/test_files/.histomicsui_config.yaml b/tests/test_files/.histomicsui_config.yaml
new file mode 100644
index 00000000..635e259b
--- /dev/null
+++ b/tests/test_files/.histomicsui_config.yaml
@@ -0,0 +1,44 @@
+---
+annotationGroups:
+  # If replaceGroups isn't present or false, groups will be merged with
+  # existing groups.  Those that have the same name will be updated.  If
+  # true, then all existing groups are removed.
+  replaceGroups: true
+  # The name of the default group.  If an existing annotation does not have
+  # a group, it will be given this group name.
+  defaultGroup: default
+  groups:
+    -
+      # The id is required.
+      id: default
+      # The fillColor and lineColor are of the form
+      # "rgb(<red>, <green>, <blue>)" or
+      # "rgba(<red>, <green>, <blue>, <alpha>)"
+      # where the colors are on a scale of 0 to 255 and the alpha is from
+      # 0 to 1.
+      fillColor: rgba(0,0,0,0)
+      lineColor: rgb(0,0,0)
+      # lineWidth is in pixels
+      lineWidth: 2
+    -
+      id: Sample
+      # Label is optional.
+      label: Sample Label
+      fillColor: rgba(0, 0, 0, 0.47)
+      lineColor: rgb(255, 80, 238)
+      lineWidth: 4
+    -
+      id: Red
+      fillColor: rgba(255, 0, 0, 0.25)
+      lineColor: rgb(255, 0, 0)
+      lineWidth: 2
+    -
+      id: Green
+      fillColor: rgba(0, 128, 0, 0.25)
+      lineColor: rgb(0, 128, 0)
+      lineWidth: 2
+    -
+      id: Blue
+      fillColor: rgba(0, 0, 255, 0.25)
+      lineColor: rgb(0, 0, 255)
+      lineWidth: 2
diff --git a/tests/test_web_client.py b/tests/test_web_client.py
index b8056e18..df3cd25c 100644
--- a/tests/test_web_client.py
+++ b/tests/test_web_client.py
@@ -42,6 +42,12 @@ def makeResources(server, fsAssetstore, admin, user):
     annotation = Annotation().setAccessList(annotation, {}, force=True, save=False)
     annotation = Annotation().setPublic(annotation, True, save=True)
 
+    Folder().createFolder(user, 'subfolder', parentType='user', public=True, creator=user)
+    utilities.uploadTestFile('.histomicsui_config.yaml', user, fsAssetstore, 'subfolder')
+    utilities.uploadExternalFile(
+        'sample_svs_image.TCGA-DU-6399-01A-01-TS1.e8eb65de-d63e-42db-af6f-14fefbbdf7bd.svs',
+        user, fsAssetstore, folderName='subfolder', name='image.svs')
+
 
 class MockSlicerCLIWebResource(Resource):
     """
diff --git a/tests/web_client_specs/annotationSpec.js b/tests/web_client_specs/annotationSpec.js
index e89833be..94833d13 100644
--- a/tests/web_client_specs/annotationSpec.js
+++ b/tests/web_client_specs/annotationSpec.js
@@ -538,7 +538,7 @@ girderTest.promise.done(function () {
                 }, 'two annotations to merge into one');
             });
             it('test a brush operation', function () {
-                var annotCount, toggle;
+                var annotCount;
                 runs(function () {
                     annotCount = $('.h-elements-container .h-element').length;
                     $('.h-draw[data-type="brush"]').click();
@@ -1755,4 +1755,49 @@ girderTest.promise.done(function () {
             });
         });
     });
+
+    describe('Effect of config file', function () {
+        it('logout', girderTest.logout());
+        it('login', function () {
+            $('.modal-dialog .modal-footer .btn-default').click();
+            huiTest.login();
+        });
+        it('open image', function () {
+            huiTest.openImage('image.svs', ['subfolder']);
+        });
+        it('create a new annotation', function () {
+            girderTest.waitForLoad();
+            runs(function () {
+                $('.h-create-annotation').click();
+            });
+            girderTest.waitForDialog();
+            runs(function () {
+                var nameInput = $('#h-annotation-name');
+                expect(nameInput.length).toBe(1);
+
+                nameInput.val('drawn c1');
+                $('.h-submit').click();
+            });
+            girderTest.waitForLoad();
+            runs(function () {
+                expect($('.h-style-group option[value="Red"]').length).toBe(1);
+            });
+        });
+        it('delete the annotation we just created', function () {
+            runs(function () {
+                $('.h-annotation-selector .h-annotation:contains("drawn c1") .h-delete-annotation').click();
+            });
+            girderTest.waitForDialog();
+            runs(function () {
+                $('.h-submit').click();
+            });
+            girderTest.waitForLoad();
+            waitsFor(function () {
+                return $('.h-annotation-selector .h-annotation:contains("drawn c1")').length === 0;
+            }, '"drawn c1" to be deleted');
+        });
+        it('open original image', function () {
+            huiTest.openImage('image');
+        });
+    });
 });
diff --git a/tests/web_client_specs/huiTest.js b/tests/web_client_specs/huiTest.js
index 3a1d5ac4..9e57bd71 100644
--- a/tests/web_client_specs/huiTest.js
+++ b/tests/web_client_specs/huiTest.js
@@ -40,7 +40,7 @@
         });
     }
 
-    function openImage(name) {
+    function openImage(name, folders) {
         var imageId;
         var deferred = $.Deferred();
 
@@ -65,9 +65,18 @@
             return $('#g-dialog-container .g-folder-list-link').length > 0;
         }, 'Hierarchy widget to render');
 
-        runs(function () {
-            $('.g-folder-list-link:contains("Public")').click();
-        });
+        if (!folders) {
+            runs(function () {
+                $('.g-folder-list-link:contains("Public")').click();
+            });
+        } else {
+            folders.forEach(function (name) {
+                runs(function () {
+                    $('.g-folder-list-link:contains("' + name + '")').click();
+                });
+                girderTest.waitForDialog();
+            });
+        }
 
         waitsFor(function () {
             return $('.g-item-list-link').length > 0;