From f1cb4a6dc0d9d0c11618b5560fecc5009de870c7 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sun, 11 Oct 2026 13:13:06 -0700 Subject: [PATCH 1/4] Fix chromatogram and QC outlier exceptions reported to mothership Handle small-molecule transition/index count mismatches, duplicate molecule series labels, sample files not found in the current container, QC folders with no enabled metrics, replicate annotation filters in the precursor pivot query, and runs deleted while their import finishes. --- .../labkey/targetedms/SkylineDocImporter.java | 5 ++++- .../targetedms/TargetedMSController.java | 19 +++++++++++++++-- .../targetedms/chart/ChromatogramDataset.java | 21 ++++++++++++------- .../targetedms/outliers/OutlierGenerator.java | 2 ++ ...leculePrecursorChromatogramsTableInfo.java | 2 +- 5 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/org/labkey/targetedms/SkylineDocImporter.java b/src/org/labkey/targetedms/SkylineDocImporter.java index 2cafc1a2a..2c4405e49 100644 --- a/src/org/labkey/targetedms/SkylineDocImporter.java +++ b/src/org/labkey/targetedms/SkylineDocImporter.java @@ -261,7 +261,10 @@ public TargetedMSRun importRun(RunInfo runInfo, PipelineJob job) throws IOExcept TargetedMSManager.get().clearQCMetricCache(run.getContainer(), true); _progressMonitor.complete(); - return TargetedMSManager.getRun(_runId); + TargetedMSRun importedRun = TargetedMSManager.getRun(_runId); + if (importedRun == null) + throw new PipelineJobException("Could not find record for runId " + _runId + ", it may have already been deleted. Please try importing again."); + return importedRun; } catch (FileNotFoundException fnfe) { diff --git a/src/org/labkey/targetedms/TargetedMSController.java b/src/org/labkey/targetedms/TargetedMSController.java index a32b55fbc..cff8efc77 100644 --- a/src/org/labkey/targetedms/TargetedMSController.java +++ b/src/org/labkey/targetedms/TargetedMSController.java @@ -145,6 +145,7 @@ import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.UpdatePermission; +import org.labkey.api.sql.LabKeySql; import org.labkey.api.targetedms.RepresentativeDataState; import org.labkey.api.targetedms.RunRepresentativeDataState; import org.labkey.api.targetedms.TargetedMSService; @@ -3067,6 +3068,12 @@ public void setUpdate(boolean update) } public void appendReplicateFilters(SQLFragment sql, String colName) + { + appendReplicateFilters(sql, colName, false); + } + + /** @param labKeySql inline annotation values as literals, since LabKey SQL text can't carry parameters */ + public void appendReplicateFilters(SQLFragment sql, String colName, boolean labKeySql) { List replicateIds = getReplicatesFilterList(); if (!replicateIds.isEmpty()) @@ -3091,8 +3098,16 @@ public void appendReplicateFilters(SQLFragment sql, String colName) { if (!first) sql.append(" OR "); - sql.append(" (name = ").appendValue(annotation.getName()) - .append(" AND value = ").appendValue(annotation.getValue()).append(")"); + if (labKeySql) + { + sql.append(" (name = ").append(LabKeySql.quoteString(annotation.getName())) + .append(" AND value = ").append(LabKeySql.quoteString(annotation.getValue())).append(")"); + } + else + { + sql.append(" (name = ").appendValue(annotation.getName()) + .append(" AND value = ").appendValue(annotation.getValue()).append(")"); + } first = false; } sql.append(")"); diff --git a/src/org/labkey/targetedms/chart/ChromatogramDataset.java b/src/org/labkey/targetedms/chart/ChromatogramDataset.java index 4f00f2372..9b31657a9 100644 --- a/src/org/labkey/targetedms/chart/ChromatogramDataset.java +++ b/src/org/labkey/targetedms/chart/ChromatogramDataset.java @@ -29,6 +29,7 @@ import org.labkey.api.security.User; import org.labkey.api.util.Formats; import org.labkey.api.util.logging.LogHelper; +import org.labkey.api.view.NotFoundException; import org.labkey.api.view.ViewContext; import org.labkey.targetedms.TargetedMSManager; import org.labkey.targetedms.TargetedMSRun; @@ -322,7 +323,9 @@ public SampleFileDataset(SampleFileChromInfo chromInfo, User user, Container con super(user, container); _chromInfo = chromInfo; SampleFile sampleFile = TargetedMSManager.getSampleFile(chromInfo.getSampleFileId(), container); - Replicate replicate = TargetedMSManager.getReplicate(sampleFile.getReplicateId(), container); + Replicate replicate = sampleFile == null ? null : TargetedMSManager.getReplicate(sampleFile.getReplicateId(), container); + if (replicate == null) + throw new NotFoundException("Could not find sample file " + chromInfo.getSampleFileId() + " in " + container.getPath()); _run = TargetedMSManager.getRun(replicate.getRunId()); } @@ -645,7 +648,12 @@ public void build() Chromatogram chromatogram = pChromInfo.createChromatogram(_run); if (chromatogram != null) { - XYSeries series = new XYSeries(getLabel(pChromInfo)); + // Molecule labels omit isotope label and adduct, so two precursors can share one + String label = getLabel(pChromInfo); + String key = label; + for (int suffix = 2; _jfreeDataset.getSeriesIndex(key) >= 0; suffix++) + key = label + " (" + suffix + ")"; + XYSeries series = new XYSeries(key); _jfreeDataset.addSeries(series); // Instead of displaying separate peaks for each transition of this precursor, // we will sum up the intensities and display a single peak for the precursor @@ -1222,17 +1230,14 @@ protected void buildJFreedataset(Chromatogram chromatogram, RtRange chromatogram if (tciList.isEmpty() && _pChromInfo.getTransitionChromatogramIndicesList() != null) { List transitions = MoleculeTransitionManager.getTransitionsForPrecursor(_molPrecursor.getId(), _user, _container); - if (transitions.size() != _pChromInfo.getTransitionChromatogramIndicesList().size()) - { - throw new IllegalStateException("Mismatch in transitions and indices lengths: " + transitions.size() + " vs " + _pChromInfo.getTransitionChromatogramIndicesList().size()); - } int index = 0; for (MoleculeTransition transition : transitions) { - if (include(transition)) + if (include(transition) && index < _pChromInfo.getTransitionChromatogramIndicesList().size()) { - tciList.add(new MoleculeTransChromInfoPlusTransition(_pChromInfo.makeDummyTransitionChromInfo(index++), transition)); + tciList.add(new MoleculeTransChromInfoPlusTransition(_pChromInfo.makeDummyTransitionChromInfo(index), transition)); } + index++; } } diff --git a/src/org/labkey/targetedms/outliers/OutlierGenerator.java b/src/org/labkey/targetedms/outliers/OutlierGenerator.java index 94ea18405..ff2ae05b8 100644 --- a/src/org/labkey/targetedms/outliers/OutlierGenerator.java +++ b/src/org/labkey/targetedms/outliers/OutlierGenerator.java @@ -199,6 +199,8 @@ public void cachePrecursorMetricValues(TargetedMSSchema schema, List getRawMetricDataSets(TargetedMSSchema schema, List configurations, Date startDate, Date endDate, List annotationGroups, boolean showExcluded, boolean showExcludedPrecursors) { List result = new ArrayList<>(); + if (configurations.isEmpty()) + return result; TableInfo sampleFileForQC = schema.getTable("SampleFileForQC"); List sfs = new TableSelector(sampleFileForQC).getArrayList(SampleFileQCMetadata.class); diff --git a/src/org/labkey/targetedms/query/GeneralMoleculePrecursorChromatogramsTableInfo.java b/src/org/labkey/targetedms/query/GeneralMoleculePrecursorChromatogramsTableInfo.java index 4d735fb8d..8103a6a75 100644 --- a/src/org/labkey/targetedms/query/GeneralMoleculePrecursorChromatogramsTableInfo.java +++ b/src/org/labkey/targetedms/query/GeneralMoleculePrecursorChromatogramsTableInfo.java @@ -154,7 +154,7 @@ private static TableInfo getPivotByPrecursorChromInfoTable(Container container, sql.append(" WHERE ").append(precursorIdKey).append(".GeneralMoleculeId=").appendValue(generalMolecule.getId()); sql.append(" AND OptimizationStep IS NULL "); // Ignore precursorChromInfos for optimization peaks (e.g. Collision energy optimization) - form.appendReplicateFilters(sql, "SampleFileId.ReplicateId"); + form.appendReplicateFilters(sql, "SampleFileId.ReplicateId", true); sql.append(" ) X"); sql.append("\n GROUP BY replicate, sample, ").append(_generalMoleculeChromInfoCol).append(", isotopecharge") From 38683901dcd1c3c6b64f75b2240b1f9591769add Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sun, 11 Oct 2026 13:24:28 -0700 Subject: [PATCH 2/4] Fix QC plot and QC summary JS exceptions reported to mothership --- resources/views/peptideSummary.html | 10 ++++---- webapp/TargetedMS/js/QCPlotHelperBase.js | 3 +++ webapp/TargetedMS/js/QCSummaryPanel.js | 11 ++++++++- webapp/TargetedMS/js/QCTrendPlotPanel.js | 31 ++++++++++++++++++------ 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/resources/views/peptideSummary.html b/resources/views/peptideSummary.html index 19a97e173..5c889fb95 100644 --- a/resources/views/peptideSummary.html +++ b/resources/views/peptideSummary.html @@ -271,7 +271,8 @@ function applyHeatmapColors() { const table = document.getElementById('heatmap-table'); - const values = []; + let min = Infinity; + let max = 0; // Extract values from table cells, excluding the last row (totals row) for (let i = 1; i < table.rows.length - 1; i++) { // skip header @@ -279,13 +280,12 @@ for (let j = 1; j < row.cells.length - 1; j++) { // skip the first and last column const value = parseFloat(row.cells[j].textContent); if (!isNaN(value)) { - values.push(value); + min = Math.min(min, value); + max = Math.max(max, value); } } } - const min = Math.min(...values); - const max = Math.max(...values, 0); document.getElementById('max-outliers').innerHTML = ' ' + max + ' outliers'; // Apply colors based on values, excluding the last row (totals row) for (let i = 1; i < table.rows.length - 1; i++) { // skip header @@ -303,7 +303,7 @@ } function getColor(value, min, max) { - const percent = (value - min) / (max - min); + const percent = max > min ? (value - min) / (max - min) : 1; const red = 255; const green = Math.floor(255 * (1 - percent)); const blue = Math.floor(255 * (1 - percent)); diff --git a/webapp/TargetedMS/js/QCPlotHelperBase.js b/webapp/TargetedMS/js/QCPlotHelperBase.js index 1f2593187..9ea6a0ba3 100644 --- a/webapp/TargetedMS/js/QCPlotHelperBase.js +++ b/webapp/TargetedMS/js/QCPlotHelperBase.js @@ -213,6 +213,9 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { let trainingSeqIdx = 1; // this index is used for displaying the average number of runs in tooltip (QCPlotHoverPanel.js L110) for (let i = this.pagingStartIndex; i < this.pagingEndIndex; i++) { const plotDataRow = plotDataRows[i]; + if (!plotDataRow) { + continue; + } tempData = plotDataRow; const fragment = plotDataRow.SeriesLabel; Ext4.iterate(plotDataRow.data, function (plotData) { diff --git a/webapp/TargetedMS/js/QCSummaryPanel.js b/webapp/TargetedMS/js/QCSummaryPanel.js index 5e622be02..08a72f1ce 100644 --- a/webapp/TargetedMS/js/QCSummaryPanel.js +++ b/webapp/TargetedMS/js/QCSummaryPanel.js @@ -305,7 +305,7 @@ Ext4.define('LABKEY.targetedms.QCSummary', { }, queryContainerSampleFileStats: function (container) { - if (container.fileCount > 0) { + if (container.fileCount > 0 && container.docCount > 0) { LABKEY.Ajax.request({ url: LABKEY.ActionURL.buildURL('targetedms', 'GetQCMetricOutliers.api', container.path), params: {sampleLimit: this.sampleLimit}, @@ -323,6 +323,9 @@ Ext4.define('LABKEY.targetedms.QCSummary', { }, failure: LABKEY.Utils.getCallbackWrapper(function(response) { var sampleFilesDiv = Ext4.get('qc-summary-samplefiles-' + container.id); + if (!sampleFilesDiv) { + return; + } if (response.message) { sampleFilesDiv.update("" + Ext4.util.Format.htmlEncode(response.message) + ""); @@ -342,6 +345,9 @@ Ext4.define('LABKEY.targetedms.QCSummary', { removeSampleFilesDetailsDiv: function (container) { var sampleFilesDiv = Ext4.get('qc-summary-samplefiles-' + container.id); + if (!sampleFilesDiv) { + return; + } sampleFilesDiv.update(''); sampleFilesDiv.removeCls('sample-file-details-loading'); }, @@ -426,6 +432,9 @@ Ext4.define('LABKEY.targetedms.QCSummary', { html += '
View all ' + container.fileCount + ' replicates and utilization calendar
'; } var sampleFilesDiv = Ext4.get('qc-summary-samplefiles-' + container.id); + if (!sampleFilesDiv) { + return; + } sampleFilesDiv.update(html); sampleFilesDiv.removeCls('sample-file-details-loading'); diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index e0e59077f..040e5d58f 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -329,7 +329,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { if (this.metric) { //hiding the show All series in a single plot checkbox for run scoped metrics - this.getPlotGroupRadioGroup().setVisible(this.getMetricPropsById(this.metric).precursorScoped); + this.getPlotGroupRadioGroup().setVisible(this.isPrecursorScopedMetric(this.metric)); } // Create vertical line separator component @@ -376,8 +376,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { // Ensure single-plot checkbox visibility mirrors previous logic when metrics are run-scoped if (this.metric) { - var showSinglePlot = this.getMetricPropsById(this.metric).precursorScoped; - this.getPlotGroupRadioGroup().setVisible(showSinglePlot); + this.getPlotGroupRadioGroup().setVisible(this.isPrecursorScopedMetric(this.metric)); } const columnsToolbar = Ext4.create('Ext.toolbar.Toolbar', { @@ -462,7 +461,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { if (this.trailingRuns === undefined || this.trailingRuns === null) { this.trailingRuns = 10; } - this.getTrailingRunsField().setVisible(newValues.indexOf("Trailing Mean") > -1 || newValues.indexOf("Trailing CV") > -1); + this.getTrailingRunsField().setVisible(this.plotTypes.indexOf("Trailing Mean") > -1 || this.plotTypes.indexOf("Trailing CV") > -1); this.havePlotOptionsChanged = true; this.displayTrendPlot(); } @@ -853,6 +852,11 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { return this.endDateField; }, + isPrecursorScopedMetric: function (metricId) { + const props = this.getMetricPropsById(metricId); + return !!props && !!props.precursorScoped; + }, + assignDefaultMetricIfNull: function () { if (this.metric == null || isNaN(Number(this.metric)) || !this.getMetricPropsById(this.metric)) { this.metric = null; @@ -936,8 +940,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { } this.havePlotOptionsChanged = true; // Update single-plot checkbox visibility directly in the 3-column layout - var showAllSeriesCheckBox = this.getMetricPropsById(this.metric).precursorScoped; - this.getPlotGroupRadioGroup().setVisible(showAllSeriesCheckBox); + this.getPlotGroupRadioGroup().setVisible(this.isPrecursorScopedMetric(this.metric)); if (this.filterQCPoints) { this.resetFilterPointsIndices(); @@ -1347,7 +1350,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { if (this.pagingStartIndex < 0) this.pagingStartIndex = 0; else if (this.pagingStartIndex > plotDataRowsBySeriesLabel.length) - this.pagingStartIndex = plotDataRowsBySeriesLabel.length - this.maxCount; + this.pagingStartIndex = Math.max(0, plotDataRowsBySeriesLabel.length - this.maxCount); this.pagingEndIndex = Math.min(this.pagingStartIndex + this.maxCount, plotDataRowsBySeriesLabel.length); @@ -1655,6 +1658,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }); let observer = null; + let showTimeout = null; tippy(point, { allowHTML: true, @@ -1685,6 +1689,9 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }, onShow(instance) { observer = new MutationObserver((mutations) => { + if (!instance.state.isVisible || !instance.popperInstance) { + return; + } const hoverPanel = container.querySelector('.qc-plot-hover-panel'); if (hoverPanel) { hoverPanel.style.minWidth = 'auto'; @@ -1700,7 +1707,11 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }); observer.observe(container, { childList: true, subtree: true }); - setTimeout(() => { + showTimeout = setTimeout(() => { + showTimeout = null; + if (!instance.state.isVisible || !instance.popperInstance) { + return; + } const hoverPanel = container.querySelector('.qc-plot-hover-panel'); if (hoverPanel && instance.popper.style.visibility === 'hidden') { hoverPanel.style.minWidth = 'auto'; @@ -1717,6 +1728,10 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { observer.disconnect(); observer = null; } + if (showTimeout) { + clearTimeout(showTimeout); + showTimeout = null; + } } }); } else { From c7447892b9c48147454718ba7fe5dec6c0370718 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sun, 11 Oct 2026 16:08:02 -0700 Subject: [PATCH 3/4] Client side fixes and Claude review --- .../targetedms/chart/ChromatogramDataset.java | 43 ++++++++----------- webapp/TargetedMS/js/BaseQCPlotPanel.js | 4 ++ webapp/TargetedMS/js/QCPlotHelperBase.js | 6 +-- webapp/TargetedMS/js/QCTrendPlotPanel.js | 5 --- 4 files changed, 24 insertions(+), 34 deletions(-) diff --git a/src/org/labkey/targetedms/chart/ChromatogramDataset.java b/src/org/labkey/targetedms/chart/ChromatogramDataset.java index 9b31657a9..90e39a094 100644 --- a/src/org/labkey/targetedms/chart/ChromatogramDataset.java +++ b/src/org/labkey/targetedms/chart/ChromatogramDataset.java @@ -115,6 +115,15 @@ public ChromatogramDataset(User u, Container c) _user = u; } + /** JFreeChart rejects duplicate series keys, so suffix repeats as "X (2)", "X (3)", ... */ + static String uniqueSeriesKey(XYSeriesCollection dataset, String label) + { + String key = label; + for (int suffix = 2; dataset.getSeriesIndex(key) >= 0; suffix++) + key = label + " (" + suffix + ")"; + return key; + } + public abstract String getChartTitle(); // Start of the peak integration boundary. Shown as a vertical dotted line @@ -649,11 +658,7 @@ public void build() if (chromatogram != null) { // Molecule labels omit isotope label and adduct, so two precursors can share one - String label = getLabel(pChromInfo); - String key = label; - for (int suffix = 2; _jfreeDataset.getSeriesIndex(key) >= 0; suffix++) - key = label + " (" + suffix + ")"; - XYSeries series = new XYSeries(key); + XYSeries series = new XYSeries(uniqueSeriesKey(_jfreeDataset, getLabel(pChromInfo))); _jfreeDataset.addSeries(series); // Instead of displaying separate peaks for each transition of this precursor, // we will sum up the intensities and display a single peak for the precursor @@ -1089,7 +1094,7 @@ GeneralMoleculeDataset.PeakInChart addTransitionAsSeries(XYSeriesCollection data float[] intensities = chromatogram.getIntensities(tci.getChromatogramIndex()); assert times.length == intensities.length : "Length of times and intensities don't match"; - XYSeries series = new XYSeries(label); + XYSeries series = new XYSeries(uniqueSeriesKey(dataset, label)); double maxTraceIntensity = 0; // maximum intensity in the displayed range. @@ -1230,6 +1235,11 @@ protected void buildJFreedataset(Chromatogram chromatogram, RtRange chromatogram if (tciList.isEmpty() && _pChromInfo.getTransitionChromatogramIndicesList() != null) { List transitions = MoleculeTransitionManager.getTransitionsForPrecursor(_molPrecursor.getId(), _user, _container); + if (transitions.size() != _pChromInfo.getTransitionChromatogramIndicesList().size()) + { + LOG.warn("Precursor {} has {} transitions but {} chromatogram indices; trace labels may be mismatched", + _molPrecursor.getId(), transitions.size(), _pChromInfo.getTransitionChromatogramIndicesList().size()); + } int index = 0; for (MoleculeTransition transition : transitions) { @@ -1634,14 +1644,7 @@ public void build() GeneralMolecule gm = entry.getKey(); // Issue 48258:Some very old docs have duplicate molecule names - String seriesName = gm.getTextId(); - int suffix = 1; - while (hasSeries(_jfreeDataset, seriesName)) - { - seriesName = gm.getTextId() + " " + (++suffix); - } - - XYSeries series = new XYSeries(seriesName, true, false); + XYSeries series = new XYSeries(uniqueSeriesKey(_jfreeDataset, gm.getTextId()), true, false); _jfreeDataset.addSeries(series); _seriesColors.put(i, ColorGenerator.getColor(gm.getTextId(), _seriesColors.values())); @@ -1679,18 +1682,6 @@ public void build() _maxDisplayRt = fullRange.getMaxRt() + padding; } - private boolean hasSeries(XYSeriesCollection jfreeDataset, String seriesName) - { - for (XYSeries series : (List)jfreeDataset.getSeries()) - { - if (seriesName.equals(series.getKey())) - { - return true; - } - } - return false; - } - private RtRange getChromatogramRange(GeneralMolecule generalMolecule, List chromInfos) { List ranges = new ArrayList<>(); diff --git a/webapp/TargetedMS/js/BaseQCPlotPanel.js b/webapp/TargetedMS/js/BaseQCPlotPanel.js index 272480eb0..04630db1b 100644 --- a/webapp/TargetedMS/js/BaseQCPlotPanel.js +++ b/webapp/TargetedMS/js/BaseQCPlotPanel.js @@ -19,6 +19,10 @@ Ext4.define('LABKEY.targetedms.BaseQCPlotPanel', { return undefined; }, + isPrecursorScopedMetric: function(id) { + return !!this.getMetricPropsById(id)?.precursorScoped; + }, + isMultiSeries : function() { return this.metric && this.metric2; }, diff --git a/webapp/TargetedMS/js/QCPlotHelperBase.js b/webapp/TargetedMS/js/QCPlotHelperBase.js index 9ea6a0ba3..598a796b0 100644 --- a/webapp/TargetedMS/js/QCPlotHelperBase.js +++ b/webapp/TargetedMS/js/QCPlotHelperBase.js @@ -484,7 +484,7 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { metricProps[this.metric2] = this.getMetricPropsById(this.metric2); } - if (this.singlePlot && this.getMetricPropsById(this.metric).precursorScoped) { + if (this.singlePlot && this.isPrecursorScopedMetric(this.metric)) { this.peptideGroups = this.buildPeptideGroups(); addedPlot = this.addCombinedPeptideSinglePlot(metricProps); } @@ -869,7 +869,7 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { getSubtitle: function(precursor) { if (!this.isMultiSeries()) { - return (precursor ? (precursor + ' - ') : '') + this.getMetricPropsById(this.metric).name; + return (precursor ? (precursor + ' - ') : '') + (this.getMetricPropsById(this.metric)?.name ?? ''); } return precursor; }, @@ -1205,7 +1205,7 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { } const extraMargin = this.showInPlotLegends() ? 0 : 10 * this.longestLegendText; - this.attachPlotExportIcons(id, mainTitle + '-' + this.precursors[precursorIndex] + '-' + this.getMetricPropsById(this.metric).name, plotIndex, this.getPlotWidth(), extraMargin); + this.attachPlotExportIcons(id, mainTitle + '-' + this.precursors[precursorIndex] + '-' + (this.getMetricPropsById(this.metric)?.name ?? ''), plotIndex, this.getPlotWidth(), extraMargin); }, getYAxisLeftMarginOffset: function(precursorInfo) { diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index 040e5d58f..7f0905d9c 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -852,11 +852,6 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { return this.endDateField; }, - isPrecursorScopedMetric: function (metricId) { - const props = this.getMetricPropsById(metricId); - return !!props && !!props.precursorScoped; - }, - assignDefaultMetricIfNull: function () { if (this.metric == null || isNaN(Number(this.metric)) || !this.getMetricPropsById(this.metric)) { this.metric = null; From d671119bd9001d579442be1cdf57d67cf8370964 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sun, 11 Oct 2026 17:53:11 -0700 Subject: [PATCH 4/4] Claude review, automated test --- .../labkey/targetedms/TargetedMSModule.java | 2 + .../targetedms/chart/ChromatogramDataset.java | 30 +++++++++++- .../TargetedMSMultiplePeptidePlotTest.java | 48 ++++++++++++++++++- webapp/TargetedMS/js/QCPlotHelperBase.js | 1 + 4 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/org/labkey/targetedms/TargetedMSModule.java b/src/org/labkey/targetedms/TargetedMSModule.java index fd5f6ff36..c9e2425c6 100644 --- a/src/org/labkey/targetedms/TargetedMSModule.java +++ b/src/org/labkey/targetedms/TargetedMSModule.java @@ -66,6 +66,7 @@ import org.labkey.panoramapremium.PanoramaPremiumController; import org.labkey.panoramapremium.QCNotificationSender; import org.labkey.panoramapremium.View.QCSummaryMenuCustomizer; +import org.labkey.targetedms.chart.ChromatogramDataset; import org.labkey.targetedms.chart.ComparisonCategory; import org.labkey.targetedms.chart.ReplicateLabelMinimizer; import org.labkey.targetedms.datasource.MsDataSourceUtil; @@ -714,6 +715,7 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext) public @NotNull Set> getUnitTests() { return Set.of( + ChromatogramDataset.TestCase.class, ComparisonCategory.TestCase.class, ReplicateLabelMinimizer.TestCase.class, SampleFile.TestCase.class, diff --git a/src/org/labkey/targetedms/chart/ChromatogramDataset.java b/src/org/labkey/targetedms/chart/ChromatogramDataset.java index 90e39a094..c1a5248fa 100644 --- a/src/org/labkey/targetedms/chart/ChromatogramDataset.java +++ b/src/org/labkey/targetedms/chart/ChromatogramDataset.java @@ -21,6 +21,8 @@ import org.jfree.data.xy.XYDataItem; import org.jfree.data.xy.XYSeries; import org.jfree.data.xy.XYSeriesCollection; +import org.junit.Assert; +import org.junit.Test; import org.labkey.api.collections.IntHashMap; import org.labkey.api.data.Container; import org.labkey.api.data.DataRegionSelection; @@ -332,9 +334,11 @@ public SampleFileDataset(SampleFileChromInfo chromInfo, User user, Container con super(user, container); _chromInfo = chromInfo; SampleFile sampleFile = TargetedMSManager.getSampleFile(chromInfo.getSampleFileId(), container); - Replicate replicate = sampleFile == null ? null : TargetedMSManager.getReplicate(sampleFile.getReplicateId(), container); - if (replicate == null) + if (sampleFile == null) throw new NotFoundException("Could not find sample file " + chromInfo.getSampleFileId() + " in " + container.getPath()); + Replicate replicate = TargetedMSManager.getReplicate(sampleFile.getReplicateId(), container); + if (replicate == null) + throw new NotFoundException("Could not find replicate " + sampleFile.getReplicateId() + " for sample file " + sampleFile.getId() + " in " + container.getPath()); _run = TargetedMSManager.getRun(replicate.getRunId()); } @@ -1711,4 +1715,26 @@ private RtRange getChromatogramRange(GeneralMolecule generalMolecule, List return new RtRange(summary._minRt, summary._maxRt, true, _syncRt); } } + + public static class TestCase extends Assert + { + @Test + public void testUniqueSeriesKey() + { + XYSeriesCollection dataset = new XYSeriesCollection(); + assertEquals("A", uniqueSeriesKey(dataset, "A")); + + dataset.addSeries(new XYSeries("A")); + assertEquals("A (2)", uniqueSeriesKey(dataset, "A")); + assertEquals("B", uniqueSeriesKey(dataset, "B")); + + dataset.addSeries(new XYSeries("A (2)")); + assertEquals("A (3)", uniqueSeriesKey(dataset, "A")); + + // A real label that looks like a suffixed duplicate must not collide either + dataset.addSeries(new XYSeries("A (3)")); + assertEquals("A (2) (2)", uniqueSeriesKey(dataset, "A (2)")); + assertEquals("A (4)", uniqueSeriesKey(dataset, "A")); + } + } } diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSMultiplePeptidePlotTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSMultiplePeptidePlotTest.java index 878fca294..582075e91 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSMultiplePeptidePlotTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSMultiplePeptidePlotTest.java @@ -27,13 +27,14 @@ import org.openqa.selenium.support.ui.ExpectedConditions; import java.io.File; +import java.util.Arrays; import java.util.List; /** * Tests rendering of multiple peptides on the same chromatogram visualization panel. */ @Category({}) -@BaseWebDriverTest.ClassTimeout(minutes = 2) +@BaseWebDriverTest.ClassTimeout(minutes = 4) public class TargetedMSMultiplePeptidePlotTest extends TargetedMSTest { @BeforeClass @@ -119,4 +120,49 @@ public void testMultiplePeptidePlotOnSameChromatogram() // Each row will have chromatograms from two replicates. We expect 5 rows since we are displaying the first 10 replicates. checker().verifyEquals("Incorrect row count after changing row size to 2 replicates per row", 5, table.getDataRowCount()); } + + @Test + public void testReplicateAnnotationFilter() + { + // Experiment folder, since importing a second SProCoP doc into the Library folder would create conflicts + String subfolder = "AnnotationFilter"; + setupSubfolder(getProjectName(), subfolder, FolderType.Experiment); + importData(SProCoP_FILE_ANNOTATED, 1, false, false); // Six replicates; DB maintenance would only cost time + + navigateToFolder(getProjectName(), subfolder); + clickAndWait(Locator.linkWithText(SProCoP_FILE_ANNOTATED)); + clickAndWait(Locator.linkWithText("gi|14278146|pdb|1B8E|")); + clickAndWait(Locator.linkWithText("VYVEELKPTPEGDLEILLQK")); + verifyFilteredReplicates(List.of("JGB_02", "JGB_32", "JGB_38", "JGB_73", "JGB_87", "JGB_87_130815040004")); + + applyAnnotationFilter("Day : 1"); + verifyFilteredReplicates(List.of("JGB_02", "JGB_32", "JGB_38")); + checker().screenShotIfNewError("singleAnnotationFilter"); + + applyAnnotationFilter("SampleIdentifier : AnnotatedSample1", "Day : 2"); + verifyFilteredReplicates(List.of("JGB_02", "JGB_73", "JGB_87", "JGB_87_130815040004")); + checker().screenShotIfNewError("multipleAnnotationFilters"); + } + + private void applyAnnotationFilter(String... annotations) + { + log("Filtering chromatograms by replicate annotations: " + Arrays.toString(annotations)); + click(Locator.tagWithText("strong", "Display Chart Settings")); + new ComboBox.ComboBoxFinder(getDriver()).withInputNamed("annotationsFilter") + .findWhenNeeded(getDriver()).setMultiSelect(true).selectComboBoxItem(annotations); + clickButton("Update"); + } + + private void verifyFilteredReplicates(List expectedReplicates) + { + DataRegionTable table = new DataRegionTable.DataRegionFinder(getDriver()).withName("PeptidePrecursorChromatograms").waitFor(); + waitForElementToDisappear(Locator.tagWithAttributeContaining("div", "alt", "Chromatogram Q_Exactive") + .withText("Loading..."), WebDriverWrapper.WAIT_FOR_PAGE); + checker().verifyEquals("Incorrect number of replicates after annotation filter", expectedReplicates.size(), table.getDataRowCount()); + for (String replicate : expectedReplicates) + { + checker().verifyTrue("Missing chromatogram for replicate " + replicate, + isElementPresent(Locator.tagWithAttributeContaining("div", "alt", "Chromatogram Q_Exactive_08_09_2013_" + replicate))); + } + } } diff --git a/webapp/TargetedMS/js/QCPlotHelperBase.js b/webapp/TargetedMS/js/QCPlotHelperBase.js index 598a796b0..4c07cd341 100644 --- a/webapp/TargetedMS/js/QCPlotHelperBase.js +++ b/webapp/TargetedMS/js/QCPlotHelperBase.js @@ -214,6 +214,7 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { for (let i = this.pagingStartIndex; i < this.pagingEndIndex; i++) { const plotDataRow = plotDataRows[i]; if (!plotDataRow) { + console.warn('No QC plot data row at index ' + i + ' of ' + plotDataRows.length); continue; } tempData = plotDataRow;