From f66543d31c663fb0f3439e945ed16544ba53f15b Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Tue, 21 Jul 2026 08:49:41 -0400 Subject: [PATCH 1/9] Add allowedGroups utility to read/validate allowed_groups metadata --- .../web_client/utilities/allowedGroups.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 histomicsui/web_client/utilities/allowedGroups.js diff --git a/histomicsui/web_client/utilities/allowedGroups.js b/histomicsui/web_client/utilities/allowedGroups.js new file mode 100644 index 00000000..790301c5 --- /dev/null +++ b/histomicsui/web_client/utilities/allowedGroups.js @@ -0,0 +1,25 @@ +import _ from 'underscore'; + +/** + * Read and validate the `allowed_groups` metadata on an annotation. + * + * The value is expected to live at `annotation.get('annotation').attributes.allowed_groups` + * and be an array of strings. Any other value (missing, not an array, empty array, etc.) is + * treated as "unrestricted" and returns `null`. + * + * @param {AnnotationModel} annotation The annotation to check. + * @returns {string[]|null} The list of allowed group names, or null if there are no restrictions. + */ +function getAllowedGroups(annotation) { + if (!annotation) return null; + + const attributes = (annotation.get('annotation') || {}).attributes || {}; + const allowedGroups = attributes.allowed_groups; + + if (!_.isArray(allowedGroups)) return null; + + const filtered = _.uniq(allowedGroups.filter((group) => _.isString(group) && group.length)); + return filtered.length ? filtered : null; +} + +export default getAllowedGroups; From 8e5aadceab57fab1ee239f00d819cf2e054bcef2 Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Tue, 21 Jul 2026 08:55:38 -0400 Subject: [PATCH 2/9] Restrict Draw panel to allowed groups --- histomicsui/web_client/panels/DrawWidget.js | 43 ++++++++++++++++++- .../templates/panels/drawWidget.pug | 2 +- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/histomicsui/web_client/panels/DrawWidget.js b/histomicsui/web_client/panels/DrawWidget.js index 5f85572c..6781c412 100644 --- a/histomicsui/web_client/panels/DrawWidget.js +++ b/histomicsui/web_client/panels/DrawWidget.js @@ -15,6 +15,7 @@ import StyleCollection from '../collections/StyleCollection'; import StyleModel from '../models/StyleModel'; import editElement from '../dialogs/editElement'; import editStyleGroups from '../dialogs/editStyleGroups'; +import getAllowedGroups from '../utilities/allowedGroups'; import drawWidget from '../templates/panels/drawWidget.pug'; import drawWidgetElement from '../templates/panels/drawWidgetElement.pug'; import '../stylesheets/panels/drawWidget.styl'; @@ -78,6 +79,7 @@ var DrawWidget = Panel.extend({ if (this._editOptions.style && this._groups.get(this._editOptions.style)) { this._setStyleGroup(this._groups.get(this._editOptions.style).toJSON()); } + this._restrictStyleToAllowedGroups(); }); this.on('h:mouseon', (model) => { if (model && model.id) { @@ -113,7 +115,7 @@ var DrawWidget = Panel.extend({ this.$el.html(drawWidget({ title: 'Draw', elements: this.collection.models, - groups: this._groups, + groups: this._groupsForDisplay(), style: this._style.id, defaultGroup: this.parentView._defaultGroup, highlighted: this._highlighted, @@ -1071,10 +1073,49 @@ var DrawWidget = Panel.extend({ }, _handleStyleGroupsUpdate() { + this._restrictStyleToAllowedGroups(); this._debounceRender(); this.trigger('h:styleGroupsUpdated', this._groups); }, + /** + * Get the current annotation's `allowed_groups` metadata, if any. + * + * @returns {string[]|null} The list of allowed group names, or null if the current annotation + * has no valid restriction. + */ + _getAllowedGroups() { + return getAllowedGroups(this.annotation); + }, + + /** + * Return the style groups that should be offered to the user given the current annotation's + * `allowed_groups` restriction, if any, sorted alphabetically by id. + * + * @returns {object[]} A list of plain style group attribute objects. + */ + _groupsForDisplay() { + const allowed = this._getAllowedGroups(); + const groups = allowed ? this._groups.filter((group) => allowed.includes(group.id)) : this._groups.models; + return _.sortBy(groups, 'id').map((group) => group.toJSON()); + }, + + /** + * If the current annotation restricts its elements to a set of `allowed_groups` and the + * currently selected style is not one of them, switch to the first allowed group that exists. + */ + _restrictStyleToAllowedGroups() { + const allowed = this._getAllowedGroups(); + if (!allowed || allowed.includes(this._style.id)) return; + + const candidates = this._groups.filter((group) => allowed.includes(group.id)) + .map((group) => group.id) + .sort(); + if (candidates.length) { + this._setStyleGroup(this._groups.get(candidates[0]).toJSON()); + } + }, + _highlightElement(evt) { const id = $(evt.currentTarget).data('id'); const annotType = this.collection._byId[id].get('type'); diff --git a/histomicsui/web_client/templates/panels/drawWidget.pug b/histomicsui/web_client/templates/panels/drawWidget.pug index 6a2374d9..407742ef 100644 --- a/histomicsui/web_client/templates/panels/drawWidget.pug +++ b/histomicsui/web_client/templates/panels/drawWidget.pug @@ -6,7 +6,7 @@ block title block content .input-group.input-group-sm.h-style-group-row select.form-control.h-style-group - each group in groups.sortBy('id') + each group in groups option(value=group.id, selected=group.id === style) = group.id .input-group-btn From 66c37b1d527f03cd752bd4a2ff9fe6f88f399bfb Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Tue, 21 Jul 2026 08:58:45 -0400 Subject: [PATCH 3/9] Auto-create missing allowed groups --- histomicsui/web_client/panels/DrawWidget.js | 25 +++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/histomicsui/web_client/panels/DrawWidget.js b/histomicsui/web_client/panels/DrawWidget.js index 6781c412..49e160cd 100644 --- a/histomicsui/web_client/panels/DrawWidget.js +++ b/histomicsui/web_client/panels/DrawWidget.js @@ -76,6 +76,7 @@ var DrawWidget = Panel.extend({ this._groups.add(this._style.toJSON()); this._groups.get(this._style.id).save(); } + this._ensureAllowedGroupsExist(); if (this._editOptions.style && this._groups.get(this._editOptions.style)) { this._setStyleGroup(this._groups.get(this._editOptions.style).toJSON()); } @@ -1088,6 +1089,30 @@ var DrawWidget = Panel.extend({ return getAllowedGroups(this.annotation); }, + /** + * If the current annotation restricts its elements to a set of allowed_groups, create any of + * those groups that don't already exist, copying the current default group's style. + */ + _ensureAllowedGroupsExist() { + const allowed = this._getAllowedGroups(); + if (!allowed) return; + + const missing = allowed.filter((groupId) => !this._groups.has(groupId)); + if (!missing.length) return; + + const defaultGroup = this._groups.get(this.parentView._defaultGroup); + const baseAttributes = defaultGroup ? _.omit(defaultGroup.toJSON(), 'id', 'group') : {}; + const saves = missing.map((groupId) => { + this._groups.add(Object.assign({}, baseAttributes, {id: groupId})); + return this._groups.get(groupId).save(); + }); + // Let other views know new groups exist after they're persisted so that a page refresh is + // not needed. + $.when(...saves).done(() => { + this.parentView.trigger('h:styleGroupsEdited', this._groups); + }); + }, + /** * Return the style groups that should be offered to the user given the current annotation's * `allowed_groups` restriction, if any, sorted alphabetically by id. From d3a47a35d9bb5761412698063f5399ec85e63ad2 Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Tue, 21 Jul 2026 08:59:57 -0400 Subject: [PATCH 4/9] React live to annotation metadata edits --- histomicsui/web_client/panels/DrawWidget.js | 23 ++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/histomicsui/web_client/panels/DrawWidget.js b/histomicsui/web_client/panels/DrawWidget.js index 49e160cd..71552cc2 100644 --- a/histomicsui/web_client/panels/DrawWidget.js +++ b/histomicsui/web_client/panels/DrawWidget.js @@ -65,9 +65,13 @@ var DrawWidget = Panel.extend({ this._groups = new StyleCollection(); 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._groups, 'remove', this._handleStyleGroupsRemoved); this.listenTo(this.collection, 'add remove reset', this._recalculateGroupAggregation); this.listenTo(this.collection, 'change update reset', this.render); + // if the annotation's metadata (including `allowed_groups`) is + // edited while this annotation is active, react immediately instead + // of requiring the annotation to be reselected or the page reloaded + this.listenTo(this.annotation, 'change:annotation', this._handleAnnotationAttributesChange); this._groups.fetch().done(() => { // ensure the default style exists if (this._groups.has(this.parentView._defaultGroup)) { @@ -81,6 +85,7 @@ var DrawWidget = Panel.extend({ this._setStyleGroup(this._groups.get(this._editOptions.style).toJSON()); } this._restrictStyleToAllowedGroups(); + this._debounceRender(); }); this.on('h:mouseon', (model) => { if (model && model.id) { @@ -1079,6 +1084,11 @@ var DrawWidget = Panel.extend({ this.trigger('h:styleGroupsUpdated', this._groups); }, + _handleStyleGroupsRemoved() { + this._restrictStyleToAllowedGroups(); + this.render(); + }, + /** * Get the current annotation's `allowed_groups` metadata, if any. * @@ -1089,6 +1099,17 @@ var DrawWidget = Panel.extend({ return getAllowedGroups(this.annotation); }, + /** + * Respond to the active annotation's metadata being edited (e.g. via the + * "Edit annotation" dialog), which may have changed its `allowed_groups` + * restriction. + */ + _handleAnnotationAttributesChange() { + this._ensureAllowedGroupsExist(); + this._restrictStyleToAllowedGroups(); + this._debounceRender(); + }, + /** * If the current annotation restricts its elements to a set of allowed_groups, create any of * those groups that don't already exist, copying the current default group's style. From 26b3ea999c43459b0dda26742850a785fc8a0a15 Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Tue, 21 Jul 2026 09:01:07 -0400 Subject: [PATCH 5/9] Restrict context menu + guard overlapping style refetches --- histomicsui/web_client/panels/DrawWidget.js | 9 +++---- .../views/popover/AnnotationContextMenu.js | 24 +++++++++++++++++-- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/histomicsui/web_client/panels/DrawWidget.js b/histomicsui/web_client/panels/DrawWidget.js index 71552cc2..50a7d1de 100644 --- a/histomicsui/web_client/panels/DrawWidget.js +++ b/histomicsui/web_client/panels/DrawWidget.js @@ -68,9 +68,7 @@ var DrawWidget = Panel.extend({ this.listenTo(this._groups, 'remove', this._handleStyleGroupsRemoved); this.listenTo(this.collection, 'add remove reset', this._recalculateGroupAggregation); this.listenTo(this.collection, 'change update reset', this.render); - // if the annotation's metadata (including `allowed_groups`) is - // edited while this annotation is active, react immediately instead - // of requiring the annotation to be reselected or the page reloaded + // if the annotation's metadata is edited while it is active, react immediately this.listenTo(this.annotation, 'change:annotation', this._handleAnnotationAttributesChange); this._groups.fetch().done(() => { // ensure the default style exists @@ -1100,9 +1098,8 @@ var DrawWidget = Panel.extend({ }, /** - * Respond to the active annotation's metadata being edited (e.g. via the - * "Edit annotation" dialog), which may have changed its `allowed_groups` - * restriction. + * Respond to the active annotation's metadata being edited, which may have changed its + * `allowed_groups` restriction. */ _handleAnnotationAttributesChange() { this._ensureAllowedGroupsExist(); diff --git a/histomicsui/web_client/views/popover/AnnotationContextMenu.js b/histomicsui/web_client/views/popover/AnnotationContextMenu.js index 390b1669..5ee15f80 100644 --- a/histomicsui/web_client/views/popover/AnnotationContextMenu.js +++ b/histomicsui/web_client/views/popover/AnnotationContextMenu.js @@ -1,6 +1,7 @@ import $ from 'jquery'; import StyleCollection from '../../collections/StyleCollection'; +import getAllowedGroups from '../../utilities/allowedGroups'; import View from '../View'; import template from '../../templates/popover/annotationContextMenu.pug'; @@ -19,6 +20,8 @@ const AnnotationContextMenu = View.extend({ this.styles = new StyleCollection(); this.styles.fetch().done(() => this.render()); this.listenTo(this.collection, 'add remove reset', this.render); + // react immediately if any annotation's metadata is edited + this.listenTo(this.parentView.annotations, 'change:annotation', this.render); }, render() { this.$el.html(template({ @@ -28,7 +31,16 @@ const AnnotationContextMenu = View.extend({ return this; }, refetchStyles() { - this.styles.fetch().done(() => this.render()); + // Prevent race conditions when multiple fetches happen in quick succession. + const requestId = (this._styleFetchRequestId = (this._styleFetchRequestId || 0) + 1); + this.styles.fetch({ + success: (collection, resp, options) => { + if (requestId === this._styleFetchRequestId) { + collection.set(resp, options); + this.render(); + } + } + }); }, setGroupCount(groupCount) { this._cachedGroupCount = groupCount; @@ -96,7 +108,15 @@ const AnnotationContextMenu = View.extend({ } }, _getAnnotationGroups() { - const groups = this.styles.map((style) => style.id); + // restrict to the allowed groups of the annotation that owns the selected/right-clicked + // element, not whichever annotation happens to be active in the Annotations panel + const referenceElement = this.collection.at(0); + const referenceAnnotation = (referenceElement && referenceElement.originalAnnotation) || this.parentView.activeAnnotation; + const allowed = getAllowedGroups(referenceAnnotation); + let groups = this.styles.map((style) => style.id); + if (allowed) { + groups = groups.filter((groupId) => allowed.includes(groupId)); + } groups.sort((a, b) => { const countA = this._cachedGroupCount[a] || 0; const countB = this._cachedGroupCount[b] || 0; From f59b03bbc628cf0d723fdef96d5e367872ec1d8e Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Tue, 21 Jul 2026 09:01:38 -0400 Subject: [PATCH 6/9] Add allowed_groups web client tests --- tests/test_web_client.py | 1 + tests/web_client_specs/allowedGroupsSpec.js | 309 ++++++++++++++++++++ 2 files changed, 310 insertions(+) create mode 100644 tests/web_client_specs/allowedGroupsSpec.js diff --git a/tests/test_web_client.py b/tests/test_web_client.py index 577aa040..27a17cf5 100644 --- a/tests/test_web_client.py +++ b/tests/test_web_client.py @@ -133,6 +133,7 @@ def testAnalysisRun(self, params): @pytest.mark.plugin('histomicsui') @pytest.mark.parametrize('spec', [ + 'allowedGroupsSpec.js', 'analysisSpec.js', 'annotationSpec.js', 'girderUISpec.js', diff --git a/tests/web_client_specs/allowedGroupsSpec.js b/tests/web_client_specs/allowedGroupsSpec.js new file mode 100644 index 00000000..abbc526c --- /dev/null +++ b/tests/web_client_specs/allowedGroupsSpec.js @@ -0,0 +1,309 @@ +/* globals describe, it, expect, waitsFor, runs, huiTest, girderTest */ + +girderTest.importPlugin( + 'jobs', 'large_image', 'large_image_annotation', 'slicer_cli_web', 'histomicsui' +); +girderTest.addScripts([ + '/static/built/plugins/histomicsui/huiTest.js', + '/static/built/plugins/histomicsui/extra/sinon.js' +]); + +girderTest.promise.done(function () { + huiTest.startApp(); + + describe('allowed_groups annotation metadata tests', function () { + var girder, largeImageAnnotation, histomicsUI; + + /** + * POST a new annotation with the given name/attributes/elements and fetch it back into a + * fresh AnnotationModel, storing the result on `result.annotation`. + */ + function createAnnotation(name, attributes, elements, result) { + var annotationId; + runs(function () { + girder.rest.restRequest({ + url: 'annotation?itemId=' + huiTest.imageId(), + contentType: 'application/json', + processData: false, + type: 'POST', + data: JSON.stringify({ + name: name, + attributes: attributes, + elements: elements + }) + }).then(function (resp) { + annotationId = resp._id; + return null; + }); + }); + waitsFor(function () { + return annotationId !== undefined; + }); + girderTest.waitForLoad(); + runs(function () { + result.annotation = new largeImageAnnotation.models.AnnotationModel({ + _id: annotationId + }); + result.fetched = false; + result.annotation.fetch().then(function () { + result.fetched = true; + return null; + }); + }); + waitsFor(function () { + return result.fetched; + }); + } + + function rectangleElement(x, y) { + return {type: 'rectangle', center: [x, y, 0], width: 4, height: 4}; + } + + describe('setup', function () { + it('login', function () { + huiTest.login(); + }); + + it('open image', function () { + huiTest.openImage('image'); + }); + + it('access plugin namespaces', function () { + girder = window.girder; + largeImageAnnotation = girder.plugins.large_image_annotation; + histomicsUI = girder.plugins.histomicsui; + }); + }); + + describe('#1/#2: no restriction (missing or empty allowed_groups)', function () { + var result = {}; + var drawWidget; + + it('creates an annotation with no allowed_groups key', function () { + createAnnotation('unrestricted annotation', {}, [rectangleElement(0, 0)], result); + }); + + it('offers every existing group in the Draw panel dropdown', function () { + var bodyView = huiTest.app.bodyView; + bodyView.annotations.add(result.annotation); + bodyView._editAnnotation(result.annotation); + drawWidget = bodyView.drawWidget; + waitsFor(function () { + return !!drawWidget._groups.length; + }); + runs(function () { + expect(drawWidget._getAllowedGroups()).toBe(null); + var expectedIds = drawWidget._groups.map(function (m) { return m.id; }).sort(); + var values = drawWidget.$('.h-style-group option').map(function () { + return this.value; + }).get().sort(); + expect(values).toEqual(expectedIds); + }); + }); + + it('offers every existing group in the context menu', function () { + var bodyView = huiTest.app.bodyView; + var element = result.annotation.elements().first(); + bodyView._resetSelection(); + bodyView._selectElement(element); + var groups = bodyView.contextMenu._getAnnotationGroups(); + var expectedIds = bodyView.contextMenu.styles.map(function (m) { return m.id; }); + expect(groups.sort()).toEqual(expectedIds.sort()); + }); + + it('treats an empty allowed_groups list the same as no restriction', function () { + result.annotation.get('annotation').attributes = {allowed_groups: []}; + expect(drawWidget._getAllowedGroups()).toBe(null); + result.annotation.get('annotation').attributes = {allowed_groups: 'not-an-array'}; + expect(drawWidget._getAllowedGroups()).toBe(null); + }); + }); + + describe('#3/#4: restricted annotation, including auto-created groups', function () { + var result = {}; + var drawWidget; + var defaultStyle; + + it('records the default group style for comparison', function () { + // StyleModel has no url/urlRoot; styles are only persisted via + // backbone.localStorage, which is patched onto the *collection*. So a + // bare model must be fetched through a StyleCollection, not directly. + var styles = new histomicsUI.collections.StyleCollection(); + var fetched = false; + runs(function () { + styles.fetch().always(function () { + fetched = true; + }); + }); + waitsFor(function () { + return fetched; + }); + runs(function () { + defaultStyle = styles.get('default'); + }); + }); + + it('creates an annotation restricted to a mix of missing groups', function () { + createAnnotation('restricted annotation', { + allowed_groups: ['groupA', 'groupB'] + }, [rectangleElement(10, 10)], result); + }); + + it('#4a/#4b: auto-creates every missing allowed group using the default style', function () { + var bodyView = huiTest.app.bodyView; + bodyView.annotations.add(result.annotation); + bodyView._editAnnotation(result.annotation); + drawWidget = bodyView.drawWidget; + waitsFor(function () { + return drawWidget._groups.has('groupA') && drawWidget._groups.has('groupB'); + }); + runs(function () { + ['groupA', 'groupB'].forEach(function (groupId) { + var created = drawWidget._groups.get(groupId).toJSON(); + expect(created.fillColor).toBe(defaultStyle.get('fillColor')); + expect(created.lineColor).toBe(defaultStyle.get('lineColor')); + expect(created.lineWidth).toBe(defaultStyle.get('lineWidth')); + expect(created.pattern).toBe(defaultStyle.get('pattern')); + }); + // the active style switches to the first allowed group + expect(drawWidget._style.id).toBe('groupA'); + }); + }); + + it('#3/#4c: restriction is immediately reflected in the Draw panel dropdown', function () { + var values = drawWidget.$('.h-style-group option').map(function () { + return this.value; + }).get(); + expect(values).toEqual(['groupA', 'groupB']); + }); + + it('#3/#4c: restriction is immediately reflected in the context menu', function () { + var bodyView = huiTest.app.bodyView; + // select an element of the restricted annotation so the context menu's group + // list reflects it, rather than a stale selection left over from a previous + // describe block + var element = result.annotation.elements().first(); + bodyView._resetSelection(); + bodyView._selectElement(element); + + var groups = bodyView.contextMenu._getAnnotationGroups(); + expect(groups.sort()).toEqual(['groupA', 'groupB']); + }); + }); + + describe('#5: context menu ignores active selection, uses the clicked annotation', function () { + var restricted = {}; + var unrestricted = {}; + + it('creates a restricted and an unrestricted annotation', function () { + createAnnotation('restricted annotation for #5', { + allowed_groups: ['groupE', 'groupF'] + }, [rectangleElement(30, 30)], restricted); + }); + + it('creates the unrestricted annotation', function () { + createAnnotation('unrestricted annotation for #5', {}, [rectangleElement(40, 40)], unrestricted); + }); + + it('auto-creates the restricted annotation\'s allowed groups (groupE/groupF) as styles', function () { + // _getAnnotationGroups() only ever returns groups that already exist as + // StyleModels; it never auto-creates them. Auto-creation only happens via the Draw + // panel's _editAnnotation() flow, so briefly edit the restricted annotation here + // to create groupE and groupF before switching the active annotation below. + var bodyView = huiTest.app.bodyView; + bodyView.annotations.add(restricted.annotation); + bodyView._editAnnotation(restricted.annotation); + waitsFor(function () { + return !!bodyView.drawWidget && + bodyView.drawWidget._groups.has('groupE') && + bodyView.drawWidget._groups.has('groupF'); + }); + }); + + it('restricts the context menu to the clicked element\'s annotation regardless of the active annotation/group/shape', function () { + var bodyView = huiTest.app.bodyView; + + runs(function () { + bodyView.annotations.add(unrestricted.annotation); + bodyView._editAnnotation(unrestricted.annotation); + }); + waitsFor(function () { + return !!bodyView.drawWidget && !!bodyView.drawWidget._groups.length; + }); + runs(function () { + // pick an arbitrary style group in the Draw panel to prove the context menu + // restriction is independent of it + bodyView.drawWidget.setStyleGroupById(bodyView.drawWidget._groups.first().id); + + var element = restricted.annotation.elements().first(); + bodyView._resetSelection(); + bodyView._selectElement(element); + + var groups = bodyView.contextMenu._getAnnotationGroups(); + expect(groups.sort()).toEqual(['groupE', 'groupF']); + }); + }); + }); + + describe('#3/#5: live metadata updates without reselecting or reloading', function () { + var result = {}; + var drawWidget; + + it('creates an annotation with no restriction', function () { + createAnnotation('live-update annotation', {}, [rectangleElement(50, 50)], result); + }); + + it('opens the annotation while unrestricted', function () { + var bodyView = huiTest.app.bodyView; + bodyView.annotations.add(result.annotation); + bodyView._editAnnotation(result.annotation); + drawWidget = bodyView.drawWidget; + waitsFor(function () { + return !!drawWidget._groups.length; + }); + runs(function () { + expect(drawWidget._getAllowedGroups()).toBe(null); + }); + }); + + it('immediately restricts the Draw panel and context menu when allowed_groups is added, without reselecting', function () { + var bodyView = huiTest.app.bodyView; + + var element = result.annotation.elements().first(); + bodyView._resetSelection(); + bodyView._selectElement(element); + + // simulate editing the annotation's metadata via the "Edit annotation" dialog, + // which mutates attributes directly and triggers 'change:annotation' rather than + // calling .set() + result.annotation.get('annotation').attributes = {allowed_groups: ['groupG', 'groupH']}; + result.annotation.trigger('change:annotation', result.annotation, {}); + + waitsFor(function () { + // the Draw panel's `_groups` collection updates synchronously, but its dropdown + // DOM is refreshed via a debounced render, so also wait for the DOM to catch up + // before asserting on it + return drawWidget._groups.has('groupG') && drawWidget._groups.has('groupH') && + drawWidget.$('.h-style-group option').length === 2; + }); + runs(function () { + expect(drawWidget._style.id).toBe('groupG'); + var drawValues = drawWidget.$('.h-style-group option').map(function () { + return this.value; + }).get(); + expect(drawValues).toEqual(['groupG', 'groupH']); + }); + // the context menu keeps its own StyleCollection, refetched asynchronously via the + // 'h:styleGroupsEdited' event once the Draw panel auto-creates the newly allowed + // groups; wait for that refetch to complete before checking it + waitsFor(function () { + return bodyView.contextMenu.styles.get('groupG') && bodyView.contextMenu.styles.get('groupH'); + }); + runs(function () { + var contextGroups = bodyView.contextMenu._getAnnotationGroups(); + expect(contextGroups.sort()).toEqual(['groupG', 'groupH']); + }); + }); + }); + }); +}); From a532893dd4483cadf8a91b57d4b12eafbea9d923 Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Tue, 4 Aug 2026 16:23:16 -0400 Subject: [PATCH 7/9] Extract missing-group creation logic from the Draw panel into a shared utility fn --- histomicsui/web_client/panels/DrawWidget.js | 19 ++++------- .../web_client/utilities/allowedGroups.js | 32 +++++++++++++++++++ .../views/popover/AnnotationContextMenu.js | 21 +++++++++++- 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/histomicsui/web_client/panels/DrawWidget.js b/histomicsui/web_client/panels/DrawWidget.js index 50a7d1de..534569e3 100644 --- a/histomicsui/web_client/panels/DrawWidget.js +++ b/histomicsui/web_client/panels/DrawWidget.js @@ -15,7 +15,7 @@ import StyleCollection from '../collections/StyleCollection'; import StyleModel from '../models/StyleModel'; import editElement from '../dialogs/editElement'; import editStyleGroups from '../dialogs/editStyleGroups'; -import getAllowedGroups from '../utilities/allowedGroups'; +import getAllowedGroups, {ensureAllowedGroupsExist} from '../utilities/allowedGroups'; import drawWidget from '../templates/panels/drawWidget.pug'; import drawWidgetElement from '../templates/panels/drawWidgetElement.pug'; import '../stylesheets/panels/drawWidget.styl'; @@ -1112,18 +1112,11 @@ var DrawWidget = Panel.extend({ * those groups that don't already exist, copying the current default group's style. */ _ensureAllowedGroupsExist() { - const allowed = this._getAllowedGroups(); - if (!allowed) return; - - const missing = allowed.filter((groupId) => !this._groups.has(groupId)); - if (!missing.length) return; - - const defaultGroup = this._groups.get(this.parentView._defaultGroup); - const baseAttributes = defaultGroup ? _.omit(defaultGroup.toJSON(), 'id', 'group') : {}; - const saves = missing.map((groupId) => { - this._groups.add(Object.assign({}, baseAttributes, {id: groupId})); - return this._groups.get(groupId).save(); - }); + const saves = ensureAllowedGroupsExist( + this._groups, this._getAllowedGroups(), this.parentView._defaultGroup); + if (!saves.length) { + return; + } // Let other views know new groups exist after they're persisted so that a page refresh is // not needed. $.when(...saves).done(() => { diff --git a/histomicsui/web_client/utilities/allowedGroups.js b/histomicsui/web_client/utilities/allowedGroups.js index 790301c5..8b38dabc 100644 --- a/histomicsui/web_client/utilities/allowedGroups.js +++ b/histomicsui/web_client/utilities/allowedGroups.js @@ -22,4 +22,36 @@ function getAllowedGroups(annotation) { return filtered.length ? filtered : null; } +/** + * Ensure that every group in an `allowed_groups` restriction exists as a persisted style group. + * Create any that are missing by copying the style of the default group. + * + * Newly created groups are added to the collection synchronously and each is persisted + * asynchronously. The caller is responsible for reacting to the returned save promises. + * + * @param {StyleCollection} styles The style-group collection to populate. + * @param {string[]|null} allowed The validated `allowed_groups` restriction, or `null` for + * "unrestricted" (in which case nothing is created). + * @param {string} defaultGroupId The id of the default style group to copy. + * @returns {Array} The list of save promises for the newly created groups. + */ +function ensureAllowedGroupsExist(styles, allowed, defaultGroupId) { + if (!allowed) { + return []; + } + const missing = allowed.filter((groupId) => !styles.get(groupId)); + if (!missing.length) { + return []; + } + // we assume the default group always exists; if it somehow does not, new groups are created + // with no inherited style rather than failing + const defaultGroup = styles.get(defaultGroupId); + const baseAttributes = defaultGroup ? _.omit(defaultGroup.toJSON(), 'id', 'group') : {}; + return missing.map((groupId) => { + styles.add(Object.assign({}, baseAttributes, {id: groupId})); + return styles.get(groupId).save(); + }); +} + export default getAllowedGroups; +export {ensureAllowedGroupsExist}; diff --git a/histomicsui/web_client/views/popover/AnnotationContextMenu.js b/histomicsui/web_client/views/popover/AnnotationContextMenu.js index 5ee15f80..77da6525 100644 --- a/histomicsui/web_client/views/popover/AnnotationContextMenu.js +++ b/histomicsui/web_client/views/popover/AnnotationContextMenu.js @@ -1,7 +1,7 @@ import $ from 'jquery'; import StyleCollection from '../../collections/StyleCollection'; -import getAllowedGroups from '../../utilities/allowedGroups'; +import getAllowedGroups, {ensureAllowedGroupsExist} from '../../utilities/allowedGroups'; import View from '../View'; import template from '../../templates/popover/annotationContextMenu.pug'; @@ -113,6 +113,7 @@ const AnnotationContextMenu = View.extend({ const referenceElement = this.collection.at(0); const referenceAnnotation = (referenceElement && referenceElement.originalAnnotation) || this.parentView.activeAnnotation; const allowed = getAllowedGroups(referenceAnnotation); + this._ensureAllowedGroupsExist(allowed); let groups = this.styles.map((style) => style.id); if (allowed) { groups = groups.filter((groupId) => allowed.includes(groupId)); @@ -133,6 +134,24 @@ const AnnotationContextMenu = View.extend({ }); return groups; }, + /** + * Create any style groups required by the given `allowed_groups` restriction that don't + * already exist. Once the new groups are persisted, notify the other views so their style + * collections stay in sync. + * + * @param {string[]|null} allowed The validated `allowed_groups` restriction, or `null` when + * unrestricted. + */ + _ensureAllowedGroupsExist(allowed) { + const saves = ensureAllowedGroupsExist( + this.styles, allowed, this.parentView._defaultGroup); + if (!saves.length) { + return; + } + $.when(...saves).done(() => { + this.parentView.trigger('h:styleGroupsEdited', this.styles); + }); + }, _setGroup(evt) { evt.preventDefault(); evt.stopPropagation(); From 856c4f06c28bdaae40b3eb4c0242b4008f97ad61 Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Tue, 4 Aug 2026 16:23:56 -0400 Subject: [PATCH 8/9] Honor the clicked annotation's group restriction in the context menu Assign `originalAnnotation` on the model before it enters the collection so the menu reflects the clicked annotation rather than the panel selection. --- histomicsui/web_client/views/body/ImageView.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/histomicsui/web_client/views/body/ImageView.js b/histomicsui/web_client/views/body/ImageView.js index a77f6f5f..988b8f71 100644 --- a/histomicsui/web_client/views/body/ImageView.js +++ b/histomicsui/web_client/views/body/ImageView.js @@ -1553,8 +1553,12 @@ var ImageView = View.extend({ return; } - var elementModel = this.selectedElements.add(element.attributes, options); + // Assign `originalAnnotation` before the element enters the collection. Setting it after + // `add` would leave that first render seeing an undefined reference and falling back to + // the panel's active annotation instead of the clicked one. + var elementModel = new this.selectedElements.model(element.attributes); elementModel.originalAnnotation = annotation; + this.selectedElements.add(elementModel, options); this.viewerWidget.highlightAnnotation(this.selectedAnnotation.id); }, From 2eeb0ddd60e98f9894e18c76b37380e4507ba997 Mon Sep 17 00:00:00 2001 From: Brianna Major Date: Tue, 4 Aug 2026 16:24:20 -0400 Subject: [PATCH 9/9] Add tests for context-menu auto-create and clicked-annotation restriction - Context menu auto-creates a restricted annotation's allowed groups the first time it encounters them - Regression test asserting context menu renders clicked annotation's restriction on selection rather than the active annotation in the panel --- tests/web_client_specs/allowedGroupsSpec.js | 61 +++++++++++++++++++-- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/tests/web_client_specs/allowedGroupsSpec.js b/tests/web_client_specs/allowedGroupsSpec.js index abbc526c..1ee5db18 100644 --- a/tests/web_client_specs/allowedGroupsSpec.js +++ b/tests/web_client_specs/allowedGroupsSpec.js @@ -206,10 +206,8 @@ girderTest.promise.done(function () { }); it('auto-creates the restricted annotation\'s allowed groups (groupE/groupF) as styles', function () { - // _getAnnotationGroups() only ever returns groups that already exist as - // StyleModels; it never auto-creates them. Auto-creation only happens via the Draw - // panel's _editAnnotation() flow, so briefly edit the restricted annotation here - // to create groupE and groupF before switching the active annotation below. + // briefly edit the restricted annotation in the Draw panel to create groupE and + // groupF before switching the active annotation below. var bodyView = huiTest.app.bodyView; bodyView.annotations.add(restricted.annotation); bodyView._editAnnotation(restricted.annotation); @@ -243,6 +241,33 @@ girderTest.promise.done(function () { expect(groups.sort()).toEqual(['groupE', 'groupF']); }); }); + + it('renders the clicked annotation\'s restriction on selection, not the active annotation\'s (regression)', function () { + var bodyView = huiTest.app.bodyView; + + runs(function () { + // keep the *unrestricted* annotation active in the panel + bodyView._editAnnotation(unrestricted.annotation); + }); + waitsFor(function () { + return !!bodyView.drawWidget && !!bodyView.drawWidget._groups.length; + }); + runs(function () { + // Selecting an element fires the context menu's render synchronously. + // This asserts on the DOM produced by that render, which is the path that + // regressed. Previously `originalAnnotation` was assigned only after `add`, + // so the render fell back to the active annotation and offered the wrong + // groups when the active annotation did not match the selected one. + var element = restricted.annotation.elements().first(); + bodyView._resetSelection(); + bodyView._selectElement(element); + + var renderedGroups = bodyView.contextMenu.$('.h-set-group').map(function () { + return window.$(this).data('group'); + }).get(); + expect(renderedGroups.sort()).toEqual(['groupE', 'groupF']); + }); + }); }); describe('#3/#5: live metadata updates without reselecting or reloading', function () { @@ -305,5 +330,33 @@ girderTest.promise.done(function () { }); }); }); + + describe('#3(consistency): the context menu auto-creates missing allowed groups', function () { + var restricted = {}; + + it('creates a restricted annotation that is never opened in the Draw panel', function () { + createAnnotation('context-only restricted annotation', { + allowed_groups: ['groupK', 'groupL'] + }, [rectangleElement(70, 70)], restricted); + }); + + it('auto-creates the allowed groups the first time the context menu sees them', function () { + var bodyView = huiTest.app.bodyView; + // the groups must not already exist from an earlier spec + expect(bodyView.contextMenu.styles.get('groupK')).toBe(undefined); + expect(bodyView.contextMenu.styles.get('groupL')).toBe(undefined); + + // select an element of the restricted annotation without ever opening it in the + // Draw panel, so the context menu is the only code path that can create its groups + var element = restricted.annotation.elements().first(); + bodyView._resetSelection(); + bodyView._selectElement(element); + + var groups = bodyView.contextMenu._getAnnotationGroups(); + expect(groups.sort()).toEqual(['groupK', 'groupL']); + expect(bodyView.contextMenu.styles.get('groupK')).toBeTruthy(); + expect(bodyView.contextMenu.styles.get('groupL')).toBeTruthy(); + }); + }); }); });