From 814fe371ab4ad024c04f2dea1e28d37109b7f217 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Thu, 19 Mar 2026 10:16:06 -0700 Subject: [PATCH 01/16] Share instrument annotations across QC folders --- .../targetedms/qcannotation/.qview.xml | 2 + .../targetedms/qcannotationtype/.qview.xml | 1 + .../postgresql/targetedms-26.002-26.003.sql | 5 + resources/schemas/targetedms.xml | 3 + .../labkey/targetedms/TargetedMSManager.java | 68 +++- .../labkey/targetedms/TargetedMSModule.java | 2 +- .../targetedms/query/QCAnnotationTable.java | 74 +++++ .../test/components/targetedms/QCPlot.java | 28 +- .../tests/targetedms/TargetedMSQCTest.java | 56 +++- .../TargetedMSUtilizationCalendarTest.java | 2 +- webapp/TargetedMS/js/QCTrendPlotPanel.js | 302 ++++++++++++++---- 11 files changed, 466 insertions(+), 77 deletions(-) create mode 100644 resources/schemas/dbscripts/postgresql/targetedms-26.002-26.003.sql diff --git a/resources/queries/targetedms/qcannotation/.qview.xml b/resources/queries/targetedms/qcannotation/.qview.xml index 402f7a544..c593875bf 100644 --- a/resources/queries/targetedms/qcannotation/.qview.xml +++ b/resources/queries/targetedms/qcannotation/.qview.xml @@ -4,6 +4,8 @@ + + diff --git a/resources/queries/targetedms/qcannotationtype/.qview.xml b/resources/queries/targetedms/qcannotationtype/.qview.xml index 920ef55d0..36f49f671 100644 --- a/resources/queries/targetedms/qcannotationtype/.qview.xml +++ b/resources/queries/targetedms/qcannotationtype/.qview.xml @@ -4,6 +4,7 @@ + diff --git a/resources/schemas/dbscripts/postgresql/targetedms-26.002-26.003.sql b/resources/schemas/dbscripts/postgresql/targetedms-26.002-26.003.sql new file mode 100644 index 000000000..0f073a6da --- /dev/null +++ b/resources/schemas/dbscripts/postgresql/targetedms-26.002-26.003.sql @@ -0,0 +1,5 @@ +ALTER TABLE targetedms.QCAnnotationType ADD COLUMN Shareable BOOLEAN DEFAULT FALSE; +ALTER TABLE targetedms.QCAnnotation ADD COLUMN instrumentModel VARCHAR(255); +ALTER TABLE targetedms.QCAnnotation ADD COLUMN instrumentSerialNumber VARCHAR(255); + +UPDATE targetedms.QCAnnotationType SET Shareable = TRUE WHERE Name = 'Instrumentation Change'; \ No newline at end of file diff --git a/resources/schemas/targetedms.xml b/resources/schemas/targetedms.xml index 569b890e1..8fddaf6bf 100644 --- a/resources/schemas/targetedms.xml +++ b/resources/schemas/targetedms.xml @@ -1262,6 +1262,7 @@ + @@ -1307,6 +1308,8 @@ Annotation Type The category of the event + + diff --git a/src/org/labkey/targetedms/TargetedMSManager.java b/src/org/labkey/targetedms/TargetedMSManager.java index c2c228a33..f9538e185 100644 --- a/src/org/labkey/targetedms/TargetedMSManager.java +++ b/src/org/labkey/targetedms/TargetedMSManager.java @@ -17,6 +17,8 @@ package org.labkey.targetedms; import com.google.common.base.Joiner; +import lombok.Getter; +import lombok.Setter; import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; @@ -207,7 +209,8 @@ public static TargetedMSManager get() public static List getSampleFileChromInfos(SampleFile sampleFile) { - return new TableSelector(getTableInfoSampleFileChromInfo(), new SimpleFilter(FieldKey.fromParts("SampleFileId"), sampleFile.getId()), new Sort("TextId")).getArrayList(SampleFileChromInfo.class); } + return new TableSelector(getTableInfoSampleFileChromInfo(), new SimpleFilter(FieldKey.fromParts("SampleFileId"), sampleFile.getId()), new Sort("TextId")).getArrayList(SampleFileChromInfo.class); + } public static SampleFileChromInfo getSampleFileChromInfo(int id, Container c) { @@ -548,7 +551,8 @@ public static TableInfo getTableInfoQuantificationSettings() return getSchema().getTable(TargetedMSSchema.TABLE_QUANTIIFICATION_SETTINGS); } - public static TableInfo getTableInfoCalibrationCurve() { + public static TableInfo getTableInfoCalibrationCurve() + { return getSchema().getTable(TargetedMSSchema.TABLE_CALIBRATION_CURVE); } @@ -628,19 +632,23 @@ public static TableInfo getTableInfoSkylineAuditLogMessage() return getSchema().getTable(TargetedMSSchema.TABLE_SKYLINE_AUDITLOG_MESSAGE); } - public static TableInfo getTableInfoListDefinition() { + public static TableInfo getTableInfoListDefinition() + { return getSchema().getTable(TargetedMSSchema.TABLE_LIST_DEFINITION); } - public static TableInfo getTableInfoListColumnDefinition() { + public static TableInfo getTableInfoListColumnDefinition() + { return getSchema().getTable(TargetedMSSchema.TABLE_LIST_COLUMN_DEFINITION); } - public static TableInfo getTableInfoListItem() { + public static TableInfo getTableInfoListItem() + { return getSchema().getTable(TargetedMSSchema.TABLE_LIST_ITEM); } - public static TableInfo getTableInfoListItemValue() { + public static TableInfo getTableInfoListItemValue() + { return getSchema().getTable(TargetedMSSchema.TABLE_LIST_ITEM_VALUE); } @@ -1246,7 +1254,7 @@ public List getNickname(String name, TargetedMSSchema schema } List result = new ArrayList<>(dedupeAcrossContainers.values()); - + if (matches.isEmpty()) { String sql = "SELECT DISTINCT InstrumentNickname, " + @@ -3046,6 +3054,41 @@ private QueryUpdateService getNicknameUpdateService(User user, Container contain return Objects.requireNonNull(table.getUpdateService()); } + public static class InstrumentDetails + { + @Getter @Setter + private String instrumentSerialNumber; + @Getter @Setter + private String model; + @Getter @Setter + private Long instrumentId; + + public InstrumentDetails() + { + } + } + + public static List getInstrumentDetails(Container container) + { + SQLFragment sql = new SQLFragment("SELECT DISTINCT sf.InstrumentSerialNumber, i.Model, i.Id AS InstrumentId FROM "); + sql.append(getTableInfoSampleFile(), "sf"); + sql.append(" INNER JOIN "); + sql.append(getTableInfoInstrument(), "i"); + sql.append(" ON sf.InstrumentId = i.Id "); + sql.append(" INNER JOIN "); + sql.append(getTableInfoReplicate(), "rep"); + sql.append(" ON sf.ReplicateId = rep.Id "); + sql.append(" INNER JOIN "); + sql.append(getTableInfoRuns(), "r"); + sql.append(" ON rep.RunId = r.Id "); + sql.append(" WHERE r.Container = ?"); + sql.add(container); + + return new SqlSelector(getSchema(), sql).getArrayList(InstrumentDetails.class); + + } + + public void deleteNickname(InstrumentNickname name, User user) throws SQLException, BatchValidationException, QueryUpdateServiceException, InvalidKeyException { getNicknameUpdateService(user, name.getContainer()). @@ -3071,4 +3114,15 @@ public void saveNickname(InstrumentNickname name, User user) throws SQLException insertRows(user, name.getContainer(), Arrays.asList(row), errors, null, null); } } + + public static boolean isQCAnnotationTypeShareable(int qcAnnotationTypeId) + { + SQLFragment sql = new SQLFragment("SELECT Shareable FROM "); + sql.append(getTableInfoQCAnnotationType()); + sql.append(" WHERE Id = ?"); + sql.add(qcAnnotationTypeId); + + Boolean isShareable = new SqlSelector(getSchema(), sql).getObject(Boolean.class); + return isShareable != null && isShareable; + } } diff --git a/src/org/labkey/targetedms/TargetedMSModule.java b/src/org/labkey/targetedms/TargetedMSModule.java index 5a14f90d9..921e46202 100644 --- a/src/org/labkey/targetedms/TargetedMSModule.java +++ b/src/org/labkey/targetedms/TargetedMSModule.java @@ -231,7 +231,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 26.002; + return 26.003; } @Override diff --git a/src/org/labkey/targetedms/query/QCAnnotationTable.java b/src/org/labkey/targetedms/query/QCAnnotationTable.java index bac7b231e..5af2f2484 100644 --- a/src/org/labkey/targetedms/query/QCAnnotationTable.java +++ b/src/org/labkey/targetedms/query/QCAnnotationTable.java @@ -15,13 +15,27 @@ */ package org.labkey.targetedms.query; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.TableInfo; import org.labkey.api.gwt.client.AuditBehaviorType; +import org.labkey.api.query.BatchValidationException; +import org.labkey.api.query.DefaultQueryUpdateService; +import org.labkey.api.query.DuplicateKeyException; import org.labkey.api.query.QueryForeignKey; +import org.labkey.api.query.QueryUpdateService; +import org.labkey.api.query.QueryUpdateServiceException; import org.labkey.api.query.SimpleUserSchema; +import org.labkey.api.query.ValidationException; +import org.labkey.api.security.User; import org.labkey.targetedms.TargetedMSManager; import org.labkey.targetedms.TargetedMSSchema; +import java.sql.SQLException; +import java.util.List; +import java.util.Map; + import static org.labkey.targetedms.query.GuideSetTable.appendFormatLabel; /** @@ -44,4 +58,64 @@ public QCAnnotationTable(TargetedMSSchema schema, ContainerFilter cf) appendFormatLabel(getMutableColumn("EndDate")); setAuditBehavior(AuditBehaviorType.DETAILED); } + + @Override + public QueryUpdateService getUpdateService() + { + TableInfo table = getRealTable(); + if (table != null) + { + return new DefaultQueryUpdateService(this, getRealTable()) + { + @Override + public List> insertRows(User user, Container container, List> rows, BatchValidationException errors, @Nullable Map configParameters, @Nullable Map extraScriptContext) throws SQLException, QueryUpdateServiceException, DuplicateKeyException + { + List> resultRows = new java.util.ArrayList<>(); + for (Map row : rows) + { + try + { + // Check if the QCAnnotationType is shareable + int qcAnnotationTypeId = (Integer) row.get("QCAnnotationTypeId"); + boolean isShareable = TargetedMSManager.isQCAnnotationTypeShareable(qcAnnotationTypeId); + + if (isShareable) + { + List instruments = TargetedMSManager.getInstrumentDetails(getContainer()); + if (instruments.isEmpty()) + { + resultRows.add(super.insertRow(user, container, row)); + } + else + { + for (TargetedMSManager.InstrumentDetails instrument : instruments) + { + Map newRow = new java.util.HashMap<>(row); + newRow.put("instrumentModel", instrument.getModel()); + newRow.put("instrumentSerialNumber", instrument.getInstrumentSerialNumber()); + newRow.put("Container", getContainer().getId()); + resultRows.add(super.insertRow(user, container, newRow)); + } + } + } + else + { + resultRows.add(super.insertRow(user, container, row)); + } + } + catch (ValidationException e) + { + errors.addRowError(e); + } + } + + if (errors.hasErrors()) + return null; + + return resultRows; + } + }; + } + return null; + } } diff --git a/test/src/org/labkey/test/components/targetedms/QCPlot.java b/test/src/org/labkey/test/components/targetedms/QCPlot.java index 1ee193246..807ae3223 100644 --- a/test/src/org/labkey/test/components/targetedms/QCPlot.java +++ b/test/src/org/labkey/test/components/targetedms/QCPlot.java @@ -16,6 +16,7 @@ package org.labkey.test.components.targetedms; import org.junit.Assert; +import org.labkey.test.BaseWebDriverTest; import org.labkey.test.Locator; import org.labkey.test.util.targetedms.QCHelper; import org.openqa.selenium.WebElement; @@ -27,6 +28,9 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.labkey.test.BaseWebDriverTest.getCurrentTest; +import static org.labkey.test.util.TestLogger.log; + public class QCPlot { final WebElement plot; @@ -64,19 +68,29 @@ public List getAnnotations() private QCHelper.Annotation parseAnnotation(WebElement annotationEl) { - String annotationString = annotationEl.getText(); - String annotationRegex = "Created By: (.+)\\s*, " + - "Date: (\\d\\d\\d\\d-\\d\\d-\\d\\d \\d\\d:\\d\\d:\\d\\d)\\s*, " + - "Description: (.+)"; + getCurrentTest().scrollIntoView(annotationEl); + getCurrentTest().mouseOver(annotationEl); + Locator tippyLocator = Locator.tagWithClass("div", "tippy-content"); + getCurrentTest().waitForElement(tippyLocator); + WebElement tippyContent = tippyLocator.findElement(getCurrentTest().getDriver()); + String annotationString = tippyContent.getText(); + + String annotationRegex = "(?s)Created By:\\s*(.*?)\\s+" + + "Type:\\s*(.*?)\\s+" + + "Date:\\s*(\\d\\d\\d\\d-\\d\\d-\\d\\d(?: \\d\\d:\\d\\d:\\d\\d)?)\\s+" + + "Description:\\s*(.*?)(?:\\s+Shared From:.*|$)"; Pattern annotationPattern = Pattern.compile(annotationRegex, Pattern.MULTILINE); Matcher annotationMatcher = annotationPattern.matcher(annotationString); - Assert.assertTrue(annotationString, annotationMatcher.find()); - String date = annotationMatcher.group(2); - String description = annotationMatcher.group(3); + Assert.assertTrue("Annotation text did not match regex: " + annotationString, annotationMatcher.find()); + String description = annotationMatcher.group(4).trim(); String color = annotationEl.getCssValue("fill"); QCHelper.AnnotationType type = getAnnotationTypes().get(color); + // Hide tippy to not interfere with other elements + getCurrentTest().mouseOver(Locator.tag("body")); + getCurrentTest().waitForElementToDisappear(tippyLocator); + return new QCHelper.Annotation(type.getName(), description); } diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java index 10a15d581..830d0ff62 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java @@ -192,6 +192,7 @@ public void preTest() goToProjectHome(); PanoramaDashboard qcDashboard = new PanoramaDashboard(this); QCPlotsWebPart qcPlotsWebPart = qcDashboard.getQcPlotsWebPart(); + scrollIntoView(qcPlotsWebPart.getComponentElement()); qcPlotsWebPart.revertToDefaultView(); } @@ -234,7 +235,7 @@ private void verifyChromatogramPlot(String date, String replicate) goToProjectHome(); PanoramaDashboard qcDashboard = new PanoramaDashboard(this); QCPlotsWebPart qcPlotsWebPart = qcDashboard.getQcPlotsWebPart(); - scrollIntoView(Locator.tagWithText("span","FFVAPFPEVFGK ++, 692.8686")); + scrollIntoView(Locator.tagWithText("span", "FFVAPFPEVFGK ++, 692.8686")); qcPlotsWebPart.openExclusionBubble(date); clickAndWait(Locator.linkWithText("view chromatogram")); assertTrue("Navigated to incorrect replicate", isTextPresent(replicate)); @@ -959,6 +960,58 @@ public void testSamplesExcludedForSingleMetric() verifyExclusionButtonSelection(acquiredDate, QCPlotsWebPart.QCPlotExclusionState.Include); } + @Test + public void testShareableAnnotations() + { + String folderA = "Folder A"; + String folderB = "Folder B"; + + setupSubfolder(getProjectName(), folderA, FolderType.QC); + importData(ISOTOPOLOGUE_FILE_ANNOTATED); + + setupSubfolder(getProjectName(), folderB, FolderType.QC); + importData(ISOTOPOLOGUE_FILE_ANNOTATED); + + clickFolder(folderA); + clickTab("Annotations"); + + QCAnnotationWebPart annotationWebPart = new PanoramaAnnotations(this).getQcAnnotationWebPart(); + QCHelper.Annotation shareableAnnotation = new QCHelper.Annotation("Instrumentation Change", "This is a shareable annotation", "2018-08-25"); + annotationWebPart.startInsert().insert(shareableAnnotation); + + clickTab("Annotations"); + annotationWebPart = new PanoramaAnnotations(this).getQcAnnotationWebPart(); + // expect two annotations because the data has association with two instruments + assertEquals("Expected two annotation rows to be created for shareable annotation", 2, annotationWebPart.getDataRegion().getDataRowCount()); + + clickFolder(folderB); + clickTab("Panorama Dashboard"); + PanoramaDashboard qcDashboard = new PanoramaDashboard(this); + QCPlotsWebPart qcPlotsWebPart = qcDashboard.getQcPlotsWebPart(); + qcPlotsWebPart.waitForPlots(35); + + // Verify the shareable annotation from Folder A appears in Folder B's QC plots + List qcPlots = qcPlotsWebPart.getPlots(); + boolean shareableAnnotationFound = false; + for (QCPlot plot : qcPlots) + { + List annotations = plot.getAnnotations(); + for (QCHelper.Annotation annotation : annotations) + { + if (annotation.getType().equals(shareableAnnotation.getType()) && + annotation.getDescription().equals(shareableAnnotation.getDescription())) + { + shareableAnnotationFound = true; + break; + } + } + if (shareableAnnotationFound) + break; + } + assertTrue("Shareable annotation from Folder A should appear in Folder B QC plots", shareableAnnotationFound); + + } + private void verifyQCSummarySampleFileOutliers(String acquiredDate, String outlierInfo) { PanoramaDashboard qcDashboard = new PanoramaDashboard(this); @@ -1059,6 +1112,7 @@ private void checkForCorrectAnnotations(String plotType, QCPlotsWebPart qcPlotsW expectedAnnotations.add(candyChange); for (QCPlot plot : qcPlots) { + log("verifying for qc plot - " + plot.getPlot().getText()); Bag plotAnnotations = new HashBag<>(plot.getAnnotations()); assertEquals("Wrong annotations in " + plotType + ":" + plot.getPrecursor(), expectedAnnotations, plotAnnotations); } diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java index 08090bcc0..e5ab284c3 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java @@ -59,7 +59,7 @@ public void testUtilizationCalendarActions() log("Input data validation checks"); utilizationCalendar.setDisplay("1") - .markOfflineExpectingError("2013-08-2", null, null, "Error saving. Missing value for required property: Description") + .markOfflineExpectingError("2013-08-2", null, null, "Error saving. A value is required for field 'Description'") .markOfflineExpectingError("2013-08-2", "2013-0802", null, "Error saving. " + getConversionErrorMessage("2013-0802", "EndDate", Timestamp.class)) .markOfflineExpectingError("2013-08-2", "2013-08-01", null, "Error saving. End date cannot be before the start date"); diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index acbf03751..1ff20d55c 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -1301,19 +1301,103 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { getAnnotationData: function() { this.setLoadingMsg(); - var config = this.getReportConfig(); + let config = this.getReportConfig(); - var annotationSql = "SELECT qca.Id AS qcAnnotationId, qca.Date, qca.Description, qca.Created, qca.CreatedBy.DisplayName, qcat.Id AS qcAnnotationTypeId, qcat.Name, qcat.Color FROM qcannotation qca JOIN qcannotationtype qcat ON qcat.Id = qca.QCAnnotationTypeId"; + let annotationSql = "SELECT qca.Id AS qcAnnotationId, qca.Date, qca.Description, qca.Created, qca.CreatedBy.DisplayName, qcat.Id AS qcAnnotationTypeId, qcat.Name, qcat.Color, qca.container.Path AS ContainerPath FROM qcannotation qca JOIN qcannotationtype qcat ON qcat.Id = qca.QCAnnotationTypeId"; // Filter on start/end dates - var separator = " WHERE "; + let dateFilter = ""; if (config.StartDate) { - annotationSql += separator + "CAST(Date AS Date) >= '" + config.StartDate + "'"; - separator = " AND "; + dateFilter += " AND CAST(Date AS Date) >= '" + config.StartDate + "'"; } if (config.EndDate) { - annotationSql += separator + "CAST(Date AS Date) <= '" + config.EndDate + "'"; - } + dateFilter += " AND CAST(Date AS Date) <= '" + config.EndDate + "'"; + } + annotationSql += " WHERE 1=1 " + dateFilter; + + let handleAnnotationData = function(data) { + let annotationData = data ? data.rows : []; + + + // Check if there is an instrument attached to the current container from samplefile table + // check the exact instruments in the current container & + // any other instruments that share a nickname with an instrument used in the current folder. + let getInstrumentsSql = "SELECT DISTINCT Model, SerialNumber AS InstrumentSerialNumber FROM InstrumentNickname " + + "WHERE Nickname IN (SELECT DISTINCT Nickname FROM InstrumentNickname " + + "WHERE (Model || '-' || SerialNumber) IN (SELECT DISTINCT (InstrumentId.Model || '-' || InstrumentSerialNumber) FROM samplefile)) " + + "UNION " + + "SELECT DISTINCT InstrumentId.Model, InstrumentSerialNumber FROM samplefile"; + + LABKEY.Query.executeSql({ + schemaName: 'targetedms', + sql: getInstrumentsSql, + scope: this, + success: function(instrumentData) { + if (instrumentData && instrumentData.rows && instrumentData.rows.length > 0) { + let instrumentFilter = ""; + let separator = ""; + for (let i = 0; i < instrumentData.rows.length; i++) { + let row = instrumentData.rows[i]; + let model = row["Model"]; + let serial = row["InstrumentSerialNumber"]; + + instrumentFilter += separator + "("; + let innerSep = ""; + if (model) { + instrumentFilter += "(qca.instrumentModel = '" + model + "'"; + innerSep = " AND "; + } else { + instrumentFilter += "(qca.instrumentModel IS NULL"; + innerSep = " AND "; + } + + if (serial) { + instrumentFilter += innerSep + "qca.instrumentSerialNumber = '" + serial + "')"; + } else { + instrumentFilter += innerSep + "qca.instrumentSerialNumber IS NULL)"; + } + + instrumentFilter += ")"; + separator = " OR "; + } + + let sharedAnnotationSql = "SELECT qca.Id AS qcAnnotationId, qca.Date, qca.Description, qca.Created, qca.CreatedBy.DisplayName, qcat.Id AS qcAnnotationTypeId, qcat.Name, qcat.Color, qca.container.Path AS ContainerPath " + + "FROM qcannotation qca " + + "JOIN qcannotationtype qcat ON qcat.Id = qca.QCAnnotationTypeId " + + "WHERE qcat.Shareable = true AND (" + instrumentFilter + ")" + dateFilter; + + LABKEY.Query.executeSql({ + schemaName: 'targetedms', + sql: sharedAnnotationSql, + containerFilter: LABKEY.Query.containerFilter.allFolders, + scope: this, + success: function(sharedData) { + if (sharedData && sharedData.rows) { + // add shared annotations but avoid duplicates if they were already in the first list + let existingIds = {}; + for (let j = 0; j < annotationData.length; j++) { + existingIds[annotationData[j].qcAnnotationId] = true; + } + for (let k = 0; k < sharedData.rows.length; k++) { + if (!existingIds[sharedData.rows[k].qcAnnotationId]) { + annotationData.push(sharedData.rows[k]); + } + } + } + this.processAnnotationData({rows: annotationData}); + }, + failure: this.failureHandler + }); + } else { + this.processAnnotationData({rows: annotationData}); + } + }, + failure: function() { + // if instrument fetch fails, just proceed with what we have + this.processAnnotationData({rows: annotationData}); + } + }); + }; LABKEY.Query.executeSql({ schemaName: 'targetedms', @@ -1321,48 +1405,64 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { sort: 'Date', containerFilter: LABKEY.Query.containerFilter.currentPlusProjectAndShared, scope: this, - success: this.processAnnotationData, + success: handleAnnotationData, failure: this.failureHandler }); }, processAnnotationData: function(data) { if (data) { - this.annotationData = data.rows; this.annotationShape = LABKEY.vis.Scale.Shape()[4]; // 0: circle, 1: triangle, 2: square, 3: diamond, 4: X + this.legendData = []; + + const collapsedData = []; + const collapsedMap = {}; + + for (let i = 0; i < data.rows.length; i++) { + const row = data.rows[i]; + const key = row['Date'] + '|' + row['Description'] + '|' + row['qcAnnotationTypeId']; + if (collapsedMap[key] === undefined) { + collapsedMap[key] = collapsedData.length; + row.qcAnnotationIds = [row.qcAnnotationId]; + collapsedData.push(row); + } + else { + collapsedData[collapsedMap[key]].qcAnnotationIds.push(row.qcAnnotationId); + } + } + + this.annotationData = collapsedData; var dateCount = {}; - this.legendData = []; // if more than one type of legend present, add a legend header for annotations if (this.annotationData.length > 0 && (this.singlePlot || this.showMeanCUSUMPlot() || this.showVariableCUSUMPlot())) { - this.legendData.push({ - text: 'Annotations', - separator: true - }); + this.legendData.push({ + text: 'Annotations', + separator: true + }); } - for (let i = 0; i < this.annotationData.length; i++) - { + for (let i = 0; i < this.annotationData.length; i++) { const annotation = this.annotationData[i]; const annotationDate = this.formatDate(new Date(annotation['Date']), !this.groupedX); - // track if we need to stack annotations that fall on the same date - if (!dateCount[annotationDate]) { - dateCount[annotationDate] = 0; - } - annotation.yStepIndex = dateCount[annotationDate]; - dateCount[annotationDate]++; + // track if we need to stack annotations that fall on the same date + if (!dateCount[annotationDate]) { + dateCount[annotationDate] = 0; + } + annotation.yStepIndex = dateCount[annotationDate]; + dateCount[annotationDate]++; - // get unique annotation names and colors for the legend + // get unique annotation names and colors for the legend if (Ext4.Array.pluck(this.legendData, "text").indexOf(annotation['Name']) === -1) { - this.legendData.push({ - text: annotation['Name'], - color: '#' + annotation['Color'], - shape: this.annotationShape - }); - } + this.legendData.push({ + text: annotation['Name'], + color: '#' + annotation['Color'], + shape: this.annotationShape + }); } + } this.getPlotsData(); } @@ -1909,23 +2009,54 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { .attr("d", this.annotationShape(4)).attr('transform', transformAcc) .style("fill", colorAcc).style("stroke", colorAcc); - // add hover text for the annotation details - annotations.append("title") - .text(function(d) { - return "Created By: " + d['DisplayName'] + ", " - + "\nType: " + d['Name'] + ", " - + "\nDate: " + me.formatDate(new Date(d['Date']), true) + ", " - + "\nDescription: " + d['Description']; - }); - - // add some mouseover effects for fun - var mouseOn = function(pt, strokeWidth) { + // 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:" + dateStr + "
Description:" + LABKEY.Utils.encodeHtml(d['Description']) + "
Shared From:" + containerPath + "
"; + + tippy(pt, { + content: content, + allowHTML: true, + arrow: true, + theme: 'light-border', + placement: 'top', + onMount(instance) { + const tippyBox = instance.popper.querySelector('.tippy-box'); + const tippyContent = instance.popper.querySelector('.tippy-content'); + + if (tippyBox) { + tippyBox.style.color = 'black'; + tippyBox.style.backgroundColor = 'white'; + tippyBox.style.border = '1px solid black'; + } + if (tippyContent) { + tippyContent.style.padding = '2px'; + } + } + }); + } }; var mouseOff = function(pt) { d3.select(pt).transition().duration(800).attr("stroke-width", 1).ease("elastic"); }; - annotations.on("mouseover", function(){ return mouseOn(this, 3); }); + annotations.on("mouseover", function(d){ return mouseOn(this, 3, d); }); annotations.on("mouseout", function(){ return mouseOff(this); }); if (this.canUserEdit()) { @@ -1962,10 +2093,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { .style("opacity", 0); // Add mouseover effects for add-annotations - nonAnnotationGroups.append("title") - .text("Add annotation"); - - nonAnnotationGroups.on("mouseover", function () { + nonAnnotationGroups.on("mouseover", function (d) { d3.select(this).select(".add-annotation-background") .transition().duration(300) .style("opacity", 0) @@ -1974,6 +2102,23 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { .transition().duration(300) .style("opacity", 1) .style("cursor", "pointer"); + + if (!this._tippy) { + tippy(this, { + content: "Add annotation", + arrow: true, + theme: 'light-border', + placement: 'top', + onMount(instance) { + const tippyBox = instance.popper.querySelector('.tippy-box'); + if (tippyBox) { + tippyBox.style.color = 'black'; + tippyBox.style.backgroundColor = 'white'; + tippyBox.style.border = '1px solid black'; + } + } + }); + } }); nonAnnotationGroups.on("mouseout", function () { d3.select(this).select(".add-annotation-background") @@ -2006,7 +2151,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { return Ext4.create('Ext.window.Window', { title: title, width: 400, - height: 200, + height: 230, modal: true, items: [{ xtype: 'labkey-combo', @@ -2018,15 +2163,48 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { store: Ext4.create('LABKEY.ext4.data.Store', { schemaName: 'targetedms', queryName: 'QCAnnotationType', - columns: 'Id,Name', - autoLoad: true + columns: 'Id,Name,Shareable', + autoLoad: true, + listeners: { + load: function() { + const field = me.sharedAnnotationDisplayField; + if (field && field.rendered) { + field.updateVisibility(); + } + } + } }), displayField: 'Name', valueField: 'Id', editable: false, allowBlank: false, value: addNew ? null : data['qcAnnotationTypeId'], - + listeners: { + change: function (combo, newValue) { + const record = combo.getStore().findRecord('Id', newValue); + const isShared = record ? record.get('Shareable') : false; + const field = combo.up('window').down('#shared-annotation-display'); + field.setVisible(isShared); + } + } + }, { + xtype: 'displayfield', + itemId: 'shared-annotation-display', + value: ' Shared', + margin: '0 0 10 165', + hidden: true, + listeners: { + afterrender: function (field) { + me.sharedAnnotationDisplayField = field; + field.updateVisibility = function() { + const combo = field.up('window').down('labkey-combo[name=annotationType]'); + const record = combo.getStore().findRecord('Id', combo.getValue()); + const isShared = record ? record.get('Shareable') : false; + field.setVisible(isShared); + }; + field.updateVisibility(); + } + } }, { xtype: 'textarea', labelWidth: 150, @@ -2084,7 +2262,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { return; } - me.updateAnnotation(data['qcAnnotationId'], annotationType, description, annotationDate, win); + me.updateAnnotation(data['qcAnnotationIds'], annotationType, description, annotationDate, win); } }, { text: 'Delete', @@ -2094,7 +2272,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { const win = this.up('window'); Ext4.Msg.confirm('Confirm Delete', 'Are you sure you want to delete this annotation?', function (btn) { if (btn === 'yes') { - me.deleteAnnotation(data['qcAnnotationId'], win); + me.deleteAnnotation(data['qcAnnotationIds'], win); } }); } @@ -2139,19 +2317,21 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }); }, - updateAnnotation: function (annotationId, annotationType, description, annotationDate, win) { + updateAnnotation: function (annotationIds, annotationType, description, annotationDate, win) { // Format date as UTC string (YYYY-MM-DD) to avoid timezone conversion const dateStr = Ext4.util.Format.date(annotationDate, 'Y-m-d'); + const rows = annotationIds.map(id => ({ + Id: id, + QCAnnotationTypeId: annotationType, + Description: description, + Date: dateStr + })); + LABKEY.Query.updateRows({ schemaName: 'targetedms', queryName: 'QCAnnotation', - rows: [{ - Id: annotationId, - QCAnnotationTypeId: annotationType, - Description: description, - Date: dateStr - }], + rows: rows, success: function () { win.close(); this.displayTrendPlot(); @@ -2170,11 +2350,13 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }); }, - deleteAnnotation: function (annotationId, win) { + deleteAnnotation: function (annotationIds, win) { + const rows = annotationIds.map(id => ({ Id: id })); + LABKEY.Query.deleteRows({ schemaName: 'targetedms', queryName: 'QCAnnotation', - rows: [{ Id: annotationId }], + rows: rows, success: function () { win.close(); this.displayTrendPlot(); From 7a0d418d5e1d6c8136dbe5337be779c0687d57e5 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Fri, 20 Mar 2026 13:38:35 -0700 Subject: [PATCH 02/16] add wait for element to disappear --- test/src/org/labkey/test/components/targetedms/QCPlot.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/org/labkey/test/components/targetedms/QCPlot.java b/test/src/org/labkey/test/components/targetedms/QCPlot.java index 807ae3223..faeb81333 100644 --- a/test/src/org/labkey/test/components/targetedms/QCPlot.java +++ b/test/src/org/labkey/test/components/targetedms/QCPlot.java @@ -89,7 +89,7 @@ private QCHelper.Annotation parseAnnotation(WebElement annotationEl) // Hide tippy to not interfere with other elements getCurrentTest().mouseOver(Locator.tag("body")); - getCurrentTest().waitForElementToDisappear(tippyLocator); + getCurrentTest().waitForElementToDisappear(tippyLocator, 2000); return new QCHelper.Annotation(type.getName(), description); } From d8ea15320551e722c38bf7e51528d792022622d9 Mon Sep 17 00:00:00 2001 From: Ankur Juneja Date: Mon, 23 Mar 2026 12:36:37 -0700 Subject: [PATCH 03/16] Apply suggestions from code review Co-authored-by: Josh Eckels --- webapp/TargetedMS/js/QCTrendPlotPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index 1ff20d55c..d545b0b4e 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -2190,7 +2190,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }, { xtype: 'displayfield', itemId: 'shared-annotation-display', - value: ' Shared', + value: ' Shared with other folders using this instrument', margin: '0 0 10 165', hidden: true, listeners: { From b7187e07ef1fd8830b52844e75faf6e84f27cf98 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Mon, 23 Mar 2026 16:28:46 -0700 Subject: [PATCH 04/16] code review + verification testing code changes --- .../postgresql/targetedms-26.002-26.003.sql | 4 +-- webapp/TargetedMS/js/QCTrendPlotPanel.js | 29 ++++++++++++++----- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/resources/schemas/dbscripts/postgresql/targetedms-26.002-26.003.sql b/resources/schemas/dbscripts/postgresql/targetedms-26.002-26.003.sql index 0f073a6da..9c49a31f7 100644 --- a/resources/schemas/dbscripts/postgresql/targetedms-26.002-26.003.sql +++ b/resources/schemas/dbscripts/postgresql/targetedms-26.002-26.003.sql @@ -1,5 +1,5 @@ ALTER TABLE targetedms.QCAnnotationType ADD COLUMN Shareable BOOLEAN DEFAULT FALSE; -ALTER TABLE targetedms.QCAnnotation ADD COLUMN instrumentModel VARCHAR(255); -ALTER TABLE targetedms.QCAnnotation ADD COLUMN instrumentSerialNumber VARCHAR(255); +ALTER TABLE targetedms.QCAnnotation ADD COLUMN instrumentModel VARCHAR(300); +ALTER TABLE targetedms.QCAnnotation ADD COLUMN instrumentSerialNumber VARCHAR(200); UPDATE targetedms.QCAnnotationType SET Shareable = TRUE WHERE Name = 'Instrumentation Change'; \ No newline at end of file diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index 1ff20d55c..f25aff35e 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -1344,7 +1344,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { instrumentFilter += separator + "("; let innerSep = ""; if (model) { - instrumentFilter += "(qca.instrumentModel = '" + model + "'"; + instrumentFilter += "(qca.instrumentModel = '" + LABKEY.Utils.encodeHtml(model) + "'"; innerSep = " AND "; } else { instrumentFilter += "(qca.instrumentModel IS NULL"; @@ -1352,7 +1352,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { } if (serial) { - instrumentFilter += innerSep + "qca.instrumentSerialNumber = '" + serial + "')"; + instrumentFilter += innerSep + "qca.instrumentSerialNumber = '" + LABKEY.Utils.encodeHtml(serial) + "')"; } else { instrumentFilter += innerSep + "qca.instrumentSerialNumber IS NULL)"; } @@ -2037,6 +2037,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { arrow: true, theme: 'light-border', placement: 'top', + offset: [0, 9], onMount(instance) { const tippyBox = instance.popper.querySelector('.tippy-box'); const tippyContent = instance.popper.querySelector('.tippy-content'); @@ -2047,7 +2048,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { tippyBox.style.border = '1px solid black'; } if (tippyContent) { - tippyContent.style.padding = '2px'; + tippyContent.style.padding = '6px'; } } }); @@ -2109,13 +2110,18 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { arrow: true, theme: 'light-border', placement: 'top', + offset: [0, 9], onMount(instance) { const tippyBox = instance.popper.querySelector('.tippy-box'); + const tippyContent = instance.popper.querySelector('.tippy-content'); if (tippyBox) { tippyBox.style.color = 'black'; tippyBox.style.backgroundColor = 'white'; tippyBox.style.border = '1px solid black'; } + if (tippyContent) { + tippyContent.style.padding = '6px'; + } } }); } @@ -2147,13 +2153,20 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { const date = this.formatDate(data['Date'], false); const title = addNew ? 'Add Annotation' : 'Edit Annotation'; const me = this; + const currentContainer = LABKEY.ActionURL.getContainer(); + const fromOtherContainer = !addNew && data['ContainerPath'] && data['ContainerPath'] !== currentContainer; return Ext4.create('Ext.window.Window', { title: title, width: 400, - height: 230, + height: fromOtherContainer ? 280 : 230, modal: true, items: [{ + xtype: 'displayfield', + value: '
This annotation is shared from another container (' + LABKEY.Utils.encodeHtml(data['ContainerPath']) + ') and cannot be edited or deleted here.
', + margin: '10 10 10 10', + hidden: !fromOtherContainer + }, { xtype: 'labkey-combo', fieldLabel: 'Annotation Type', name: 'annotationType', @@ -2176,6 +2189,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }), displayField: 'Name', valueField: 'Id', + readOnly: !addNew, editable: false, allowBlank: false, value: addNew ? null : data['qcAnnotationTypeId'], @@ -2214,6 +2228,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { margin: '10 10 10 10', name: 'description', allowBlank: false, + readOnly: fromOtherContainer, value: addNew ? null : data['Description'] }, { xtype: 'datefield', @@ -2224,6 +2239,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { name: 'annotationDate', format: 'Y-m-d', allowBlank: false, + readOnly: fromOtherContainer, value: date, submitFormat: 'Y-m-d' }], @@ -2249,7 +2265,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }, { text: 'Update', hidden: addNew, - disabled: !me.canUserEdit(), + disabled: !me.canUserEdit() || fromOtherContainer, handler: function () { const win = this.up('window'); const form = win.down('form') || win; @@ -2267,7 +2283,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { }, { text: 'Delete', hidden: addNew, - disabled: !me.canUserEdit(), + disabled: !me.canUserEdit() || fromOtherContainer, handler: function () { const win = this.up('window'); Ext4.Msg.confirm('Confirm Delete', 'Are you sure you want to delete this annotation?', function (btn) { @@ -2282,7 +2298,6 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { this.up('window').close(); } }] - }); }, From 7fac9ec074fea40753e19e079730a177a9f2e2ad Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Mon, 23 Mar 2026 16:34:32 -0700 Subject: [PATCH 05/16] fix the triangle pointing down to be one pixel lower so that it doesn't extend in the main box --- webapp/TargetedMS/js/QCTrendPlotPanel.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index ae48d8275..6fc9b1a3c 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -2037,10 +2037,11 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { arrow: true, theme: 'light-border', placement: 'top', - offset: [0, 9], + 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'; @@ -2050,6 +2051,9 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { if (tippyContent) { tippyContent.style.padding = '6px'; } + if (tippyArrow) { + tippyArrow.style.bottom = '-1px'; + } } }); } @@ -2110,10 +2114,12 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { arrow: true, theme: 'light-border', placement: 'top', - offset: [0, 9], + 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'; @@ -2122,6 +2128,9 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { if (tippyContent) { tippyContent.style.padding = '6px'; } + if (tippyArrow) { + tippyArrow.style.bottom = '-1px'; + } } }); } From 54f7b34c4a7a5fc243121fa2c982511de9ba3427 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Mon, 23 Mar 2026 17:04:02 -0700 Subject: [PATCH 06/16] Fix duplicate QCAnnotations from multiple Skyline imports --- src/org/labkey/targetedms/TargetedMSManager.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/org/labkey/targetedms/TargetedMSManager.java b/src/org/labkey/targetedms/TargetedMSManager.java index f9538e185..73b423668 100644 --- a/src/org/labkey/targetedms/TargetedMSManager.java +++ b/src/org/labkey/targetedms/TargetedMSManager.java @@ -3060,8 +3060,6 @@ public static class InstrumentDetails private String instrumentSerialNumber; @Getter @Setter private String model; - @Getter @Setter - private Long instrumentId; public InstrumentDetails() { @@ -3070,7 +3068,7 @@ public InstrumentDetails() public static List getInstrumentDetails(Container container) { - SQLFragment sql = new SQLFragment("SELECT DISTINCT sf.InstrumentSerialNumber, i.Model, i.Id AS InstrumentId FROM "); + SQLFragment sql = new SQLFragment("SELECT DISTINCT sf.InstrumentSerialNumber, i.Model FROM "); sql.append(getTableInfoSampleFile(), "sf"); sql.append(" INNER JOIN "); sql.append(getTableInfoInstrument(), "i"); From 13988eb7f479f768fe0aa272df7a7d2b809f7dd5 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Mon, 23 Mar 2026 17:11:04 -0700 Subject: [PATCH 07/16] increase height for shared annotation dialog in another folder --- webapp/TargetedMS/js/QCTrendPlotPanel.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index 6fc9b1a3c..1aef45dde 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -2168,11 +2168,11 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { return Ext4.create('Ext.window.Window', { title: title, width: 400, - height: fromOtherContainer ? 280 : 230, + height: fromOtherContainer ? 350 : 230, modal: true, items: [{ xtype: 'displayfield', - value: '
This annotation is shared from another container (' + LABKEY.Utils.encodeHtml(data['ContainerPath']) + ') and cannot be edited or deleted here.
', + value: '
This annotation is shared from another folder (' + LABKEY.Utils.encodeHtml(data['ContainerPath']) + ') and cannot be edited or deleted here.
', margin: '10 10 10 10', hidden: !fromOtherContainer }, { From 6bcd0223cf5fd4529cb3367e21d8d7049d91e939 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Mon, 23 Mar 2026 19:35:43 -0700 Subject: [PATCH 08/16] test user access with shareable annotations --- .../tests/targetedms/TargetedMSQCTest.java | 60 ++++++++++++++++--- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java index 830d0ff62..14d7262ee 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java @@ -41,6 +41,7 @@ import org.labkey.test.util.Ext4Helper; import org.labkey.test.util.LogMethod; import org.labkey.test.util.LoggedParam; +import org.labkey.test.util.PermissionsHelper; import org.labkey.test.util.PipelineStatusTable; import org.labkey.test.util.PortalHelper; import org.labkey.test.util.targetedms.QCHelper; @@ -965,6 +966,7 @@ public void testShareableAnnotations() { String folderA = "Folder A"; String folderB = "Folder B"; + String folderC = "Folder C"; setupSubfolder(getProjectName(), folderA, FolderType.QC); importData(ISOTOPOLOGUE_FILE_ANNOTATED); @@ -972,6 +974,9 @@ public void testShareableAnnotations() setupSubfolder(getProjectName(), folderB, FolderType.QC); importData(ISOTOPOLOGUE_FILE_ANNOTATED); + setupSubfolder(getProjectName(), folderC, FolderType.QC); + importData(ISOTOPOLOGUE_FILE_ANNOTATED); + clickFolder(folderA); clickTab("Annotations"); @@ -983,16 +988,31 @@ public void testShareableAnnotations() annotationWebPart = new PanoramaAnnotations(this).getQcAnnotationWebPart(); // expect two annotations because the data has association with two instruments assertEquals("Expected two annotation rows to be created for shareable annotation", 2, annotationWebPart.getDataRegion().getDataRowCount()); - + + String testUser = "test_shareable_annotations@targetedms.test"; + _userHelper.createUser(testUser); + String testUserNoAccess = "test_shareable_annotations_no_access@targetedms.test"; + _userHelper.createUser(testUserNoAccess); + + ApiPermissionsHelper permissionsHelper = new ApiPermissionsHelper(this); + // Test user has access to Folder B and Folder A (so should see Folder A's annotation in Folder B) + permissionsHelper.addMemberToRole(testUser, READER_ROLE, PermissionsHelper.MemberType.user, getProjectName() + "/" + folderA); + permissionsHelper.addMemberToRole(testUser, READER_ROLE, PermissionsHelper.MemberType.user, getProjectName() + "/" + folderB); + + // Test user with NO access to Folder A, only access to Folder C + permissionsHelper.addMemberToRole(testUserNoAccess, READER_ROLE, PermissionsHelper.MemberType.user, getProjectName() + "/" + folderC); + + impersonate(testUser); + + // Case 1: In Folder B, user HAS access to Folder A, so they SHOULD see the shareable annotation clickFolder(folderB); clickTab("Panorama Dashboard"); PanoramaDashboard qcDashboard = new PanoramaDashboard(this); QCPlotsWebPart qcPlotsWebPart = qcDashboard.getQcPlotsWebPart(); qcPlotsWebPart.waitForPlots(35); - // Verify the shareable annotation from Folder A appears in Folder B's QC plots List qcPlots = qcPlotsWebPart.getPlots(); - boolean shareableAnnotationFound = false; + boolean shareableAnnotationFoundInB = false; for (QCPlot plot : qcPlots) { List annotations = plot.getAnnotations(); @@ -1001,15 +1021,39 @@ public void testShareableAnnotations() if (annotation.getType().equals(shareableAnnotation.getType()) && annotation.getDescription().equals(shareableAnnotation.getDescription())) { - shareableAnnotationFound = true; - break; + shareableAnnotationFoundInB = true; } } - if (shareableAnnotationFound) - break; } - assertTrue("Shareable annotation from Folder A should appear in Folder B QC plots", shareableAnnotationFound); + assertTrue("Shareable annotation from Folder A should appear in Folder B QC plots", shareableAnnotationFoundInB); + + stopImpersonating(); + impersonate(testUserNoAccess); + + // Case 2: In Folder C, user does NOT have access to Folder A, so they SHOULD NOT see the shareable annotation + clickFolder(folderC); + clickTab("Panorama Dashboard"); + qcDashboard = new PanoramaDashboard(this); + qcPlotsWebPart = qcDashboard.getQcPlotsWebPart(); + qcPlotsWebPart.waitForPlots(35); + qcPlots = qcPlotsWebPart.getPlots(); + boolean shareableAnnotationFoundInC = false; + for (QCPlot plot : qcPlots) + { + List annotations = plot.getAnnotations(); + for (QCHelper.Annotation annotation : annotations) + { + if (annotation.getType().equals(shareableAnnotation.getType()) && + annotation.getDescription().equals(shareableAnnotation.getDescription())) + { + shareableAnnotationFoundInC = true; + } + } + } + assertFalse("Shareable annotation from Folder A should NOT appear in Folder C QC plots for user without read access to Folder A", shareableAnnotationFoundInC); + + stopImpersonating(); } private void verifyQCSummarySampleFileOutliers(String acquiredDate, String outlierInfo) From ff3aacf630a70f5310d1b7cbca7d0e328d485a0d Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Tue, 24 Mar 2026 09:11:20 -0700 Subject: [PATCH 09/16] test change --- test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java index 14d7262ee..7c2eb7d08 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java @@ -1028,10 +1028,10 @@ public void testShareableAnnotations() assertTrue("Shareable annotation from Folder A should appear in Folder B QC plots", shareableAnnotationFoundInB); stopImpersonating(); - impersonate(testUserNoAccess); // Case 2: In Folder C, user does NOT have access to Folder A, so they SHOULD NOT see the shareable annotation clickFolder(folderC); + impersonate(testUserNoAccess); clickTab("Panorama Dashboard"); qcDashboard = new PanoramaDashboard(this); qcPlotsWebPart = qcDashboard.getQcPlotsWebPart(); From 6046bdef59e33117cab3fa5c8e4578b267d2c225 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Tue, 24 Mar 2026 09:14:34 -0700 Subject: [PATCH 10/16] encode datastr --- webapp/TargetedMS/js/QCTrendPlotPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/TargetedMS/js/QCTrendPlotPanel.js b/webapp/TargetedMS/js/QCTrendPlotPanel.js index 1aef45dde..529153071 100644 --- a/webapp/TargetedMS/js/QCTrendPlotPanel.js +++ b/webapp/TargetedMS/js/QCTrendPlotPanel.js @@ -2019,7 +2019,7 @@ Ext4.define('LABKEY.targetedms.QCTrendPlotPanel', { let content = "" + "" + "" - + "" + + "" + ""; if (d['ContainerPath'] && d['ContainerPath'] !== LABKEY.ActionURL.getContainer()) { From 5feee7d4bc4d477c553713b0bdf656a9da044d24 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Tue, 24 Mar 2026 13:02:50 -0700 Subject: [PATCH 11/16] fix test --- test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java index 7c2eb7d08..6493d7224 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSQCTest.java @@ -1030,6 +1030,7 @@ public void testShareableAnnotations() stopImpersonating(); // Case 2: In Folder C, user does NOT have access to Folder A, so they SHOULD NOT see the shareable annotation + goToProjectHome(); clickFolder(folderC); impersonate(testUserNoAccess); clickTab("Panorama Dashboard"); From e40f771ba6b47d34e33a577ade6c7680ea5b6d63 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Tue, 24 Mar 2026 13:13:40 -0700 Subject: [PATCH 12/16] fix test - testQCAnnotations --- test/src/org/labkey/test/components/targetedms/QCPlot.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/org/labkey/test/components/targetedms/QCPlot.java b/test/src/org/labkey/test/components/targetedms/QCPlot.java index faeb81333..8eecd3389 100644 --- a/test/src/org/labkey/test/components/targetedms/QCPlot.java +++ b/test/src/org/labkey/test/components/targetedms/QCPlot.java @@ -88,7 +88,7 @@ private QCHelper.Annotation parseAnnotation(WebElement annotationEl) QCHelper.AnnotationType type = getAnnotationTypes().get(color); // Hide tippy to not interfere with other elements - getCurrentTest().mouseOver(Locator.tag("body")); + getCurrentTest().mouseOver(Locator.tagWithClass("table", "labkey-wp qc-plot-wp")); getCurrentTest().waitForElementToDisappear(tippyLocator, 2000); return new QCHelper.Annotation(type.getName(), description); From ab3d6c448a3a9e3deb7e484a6b71a218e0699fb8 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Wed, 25 Mar 2026 09:35:34 -0700 Subject: [PATCH 13/16] fix test - testUtilizationCalendarActions --- .../tests/targetedms/TargetedMSUtilizationCalendarTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java index e5ab284c3..05ed29770 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java @@ -60,7 +60,7 @@ public void testUtilizationCalendarActions() log("Input data validation checks"); utilizationCalendar.setDisplay("1") .markOfflineExpectingError("2013-08-2", null, null, "Error saving. A value is required for field 'Description'") - .markOfflineExpectingError("2013-08-2", "2013-0802", null, "Error saving. " + getConversionErrorMessage("2013-0802", "EndDate", Timestamp.class)) + .markOfflineExpectingError("2013-08-2", "2013-0802", null, "Error saving. Unable to convert value '2013-0802' to Date and Time") .markOfflineExpectingError("2013-08-2", "2013-08-01", null, "Error saving. End date cannot be before the start date"); log("Marking single day offline"); From cf4d49d7930546a5f3458890585f71d250311bca Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Wed, 25 Mar 2026 11:21:06 -0700 Subject: [PATCH 14/16] fix test --- .../tests/targetedms/TargetedMSUtilizationCalendarTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java index 05ed29770..d31fa8b43 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java @@ -61,7 +61,7 @@ public void testUtilizationCalendarActions() utilizationCalendar.setDisplay("1") .markOfflineExpectingError("2013-08-2", null, null, "Error saving. A value is required for field 'Description'") .markOfflineExpectingError("2013-08-2", "2013-0802", null, "Error saving. Unable to convert value '2013-0802' to Date and Time") - .markOfflineExpectingError("2013-08-2", "2013-08-01", null, "Error saving. End date cannot be before the start date"); + .markOfflineExpectingError("2013-08-2", "2013-08-01", "Offline", "Error saving. End date cannot be before the start date"); log("Marking single day offline"); String offlineDate = "2013-08-13"; From 34d08629ba45f69b0413480e16706c31c9b57831 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Wed, 25 Mar 2026 11:29:58 -0700 Subject: [PATCH 15/16] fix test --- test/src/org/labkey/test/components/targetedms/QCPlot.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/src/org/labkey/test/components/targetedms/QCPlot.java b/test/src/org/labkey/test/components/targetedms/QCPlot.java index 8eecd3389..c4c485af1 100644 --- a/test/src/org/labkey/test/components/targetedms/QCPlot.java +++ b/test/src/org/labkey/test/components/targetedms/QCPlot.java @@ -87,10 +87,6 @@ private QCHelper.Annotation parseAnnotation(WebElement annotationEl) String color = annotationEl.getCssValue("fill"); QCHelper.AnnotationType type = getAnnotationTypes().get(color); - // Hide tippy to not interfere with other elements - getCurrentTest().mouseOver(Locator.tagWithClass("table", "labkey-wp qc-plot-wp")); - getCurrentTest().waitForElementToDisappear(tippyLocator, 2000); - return new QCHelper.Annotation(type.getName(), description); } From b1fbe1f1bb3941deb2b22df48d527c745711b806 Mon Sep 17 00:00:00 2001 From: ankurjuneja Date: Wed, 25 Mar 2026 16:40:14 -0700 Subject: [PATCH 16/16] fix test --- .../targetedms/query/QCAnnotationTable.java | 48 ++++++++----------- .../TargetedMSUtilizationCalendarTest.java | 2 +- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/org/labkey/targetedms/query/QCAnnotationTable.java b/src/org/labkey/targetedms/query/QCAnnotationTable.java index 5af2f2484..781e19eb4 100644 --- a/src/org/labkey/targetedms/query/QCAnnotationTable.java +++ b/src/org/labkey/targetedms/query/QCAnnotationTable.java @@ -73,46 +73,36 @@ public List> insertRows(User user, Container container, List List> resultRows = new java.util.ArrayList<>(); for (Map row : rows) { - try - { - // Check if the QCAnnotationType is shareable - int qcAnnotationTypeId = (Integer) row.get("QCAnnotationTypeId"); - boolean isShareable = TargetedMSManager.isQCAnnotationTypeShareable(qcAnnotationTypeId); + // Check if the QCAnnotationType is shareable + int qcAnnotationTypeId = (Integer) row.get("QCAnnotationTypeId"); + boolean isShareable = TargetedMSManager.isQCAnnotationTypeShareable(qcAnnotationTypeId); - if (isShareable) + if (isShareable) + { + List instruments = TargetedMSManager.getInstrumentDetails(getContainer()); + if (instruments.isEmpty()) { - List instruments = TargetedMSManager.getInstrumentDetails(getContainer()); - if (instruments.isEmpty()) - { - resultRows.add(super.insertRow(user, container, row)); - } - else - { - for (TargetedMSManager.InstrumentDetails instrument : instruments) - { - Map newRow = new java.util.HashMap<>(row); - newRow.put("instrumentModel", instrument.getModel()); - newRow.put("instrumentSerialNumber", instrument.getInstrumentSerialNumber()); - newRow.put("Container", getContainer().getId()); - resultRows.add(super.insertRow(user, container, newRow)); - } - } + resultRows.add(row); } else { - resultRows.add(super.insertRow(user, container, row)); + for (TargetedMSManager.InstrumentDetails instrument : instruments) + { + Map newRow = new java.util.HashMap<>(row); + newRow.put("instrumentModel", instrument.getModel()); + newRow.put("instrumentSerialNumber", instrument.getInstrumentSerialNumber()); + newRow.put("Container", getContainer().getId()); + resultRows.add(newRow); + } } } - catch (ValidationException e) + else { - errors.addRowError(e); + resultRows.add(row); } } - if (errors.hasErrors()) - return null; - - return resultRows; + return super.insertRows(user, container, resultRows, errors, configParameters, extraScriptContext); } }; } diff --git a/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java b/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java index d31fa8b43..05ed29770 100644 --- a/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java +++ b/test/src/org/labkey/test/tests/targetedms/TargetedMSUtilizationCalendarTest.java @@ -61,7 +61,7 @@ public void testUtilizationCalendarActions() utilizationCalendar.setDisplay("1") .markOfflineExpectingError("2013-08-2", null, null, "Error saving. A value is required for field 'Description'") .markOfflineExpectingError("2013-08-2", "2013-0802", null, "Error saving. Unable to convert value '2013-0802' to Date and Time") - .markOfflineExpectingError("2013-08-2", "2013-08-01", "Offline", "Error saving. End date cannot be before the start date"); + .markOfflineExpectingError("2013-08-2", "2013-08-01", null, "Error saving. End date cannot be before the start date"); log("Marking single day offline"); String offlineDate = "2013-08-13";
Created By:" + LABKEY.Utils.encodeHtml(d['DisplayName']) + "
Type:" + LABKEY.Utils.encodeHtml(d['Name']) + "
Date:" + dateStr + "
Date:" + LABKEY.Utils.encodeHtml(dateStr) + "
Description:" + LABKEY.Utils.encodeHtml(d['Description']) + "