From 398484184a223ead6244a9c2f4ead6d5faef9875 Mon Sep 17 00:00:00 2001 From: Ankur Juneja Date: Tue, 2 Jun 2026 07:06:51 -0700 Subject: [PATCH 1/9] Click/drag zooming for QC plots (#1209) --- .../components/targetedms/QCPlotsWebPart.java | 33 +++ .../tests/targetedms/TargetedMSQCTest.java | 50 ++++ webapp/TargetedMS/css/qcTrendPlotReport.css | 22 +- webapp/TargetedMS/js/QCPlotHelperBase.js | 261 ++++++++++++++++++ webapp/TargetedMS/js/QCTrendPlotPanel.js | 53 +++- 5 files changed, 416 insertions(+), 3 deletions(-) diff --git a/test/src/org/labkey/test/components/targetedms/QCPlotsWebPart.java b/test/src/org/labkey/test/components/targetedms/QCPlotsWebPart.java index 1a2792f5c..768f84a9e 100644 --- a/test/src/org/labkey/test/components/targetedms/QCPlotsWebPart.java +++ b/test/src/org/labkey/test/components/targetedms/QCPlotsWebPart.java @@ -904,6 +904,36 @@ public String toString() } } + public void performYAxisZoom(QCPlot qcPlot) + { + WebElement plotEl = qcPlot.getPlot(); + WebElement overlay = elementCache().yZoomOverlay.findElement(plotEl); + getWrapper().scrollIntoView(overlay); + + int clickOffset = 40; + new Actions(getWrapper().getDriver()) + .moveToElement(overlay, 0, -clickOffset) + .click() + .moveToElement(overlay, 0, clickOffset) + .click() + .perform(); + + WebDriverWrapper.waitFor(() -> !elementCache().yZoomConfirmBtn.findElements(plotEl).isEmpty(), + "Zoom buttons did not appear after y-axis clicks", WAIT_FOR_JAVASCRIPT); + + elementCache().yZoomConfirmBtn.findElement(plotEl).click(); + } + + public boolean isZoomActive(QCPlot qcPlot) + { + return !elementCache().yZoomBorder.findElements(qcPlot.getPlot()).isEmpty(); + } + + public void clickResetZoom(QCPlot qcPlot) + { + elementCache().yZoomOverlay.findElement(qcPlot.getPlot()).click(); + } + public class Elements extends BodyWebPart.ElementCache { WebElement startDate = Locator.css("#start-date-field input").findWhenNeeded(this); @@ -939,6 +969,9 @@ public class Elements extends BodyWebPart.ElementCache WebElement plotPanel = Locator.css("div.tiledPlotPanel").findWhenNeeded(this); WebElement paginationPanel = Locator.css("div.plotPaginationHeaderPanel").findWhenNeeded(this); Locator extFormDisplay = Locator.css("div.x4-form-display-field"); + Locator.CssLocator yZoomOverlay = Locator.css("svg rect.y-zoom-overlay"); + Locator.CssLocator yZoomConfirmBtn = Locator.css("svg g.y-zoom-btn-zoom rect"); + Locator.CssLocator yZoomBorder = Locator.css("svg rect.y-zoom-border"); Locator.CssLocator guideSetTrainingRect = Locator.css("svg rect.training"); Locator.CssLocator experimentRangeRect = Locator.css("svg rect.expRange"); Locator.CssLocator guideSetSvgButton = Locator.css("svg g.guideset-svg-button text"); diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java index b51926618..acc52709e 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java @@ -1148,6 +1148,56 @@ private void verifyRow(DataRegionTable drt, int row, String sampleName, String s assertEquals(skylineDocName, drt.getDataAsText(row, "File")); } + @Test + public void testQCPlotYAxisZoom() + { + PanoramaDashboard qcDashboard = new PanoramaDashboard(this); + QCPlotsWebPart qcPlotsWebPart = qcDashboard.getQcPlotsWebPart(); + qcPlotsWebPart.filterQCPlotsToInitialData(PRECURSORS.length, true); + + List plots = qcPlotsWebPart.getPlots(); + assertTrue("Expected at least 2 plots for y-axis zoom test", plots.size() >= 2); + + // 1. Verify zooming is possible: drag on y-axis, confirm zoom, border appears + log("Verifying y-axis zoom can be applied"); + qcPlotsWebPart.performYAxisZoom(plots.get(0)); + waitForElement(Locator.css("svg rect.y-zoom-border"), WAIT_FOR_JAVASCRIPT); + + plots = qcPlotsWebPart.getPlots(); + QCPlot firstPlot = plots.get(0); + QCPlot secondPlot = plots.get(1); + + assertTrue("Zoom border should appear on first plot after zoom", qcPlotsWebPart.isZoomActive(firstPlot)); + + // 2. Verify zoom is per-plot: second plot is unaffected + log("Verifying zoom is independent per plot"); + assertFalse("Second plot should not be zoomed", qcPlotsWebPart.isZoomActive(secondPlot)); + + // 3. Verify reset works: clicking the zoomed y-axis (zoom-out cursor) resets zoom + log("Verifying clicking the y-axis resets zoom on the target plot"); + qcPlotsWebPart.clickResetZoom(firstPlot); + waitForElementToDisappear(Locator.css("svg rect.y-zoom-border"), WAIT_FOR_JAVASCRIPT); + + plots = qcPlotsWebPart.getPlots(); + firstPlot = plots.get(0); + + assertFalse("Zoom border should be gone after reset", qcPlotsWebPart.isZoomActive(firstPlot)); + + // 4. Verify zoom is not persisted after page reload + log("Verifying zoom state is cleared on page reload"); + qcPlotsWebPart.performYAxisZoom(firstPlot); + waitForElement(Locator.css("svg rect.y-zoom-border"), WAIT_FOR_JAVASCRIPT); + + refresh(); + qcDashboard = new PanoramaDashboard(this); + qcPlotsWebPart = qcDashboard.getQcPlotsWebPart(); + + plots = qcPlotsWebPart.getPlots(); + firstPlot = plots.get(0); + + assertFalse("Zoom should not persist after page reload", qcPlotsWebPart.isZoomActive(firstPlot)); + } + private void createAndInsertAnnotations() { clickTab("Annotations"); diff --git a/webapp/TargetedMS/css/qcTrendPlotReport.css b/webapp/TargetedMS/css/qcTrendPlotReport.css index b4434fed4..51e40d084 100644 --- a/webapp/TargetedMS/css/qcTrendPlotReport.css +++ b/webapp/TargetedMS/css/qcTrendPlotReport.css @@ -115,4 +115,24 @@ .qc-combined-tree-legend .qc-tree-precursor:hover { background-color: #f0f0f0; -} \ No newline at end of file +} + +.y-zoom-overlay { + cursor: zoom-in; +} + +.y-zoom-pending-line { + stroke: rgba(20, 204, 201, 1); + stroke-width: 2px; + stroke-dasharray: 6, 3; +} + +.y-zoom-selection { + fill: rgba(20, 204, 201, 0.3); + stroke: rgba(20, 204, 201, 1); + stroke-width: 1px; +} + +.y-zoom-buttons g { + cursor: pointer; +} diff --git a/webapp/TargetedMS/js/QCPlotHelperBase.js b/webapp/TargetedMS/js/QCPlotHelperBase.js index 85cacafa4..11fb334b3 100644 --- a/webapp/TargetedMS/js/QCPlotHelperBase.js +++ b/webapp/TargetedMS/js/QCPlotHelperBase.js @@ -805,6 +805,12 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { } Ext4.apply(trendLineProps, this.getPlotTypeProperties(combinePlotData, plotType, isCUSUMMean, metricProps)); + let yZoomDomainCombined = this.getYZoomDomain ? this.getYZoomDomain(id) : null; + if (yZoomDomainCombined) { + if (yZoomDomainCombined.left) trendLineProps.yZoomDomain = yZoomDomainCombined.left; + if (yZoomDomainCombined.right) trendLineProps.yZoomDomainRight = yZoomDomainCombined.right; + } + // Suppress the mean line for multi-series plots trendLineProps.mean = undefined; @@ -860,6 +866,7 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { const plot = LABKEY.vis.TrendingLinePlot(plotConfig); plot.render(); + this.addYZoomInteraction(plot, id); this.attachCombinedLegendClickHandlers(); this.addAnnotationsToPlot(plot, combinePlotData); @@ -945,6 +952,12 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { Ext4.apply(trendLineProps, this.getPlotTypeProperties(precursorInfo, plotType, isCUSUMMean, metricProps)); + let yZoomDomain = this.getYZoomDomain ? this.getYZoomDomain(id) : null; + if (yZoomDomain) { + if (yZoomDomain.left) trendLineProps.yZoomDomain = yZoomDomain.left; + if (yZoomDomain.right) trendLineProps.yZoomDomainRight = yZoomDomain.right; + } + var plotLegendData = this.getAdditionalPlotLegend(plotType); if (Ext4.isArray(this.legendData)) { plotLegendData = plotLegendData.concat(this.legendData); @@ -1022,6 +1035,7 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { const plot = LABKEY.vis.TrendingLinePlot(plotConfig); plot.render(); + this.addYZoomInteraction(plot, id); this.addAnnotationsToPlot(plot, precursorInfo); this.addGuideSetTrainingRangeToPlot(plot, precursorInfo); @@ -1065,5 +1079,252 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { showInPlotLegends: function () { return true; + }, + + addYZoomInteraction: function(plot, plotId) { + let me = this; + let svg = this.getSvgElForPlot(plot); + let grid = plot.grid; + + if (!plot.scales.yLeft || !plot.scales.yLeft.scale || !plot.scales.yLeft.scale.invert) { + return; + } + + let gridTop = grid.topEdge; + let gridBottom = grid.bottomEdge; + let gridLeft = grid.leftEdge; + let gridRight = grid.rightEdge; + + let clampY = function(y) { + return Math.max(gridTop, Math.min(gridBottom, y)); + }; + + let zoomEntry = this.getYZoomDomain ? this.getYZoomDomain(plotId) : null; + + // Creates an independent drag/click overlay for one y-axis (left or right). + // overlayX/overlayW define where the invisible hit area sits. + // btnAnchorX is the left edge of the Zoom button. + let setupAxisOverlay = function(axis, yScale, overlayX, overlayW, btnAnchorX) { + let isZoomed = !!(zoomEntry && zoomEntry[axis]); + + let overlayEl = svg.append('rect') + .attr('class', 'y-zoom-overlay') + .attr('x', overlayX) + .attr('y', gridTop) + .attr('width', overlayW) + .attr('height', gridBottom - gridTop) + .style({'fill': 'transparent', 'cursor': isZoomed ? 'zoom-out' : 'zoom-in'}); + + if (isZoomed) { + overlayEl.on('click', function() { me.resetYZoom(plotId, axis); }); + return; + } + + let dragStartY = null, dragCurrentY = null; + let selectionRect = null, zoomButtonGroup = null, pendingLine = null, pendingStartY = null; + let interactionMask = null, plotClickCapture = null; + let moveNs = 'mousemove.yzoom-' + axis; + let keyNs = 'keydown.yzoom-' + axis; + + let removeOverlays = function() { + if (selectionRect) { selectionRect.remove(); selectionRect = null; } + if (zoomButtonGroup) { zoomButtonGroup.remove(); zoomButtonGroup = null; } + if (pendingLine) { pendingLine.remove(); pendingLine = null; } + if (interactionMask) { interactionMask.remove(); interactionMask = null; } + if (plotClickCapture) { plotClickCapture.remove(); plotClickCapture = null; } + }; + + let showZoomButtons = function(y1, y2) { + let domainMax = yScale.invert(y1); + let domainMin = yScale.invert(y2); + let yMid = y1 + (y2 - y1) / 2; + + // Block all plot interactions while zoom buttons are visible + interactionMask = svg.append('rect') + .attr('x', 0).attr('y', 0) + .attr('width', parseFloat(svg.attr('width')) || (gridRight + 80)) + .attr('height', parseFloat(svg.attr('height')) || (gridBottom + 50)) + .style({'fill': 'transparent', 'pointer-events': 'all', 'cursor': 'default'}); + + zoomButtonGroup = svg.append('g').attr('class', 'y-zoom-buttons'); + + let makeBtn = function(text, xLeft, width, onClick) { + let btnG = zoomButtonGroup.append('g').attr('class', 'y-zoom-btn-' + text.toLowerCase()); + btnG.append('rect') + .attr('x', xLeft).attr('y', yMid - 10).attr('rx', 5).attr('ry', 5) + .attr('width', width).attr('height', 20) + .style({'fill': '#ffffff', 'stroke': '#b4b4b4'}); + btnG.append('text') + .text(text) + .attr('x', xLeft + width / 2).attr('y', yMid + 4) + .style({'fill': '#126495', 'font-size': '10px', 'font-weight': 'bold', + 'text-anchor': 'middle', 'text-transform': 'uppercase', 'pointer-events': 'none'}); + btnG.on('click', onClick); + return btnG; + }; + + makeBtn('Zoom', btnAnchorX, 50, function() { + removeOverlays(); + me.applyYZoom(plotId, domainMin, domainMax, axis); + }); + + makeBtn('Cancel', btnAnchorX + 60, 55, function() { + removeOverlays(); + }); + }; + + let cancelPendingClick = function() { + pendingStartY = null; + svg.on(moveNs, null); + d3.select(document).on(keyNs, null); + removeOverlays(); + }; + + let startClickModeTracking = function(startY) { + pendingStartY = startY; + + pendingLine = svg.append('line') + .attr('class', 'y-zoom-pending-line') + .attr('x1', gridLeft).attr('y1', startY) + .attr('x2', gridRight).attr('y2', startY) + .style('pointer-events', 'none'); + + svg.on(moveNs, function() { + let currentY = clampY(d3.mouse(svg.node())[1]); + let y1 = Math.min(pendingStartY, currentY); + let y2 = Math.max(pendingStartY, currentY); + let h = y2 - y1; + + if (selectionRect) { + selectionRect.attr('x', gridLeft).attr('y', y1) + .attr('width', gridRight - gridLeft).attr('height', Math.max(1, h)); + } else { + selectionRect = svg.append('rect') + .attr('class', 'y-zoom-selection') + .attr('x', gridLeft).attr('y', y1) + .attr('width', gridRight - gridLeft) + .attr('height', Math.max(1, h)) + .style('pointer-events', 'none'); + } + }); + + d3.select(document).on(keyNs, function() { + if (d3.event.key === 'Escape' || d3.event.keyCode === 27) { + cancelPendingClick(); + } + }); + + plotClickCapture = svg.append('rect') + .attr('x', gridLeft).attr('y', gridTop) + .attr('width', gridRight - gridLeft).attr('height', gridBottom - gridTop) + .style({'fill': 'transparent', 'cursor': 'crosshair'}) + .on('click', function() { + let clickY = clampY(d3.mouse(svg.node())[1]); + let firstY = pendingStartY; + cancelPendingClick(); + let finalY1 = Math.min(firstY, clickY); + let finalY2 = Math.max(firstY, clickY); + if (finalY2 - finalY1 < 5) { return; } + showZoomButtons(finalY1, finalY2); + }); + }; + + let drag = d3.behavior.drag() + .on('dragstart', function() { + dragStartY = clampY(d3.mouse(svg.node())[1]); + dragCurrentY = dragStartY; + if (zoomButtonGroup) { zoomButtonGroup.remove(); zoomButtonGroup = null; } + }) + .on('drag', function() { + dragCurrentY = clampY(d3.mouse(svg.node())[1]); + + if (pendingStartY !== null && Math.abs(dragCurrentY - dragStartY) >= 5) { + cancelPendingClick(); + } + + let y1 = Math.min(dragStartY, dragCurrentY); + let y2 = Math.max(dragStartY, dragCurrentY); + let h = y2 - y1; + + if (h < 1) { return; } + + if (selectionRect) { + selectionRect.attr('x', gridLeft).attr('y', y1) + .attr('width', gridRight - gridLeft).attr('height', h); + } else { + selectionRect = svg.append('rect') + .attr('class', 'y-zoom-selection') + .attr('x', gridLeft).attr('y', y1) + .attr('width', gridRight - gridLeft) + .attr('height', h) + .style('pointer-events', 'none'); + } + }) + .on('dragend', function() { + let y1 = Math.min(dragStartY, dragCurrentY); + let y2 = Math.max(dragStartY, dragCurrentY); + + if (y2 - y1 < 5) { return; } + + if (pendingStartY !== null) { cancelPendingClick(); } + showZoomButtons(y1, y2); + }); + + overlayEl.call(drag); + + overlayEl.on('click', function() { + let clickY = clampY(d3.mouse(svg.node())[1]); + + if (pendingStartY === null) { + removeOverlays(); + startClickModeTracking(clickY); + } else { + let firstY = pendingStartY; + cancelPendingClick(); + + let finalY1 = Math.min(firstY, clickY); + let finalY2 = Math.max(firstY, clickY); + if (finalY2 - finalY1 < 5) { return; } + + showZoomButtons(finalY1, finalY2); + } + }); + }; + + // Left axis overlay + setupAxisOverlay('left', plot.scales.yLeft.scale, 0, gridLeft - 2, gridLeft + 5); + + // Right axis overlay — only when a right scale exists + if (plot.scales.yRight && plot.scales.yRight.scale && plot.scales.yRight.scale.invert) { + let svgWidth = parseFloat(svg.attr('width')) || (gridRight + 80); + let rightOverlayX = gridRight + 2; + let rightOverlayW = Math.max(1, svgWidth - rightOverlayX); + // Buttons sit just inside the plot to the left of the right axis (Zoom 50px + gap 10px + Cancel 55px = 115px) + setupAxisOverlay('right', plot.scales.yRight.scale, rightOverlayX, rightOverlayW, gridRight - 120); + } + + if (zoomEntry) { + let gridWidth = gridRight - gridLeft; + let gridHeight = gridBottom - gridTop; + let clipId = (plot.renderTo || plotId) + '-yzoom-clip'; + + let svgDefs = svg.select('defs'); + if (svgDefs.empty()) { + svgDefs = svg.insert('defs', ':first-child'); + } + svgDefs.append('clipPath') + .attr('id', clipId) + .append('rect') + .attr('x', gridLeft).attr('y', gridTop) + .attr('width', gridWidth).attr('height', gridHeight); + + svg.selectAll('g.layer').attr('clip-path', 'url(#' + clipId + ')'); + + svg.append('rect') + .attr('class', 'y-zoom-border') + .attr('x', gridLeft).attr('y', gridTop) + .attr('width', gridWidth).attr('height', gridHeight) + .style({'fill': 'none', 'stroke': '#888', 'stroke-width': '1px', 'pointer-events': 'none'}); + } } }); \ No newline at end of file diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index 55ae8f4e2..90bb4f8de 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -82,6 +82,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { trailingRuns: null, minWidth: 1250, // Keep in sync with the width defined in qcTrendPlot.jsp width: '100%', + yZoomByPlot: {}, SHOW_ALL_IN_A_SINGLE_PLOT: 'Show all series in a single plot', LABEL_WIDTH: 115, @@ -1213,8 +1214,10 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { Ext4.get(this.plotDivId).mask("Loading..."); }, - displayTrendPlot: function() { - + displayTrendPlot: function(preserveZoom) { + if (!preserveZoom) { + this.yZoomByPlot = {}; + } this.setBrushingEnabled(false); this.updateSelectedAnnotations(); this.setLoadingMsg(); @@ -2223,6 +2226,52 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { } }, + getYZoomDomain: function(plotId) { + let entry = this.yZoomByPlot && this.yZoomByPlot[plotId]; + if (!entry || (!entry.left && !entry.right)) return null; + return entry; + }, + + incrementMetric: function(metricName) { + if (LABKEY.user && LABKEY.user.isGuest) { + return; + } + LABKEY.Ajax.request({ + url: LABKEY.ActionURL.buildURL('core', 'incrementClientSideMetricCount.api'), + method: 'POST', + jsonData: { featureArea: 'panoramaQCPlot', metricName: metricName }, + failure: function(response) { console.error('Failed to track metric ' + metricName + ':', response); } + }); + }, + + applyYZoom: function(plotId, yMin, yMax, axis) { + if (!this.yZoomByPlot) { + this.yZoomByPlot = {}; + } + if (!this.yZoomByPlot[plotId]) { + this.yZoomByPlot[plotId] = {}; + } + this.yZoomByPlot[plotId][axis] = [yMin, yMax]; + this.incrementMetric('yAxisZoom'); + this.processPlotData(); + }, + + resetYZoom: function(plotId, axis) { + if (this.yZoomByPlot && this.yZoomByPlot[plotId]) { + if (axis) { + delete this.yZoomByPlot[plotId][axis]; + if (!this.yZoomByPlot[plotId].left && !this.yZoomByPlot[plotId].right) { + delete this.yZoomByPlot[plotId]; + } + } + else { + delete this.yZoomByPlot[plotId]; + } + } + this.incrementMetric('yAxisZoomReset'); + this.processPlotData(); + }, + getSvgElForPlot : function(plot) { return d3.select('#' + plot.renderTo + ' svg'); }, From dc33cdf0beb0eded17f5abd099269a40fb6c93a4 Mon Sep 17 00:00:00 2001 From: Ankur Juneja Date: Tue, 2 Jun 2026 07:07:25 -0700 Subject: [PATCH 2/9] Fix accessibility issues suggested by WAVE (#1220) --- .../view/precursorConflictResolutionView.jsp | 8 +++--- webapp/TargetedMS/css/qcTrendPlotReport.css | 1 + webapp/TargetedMS/js/QCTrendPlotPanel.js | 28 +++++++++---------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/org/labkey/targetedms/view/precursorConflictResolutionView.jsp b/src/org/labkey/targetedms/view/precursorConflictResolutionView.jsp index 90a66ebdc..caa2fcb10 100644 --- a/src/org/labkey/targetedms/view/precursorConflictResolutionView.jsp +++ b/src/org/labkey/targetedms/view/precursorConflictResolutionView.jsp @@ -119,12 +119,12 @@ $(document).ready(function () { { row.child.hide(); tr.removeClass('shown'); - $("." + cls).children('img').attr('src', "<%=getWebappURL("_images/plus.gif")%>"); + $("." + cls).children('img').attr('src', "<%=getWebappURL("_images/plus.gif")%>").attr('alt', 'Expand row details'); } else { row.child.show(); tr.addClass('shown'); - $("." + cls).children('img').attr('src', "<%=getWebappURL("_images/minus.gif")%>"); + $("." + cls).children('img').attr('src', "<%=getWebappURL("_images/minus.gif")%>").attr('alt', 'Collapse row details'); } if(!srcTd.hasClass('content_loaded')) @@ -271,7 +271,7 @@ function toggleCheckboxSelection(element) - "/> + " alt="Expand row details"/> @@ -291,7 +291,7 @@ function toggleCheckboxSelection(element) - "/> + " alt="Expand row details"/> diff --git a/webapp/TargetedMS/css/qcTrendPlotReport.css b/webapp/TargetedMS/css/qcTrendPlotReport.css index 51e40d084..2efcc589a 100644 --- a/webapp/TargetedMS/css/qcTrendPlotReport.css +++ b/webapp/TargetedMS/css/qcTrendPlotReport.css @@ -70,6 +70,7 @@ font-size: 18px; padding: 0 8px; border: solid #c0c0c0 1px; + background: none; } .qc-paging-prev { border-right-width: 0; diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index 90bb4f8de..ec699977d 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -1021,8 +1021,8 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { columns: 2, vertical: false, items: [ - { boxLabel: 'per replicate', id: 'x-axis-grouping-replicate', name: 'xAxisGrouping', inputValue: 'replicate', checked: this.groupedX === false }, - { boxLabel: 'per date', id: 'x-axis-grouping-date', name: 'xAxisGrouping', inputValue: 'date', checked: this.groupedX === true } + { boxLabel: 'per replicate', id: 'x-axis-grouping-replicate', name: 'xAxisGrouping', inputValue: 'replicate', checked: this.groupedX === false, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'X-axis grouping: per replicate'}); } } }, + { boxLabel: 'per date', id: 'x-axis-grouping-date', name: 'xAxisGrouping', inputValue: 'date', checked: this.groupedX === true, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'X-axis grouping: per date'}); } } } ], listeners: { scope: this, @@ -1051,8 +1051,8 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { columns: 2, vertical: false, items: [ - { boxLabel: 'per precursor', name: 'showPlots', id: 'plots-per-precursor', inputValue: 'per-precursor', checked: this.singlePlot === false }, - { boxLabel: 'combined', name: 'showPlots', id: 'plots-combined', inputValue: 'combined', checked: this.singlePlot === true } + { boxLabel: 'per precursor', name: 'showPlots', id: 'plots-per-precursor', inputValue: 'per-precursor', checked: this.singlePlot === false, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'Plots: per precursor'}); } } }, + { boxLabel: 'combined', name: 'showPlots', id: 'plots-combined', inputValue: 'combined', checked: this.singlePlot === true, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'Plots: combined'}); } } } ], listeners: { scope: this, @@ -1082,8 +1082,8 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { columns: 2, vertical: false, items: [ - { boxLabel: 'show', id: 'excluded-replicates-show', name: 'excludedSamples', inputValue: 'show', checked: this.showExcluded === true }, - { boxLabel: 'hide', id: 'excluded-replicates-hide', name: 'excludedSamples', inputValue: 'hide', checked: this.showExcluded === false } + { boxLabel: 'show', id: 'excluded-replicates-show', name: 'excludedSamples', inputValue: 'show', checked: this.showExcluded === true, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'Excluded replicates: show'}); } } }, + { boxLabel: 'hide', id: 'excluded-replicates-hide', name: 'excludedSamples', inputValue: 'hide', checked: this.showExcluded === false, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'Excluded replicates: hide'}); } } } ], listeners: { scope: this, @@ -1111,8 +1111,8 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { columns: 2, vertical: false, items: [ - { boxLabel: 'show', id: 'excluded-precursors-show', name: 'excludedPrecursors', inputValue: 'show', checked: this.showExcludedPrecursors === true }, - { boxLabel: 'hide', id: 'excluded-precursors-hide', name: 'excludedPrecursors', inputValue: 'hide', checked: this.showExcludedPrecursors === false } + { boxLabel: 'show', id: 'excluded-precursors-show', name: 'excludedPrecursors', inputValue: 'show', checked: this.showExcludedPrecursors === true, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'Excluded precursors: show'}); } } }, + { boxLabel: 'hide', id: 'excluded-precursors-hide', name: 'excludedPrecursors', inputValue: 'hide', checked: this.showExcludedPrecursors === false, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'Excluded precursors: hide'}); } } } ], listeners: { scope: this, @@ -1145,8 +1145,8 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { columns: 2, vertical: false, items: [ - { boxLabel: 'always show', id: 'reference-guide-set-show', name: 'referenceGuideSets', inputValue: 'show', checked: this.showReferenceGS === true }, - { boxLabel: 'when in date range', id: 'reference-guide-set-hide', name: 'referenceGuideSets', inputValue: 'hide', checked: this.showReferenceGS === false } + { boxLabel: 'always show', id: 'reference-guide-set-show', name: 'referenceGuideSets', inputValue: 'show', checked: this.showReferenceGS === true, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'Reference guide sets: always show'}); } } }, + { boxLabel: 'when in date range', id: 'reference-guide-set-hide', name: 'referenceGuideSets', inputValue: 'hide', checked: this.showReferenceGS === false, listeners: { afterrender: function(r) { r.inputEl.set({'aria-label': 'Reference guide sets: when in date range'}); } } } ], listeners: { scope: this, @@ -1286,11 +1286,11 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { getPaginationBtns: function(numOfPrecursors) { var btnHtml = ''; - btnHtml += ''; + btnHtml += ''; - btnHtml += ''; + btnHtml += ''; return btnHtml; }, From 8d289d2af0e8a9296027ca841ffc1d12ea01fa69 Mon Sep 17 00:00:00 2001 From: Ankur Juneja Date: Sun, 7 Jun 2026 09:16:48 -0700 Subject: [PATCH 3/9] Add Annotation makes it hard to interact with existing annotations on dense plots (#1225) #### Rationale https://github.com/LabKey/internal-issues/issues/1207 - Annotations on dense plots gets hindered by 'Add Annotation' markers making it difficult to hover or click the annotation to see/modify the details. #### Changes * Render annotation glyphs after add annotation glyphs --- webapp/TargetedMS/js/QCTrendPlotPanel.js | 134 ++++++++++++----------- 1 file changed, 68 insertions(+), 66 deletions(-) diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index ec699977d..90b25a755 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -2547,72 +2547,6 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { return '#' + d['Color']; }; - let annotations = this.getSvgElForPlot(plot).selectAll("path.annotation").data(this.annotationData) - .enter().append("path").attr("class", "annotation") - .attr("d", this.annotationShape(4)).attr('transform', transformAcc) - .style("fill", colorAcc).style("stroke", colorAcc); - - // add mouseover effects for fun - let mouseOn = function(pt, strokeWidth, d) { - d3.select(pt).transition().duration(800).attr("stroke-width", strokeWidth).ease("elastic"); - - if (!pt._tippy) { - let date = new Date(d['Date']); - let dateStr = me.formatDate(date, date.getHours() !== 0 || date.getMinutes() !== 0 || date.getSeconds() !== 0); - let content = "" - + "" - + "" - + "" - + ""; - - if (d['ContainerPath'] && d['ContainerPath'] !== LABKEY.ActionURL.getContainer()) { - let containerPath = LABKEY.Utils.encodeHtml(d['ContainerPath']); - if (!containerPath.startsWith('/')) { - containerPath = '/' + containerPath; - } - content += ""; - } - content += "
Created By:" + LABKEY.Utils.encodeHtml(d['DisplayName']) + "
Type:" + LABKEY.Utils.encodeHtml(d['Name']) + "
Date:" + LABKEY.Utils.encodeHtml(dateStr) + "
Description:" + LABKEY.Utils.encodeHtml(d['Description']) + "
Shared From:" + containerPath + "
"; - - tippy(pt, { - content: content, - allowHTML: true, - arrow: true, - theme: 'light-border', - placement: 'top', - offset: [0, 8], - onMount(instance) { - const tippyBox = instance.popper.querySelector('.tippy-box'); - const tippyContent = instance.popper.querySelector('.tippy-content'); - const tippyArrow = instance.popper.querySelector('.tippy-arrow'); - - if (tippyBox) { - tippyBox.style.color = 'black'; - tippyBox.style.backgroundColor = 'white'; - tippyBox.style.border = '1px solid black'; - } - if (tippyContent) { - tippyContent.style.padding = '6px'; - } - if (tippyArrow) { - tippyArrow.style.bottom = '-1px'; - } - } - }); - } - }; - var mouseOff = function(pt) { - d3.select(pt).transition().duration(800).attr("stroke-width", 1).ease("elastic"); - }; - annotations.on("mouseover", function(d){ return mouseOn(this, 3, d); }); - annotations.on("mouseout", function(){ return mouseOff(this); }); - - if (this.canUserEdit()) { - annotations.on("click", function (d) { - me.openAnnotationDialog(false, d).show(); - }); - } - // Add add-annotation markers with '+' shape const addShape = function (size) { var s = size / 2; @@ -2699,6 +2633,74 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { if (!this.canUserEdit()) { nonAnnotationGroups.style("display", "none"); } + + // Render the existing annotation glyphs after the add-annotation markers so they paint on + // top and take precedence for hover/click when the markers overlap them. + let annotations = this.getSvgElForPlot(plot).selectAll("path.annotation").data(this.annotationData) + .enter().append("path").attr("class", "annotation") + .attr("d", this.annotationShape(4)).attr('transform', transformAcc) + .style("fill", colorAcc).style("stroke", colorAcc); + + // add mouseover effects for fun + let mouseOn = function(pt, strokeWidth, d) { + d3.select(pt).transition().duration(800).attr("stroke-width", strokeWidth).ease("elastic"); + + if (!pt._tippy) { + let date = new Date(d['Date']); + let dateStr = me.formatDate(date, date.getHours() !== 0 || date.getMinutes() !== 0 || date.getSeconds() !== 0); + let content = "" + + "" + + "" + + "" + + ""; + + if (d['ContainerPath'] && d['ContainerPath'] !== LABKEY.ActionURL.getContainer()) { + let containerPath = LABKEY.Utils.encodeHtml(d['ContainerPath']); + if (!containerPath.startsWith('/')) { + containerPath = '/' + containerPath; + } + content += ""; + } + content += "
Created By:" + LABKEY.Utils.encodeHtml(d['DisplayName']) + "
Type:" + LABKEY.Utils.encodeHtml(d['Name']) + "
Date:" + LABKEY.Utils.encodeHtml(dateStr) + "
Description:" + LABKEY.Utils.encodeHtml(d['Description']) + "
Shared From:" + containerPath + "
"; + + tippy(pt, { + content: content, + allowHTML: true, + arrow: true, + theme: 'light-border', + placement: 'top', + offset: [0, 8], + onMount(instance) { + const tippyBox = instance.popper.querySelector('.tippy-box'); + const tippyContent = instance.popper.querySelector('.tippy-content'); + const tippyArrow = instance.popper.querySelector('.tippy-arrow'); + + if (tippyBox) { + tippyBox.style.color = 'black'; + tippyBox.style.backgroundColor = 'white'; + tippyBox.style.border = '1px solid black'; + } + if (tippyContent) { + tippyContent.style.padding = '6px'; + } + if (tippyArrow) { + tippyArrow.style.bottom = '-1px'; + } + } + }); + } + }; + var mouseOff = function(pt) { + d3.select(pt).transition().duration(800).attr("stroke-width", 1).ease("elastic"); + }; + annotations.on("mouseover", function(d){ return mouseOn(this, 3, d); }); + annotations.on("mouseout", function(){ return mouseOff(this); }); + + if (this.canUserEdit()) { + annotations.on("click", function (d) { + me.openAnnotationDialog(false, d).show(); + }); + } }, openAnnotationDialog: function (addNew, data) { From 5cf0aac02c3c0636d8534f48a22afdd57acaf2eb Mon Sep 17 00:00:00 2001 From: vagisha Date: Mon, 8 Jun 2026 17:43:50 -0700 Subject: [PATCH 4/9] Restrict library spectra for guest users on large spectrum libraries (#1226) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Do not show library spectra to guests when the spectrum library file is ≥ 500 MB; show the existing "Login to view this data" message instead. - The restriction applies to all library-read paths (the spectrum panel, the spectrum AJAX endpoint, and the chromatogram peptide-ID retention-time markers). - Logged-in users and small libraries are unaffected. Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../targetedms/TargetedMSController.java | 34 +++++++++++ .../targetedms/chart/ChromatogramDataset.java | 7 +++ .../spectrum/LibrarySpectrumMatchGetter.java | 60 +++++++++++++++++++ 3 files changed, 101 insertions(+) diff --git a/src/org/labkey/targetedms/TargetedMSController.java b/src/org/labkey/targetedms/TargetedMSController.java index 7ae73decc..d75d0d8c7 100644 --- a/src/org/labkey/targetedms/TargetedMSController.java +++ b/src/org/labkey/targetedms/TargetedMSController.java @@ -3232,8 +3232,31 @@ public void addNavTrail(NavTree root) } } + /** + * Reading library spectra from a large library file can take many seconds over network storage, especially for large + * EncyclopeDIA libraries. To protect public folders from aggressive bots, do not show library spectra to guests when + * the library is large. Show the login prompt instead. + * Returns true (and adds the login view) when the library spectrum should be withheld. + */ + private boolean addGuestSpectrumGate(TargetedMSRun run, VBox vbox) + { + if (!LibrarySpectrumMatchGetter.blockSpectraForGuest(getUser(), run.getId())) + { + return false; + } + HtmlView loginView = getLoginView(getViewContext(), getContainer()); + loginView.setTitle("Library Spectrum"); + loginView.setFrame(WebPartView.FrameType.PORTAL); + vbox.addView(loginView); + return true; + } + private void addSpectrumViews(TargetedMSRun run, VBox vbox, Precursor precursor, BindException errors) { + if (addGuestSpectrumGate(run, vbox)) + { + return; + } PipeRoot root = PipelineService.get().getPipelineRootSetting(getContainer()); if (null != root) { @@ -3252,6 +3275,10 @@ private void addSpectrumViews(TargetedMSRun run, VBox vbox, Precursor precursor, private void addSpectrumViews(TargetedMSRun run, VBox vbox, Peptide peptide, BindException errors) { + if (addGuestSpectrumGate(run, vbox)) + { + return; + } PipeRoot root = PipelineService.get().getPipelineRootSetting(getContainer()); if (null != root) { @@ -3336,6 +3363,13 @@ public Object execute(SpectrumDataForm form, BindException errors) } TargetedMSRun run = TargetedMSManager.getRunForGeneralMolecule(peptide.getId()); + // Apply the same guest gate as the spectrum views (see LibrarySpectrumMatchGetter.blockSpectraForGuest). + if (LibrarySpectrumMatchGetter.blockSpectraForGuest(getUser(), run.getId())) + { + response.put("error", "Login to view this data"); + return response; + } + List libraries = LibraryManager.getLibraries(run.getId()); PeptideSettings.SpectrumLibrary library = null; for (PeptideSettings.SpectrumLibrary lib : libraries) diff --git a/src/org/labkey/targetedms/chart/ChromatogramDataset.java b/src/org/labkey/targetedms/chart/ChromatogramDataset.java index afeba8579..c78fd36cc 100644 --- a/src/org/labkey/targetedms/chart/ChromatogramDataset.java +++ b/src/org/labkey/targetedms/chart/ChromatogramDataset.java @@ -901,6 +901,13 @@ public void build() protected List getPeptideIdRetentionTimes() { + // Skip peptide-ID retention-time markers for guests when the library is large. Reading large libraries can be slow over network storage. + // See LibrarySpectrumMatchGetter.blockSpectraForGuest. + if (LibrarySpectrumMatchGetter.blockSpectraForGuest(_user, _run.getId())) + { + return Collections.emptyList(); + } + SampleFile sampleFile = ReplicateManager.getSampleFile(_pChromInfo.getSampleFileId()); // TODO: May want to move LocalDirectory up to controller, where others are created. Sharing probably desired. diff --git a/src/org/labkey/targetedms/view/spectrum/LibrarySpectrumMatchGetter.java b/src/org/labkey/targetedms/view/spectrum/LibrarySpectrumMatchGetter.java index 531e9f161..cd5e4178a 100644 --- a/src/org/labkey/targetedms/view/spectrum/LibrarySpectrumMatchGetter.java +++ b/src/org/labkey/targetedms/view/spectrum/LibrarySpectrumMatchGetter.java @@ -16,6 +16,7 @@ package org.labkey.targetedms.view.spectrum; import org.apache.commons.io.FilenameUtils; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.cache.BlockingCache; @@ -24,6 +25,7 @@ import org.labkey.api.data.Container; import org.labkey.api.security.User; import org.labkey.api.util.FileUtil; +import org.labkey.api.util.logging.LogHelper; import org.labkey.targetedms.TargetedMSManager; import org.labkey.targetedms.TargetedMSRun; import org.labkey.targetedms.TargetedMSSchema; @@ -42,6 +44,8 @@ import org.labkey.targetedms.query.PeptideManager; import org.labkey.targetedms.query.PrecursorManager; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.sql.SQLException; import java.util.ArrayList; @@ -59,8 +63,64 @@ */ public class LibrarySpectrumMatchGetter { + private static final Logger LOG = LogHelper.getLogger(LibrarySpectrumMatchGetter.class, "Matches library spectra and retention times for the library spectrum viewer"); + private static final int CACHE_SIZE = 10; + // Reading library spectra and retention times from large spectrum libraries can be slow over network storage. + // For EncyclopeDIA .elib we read one row per source file for the peptide. This can be hundreds of rows and the needed + // columns are not in the index, so each table row lookup is a separate network round-trip on GPFS. + // For BiblioSpec .blib we scan the unindexed RetentionTimes table for the RT of the peptide in all the scans and source + // files. + // PanoramaWeb has large files of both types, so the size gate covers both library types. To protect public folders from + // aggressive bots, library spectra are not shown to guests when the library file is at or above this size. Guests are + // asked to log in instead. + private static final long GUEST_SPECTRUM_LIBRARY_SIZE_LIMIT = 500L * 1024 * 1024; // 500 MB + + /** + * Returns true if library spectra should NOT be shown to the given user for the given run, + * i.e. the user is a guest and the run references a supported spectrum library file that is at + * or above {@link #GUEST_SPECTRUM_LIBRARY_SIZE_LIMIT}. Logged-in users are never blocked, and + * small libraries are read in place as before. + */ + public static boolean blockSpectraForGuest(User user, long runId) + { + if (!user.isGuest()) + { + return false; + } + for (Path libPath : LibraryManager.getLibraryFilePaths(runId).values()) + { + if (isLargeSpectrumLibrary(libPath)) + { + return true; + } + } + return false; + } + + private static boolean isLargeSpectrumLibrary(Path libPath) + { + // Only .elib/.blib libraries are read for spectra; ignore anything we cannot read. + if (libPath == null || getReaderForLibrary(FileUtil.getFileName(libPath)) == null) + { + return false; + } + try + { + // Files.size throws NoSuchFileException if the file is missing, so a separate Files.exists + // check is unnecessary and would add a second filesystem round-trip on network storage. + return Files.size(libPath) >= GUEST_SPECTRUM_LIBRARY_SIZE_LIMIT; + } + catch (IOException e) + { + // If we cannot stat the file it is missing or unreadable, in which case the + // downstream library read will fail too. + LOG.warn("Could not determine size of spectrum library file " + libPath, e); + return false; + } + } + private static final BlockingCache> _peptideIdRtsCache = CacheManager.getBlockingCache(CACHE_SIZE, CacheManager.DAY, "TargetedMS peptide ID retention times", (precursor, argument) -> { From ffabd0c20d14ff51208e504cea96a206abd7aa32 Mon Sep 17 00:00:00 2001 From: vagisha Date: Wed, 10 Jun 2026 18:32:14 -0700 Subject: [PATCH 5/9] Reduce log noise when a spectrum library file is missing or unreadable (#1231) - Log a single WARN line (with the exception type) instead of the full trace. - Keep the full stack trace at DEBUG (off by default at INFO) for when it is needed. --- .../view/spectrum/LibrarySpectrumMatchGetter.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/org/labkey/targetedms/view/spectrum/LibrarySpectrumMatchGetter.java b/src/org/labkey/targetedms/view/spectrum/LibrarySpectrumMatchGetter.java index cd5e4178a..5d7d6c9ab 100644 --- a/src/org/labkey/targetedms/view/spectrum/LibrarySpectrumMatchGetter.java +++ b/src/org/labkey/targetedms/view/spectrum/LibrarySpectrumMatchGetter.java @@ -114,9 +114,10 @@ private static boolean isLargeSpectrumLibrary(Path libPath) } catch (IOException e) { - // If we cannot stat the file it is missing or unreadable, in which case the - // downstream library read will fail too. - LOG.warn("Could not determine size of spectrum library file " + libPath, e); + // The file is missing or unreadable; the downstream library read would fail too. + // Log a single WARN line and keep the stack trace at DEBUG so frequent hits do not flood the primary log. + LOG.warn("Could not determine size of spectrum library file " + libPath + " (" + e.getClass().getSimpleName() + ")"); + LOG.debug("Could not determine size of spectrum library file " + libPath, e); return false; } } From bc1bd19618532efb1e02851cf98439e2945f7294 Mon Sep 17 00:00:00 2001 From: vagisha Date: Sun, 14 Jun 2026 10:46:10 -0700 Subject: [PATCH 6/9] Update psi-ms-PARSED.xml to include new instruments added to the PSI-MS CV (#1232) - ~120 instruments were added in a single release in Sept 2025 (driven by metabolomics data repositories) - Recently, new instruments announced at ASMS 2026 were added in release 4.1.254 --- resources/psi-ms-PARSED.xml | 137 ++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/resources/psi-ms-PARSED.xml b/resources/psi-ms-PARSED.xml index fa197bf67..66399935f 100644 --- a/resources/psi-ms-PARSED.xml +++ b/resources/psi-ms-PARSED.xml @@ -342,4 +342,141 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file From 4c77c77da5eadb556eabb5e8c1b61523915e1ec4 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Mon, 15 Jun 2026 09:51:19 -0700 Subject: [PATCH 7/9] Fix PassportTest and improve scoping (#1233) #### Rationale PassportTest needs an update to expect the updated data being pulled dynamically from UniProt #### Changes * Be tolerant of changes in protein metadata from third-party service * Add container checks * Backport query name quoting --- .../targetedms/SkylineAuditLogManager.java | 2 +- .../TargetedMSContainerScopingTest.java | 143 ++++++++++++++++++ .../targetedms/TargetedMSController.java | 6 +- .../labkey/targetedms/TargetedMSModule.java | 3 +- .../targetedms/outliers/OutlierGenerator.java | 3 +- .../parser/skyaudit/AuditLogEntry.java | 24 ++- .../targetedms/passport/PassportTest.java | 19 ++- 7 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 src/org/labkey/targetedms/TargetedMSContainerScopingTest.java diff --git a/src/org/labkey/targetedms/SkylineAuditLogManager.java b/src/org/labkey/targetedms/SkylineAuditLogManager.java index a3d9077f7..15aa690da 100644 --- a/src/org/labkey/targetedms/SkylineAuditLogManager.java +++ b/src/org/labkey/targetedms/SkylineAuditLogManager.java @@ -606,7 +606,7 @@ public void testEntryRetrieval() throws AuditLogException { AuditLogTree node = new SkylineAuditLogManager(_container, null).buildLogTree(_docGUID); while(node.iterator().hasNext()){ - AuditLogEntry ent = AuditLogEntry.retrieve(node.getEntryId()); + AuditLogEntry ent = AuditLogEntry.retrieve(node.getEntryId(), _container); assertNotNull(ent); node = node.iterator().next(); } diff --git a/src/org/labkey/targetedms/TargetedMSContainerScopingTest.java b/src/org/labkey/targetedms/TargetedMSContainerScopingTest.java new file mode 100644 index 000000000..cb6d13999 --- /dev/null +++ b/src/org/labkey/targetedms/TargetedMSContainerScopingTest.java @@ -0,0 +1,143 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.targetedms; + +import jakarta.servlet.http.HttpServletResponse; +import org.apache.logging.log4j.Logger; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.labkey.api.data.Container; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.Table; +import org.labkey.api.data.TableSelector; +import org.labkey.api.exp.api.ExpRun; +import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.pipeline.PipeRoot; +import org.labkey.api.pipeline.PipelineService; +import org.labkey.api.query.FieldKey; +import org.labkey.api.security.permissions.AbstractContainerScopingTest; +import org.labkey.api.util.GUID; +import org.labkey.api.util.logging.LogHelper; +import org.labkey.api.view.ActionURL; +import org.labkey.api.view.ViewBackgroundInfo; +import org.labkey.targetedms.parser.skyaudit.UnitTestUtil; + +import java.io.File; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +/** + * Container-scoping integration tests for {@link TargetedMSController} actions that resolve an object by a + * global id. Each test sets up the same object in one folder and confirms the action rejects a request that addresses + * via the wrong container + */ +public class TargetedMSContainerScopingTest extends AbstractContainerScopingTest +{ + private static final Logger LOG = LogHelper.getLogger(TargetedMSContainerScopingTest.class, "TargetedMS container scoping tests"); + private static final GUID DOC_GUID = new GUID("8f1c2a44-3c5e-4f0a-9d2b-6e7a1b3c5d9e"); + + @Before + @After + public void cleanupAuditLog() + { + UnitTestUtil.cleanupDatabase(DOC_GUID); + } + + /** + * RemoveLinkVersionAction relinks a Skyline document-version chain by run rowId. It must reject a run that lives in + * a different container (mirroring its sibling SaveLinkVersionsAction), otherwise an editor in one folder could + * rewrite the version chain of documents they can't see. + */ + @Test + public void removeLinkVersionIsContainerScoped() throws Exception + { + Container owner = createContainer("LinkOwner"); + Container other = createContainer("LinkOther"); + + // Negative: a run that lives in 'owner' cannot be relinked through the 'other' folder + ExpRun foreignRun = createRun(owner, "foreign-version"); + ActionURL foreign = new ActionURL(TargetedMSController.RemoveLinkVersionAction.class, other) + .addParameter("rowId", foreignRun.getRowId()); + assertStatus(HttpServletResponse.SC_BAD_REQUEST, post(foreign, getAdmin())); + + // Positive: a run that lives in the acting folder is accepted + ExpRun localRun = createRun(other, "local-version"); + ActionURL local = new ActionURL(TargetedMSController.RemoveLinkVersionAction.class, other) + .addParameter("rowId", localRun.getRowId()); + assertStatus(HttpServletResponse.SC_OK, post(local, getAdmin())); + } + + /** + * ShowSkylineAuditLogExtraInfoAJAXAction renders the full Skyline audit detail (user, timestamp, extra info) for an + * audit entry resolved by global EntryId. It must only render entries whose owning run is in the requested folder, + * otherwise any reader could read another folder's document audit history by guessing entry ids. + */ + @Test + public void auditLogExtraInfoIsContainerScoped() throws Exception + { + Container owner = createContainer("AuditOwner"); + Container other = createContainer("AuditOther"); + + int entryId = importAuditLogEntry(owner); + + // Negative: the entry's run lives in 'owner', so requesting it via 'other' must 404 rather than leak detail + ActionURL foreign = new ActionURL(TargetedMSController.ShowSkylineAuditLogExtraInfoAJAXAction.class, other) + .addParameter("entryId", entryId); + assertStatus(HttpServletResponse.SC_NOT_FOUND, get(foreign, getAdmin())); + + // Positive: requesting the entry through its owning folder renders normally + ActionURL owned = new ActionURL(TargetedMSController.ShowSkylineAuditLogExtraInfoAJAXAction.class, owner) + .addParameter("entryId", entryId); + assertStatus(HttpServletResponse.SC_OK, get(owned, getAdmin())); + } + + /** Persist a minimal, saved {@link ExpRun} in the given container so the controller can resolve it by rowId. */ + private ExpRun createRun(Container c, String name) throws Exception + { + ExperimentService svc = ExperimentService.get(); + ExpRun run = svc.createExperimentRun(c, name); + PipeRoot pipeRoot = PipelineService.get().findPipelineRoot(c); + assertNotNull("No pipeline root for " + c.getPath(), pipeRoot); + run.setFilePathRoot(pipeRoot.getRootPath()); + run.setProtocol(svc.ensureSampleDerivationProtocol(getAdmin())); + ViewBackgroundInfo info = new ViewBackgroundInfo(c, getAdmin(), null); + return svc.saveSimpleExperimentRun(run, Collections.emptyMap(), Collections.emptyMap(), + Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap(), info, null, false); + } + + /** Insert a TargetedMS run in the given container, import a sample Skyline audit log for it, and return an EntryId. */ + private int importAuditLogEntry(Container c) throws Exception + { + TargetedMSRun run = new TargetedMSRun(); + run.setContainer(c); + run.setDocumentGUID(DOC_GUID); + Table.insert(getAdmin(), TargetedMSManager.getTableInfoRuns(), run); + + File zip = UnitTestUtil.getSampleDataFile("AuditLogFiles/MethodEdit_v1.zip"); + File logFile = UnitTestUtil.extractLogFromZip(zip, LOG); + new SkylineAuditLogManager(c, null).importAuditLogFile(getAdmin(), logFile, DOC_GUID, run); + + List entryIds = new TableSelector( + TargetedMSManager.getTableInfoSkylineRunAuditLogEntry(), + Set.of("AuditLogEntryId"), + new SimpleFilter(FieldKey.fromParts("VersionId"), run.getId()), null) + .getArrayList(Integer.class); + assertFalse("No audit log entries were imported", entryIds.isEmpty()); + return entryIds.get(0); + } +} diff --git a/src/org/labkey/targetedms/TargetedMSController.java b/src/org/labkey/targetedms/TargetedMSController.java index 58719e055..85c04b12d 100644 --- a/src/org/labkey/targetedms/TargetedMSController.java +++ b/src/org/labkey/targetedms/TargetedMSController.java @@ -4957,7 +4957,9 @@ public static class ShowSkylineAuditLogExtraInfoAJAXAction extends SimpleViewAct @Override public ModelAndView getView(SkylineAuditLogExtraInfoForm form, BindException errors) { - AuditLogEntry ent = AuditLogEntry.retrieve(form.getEntryId()); + AuditLogEntry ent = AuditLogEntry.retrieve(form.getEntryId(), getContainer()); + if (ent == null) + throw new NotFoundException("No audit log entry found for id " + form.getEntryId() + " in this folder."); getPageConfig().setTemplate(PageConfig.Template.None); return new JspView<>("/org/labkey/targetedms/view/skylineAuditLogExtraInfoView.jsp", ent); } @@ -7657,7 +7659,7 @@ public void validateForm(RowIdForm form, Errors errors) // verify that the run rowId is valid and matches an existing run // and if the run replaces any other runs, it should only replace one ExpRun run = ExperimentService.get().getExpRun(form.getRowId()); - if (run == null) + if (run == null || !run.getContainer().equals(getContainer())) errors.reject(ERROR_MSG, "No run found for id " + form.getRowId() + "."); else if (!run.getReplacesRuns().isEmpty() && run.getReplacesRuns().size() > 1) errors.reject(ERROR_MSG, "Run " + form.getRowId() + " replaces more than one run."); diff --git a/src/org/labkey/targetedms/TargetedMSModule.java b/src/org/labkey/targetedms/TargetedMSModule.java index d0469a0a9..533785b62 100644 --- a/src/org/labkey/targetedms/TargetedMSModule.java +++ b/src/org/labkey/targetedms/TargetedMSModule.java @@ -687,7 +687,8 @@ public Set getIntegrationTests() { return Set.of( MsDataSourceUtil.TestCase.class, - SkylineAuditLogManager.TestCase.class + SkylineAuditLogManager.TestCase.class, + TargetedMSContainerScopingTest.class ); } diff --git a/src/org/labkey/targetedms/outliers/OutlierGenerator.java b/src/org/labkey/targetedms/outliers/OutlierGenerator.java index 6d5ff96ec..994da0615 100644 --- a/src/org/labkey/targetedms/outliers/OutlierGenerator.java +++ b/src/org/labkey/targetedms/outliers/OutlierGenerator.java @@ -26,6 +26,7 @@ import org.labkey.api.data.TableSelector; import org.labkey.api.query.QueryService; import org.labkey.api.security.User; +import org.labkey.api.sql.LabKeySql; import org.labkey.api.targetedms.model.SampleFileInfo; import org.labkey.api.util.Pair; import org.labkey.targetedms.TargetedMSManager; @@ -112,7 +113,7 @@ private String getEachSeriesTypePlotDataSql(int seriesIndex, QCMetricConfigurati sql.append(" CAST(IFDEFINED(SeriesLabel) AS VARCHAR) AS SeriesLabel, "); sql.append("\nMetricValue, 0 as metric, ").append(seriesIndex).append(" AS MetricSeriesIndex, ").append(configuration.getId()).append(" AS MetricId"); - sql.append("\n FROM ").append(schemaName).append('.').append(queryName); + sql.append("\n FROM ").append(schemaName).append('.').append(LabKeySql.quoteIdentifier(queryName)); if (!annotationGroups.isEmpty()) { diff --git a/src/org/labkey/targetedms/parser/skyaudit/AuditLogEntry.java b/src/org/labkey/targetedms/parser/skyaudit/AuditLogEntry.java index 2d0e35445..3743c1e6d 100644 --- a/src/org/labkey/targetedms/parser/skyaudit/AuditLogEntry.java +++ b/src/org/labkey/targetedms/parser/skyaudit/AuditLogEntry.java @@ -15,6 +15,7 @@ */ package org.labkey.targetedms.parser.skyaudit; +import org.labkey.api.data.Container; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Table; import org.labkey.api.data.TableSelector; @@ -22,6 +23,7 @@ import org.labkey.api.security.User; import org.labkey.api.util.GUID; import org.labkey.targetedms.TargetedMSManager; +import org.labkey.targetedms.TargetedMSRun; import java.math.BigDecimal; import java.math.BigInteger; @@ -70,13 +72,25 @@ public AuditLogEntry(BigDecimal documentFormatVersion) _documentFormatVersion = documentFormatVersion; } - public static AuditLogEntry retrieve(int pEntryId) + /** + * Container-scoped retrieve. Returns the entry only if its owning Skyline run lives in the supplied container, + * preventing callers from reading audit detail for documents in folders the user can't access. An entryId can + * map to more than one run when documents share an audit history, so check every match for one in the container. + */ + public static AuditLogEntry retrieve(int pEntryId, Container container) { TableSelector sel = new TableSelector(TargetedMSManager.getTableInfoSkylineAuditLog(), new SimpleFilter(FieldKey.fromParts("EntryId"), pEntryId), null); - List results = sel.getArrayList(AuditLogEntry.class); - // Possible to get more than one match if two documents share an audit history. In this case, we don't care - // which we use - return results.isEmpty() ? null : results.get(0); + for (AuditLogEntry entry : sel.getArrayList(AuditLogEntry.class)) + { + Long runId = entry.getRunId(); + if (runId != null) + { + TargetedMSRun run = TargetedMSManager.getRun(runId); + if (run != null && container.equals(run.getContainer())) + return entry; + } + } + return null; } public AuditLogTree getTreeEntry(){ diff --git a/test/src/org/labkey/test/tests/targetedms/passport/PassportTest.java b/test/src/org/labkey/test/tests/targetedms/passport/PassportTest.java index 76554c4d2..c85b5c3b2 100644 --- a/test/src/org/labkey/test/tests/targetedms/passport/PassportTest.java +++ b/test/src/org/labkey/test/tests/targetedms/passport/PassportTest.java @@ -24,6 +24,8 @@ import java.util.List; +import static org.junit.Assert.assertTrue; + @Category({}) public class PassportTest extends PassportTestPart { @@ -91,13 +93,20 @@ private void testNormalStuff() waitForElement(Locator.tagWithId("span", "filteredPeptideCount").childTag("green")); assertElementContains(Locator.xpath("//span[@id='filteredPeptideCount']//green"), "19"); - //features - waitForElements(Locator.tagWithClass("td", "feature-sequencevariant"), 5); - waitForElements(Locator.tagWithClass("td", "feature-glycosylationsite"), 4); - waitForElements(Locator.tagWithClass("td", "feature-helix"), 7); - waitForElements(Locator.tagWithClass("td", "feature-turn"), 6); + // Features come from a live query against Uniprot, so be tolerant of any number above our baseline + waitForElementsAtLeast(Locator.tagWithClass("td", "feature-sequencevariant"), 3); + waitForElementsAtLeast(Locator.tagWithClass("td", "feature-glycosylationsite"), 3); + waitForElementsAtLeast(Locator.tagWithClass("td", "feature-helix"), 3); + waitForElementsAtLeast(Locator.tagWithClass("td", "feature-turn"), 3); } + public void waitForElementsAtLeast(final Locator loc, final int count) + { + waitFor(() -> loc.findElements(getDriver()).size() >= count, WAIT_FOR_JAVASCRIPT); + assertTrue("Element not present at least the expected number of times", loc.findElements(getDriver()).size() >= count); + } + + @LogMethod protected void testAsNormalUser() { From bc5da60564c7af72e2bf60473b327ea0a607a6f8 Mon Sep 17 00:00:00 2001 From: Ankur Juneja Date: Wed, 17 Jun 2026 10:16:09 -0700 Subject: [PATCH 8/9] Series highlight doesn't work when plot doesn't show replicate data points (#1234) --- webapp/TargetedMS/js/QCTrendPlotPanel.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index 90b25a755..2ee71802f 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -1509,7 +1509,8 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { pathMouseOver : function(event, pathData, layerSel, path, valueName, config) { if (pathData.group) { - this.highlightFragmentSeries(pathData.group); + // pass base fragment, like the other highlight triggers + this.highlightFragmentSeries(pathData.group.split(LABKEY.targetedms.QCPlotHelperBase.SERIES_NAME_SEP)[0]); } }, @@ -2069,6 +2070,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { let label = checkbox.closest('label'); if (label) { label.addEventListener('mouseenter', function() { + clearTimeout(precursorLeaveTimer); // cancel a pending precursor reset when moving sub-item -> header let hidden = me.hiddenPrecursorSeries || {}; let hiddenFragments = group.fragments.filter(function(f) { return !!hidden[f]; }); if (hiddenFragments.length > 0) { From 447c496e20c6a4fce473d99be10acb8633112e33 Mon Sep 17 00:00:00 2001 From: Dan Duffek Date: Wed, 17 Jun 2026 13:36:17 -0700 Subject: [PATCH 9/9] Back porting test fix. (#1238) --- .../test/tests/targetedms/InstrumentSchedulingTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/src/org/labkey/test/tests/targetedms/InstrumentSchedulingTest.java b/test/src/org/labkey/test/tests/targetedms/InstrumentSchedulingTest.java index f173a348d..f59935900 100644 --- a/test/src/org/labkey/test/tests/targetedms/InstrumentSchedulingTest.java +++ b/test/src/org/labkey/test/tests/targetedms/InstrumentSchedulingTest.java @@ -134,13 +134,8 @@ public void testSchedule() clickAndWait(Locator.linkWithText(PROJECT_1)); waitAndClickAndWait(Locator.linkWithText("Schedule instrument time")); - String yearMonth = Calendar.getInstance().get(Calendar.YEAR) + "-"; int month = (Calendar.getInstance().get(Calendar.MONTH) + 1); - if (month < 10) - { - yearMonth += yearMonth; - } - yearMonth += month; + String yearMonth = Calendar.getInstance().get(Calendar.YEAR) + "-" + (month < 10 ? "0" + month : "" + month); scheduleInstrument(yearMonth + "-02"); scheduleInstrument(yearMonth + "-03");