diff --git a/signalData/resources/assay/signalData/domains/result.xml b/signalData/resources/assay/signalData/domains/result.xml index 3c859714d..6dc81ccb8 100644 --- a/signalData/resources/assay/signalData/domains/result.xml +++ b/signalData/resources/assay/signalData/domains/result.xml @@ -7,7 +7,6 @@ DataFile - true http://cpas.fhcrc.org/exp/xml#fileLink Data File diff --git a/signalData/resources/schemas/dbscripts/postgresql/signaldata-25.000-25.001.sql b/signalData/resources/schemas/dbscripts/postgresql/signaldata-25.000-25.001.sql new file mode 100644 index 000000000..58caa6d82 --- /dev/null +++ b/signalData/resources/schemas/dbscripts/postgresql/signaldata-25.000-25.001.sql @@ -0,0 +1 @@ +SELECT core.executeJavaUpgradeCode('updateDataFileField'); diff --git a/signalData/resources/views/signalDataUpload.html b/signalData/resources/views/signalDataUpload.html index 91c86d742..3fd73f943 100644 --- a/signalData/resources/views/signalDataUpload.html +++ b/signalData/resources/views/signalDataUpload.html @@ -1,7 +1,7 @@

Signal Data Files

  1. - Import (or paste) the results file. Data must include a column that has the signal data filename (case-sensitive). + Import (or paste) the results file. Data must include a column that has the signal data filename.
    Download Spreadsheet Template
  2. diff --git a/signalData/resources/web/signaldata/QCView/DataService.js b/signalData/resources/web/signaldata/QCView/DataService.js index edaa7cd4f..11b11e741 100644 --- a/signalData/resources/web/signaldata/QCView/DataService.js +++ b/signalData/resources/web/signaldata/QCView/DataService.js @@ -247,6 +247,9 @@ Ext4.define('LABKEY.SignalData.DataService', { name = name[0]; var filePath = ""; var dataFile = run.dataRows[r]['DataFile']; + // dataFile values can be empty, indicating no file was uploaded for the results row, ignore these. + if (!dataFile) + continue; var osDelimiter = '/'; var fileName = dataFile.split(osDelimiter).pop(); //Hack to make fileLink and pipe resolve file diff --git a/signalData/resources/web/signaldata/UploadView/UploadLog.js b/signalData/resources/web/signaldata/UploadView/UploadLog.js index 1e2b50231..643372aa8 100644 --- a/signalData/resources/web/signaldata/UploadView/UploadLog.js +++ b/signalData/resources/web/signaldata/UploadView/UploadLog.js @@ -16,6 +16,7 @@ Ext4.define('LABKEY.SignalData.UploadLog', { DATA_FILE: 'DataFile', FILE_URL: 'DataFileURL', FILENAME: 'FileName', + constructor: function (config) { if (!Ext4.ModelManager.isRegistered(this.modelClass)) { Ext4.define(this.modelClass, { @@ -54,13 +55,22 @@ Ext4.define('LABKEY.SignalData.UploadLog', { getFields: function (resultFields) { if (!this.fields) { + // issue 52421 data file metadata can contain full paths + var fileNameFromPath = function(v, rec) { + if (v.indexOf('/') > -1) + return v.substring(v.lastIndexOf('/') + 1, v.length); + + return v.substring(v.lastIndexOf('\\') + 1, v.length); + }; + var fields = []; resultFields.forEach(function (field) { fields.push({ name: field.name, - type: 'string' + type: 'string', + convert: field.fieldKey === this.DATA_FILE ? fileNameFromPath : null }); - }); + }, this); this.fields = fields.concat([ {name: this.FILE_URL, type: 'string'}, @@ -158,7 +168,10 @@ Ext4.define('LABKEY.SignalData.UploadLog', { var fileName = row.get(this.DATA_FILE); var me = this; - //Delete File + // don't delete if no file has been uploaded + if (!row.get(this.UPLOAD_TIME)) + return; + this.fileSystem.deletePath({ path: this.fileSystem.concatPaths(this.getFullWorkingPath(), fileName), isFile: true, diff --git a/signalData/resources/web/signaldata/UploadView/uploadResultDataForm.js b/signalData/resources/web/signaldata/UploadView/uploadResultDataForm.js index e969dc4f0..93f8b8ac6 100644 --- a/signalData/resources/web/signaldata/UploadView/uploadResultDataForm.js +++ b/signalData/resources/web/signaldata/UploadView/uploadResultDataForm.js @@ -140,6 +140,7 @@ LABKEY.SignalData.initializeUploadForm = function(metadataFormElId, metadataFile }], submit:function(){ LABKEY.Ajax.request({ + method: 'POST', url: LABKEY.ActionURL.buildURL("assay", "assayFileUpload", LABKEY.ActionURL.getContainer()), params: { protocolId: assay.id, fileName: metadataTSVId + '.txt', fileContent: this.getTsvInput() }, success: function(response) { diff --git a/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js b/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js index a08939bb4..f51a34f37 100644 --- a/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js +++ b/signalData/resources/web/signaldata/UploadView/uploadResultFiles.js @@ -376,17 +376,20 @@ LABKEY.SignalData.initializeDataFileUploadForm = function (metadataFormId, eleme var dataRows = []; var dataInputs = []; - var rows = uploadLog.getStore().getRange(); - var runFolder = getRunFolderName(); + rows.forEach(function (row){ var dataRow = {}; row.fields.eachKey(function(key){ - dataRow[key] = row.get(key); + + // if there was no actual file uploaded, ignore the dataFile field value so we don't + // try to validate on the server + if (key !== uploadLog.DATA_FILE || row.get('file')) + dataRow[key] = row.get(key); }); - if(row.get('file')) { + if (row.get('file')) { dataRow[uploadLog.DATA_FILE] = decodeURI(dataRow[uploadLog.FILE_URL]).replace('file:',''); dataInputs.push({ diff --git a/signalData/src/org/labkey/signaldata/SignalDataController.java b/signalData/src/org/labkey/signaldata/SignalDataController.java index 228fc5b3d..b065d3301 100644 --- a/signalData/src/org/labkey/signaldata/SignalDataController.java +++ b/signalData/src/org/labkey/signaldata/SignalDataController.java @@ -39,6 +39,7 @@ import org.labkey.api.webdav.WebdavResource; import org.labkey.api.webdav.WebdavService; import org.labkey.signaldata.assay.SignalDataAssayDataHandler; +import org.labkey.vfs.FileLike; import org.springframework.validation.BindException; import java.io.File; @@ -83,11 +84,10 @@ public ApiResponse execute(Object form, BindException errors) throws Exception { containerPath = root.getContainer().getPath(); webdavURL = root.getWebdavURL(); - if (!SignalDataAssayDataHandler.NAMESPACE.isEmpty()) - webdavURL = webdavURL.resolve(SignalDataAssayDataHandler.NAMESPACE); + webdavURL = webdavURL.resolve(SignalDataAssayDataHandler.NAMESPACE); //Create folder if needed - File sdFileRoot = new File(root.getRootPath(), SignalDataAssayDataHandler.NAMESPACE); + FileLike sdFileRoot = root.getRootFileLike().resolveChild(SignalDataAssayDataHandler.NAMESPACE); if(!sdFileRoot.exists()) sdFileRoot.mkdirs(); } diff --git a/signalData/src/org/labkey/signaldata/SignalDataModule.java b/signalData/src/org/labkey/signaldata/SignalDataModule.java index a0a4a0b0c..223c01a06 100644 --- a/signalData/src/org/labkey/signaldata/SignalDataModule.java +++ b/signalData/src/org/labkey/signaldata/SignalDataModule.java @@ -17,16 +17,19 @@ package org.labkey.signaldata; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; -import org.labkey.api.module.CodeOnlyModule; +import org.labkey.api.data.UpgradeCode; +import org.labkey.api.module.DefaultModule; import org.labkey.api.module.ModuleContext; import org.labkey.api.module.ModuleProperty; import org.labkey.api.view.WebPartFactory; import java.util.Collection; import java.util.Collections; +import java.util.List; -public class SignalDataModule extends CodeOnlyModule +public class SignalDataModule extends DefaultModule { public static final String NAME = "SignalData"; public static final String QC_PROVIDER_PROPERTY_NAME = "QCViewProviderModule"; @@ -67,6 +70,30 @@ public void doStartup(ModuleContext moduleContext) { } + @Override + public @Nullable Double getSchemaVersion() + { + return 25.001; + } + + @Override + public boolean hasScripts() + { + return true; + } + + @Override + public @NotNull Collection getSchemaNames() + { + return List.of("signaldata"); + } + + @Override + public @Nullable UpgradeCode getUpgradeCode() + { + return new SignalDataUpgradeCode(); + } + @Override @NotNull public Collection getSummary(Container c) diff --git a/signalData/src/org/labkey/signaldata/SignalDataUpgradeCode.java b/signalData/src/org/labkey/signaldata/SignalDataUpgradeCode.java new file mode 100644 index 000000000..364138ed7 --- /dev/null +++ b/signalData/src/org/labkey/signaldata/SignalDataUpgradeCode.java @@ -0,0 +1,53 @@ +package org.labkey.signaldata; + +import org.apache.logging.log4j.Logger; +import org.labkey.api.assay.AssayProvider; +import org.labkey.api.assay.AssayService; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.UpgradeCode; +import org.labkey.api.exp.api.ExpProtocol; +import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.exp.property.Domain; +import org.labkey.api.exp.property.DomainProperty; +import org.labkey.api.module.Module; +import org.labkey.api.module.ModuleContext; +import org.labkey.api.module.ModuleLoader; +import org.labkey.api.security.User; +import org.labkey.api.util.logging.LogHelper; + +public class SignalDataUpgradeCode implements UpgradeCode +{ + private static final Logger LOG = LogHelper.getLogger(SignalDataUpgradeCode.class, "SignalData upgrade code"); + + /** + * Called from signaldata-25.000-25.001.sql + * Updates SignalData assay protocols to make the result domain DataFile field not required. + */ + @SuppressWarnings({"UnusedDeclaration"}) + public static void updateDataFileField(ModuleContext ctx) throws Exception + { + Module module = ModuleLoader.getInstance().getModule(SignalDataModule.NAME); + if (module != null) + { + for (Container c : ContainerManager.getAllChildrenWithModule(ContainerManager.getRoot(), module)) + { + for (ExpProtocol protocol : ExperimentService.get().getExpProtocols(c)) + { + AssayProvider provider = AssayService.get().getProvider(protocol); + if (provider != null && provider.getName().equalsIgnoreCase("Signal Data")) + { + Domain domain = provider.getResultsDomain(protocol, true); + DomainProperty dataFile = domain.getPropertyByName("DataFile"); + if (dataFile != null && dataFile.isRequired()) + { + LOG.info(String.format("Updating Signal Data assay in folder '%s'", c.getPath())); + dataFile.setRequired(false); + domain.save(User.getAdminServiceUser()); + } + } + } + } + } + } +} diff --git a/signalData/test/sampledata/signaldata/RunsMetadata/datafiles.tsv b/signalData/test/sampledata/signaldata/RunsMetadata/datafiles.tsv index 78d5974cc..9fefaef13 100644 --- a/signalData/test/sampledata/signaldata/RunsMetadata/datafiles.tsv +++ b/signalData/test/sampledata/signaldata/RunsMetadata/datafiles.tsv @@ -1,4 +1,4 @@ -Name DataFile -LGC12392.TXT LGC12392.TXT -LGC14332.TXT LGC14332.TXT -MPP82113.TXT MPP82113.TXT +Name DataFile StringValue IntegerValue +LGC12392.TXT LGC12392.TXT StringOne 1 +LGC14332.TXT LGC14332.TXT StringTwo 2 +MPP82113.TXT MPP82113.TXT StringThree 3 diff --git a/signalData/test/sampledata/signaldata/RunsMetadata/datafiles2.tsv b/signalData/test/sampledata/signaldata/RunsMetadata/datafiles2.tsv new file mode 100644 index 000000000..9e5d3cd56 --- /dev/null +++ b/signalData/test/sampledata/signaldata/RunsMetadata/datafiles2.tsv @@ -0,0 +1,5 @@ +Name DataFile +TD789-12.TXT /Users/testuser/Downloads/td789-12.txt +TD789-25.TXT /Users/testuser/Downloads/TD789-25.txt +QD123-11.TXT c:\\home\\windows\\QD123-11.TXT +QuotedPath.TXT "c:\user\windows\qd123-24.txt" diff --git a/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataAssayBeginPage.java b/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataAssayBeginPage.java index b2edfd790..031862755 100644 --- a/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataAssayBeginPage.java +++ b/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataAssayBeginPage.java @@ -54,7 +54,7 @@ public void waitForGridValue(String value, int qty) } @NotNull - private DataRegionTable getDataRegionTable() + public DataRegionTable getDataRegionTable() { return new DataRegionTable("aqwp101", _test.getDriver()); } diff --git a/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataUploadPage.java b/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataUploadPage.java index cd30d7760..fe1e6a427 100644 --- a/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataUploadPage.java +++ b/signalData/test/src/org/labkey/test/pages/signaldata/SignalDataUploadPage.java @@ -25,7 +25,7 @@ import org.openqa.selenium.support.ui.ExpectedConditions; import java.io.File; -import java.util.Arrays; +import java.util.List; import static org.labkey.test.WebDriverWrapper.WAIT_FOR_JAVASCRIPT; import static org.labkey.test.WebDriverWrapper.sleep; @@ -50,11 +50,11 @@ public void uploadMetadataFile(File file) _test.waitForElement(Locators.runIdentifier); } - public void uploadFile(File... file) + public void uploadFile(List files) { sleep(1_000); WebElement dropFileInputEl = Locators.dropFileInput.findElement(_test.getDriver()); - _test.setInput(dropFileInputEl, Arrays.asList(file)); + _test.setInput(dropFileInputEl, files); } public void waitForProgressBars(int count) @@ -68,7 +68,7 @@ public void waitForProgressBars(int count) public void uploadIncorrectFile(File file) { - uploadFile(file); + uploadFile(List.of(file)); WebElement msgBox = Locators.fileNotUploadedMsgBox(file.getName()).waitForElement(_test.getDriver(), WAIT_FOR_JAVASCRIPT); msgBox.sendKeys(Keys.ESCAPE); _test.shortWait().until(ExpectedConditions.invisibilityOf(msgBox)); diff --git a/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java b/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java index d9b231f0b..fd8f92cf3 100644 --- a/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java +++ b/signalData/test/src/org/labkey/test/tests/signaldata/SignalDataRawTest.java @@ -26,6 +26,7 @@ import org.labkey.test.pages.signaldata.SignalDataAssayBeginPage; import org.labkey.test.pages.signaldata.SignalDataRunViewerPage; import org.labkey.test.pages.signaldata.SignalDataUploadPage; +import org.labkey.test.util.DataRegionTable; import org.labkey.test.util.Ext4Helper; import org.labkey.test.util.PostgresOnlyTest; import org.labkey.test.util.signaldata.SignalDataInitializer; @@ -34,7 +35,9 @@ import java.io.File; import java.util.Collections; import java.util.List; +import java.util.Map; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @Category({Daily.class}) @@ -46,6 +49,10 @@ public class SignalDataRawTest extends BaseWebDriverTest implements PostgresOnly private static final String RESULT_FILENAME_1 = "LGC12392.TXT"; private static final String RESULT_FILENAME_2 = "LGC14332.TXT"; private static final String RESULT_FILENAME_3 = "MPP82113.TXT"; + private static final String RESULT_FILENAME_4 = "TD789-12.TXT"; + private static final String RESULT_FILENAME_5 = "TD789-25.TXT"; + private static final String RESULT_FILENAME_6 = "QD123-11.TXT"; + private static final String RESULT_FILENAME_7 = "QD123-24.TXT"; @Nullable @Override @@ -122,56 +129,103 @@ private String resultNameFromFilename(String filename) @Test public void testFileImport() { - final File METADATA_FILE = getFile("RunsMetadata/datafiles.tsv"); + File metadataFile = getFile("RunsMetadata/datafiles.tsv"); + Map> expectedData = Map.of("StringValue", List.of("StringOne", "StringTwo", "StringThree"), + "IntegerValue", List.of("1", "2", "3")); + SignalDataAssayBeginPage beginPage = importRun("importTest1", + metadataFile, + List.of(getFile(String.join("/", ASSAY_DATA_LOC, "BLANK235.TXT"))), + List.of( + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_1)), + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_2)), + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_3)) + ), + expectedData, 3); + /////////// Check clearing run /////////// + log("Check clearing a run during import"); + //Create new run + SignalDataUploadPage uploadPage = beginPage.navigateToImportPage(); + uploadPage.uploadMetadataFile(metadataFile); + uploadPage.setRunIDField("cleared run"); + assertElementPresent(Ext4Helper.Locators.getGridRow()); //Check grid has elements + uploadPage.clearRun(); + navigateToAssayLandingPage(); //Should not cause unload warning + + // test upload of metadata file with full data file paths + importRun("importTest2", + getFile("RunsMetadata/datafiles2.tsv"), + Collections.emptyList(), + List.of( + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_4)), + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_5)), + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_6)), + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_7)) + ), Collections.EMPTY_MAP, 4); + + // test import of files with a subset of the metadata files + importRun("importTest3", + getFile("RunsMetadata/datafiles.tsv"), + List.of(getFile(String.join("/", ASSAY_DATA_LOC, "BLANK235.TXT"))), + List.of( + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_1)), + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_3)) + ), + expectedData, 3); + + importRun("importTest4", + getFile("RunsMetadata/datafiles2.tsv"), + Collections.emptyList(), + List.of( + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_6)), + getFile(String.join("/", ASSAY_DATA_LOC, RESULT_FILENAME_7)) + ), Collections.EMPTY_MAP, 4); + } + + private SignalDataAssayBeginPage importRun( + String runName, + File metadataFile, + List unspecifiedDataFiles, + List dataFiles, + Map> expectedData, + int expectedResultRows + ) + { SignalDataAssayBeginPage beginPage = navigateToAssayLandingPage(); SignalDataUploadPage uploadPage = beginPage.navigateToImportPage(); log("Uploading metadata file"); - uploadPage.uploadMetadataFile(METADATA_FILE); + uploadPage.uploadMetadataFile(metadataFile); - log("Attempting to upload a data file not specified in metadata"); - uploadPage.uploadIncorrectFile(getFile(ASSAY_DATA_LOC + "/" + "BLANK235.TXT")); + for (File dataFile : unspecifiedDataFiles) + { + log("Attempting to upload a data file not specified in metadata"); + uploadPage.uploadIncorrectFile(dataFile); + } log("Uploading data files"); - int uploadCount = 3; - uploadPage.uploadFile( - getFile(ASSAY_DATA_LOC + "/" + RESULT_FILENAME_1), - getFile(ASSAY_DATA_LOC + "/" + RESULT_FILENAME_2), - getFile(ASSAY_DATA_LOC + "/" + RESULT_FILENAME_3)); - uploadPage.waitForProgressBars(3); - -// TODO: Not clear what delete should do- just make file unavailable? -// log("Delete a file"); -// uploadPage.deleteFile(RESULT_FILENAME_3); - - String runName = "importTest1"; + int uploadCount = dataFiles.size(); + uploadPage.uploadFile(dataFiles); + uploadPage.waitForProgressBars(uploadCount); + uploadPage.setRunIDField(runName); uploadPage.saveRun(); beginPage.waitForPageLoad(); log("Verifying run was added"); - beginPage.waitForGridValue(runName, uploadCount); + beginPage.waitForGridValue(runName, expectedResultRows); - // While we're here, a better test of the Run Identifier search filter now that we have 2 runs + // verify the uploaded files in the run beginPage.setSearchBox(runName); - assertEquals("Incorrect number of rows for imported run " + runName, uploadCount, beginPage.getRowCount()); - - // TODO: Not clear what delete should do- just make file unavailable? -// //Check if deleted file was added as result -// beginPage.setSearchBox(RESULT_FILENAME_3); //Filter results to deleted filename -// assertEquals("Deleted file found in search", 0, beginPage.getRowCount()); -// beginPage.clearSearchBox(); -// - /////////// Check clearing run /////////// - log("Check clearing a run during import"); - //Create new run - uploadPage = beginPage.navigateToImportPage(); - uploadPage.uploadMetadataFile(METADATA_FILE); - runName = "importTest2"; - uploadPage.setRunIDField(runName); - assertElementPresent(Ext4Helper.Locators.getGridRow()); //Check grid has elements - uploadPage.clearRun(); - navigateToAssayLandingPage(); //Should not cause unload warning + assertEquals("Incorrect number of rows for imported run " + runName, expectedResultRows, beginPage.getRowCount()); + + // verify any data row values + DataRegionTable table = beginPage.getDataRegionTable(); + for (Map.Entry> entry : expectedData.entrySet()) + { + List values = table.getColumnDataAsText(entry.getKey()); + assertArrayEquals(values.toArray(), entry.getValue().toArray()); + } + return beginPage; } private File getFile(String relativePath) diff --git a/signalData/test/src/org/labkey/test/util/signaldata/SignalDataInitializer.java b/signalData/test/src/org/labkey/test/util/signaldata/SignalDataInitializer.java index 701f3c408..9b54cc0e8 100644 --- a/signalData/test/src/org/labkey/test/util/signaldata/SignalDataInitializer.java +++ b/signalData/test/src/org/labkey/test/util/signaldata/SignalDataInitializer.java @@ -19,6 +19,7 @@ import org.labkey.test.Locator; import org.labkey.test.TestFileUtils; import org.labkey.test.pages.ReactAssayDesignerPage; +import org.labkey.test.params.FieldDefinition; import org.labkey.test.util.LogMethod; import org.labkey.test.util.core.webdav.WebDavUploadHelper; @@ -64,6 +65,9 @@ private void defineRawSignalDataAssay() assayDesigner.setDescription(RAW_SignalData_DESC); assayDesigner.setEditableRuns(true); assayDesigner.setEditableResults(true); + assayDesigner.goToResultsFields() + .addField(new FieldDefinition("StringValue", FieldDefinition.ColumnType.String)) + .addField(new FieldDefinition("IntegerValue", FieldDefinition.ColumnType.Integer)); assayDesigner.clickFinish(); new WebDavUploadHelper(_test.getPrimaryTestProject()).uploadDirectoryContents(RAW_SignalData_SAMPLE_DATA);