diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCGuideSetTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCGuideSetTest.java index 89af0cb8f..f911a0c2f 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCGuideSetTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCGuideSetTest.java @@ -113,6 +113,7 @@ public void runTestsInOrder() throws IOException, CommandException { testGuideSetStats(); testGuideSetCreateValidation(); + testCombinedPlotsGuideSetCreation(); testGuideSetPlotDisplay(); testParetoPlot(); testEmptyParetoPlot(); @@ -182,6 +183,31 @@ public void testGuideSetCreateValidation() createGuideSet(new GuideSet("", "", null, 47), overlapErrorMsg); } + + private void testCombinedPlotsGuideSetCreation() + { + preTest(); + PanoramaDashboard qcDashboard = new PanoramaDashboard(this); + QCPlotsWebPart qcPlotsWebPart = qcDashboard.getQcPlotsWebPart(); + qcPlotsWebPart.resetInitialQCPlotFields(); + qcPlotsWebPart.setShowAllPeptidesInSinglePlot(true, 1); + + GuideSet gsCombined = new GuideSet("2013/08/17 16:01:37", "2013/08/20 13:35:16", "guide set created from combined plots"); + createGuideSet(gsCombined); + + clickTab("Guide Sets"); + DataRegionTable table = new DataRegionTable.DataRegionFinder(getDriver()).waitFor(); + assertEquals("Guide set was not created from combined plots", 6, table.getDataRowCount()); + + int rowIndex = table.getRowIndex("Comment", "guide set created from combined plots"); + table.checkCheckbox(rowIndex); + doAndWaitForPageToLoad(() -> + { + table.clickHeaderButton("Delete"); + assertAlert("Are you sure you want to delete the selected row?"); + }); + } + public void testGuideSetPlotDisplay() { preTest(); diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java index c16f76235..6960810ee 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java @@ -1081,6 +1081,7 @@ private void verifyAddAnnotationsFromQCPlots() String testComment = "Test annotation from QC plot"; setFormElement(Locator.name("description"), testComment); + log("Clicking Save button in add annotation dialog"); // Click the save button addAnnotationDialog.clickButton("Save", true); _ext4Helper.waitForMaskToDisappear(); diff --git a/webapp/TargetedMS/js/QCPlotHelperBase.js b/webapp/TargetedMS/js/QCPlotHelperBase.js index 935e91b88..53e6dbf16 100644 --- a/webapp/TargetedMS/js/QCPlotHelperBase.js +++ b/webapp/TargetedMS/js/QCPlotHelperBase.js @@ -694,7 +694,7 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { return precursor; }, - addEachCombinedPrecursorPlot: function(plotIndex, id, combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, plotType, isCUSUMMean) { + addEachCombinedPrecursorPlot: function(plotIndex, id, combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, plotType, isCUSUMMean, scope) { let plotLegendData = this.getCombinedPlotLegendData(metricProps, groupColors, yAxisCount, plotType, isCUSUMMean); if (plotType !== LABKEY.vis.TrendingLinePlotType.CUSUM) { @@ -735,6 +735,7 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { color: 'fragment', defaultGuideSetLabel: 'fragment', pointSize: 2, + pointIdAttr: function(row) { return row['fullDate'] + row['fragment']; }, shapeRange: [LABKEY.vis.Scale.Shape()[0] /* circle */, LABKEY.vis.Scale.DataspaceShape()[0] /* open circle */, LABKEY.vis.Scale.Shape()[1], LABKEY.vis.Scale.Shape()[2]], shapeDomain: shapeDomain, showTrendLine: true, @@ -794,6 +795,24 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperBase", { visibility: this.isMultiSeries() ? undefined : 'hidden' } }, + brushing: !this.allowGuideSetBrushing() ? undefined : { + dimension: 'x', + fillOpacity: 0.4, + fillColor: 'rgba(20, 204, 201, 1)', + strokeColor: 'rgba(20, 204, 201, 1)', + brushstart: function(event, data, extent, plot, layerSelections) { + scope.plotBrushStartEvent(plot); + }, + brush: function(event, data, extent, plot, layerSelections) { + scope.plotBrushEvent(extent, plot, layerSelections); + }, + brushend: function(event, data, extent, plot, layerSelections) { + scope.plotBrushEndEvent(data[data.length - 1], extent, plot); + }, + brushclear: function(event, data, plot, layerSelections) { + scope.plotBrushClearEvent(data[data.length - 1], plot); + } + }, properties: trendLineProps }); diff --git a/webapp/TargetedMS/js/QCPlotHelperWrapper.js b/webapp/TargetedMS/js/QCPlotHelperWrapper.js index 0178168c7..686efef74 100644 --- a/webapp/TargetedMS/js/QCPlotHelperWrapper.js +++ b/webapp/TargetedMS/js/QCPlotHelperWrapper.js @@ -128,6 +128,7 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperWrapper", { }, addCombinedPeptideSinglePlot : function(metricProps) { + const me = this; // for plot brushing let yAxisCount = this.isMultiSeries() ? 2 : 1, //Will only have a right if there is already a left y-axis groupColors = this.getColorRange(), combinePlotData = this.getCombinedPlotInitData(), @@ -195,25 +196,25 @@ Ext4.define("LABKEY.targetedms.QCPlotHelperWrapper", { legendMargin = 300; if (this.showMetricValuePlot()) { - this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex++], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.LeveyJennings); + this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex++], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.LeveyJennings, false, me); } if (this.showMovingRangePlot()) { - this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex++], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.MovingRange); + this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex++], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.MovingRange, false, me); } if (this.showMeanCUSUMPlot()) { - this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex++], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.CUSUM, true); + this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex++], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.CUSUM, true, me); } if (this.showVariableCUSUMPlot()) { - this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.CUSUM, false); + this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.CUSUM, false, me); } if (this.showTrailingMeanPlot()) { - this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.TrailingMean, false); + this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.TrailingMean, false, me); } if (this.showTrailingCVPlot()) { - this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.TrailingCV, false); + this.addEachCombinedPrecursorPlot(plotIndex, ids[plotIndex], combinePlotData, groupColors, yAxisCount, metricProps, showLogInvalid, legendMargin, LABKEY.vis.TrendingLinePlotType.TrailingCV, false, me); } - + this.setPlotBrushingDisplayStyle(); return true; }, diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index 16b5e3aae..51e41b383 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -1169,7 +1169,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { if (!this.createGuideSetToggleButton) { this.createGuideSetToggleButton = Ext4.create('Ext.button.Button', { text: 'Create Guide Set', - tooltip: 'Enable/disable guide set creation mode. Supported for separate plots, not grouped by date, when ' + LABKEY.targetedms.QCPlotHelperBase.maxPointsPerSeries + ' or fewer samples are shown', + tooltip: 'Enable/disable guide set creation mode. Supported for plots when ' + LABKEY.targetedms.QCPlotHelperBase.maxPointsPerSeries + ' or fewer samples are shown', disabled: !this.canCreateGuideSetFromPlot(), enableToggle: true, handler: function(btn) { @@ -1183,11 +1183,11 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }, canCreateGuideSetFromPlot : function() { - return !(this.groupedX || this.singlePlot || this.isMultiSeries() || this.showExpRunRange || !this.showDataPoints); + return !(this.showExpRunRange || !this.showDataPoints); }, setBrushingEnabled : function(enabled) { - // we don't currently allow creation of guide sets in single plot mode, grouped x-axis mode, multi series mode or when showingExpRunRange + // we don't currently allow creation when showingExpRunRange this.getGuideSetCreateButton().setDisabled(!this.canCreateGuideSetFromPlot()); this.enableBrushing = enabled; @@ -1552,7 +1552,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { this.bringSvgElementToFront(plot, "g.guideset-svg-button"); }, - plotBrushClearEvent : function(data, plot) { + plotBrushClearEvent : function() { this.plotBrushSelection = undefined; }, @@ -1561,7 +1561,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }, allowGuideSetBrushing : function() { - return this.canUserEdit() && !this.groupedX; + return this.canUserEdit(); }, createGuideSetSvgButton : function(plot, text, xLeftPos, width) { @@ -1586,8 +1586,9 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { setPlotBrushingDisplayStyle : function() { // hide the brushing related components for all plots if not in "create guide set" mode var displayStyle = this.enableBrushing ? 'inline' : 'none'; - d3.selectAll('.brush').style({'display': displayStyle}); - d3.selectAll('.x-axis-handle').style({'display': displayStyle}); + // Scope the selection to only plots within the current plotDivId to avoid affecting other plot types + d3.select('#' + this.plotDivId).selectAll('.brush').style({'display': displayStyle}); + d3.select('#' + this.plotDivId).selectAll('.x-axis-handle').style({'display': displayStyle}); }, clearPlotBrush : function(plot) { @@ -2328,17 +2329,26 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }, createGuideSetBtnClick: function() { - var minGuideSetPointCount = 5; // to warn user if less than this many points are selected for the new guide set + let minGuideSetReplicateCount = 5; // to warn user if less than this many replicates are selected for the new guide set if (this.plotBrushSelection && this.plotBrushSelection.points.length > 0) { - var startDate = this.plotBrushSelection.points[0]['fullDate']; - var endDate = this.plotBrushSelection.points[this.plotBrushSelection.points.length - 1]['fullDate']; + let startDate = this.plotBrushSelection.points[0]['fullDate']; + let endDate = this.plotBrushSelection.points[this.plotBrushSelection.points.length - 1]['fullDate']; + + let distinctSampleFileIds = {}; + for (let i = 0; i < this.plotBrushSelection.points.length; i++) { + let sampleFileId = this.plotBrushSelection.points[i].SampleFileId; + if (sampleFileId !== undefined && sampleFileId !== null) { + distinctSampleFileIds[sampleFileId] = true; + } + } + let distinctCount = Object.keys(distinctSampleFileIds).length; - if (this.plotBrushSelection.points.length < minGuideSetPointCount) { + if (distinctCount < minGuideSetReplicateCount) { Ext4.Msg.show({ - title:'Create Guide Set Warning', + title: 'Create Guide Set Warning', icon: Ext4.MessageBox.WARNING, - msg: 'Fewer than ' + minGuideSetPointCount + ' data points were selected for the new guide set, which may not be statistically significant. Would you like to proceed anyway?', + msg: 'Fewer than ' + minGuideSetReplicateCount + ' replicates were selected for the new guide set, which may not be statistically significant. Would you like to proceed anyway?', buttons: Ext4.Msg.YESNO, scope: this, fn: function(btnId, text, opt){