Fix TargetedMS chromatogram, QC plot, and QC summary exceptions reported to mothership

resources/views/peptideSummary.html

@@ -271,21 +271,21 @@
 
     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
             const row = table.rows[i];
             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 = '&nbsp;' +  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));

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)
         {

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<Integer> 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(")");

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<Class<?>> getUnitTests()
     {
         return Set.of(
+            ChromatogramDataset.TestCase.class,
             ComparisonCategory.TestCase.class,
             ReplicateLabelMinimizer.TestCase.class,
             SampleFile.TestCase.class,

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;
@@ -29,6 +31,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;
@@ -114,6 +117,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
@@ -322,7 +334,11 @@ public SampleFileDataset(SampleFileChromInfo chromInfo, User user, Container con
             super(user, container);
             _chromInfo = chromInfo;
             SampleFile sampleFile = TargetedMSManager.getSampleFile(chromInfo.getSampleFileId(), container);
+            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());
         }
 
@@ -645,7 +661,8 @@ 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
+                    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
@@ -1081,7 +1098,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.
 
@@ -1224,15 +1241,17 @@ protected void buildJFreedataset(Chromatogram chromatogram, RtRange chromatogram
                 List<MoleculeTransition> 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());
+                    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)
                 {
-                    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++;
                 }
             }
 
@@ -1629,14 +1648,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()));
 
@@ -1674,18 +1686,6 @@ public void build()
             _maxDisplayRt = fullRange.getMaxRt() + padding;
         }
 
-        private boolean hasSeries(XYSeriesCollection jfreeDataset, String seriesName)
-        {
-            for (XYSeries series : (List<XYSeries>)jfreeDataset.getSeries())
-            {
-                if (seriesName.equals(series.getKey()))
-                {
-                    return true;
-                }
-            }
-            return false;
-        }
-
         private RtRange getChromatogramRange(GeneralMolecule<?, ?> generalMolecule, List<PrecursorChromInfoPlus> chromInfos)
         {
             List<RtRange> ranges = new ArrayList<>();
@@ -1715,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"));
+        }
+    }
 }

src/org/labkey/targetedms/outliers/OutlierGenerator.java

@@ -199,6 +199,8 @@ public void cachePrecursorMetricValues(TargetedMSSchema schema, List<QCMetricCon
     public List<RawMetricDataSet> getRawMetricDataSets(TargetedMSSchema schema, List<QCMetricConfiguration> configurations, Date startDate, Date endDate, List<AnnotationGroup> annotationGroups, boolean showExcluded, boolean showExcludedPrecursors)
     {
         List<RawMetricDataSet> result = new ArrayList<>();
+        if (configurations.isEmpty())
+            return result;
 
         TableInfo sampleFileForQC = schema.getTable("SampleFileForQC");
         List<SampleFileQCMetadata> sfs = new TableSelector(sampleFileForQC).getArrayList(SampleFileQCMetadata.class);

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")

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<String> 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)));
+        }
+    }
 }

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;
     },

webapp/TargetedMS/js/QCPlotHelperBase.js

@@ -213,6 +213,10 @@ 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) {
+                console.warn('No QC plot data row at index ' + i + ' of ' + plotDataRows.length);
+                continue;
+            }
             tempData = plotDataRow;
             const fragment = plotDataRow.SeriesLabel;
             Ext4.iterate(plotDataRow.data, function (plotData) {
@@ -481,7 +485,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);
         }
@@ -866,7 +870,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;
     },
@@ -1202,7 +1206,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) {