diff --git a/api/schemas/study.xsd b/api/schemas/study.xsd index 90a4ceef127..68e6ae2eee5 100644 --- a/api/schemas/study.xsd +++ b/api/schemas/study.xsd @@ -469,6 +469,13 @@ + + + + Indicates whether new timepoints can be created automatically when data is imported into the study. + + + diff --git a/api/src/org/labkey/api/study/Study.java b/api/src/org/labkey/api/study/Study.java index 634edbedf5b..1ab74ddec07 100644 --- a/api/src/org/labkey/api/study/Study.java +++ b/api/src/org/labkey/api/study/Study.java @@ -152,6 +152,7 @@ public interface Study extends StudyEntity boolean isAllowReqLocClinic(); boolean isAllowReqLocSal(); boolean isAllowReqLocEndpoint(); + boolean isFailForUndefinedTimepoints(); // "is" prefix doesn't work with "Boolean", use get /** diff --git a/pipeline/src/org/labkey/pipeline/startPipelineImport.jsp b/pipeline/src/org/labkey/pipeline/startPipelineImport.jsp index 01fcaeccc1b..0419bbb895e 100644 --- a/pipeline/src/org/labkey/pipeline/startPipelineImport.jsp +++ b/pipeline/src/org/labkey/pipeline/startPipelineImport.jsp @@ -93,7 +93,7 @@ Ext4.onReady(function() isApplyToMultipleFolders: <%=bean.isApplyToMultipleFolders()%>, isFailForUndefinedVisits: <%=bean.isFailForUndefinedVisits()%>, isCloudRoot: <%=bean.isCloudRoot()%>, // Remove as part of Issue #43835 - showFailForUndefinedVisits: <%=timepointType == null || timepointType == TimepointType.VISIT%> + showFailForUndefinedVisits: <%=(timepointType == null || timepointType == TimepointType.VISIT) && ((study == null) || !study.isFailForUndefinedTimepoints())%> }); }, failure: function(response) diff --git a/specimen/src/org/labkey/specimen/SpecimenModule.java b/specimen/src/org/labkey/specimen/SpecimenModule.java index 7b27157e957..b01650ee23d 100644 --- a/specimen/src/org/labkey/specimen/SpecimenModule.java +++ b/specimen/src/org/labkey/specimen/SpecimenModule.java @@ -38,6 +38,7 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.QueryParam; import org.labkey.api.query.QueryView; +import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.security.roles.RoleManager; import org.labkey.api.specimen.SpecimenMigrationService; @@ -189,7 +190,7 @@ public ActionURL getUpdateSpecimenQueryRowURL(Container c, String schemaName, Ta } @Override - public void importSpecimenArchive(@Nullable Path inputFile, PipelineJob job, SimpleStudyImportContext ctx, boolean merge, boolean syncParticipantVisit) throws PipelineJobException + public void importSpecimenArchive(@Nullable Path inputFile, PipelineJob job, SimpleStudyImportContext ctx, boolean merge, boolean syncParticipantVisit) throws PipelineJobException, ValidationException { AbstractSpecimenTask.doImport(inputFile, job, ctx, merge, syncParticipantVisit); } diff --git a/specimen/src/org/labkey/specimen/actions/ShowUploadSpecimensAction.java b/specimen/src/org/labkey/specimen/actions/ShowUploadSpecimensAction.java index 4671ac5e60c..ffe324433d1 100644 --- a/specimen/src/org/labkey/specimen/actions/ShowUploadSpecimensAction.java +++ b/specimen/src/org/labkey/specimen/actions/ShowUploadSpecimensAction.java @@ -25,6 +25,7 @@ import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.pipeline.PipelineUrls; import org.labkey.api.portal.ProjectUrls; +import org.labkey.api.query.ValidationException; import org.labkey.api.reader.ColumnDescriptor; import org.labkey.api.reader.TabLoader; import org.labkey.api.security.RequiresPermission; @@ -262,7 +263,7 @@ else if (study.getTimepointType() == TimepointType.VISIT && null == row.get(Simp if (!errors.hasErrors()) importer.process(specimenRows, form.isMerge()); } - catch (IllegalStateException e) + catch (IllegalStateException | ValidationException e) { errors.reject(SpringActionController.ERROR_MSG, e.getMessage()); } diff --git a/specimen/src/org/labkey/specimen/importer/AbstractSpecimenTask.java b/specimen/src/org/labkey/specimen/importer/AbstractSpecimenTask.java index 4c1b780c6e6..d48db7d168a 100644 --- a/specimen/src/org/labkey/specimen/importer/AbstractSpecimenTask.java +++ b/specimen/src/org/labkey/specimen/importer/AbstractSpecimenTask.java @@ -25,6 +25,7 @@ import org.labkey.api.pipeline.PipelineJobException; import org.labkey.api.pipeline.RecordedActionSet; import org.labkey.api.pipeline.TaskFactory; +import org.labkey.api.query.ValidationException; import org.labkey.api.specimen.importer.SpecimenImporter; import org.labkey.api.study.SpecimenService; import org.labkey.api.study.SpecimenTransform; @@ -98,7 +99,7 @@ protected SimpleStudyImportContext getImportContext(PipelineJob job) } public static void doImport(@Nullable Path inputFile, PipelineJob job, SimpleStudyImportContext ctx, boolean merge, - boolean syncParticipantVisit) throws PipelineJobException + boolean syncParticipantVisit) throws PipelineJobException, ValidationException { doImport(inputFile, job, ctx, merge, syncParticipantVisit, new DefaultImportHelper()); } @@ -115,7 +116,7 @@ private static void doPostTransform(SpecimenTransform transformer, Path inputFil } public static void doImport(@Nullable Path inputFile, PipelineJob job, SimpleStudyImportContext ctx, boolean merge, - boolean syncParticipantVisit, ImportHelper importHelper) throws PipelineJobException + boolean syncParticipantVisit, ImportHelper importHelper) throws PipelineJobException, ValidationException { // do nothing if we've specified data types and specimen is not one of them if (!ctx.isDataTypeSelected(SpecimenArchiveDataTypes.SPECIMENS)) @@ -172,7 +173,11 @@ public static void doImport(@Nullable Path inputFile, PipelineJob job, SimpleStu // Since changing specimens in this study will impact specimens in ancillary studies dependent on this study, // we need to force a participant/visit refresh in those study containers (if any): for (Study dependentStudy : StudyService.get().getAncillaryStudies(ctx.getContainer())) - VisitService.get().updateParticipantVisits(dependentStudy, ctx.getUser()); + { + ValidationException errors = VisitService.get().updateParticipantVisits(dependentStudy, ctx.getUser()); + if (errors.hasErrors()) + throw errors; + } } } diff --git a/study/api-src/org/labkey/api/specimen/SpecimenMigrationService.java b/study/api-src/org/labkey/api/specimen/SpecimenMigrationService.java index 2708bbf6b91..3a513b3b873 100644 --- a/study/api-src/org/labkey/api/specimen/SpecimenMigrationService.java +++ b/study/api-src/org/labkey/api/specimen/SpecimenMigrationService.java @@ -5,6 +5,7 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.pipeline.PipelineJob; import org.labkey.api.pipeline.PipelineJobException; +import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.services.ServiceRegistry; import org.labkey.api.study.importer.SimpleStudyImportContext; @@ -36,7 +37,7 @@ static void setInstance(SpecimenMigrationService impl) ActionURL getUpdateSpecimenQueryRowURL(Container c, String schemaName, TableInfo table); void importSpecimenArchive(@Nullable Path inputFile, PipelineJob job, SimpleStudyImportContext ctx, boolean merge, - boolean syncParticipantVisit) throws PipelineJobException; + boolean syncParticipantVisit) throws PipelineJobException, ValidationException; void clearRequestCaches(Container c); void clearGroupedValuesForColumn(Container container); diff --git a/study/api-src/org/labkey/api/specimen/importer/SpecimenImporter.java b/study/api-src/org/labkey/api/specimen/importer/SpecimenImporter.java index bfc9dab06d3..c1daf91996a 100644 --- a/study/api-src/org/labkey/api/specimen/importer/SpecimenImporter.java +++ b/study/api-src/org/labkey/api/specimen/importer/SpecimenImporter.java @@ -256,7 +256,7 @@ public SpecimenImporter(Container container, User user) SpecimenSchema.get().getTableInfoSpecimenPrimaryType(getContainer()).getSelectName(), SpecimenColumns.PRIMARYTYPE_COLUMNS); } - private void resyncStudy(boolean syncParticipantVisit) + private void resyncStudy(boolean syncParticipantVisit, boolean failForUndefinedVisits) throws ValidationException { TableInfo tableParticipant = SpecimenSchema.get().getTableInfoParticipant(); TableInfo tableSpecimen = getTableInfoSpecimen(); @@ -271,7 +271,9 @@ private void resyncStudy(boolean syncParticipantVisit) { Study study = StudyService.get().getStudy(getContainer()); info("Updating study-wide subject/visit information..."); - VisitService.get().updateParticipantVisitsWithCohortUpdate(study, getUser(), _logger); + ValidationException errors = VisitService.get().updateParticipantVisitsWithCohortUpdate(study, getUser(), failForUndefinedVisits, _logger); + if (errors.hasErrors()) + throw errors; info("Subject/visit update complete."); } @@ -302,14 +304,16 @@ public void process(VirtualFile specimensDir, boolean merge, SimpleStudyImportCo throws IOException, ValidationException { Map sifMap = populateFileMap(specimensDir, new HashMap<>()); - process(sifMap, merge, ctx.getLogger(), job, syncParticipantVisit, false, ctx.isFailForUndefinedVisits()); + Study study = StudyService.get().getStudy(getContainer()); + process(sifMap, merge, ctx.getLogger(), job, syncParticipantVisit, false, ctx.isFailForUndefinedVisits() || study.isFailForUndefinedTimepoints()); } protected void process(Map sifMap, boolean merge, Logger logger, @Nullable PipelineJob job, boolean syncParticipantVisit, boolean editingSpecimens) throws IOException, ValidationException { - process(sifMap, merge, logger, job, syncParticipantVisit, editingSpecimens, false); + Study study = StudyService.get().getStudy(getContainer()); + process(sifMap, merge, logger, job, syncParticipantVisit, editingSpecimens, study.isFailForUndefinedTimepoints()); } private void process(Map sifMap, boolean merge, Logger logger, @Nullable PipelineJob job, @@ -367,10 +371,6 @@ private void process(Map sifMap, boolean SpecimenImportFile specimenFile = sifMap.get(_specimensTableType); SpecimenLoadInfo loadInfo = populateTempSpecimensTable(specimenFile, merge); - Study study = StudyService.get().getStudy(getContainer()); - if (loadInfo.getRowCount() > 0 && failForUndefinedVisits && study.getTimepointType() == TimepointType.VISIT) - checkForUndefinedVisits(loadInfo, study); - // NOTE: if no rows were loaded in the temp table, don't remove existing materials/specimens/vials/events. if (loadInfo.getRowCount() > 0) populateSpecimenTables(loadInfo, merge); @@ -390,7 +390,7 @@ private void process(Map sifMap, boolean setStatus(GENERAL_JOB_STATUS_MSG + " (update study)"); _iTimer.setPhase(ImportPhases.ResyncStudy); - resyncStudy(syncParticipantVisit); + resyncStudy(syncParticipantVisit, failForUndefinedVisits); ensureNotCanceled(); _iTimer.setPhase(ImportPhases.SetLastSpecimenLoad); @@ -452,29 +452,6 @@ private SpecimenLoadInfo populateTempSpecimensTable(SpecimenImportFile file, boo return new SpecimenLoadInfo(getUser(), getContainer(), DbSchema.getTemp(), columns, rowCount, tempTablesHolder.getTempTableInfo()); } - private void checkForUndefinedVisits(SpecimenLoadInfo info, Study study) throws ValidationException - { - SQLFragment sql = new SQLFragment() - .append("SELECT DISTINCT VisitValue FROM ") - .append(info.getTempTableName()) - .append(" tt ") - .append("\nLEFT JOIN study.Visit v") - .append("\nON tt.VisitValue >= v.SequenceNumMin AND tt.VisitValue <=v.SequenceNumMax AND v.Container = ?") - .append("\nWHERE tt.VisitValue IS NOT NULL AND v.RowId IS NULL"); - - // shared visit container - Study visitStudy = StudyService.get().getStudyForVisits(study); - sql.add(visitStudy.getContainer().getId()); - - SqlSelector selector = new SqlSelector(SpecimenSchema.get().getSchema(), sql); - List undefinedVisits = selector.getArrayList(Double.class); - if (!undefinedVisits.isEmpty()) - { - Collections.sort(undefinedVisits); - throw new ValidationException("The following undefined visits exist in the specimen data: " + StringUtils.join(undefinedVisits, ", ")); - } - } - private void populateSpecimenTables(SpecimenLoadInfo info, boolean merge) throws ValidationException { setStatus(GENERAL_JOB_STATUS_MSG + " (populate tables)"); diff --git a/study/api-src/org/labkey/api/study/model/VisitService.java b/study/api-src/org/labkey/api/study/model/VisitService.java index 35b42f8fd8c..41ad344a9a4 100644 --- a/study/api-src/org/labkey/api/study/model/VisitService.java +++ b/study/api-src/org/labkey/api/study/model/VisitService.java @@ -2,6 +2,7 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; +import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.services.ServiceRegistry; import org.labkey.api.study.Study; @@ -27,10 +28,10 @@ static void setInstance(VisitService impl) List getVisits(Study study, Visit.Order order); - void updateParticipantVisitsWithCohortUpdate(Study study, User user, @Nullable Logger logger); + ValidationException updateParticipantVisitsWithCohortUpdate(Study study, User user, boolean failForUndefinedVisits, @Nullable Logger logger); /** * Updates this study's participant, visit, and participant visit tables. Also updates automatic cohort assignments. */ - void updateParticipantVisits(Study study, User user); + ValidationException updateParticipantVisits(Study study, User user); } diff --git a/study/resources/schemas/dbscripts/postgresql/study-23.001-23.002.sql b/study/resources/schemas/dbscripts/postgresql/study-23.001-23.002.sql new file mode 100644 index 00000000000..cd25601e129 --- /dev/null +++ b/study/resources/schemas/dbscripts/postgresql/study-23.001-23.002.sql @@ -0,0 +1 @@ +ALTER TABLE study.study ADD COLUMN FailForUndefinedTimepoints BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/study/resources/schemas/dbscripts/sqlserver/study-23.001-23.002.sql b/study/resources/schemas/dbscripts/sqlserver/study-23.001-23.002.sql new file mode 100644 index 00000000000..61d79a82605 --- /dev/null +++ b/study/resources/schemas/dbscripts/sqlserver/study-23.001-23.002.sql @@ -0,0 +1 @@ +ALTER TABLE study.study ADD FailForUndefinedTimepoints BIT NOT NULL DEFAULT 0; diff --git a/study/resources/schemas/study.xml b/study/resources/schemas/study.xml index e5e205ec2d5..9dd4ac38ff2 100644 --- a/study/resources/schemas/study.xml +++ b/study/resources/schemas/study.xml @@ -159,6 +159,9 @@ Flag to perform query validation after import. + + Flag to prevent the creation of new timepoints during data import. + diff --git a/study/src/org/labkey/study/StudyModule.java b/study/src/org/labkey/study/StudyModule.java index 515125acb07..540ab65daa1 100644 --- a/study/src/org/labkey/study/StudyModule.java +++ b/study/src/org/labkey/study/StudyModule.java @@ -209,7 +209,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 23.001; + return 23.002; } @Override diff --git a/study/src/org/labkey/study/VisitServiceImpl.java b/study/src/org/labkey/study/VisitServiceImpl.java index e665c24bc6b..8cbe1dc7882 100644 --- a/study/src/org/labkey/study/VisitServiceImpl.java +++ b/study/src/org/labkey/study/VisitServiceImpl.java @@ -1,7 +1,9 @@ package org.labkey.study; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.study.Study; import org.labkey.api.study.Visit; @@ -20,14 +22,14 @@ public List getVisits(Study study, Visit.Order order) } @Override - public void updateParticipantVisitsWithCohortUpdate(Study study, User user, @Nullable Logger logger) + public @NotNull ValidationException updateParticipantVisitsWithCohortUpdate(Study study, User user, boolean failForUndefinedVisits, @Nullable Logger logger) { - StudyManager.getInstance().getVisitManager(study).updateParticipantVisitsWithCohortUpdate(user, logger); + return StudyManager.getInstance().getVisitManager(study).updateParticipantVisitsWithCohortUpdate(user, failForUndefinedVisits, logger); } @Override - public void updateParticipantVisits(Study study, User user) + public @NotNull ValidationException updateParticipantVisits(Study study, User user) { - StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(user, Collections.emptySet()); + return StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(user, Collections.emptySet()); } } diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 26447d63559..17ce7bf8c8a 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -456,6 +456,14 @@ else if (!datasetPublishSourceId.equals(publishSource.second) || dataset.getPubl logPublishEvent(publishSource.first, source, dataMaps, user, sourceContainer, targetContainer, dataset); } + //Make sure that the study is updated with the correct timepoints. + ValidationException validationErrors = StudyManager.getInstance().getVisitManager(targetStudy).updateParticipantVisits(user, Collections.singleton(dataset)); + if (validationErrors.hasErrors()) + { + errors.add(validationErrors.getMessage()); + return null; + } + transaction.commit(); } catch (ChangePropertyDescriptorException | IOException e) @@ -463,9 +471,6 @@ else if (!datasetPublishSourceId.equals(publishSource.second) || dataset.getPubl throw UnexpectedException.wrap(e); } - //Make sure that the study is updated with the correct timepoints. - StudyManager.getInstance().getVisitManager(targetStudy).updateParticipantVisits(user, Collections.singleton(dataset)); - return PageFlowUtil.urlProvider(StudyUrls.class).getDatasetURL(targetContainer, dataset.getRowId()); } @@ -978,12 +983,17 @@ public Pair, UploadLog> importDatasetTSV(User user, StudyImpl study defaultQCState = DataStateManager.getInstance().getStateForRowId(study.getContainer(), defaultQCStateId.intValue()); lsids = StudyManager.getInstance().importDatasetData(user, dsd, dl, columnMap, errors, DatasetDefinition.CheckForDuplicates.sourceOnly, defaultQCState, insertOption, null, importLookupByAlternateKey, auditBehaviorType); + + if (!errors.hasErrors()) + { + ValidationException validationException = StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(user, Collections.singleton(dsd)); + if (validationException.hasErrors()) + errors.addRowError(validationException); + } + if (!errors.hasErrors()) transaction.commit(); } - - if (!errors.hasErrors()) - StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(user, Collections.singleton(dsd)); } catch (IOException x) { @@ -1272,6 +1282,9 @@ public void autoLinkSamples(ExpSampleType sampleType, List ExpMaterialTable.Column.RowId.toString(), publishErrors ); + + if (!publishErrors.isEmpty()) + publishErrors.forEach(LOG::error); } else { diff --git a/study/src/org/labkey/study/controllers/StudyController.java b/study/src/org/labkey/study/controllers/StudyController.java index 3d188f6ba86..3115bd82f63 100644 --- a/study/src/org/labkey/study/controllers/StudyController.java +++ b/study/src/org/labkey/study/controllers/StudyController.java @@ -1656,10 +1656,14 @@ public class ManageVisitsAction extends FormViewAction @Override public void validateCommand(StudyPropertiesForm target, Errors errors) { - if (target.getTimepointType() == TimepointType.DATE && null == target.getStartDate()) - errors.reject(ERROR_MSG, "Start date must be supplied for a date-based study."); - if (target.getDefaultTimepointDuration() < 1) - errors.reject(ERROR_MSG, "Default timepoint duration must be a positive number."); + StudyImpl study = getStudy(); + if (study.getTimepointType() == TimepointType.DATE) + { + if (target.getTimepointType() == TimepointType.DATE && null == target.getStartDate()) + errors.reject(ERROR_MSG, "Start date must be supplied for a date-based study."); + if (target.getDefaultTimepointDuration() < 1) + errors.reject(ERROR_MSG, "Default timepoint duration must be a positive number."); + } } @Override @@ -1686,8 +1690,13 @@ public boolean handlePost(StudyPropertiesForm form, BindException errors) StudyImpl study = getStudyThrowIfNull().createMutable(); redirectToSharedVisitStudy(study, getViewContext().getActionURL()); - study.setStartDate(form.getStartDate()); - study.setDefaultTimepointDuration(form.getDefaultTimepointDuration()); + if (study.getTimepointType() == TimepointType.DATE) + { + study.setStartDate(form.getStartDate()); + study.setDefaultTimepointDuration(form.getDefaultTimepointDuration()); + } + study.setFailForUndefinedTimepoints(form.isFailForUndefinedTimepoints()); + StudyManager.getInstance().updateStudy(getUser(), study); return true; @@ -2110,7 +2119,7 @@ public ModelAndView getConfirmView(IdForm idForm, BindException errors) if (visits.isEmpty()) { - sb.append("No unused visits found.
"); + sb.unsafeAppend("No unused visits found.
"); } else { @@ -5422,6 +5431,7 @@ public static class StudyPropertiesForm extends ReturnUrlForm private boolean _allowReqLocEndpoint = true; private boolean _shareDatasets = false; private boolean _shareVisits = false; + private boolean _failForUndefinedTimepoints; public String getLabel() { @@ -5662,6 +5672,16 @@ public void setShareVisits(boolean shareDatasets) { _shareVisits = shareDatasets; } + + public boolean isFailForUndefinedTimepoints() + { + return _failForUndefinedTimepoints; + } + + public void setFailForUndefinedTimepoints(boolean failForUndefinedTimepoints) + { + _failForUndefinedTimepoints = failForUndefinedTimepoints; + } } public static class IdForm diff --git a/study/src/org/labkey/study/controllers/publish/PublishController.java b/study/src/org/labkey/study/controllers/publish/PublishController.java index c46997b9a0c..e64baf31f4c 100644 --- a/study/src/org/labkey/study/controllers/publish/PublishController.java +++ b/study/src/org/labkey/study/controllers/publish/PublishController.java @@ -334,7 +334,6 @@ public String getDescription() public void run() { setStatus(TaskStatus.running); - TaskStatus finalStatus = TaskStatus.complete; try { if (_targetStudyContainer != null) @@ -374,10 +373,8 @@ public void run() catch (Throwable t) { error("Failure", t); - finalStatus = TaskStatus.error; } info("Auto link to study complete"); - setStatus(finalStatus); } } } diff --git a/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java b/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java index a1ea09f735c..cb004da04b2 100644 --- a/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java +++ b/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java @@ -272,8 +272,13 @@ public void createSnapshot(ViewContext context, QuerySnapshotDefinition qsDef, B // snapshot are updated if (!queryDef.getContainer().equals(qsDef.getContainer())) { - StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(context.getUser(), + ValidationException validationException = StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(context.getUser(), Collections.singletonList(def)); + if (validationException.hasErrors()) + { + errors.reject(SpringActionController.ERROR_MSG, validationException.getMessage()); + return; + } } transaction.commit(); } @@ -504,7 +509,14 @@ public synchronized ActionURL updateSnapshot(QuerySnapshotForm form, BindExcepti return null; if (!suppressVisitManagerRecalc) - StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(form.getViewContext().getUser(), Collections.singleton(dsDef)); + { + ValidationException validationException = StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(form.getViewContext().getUser(), Collections.singleton(dsDef)); + if (validationException.hasErrors()) + { + errors.reject(SpringActionController.ERROR_MSG, validationException.getMessage()); + return null; + } + } ViewContext context = form.getViewContext(); new DatasetDefinition.DatasetAuditHandler(dsDef).addAuditEvent(context.getUser(), context.getContainer(), AuditBehaviorType.DETAILED, @@ -816,6 +828,19 @@ public void run() BindException errors = new NullSafeBindException(new Object(), "command"); QuerySnapshotService.get(StudySchema.getInstance().getSchemaName()).updateSnapshot(form, errors, _suppressVisitManagerRecalc); + if (errors.hasErrors()) + { + // log an error as well as an audit event to match exception handling within updateSnapshot + LOG.error(errors.getMessage()); + StudyImpl study = StudyManager.getInstance().getStudy(form.getViewContext().getContainer()); + if (study != null) + { + DatasetDefinition dsDef = StudyManager.getInstance().getDatasetDefinitionByName(study, _def.getName()); + if (dsDef != null) + new DatasetDefinition.DatasetAuditHandler(dsDef).addAuditEvent(context.getUser(), context.getContainer(), AuditBehaviorType.DETAILED, + "Dataset snapshot was not updated. Cause of failure: " + errors.getMessage(), null); + } + } } catch(Exception e) { diff --git a/study/src/org/labkey/study/importer/CreateChildStudyPipelineJob.java b/study/src/org/labkey/study/importer/CreateChildStudyPipelineJob.java index 7197897b715..7210ba5db5a 100644 --- a/study/src/org/labkey/study/importer/CreateChildStudyPipelineJob.java +++ b/study/src/org/labkey/study/importer/CreateChildStudyPipelineJob.java @@ -35,6 +35,7 @@ import org.labkey.api.query.CustomView; import org.labkey.api.query.QueryDefinition; import org.labkey.api.query.QueryService; +import org.labkey.api.query.ValidationException; import org.labkey.api.query.snapshot.QuerySnapshotDefinition; import org.labkey.api.query.snapshot.QuerySnapshotService; import org.labkey.api.security.User; @@ -274,7 +275,6 @@ public boolean run(ViewContext context) // Save the snapshot RowId to the destination study StudyImpl mutableStudy = StudyManager.getInstance().getStudy(getDstContainer()).createMutable(); mutableStudy.setStudySnapshot(snapshot.getRowId()); - StudyManager.getInstance().updateStudy(user, mutableStudy); // export objects from the parent study, then import them into the new study getLogger().info("Exporting data from parent study."); @@ -309,6 +309,14 @@ public boolean run(ViewContext context) importTreatmentVisitMapData(errors, studyDir, studyImportContext); new TopLevelStudyPropertiesImporter().process(studyImportContext, studyDir, errors); + + // after the data has been imported, configure the new study setting for undefined timepoints + if (sourceStudy.isFailForUndefinedTimepoints()) + mutableStudy.setFailForUndefinedTimepoints(true); + + ValidationException validationException = StudyManager.getInstance().updateStudy(user, mutableStudy); + if (validationException.hasErrors()) + errors.reject(null, validationException.getMessage()); } if (errors.hasErrors()) diff --git a/study/src/org/labkey/study/importer/StudyImportFinalTask.java b/study/src/org/labkey/study/importer/StudyImportFinalTask.java index cee6df66191..8ca544bde14 100644 --- a/study/src/org/labkey/study/importer/StudyImportFinalTask.java +++ b/study/src/org/labkey/study/importer/StudyImportFinalTask.java @@ -21,7 +21,10 @@ import org.labkey.api.study.importer.SimpleStudyImporter; import org.labkey.api.study.importer.SimpleStudyImporter.Timing; import org.labkey.api.writer.VirtualFile; +import org.labkey.study.model.StudyImpl; +import org.labkey.study.model.StudyManager; import org.labkey.study.writer.StudySerializationRegistry; +import org.labkey.study.xml.StudyDocument; import org.springframework.validation.BindException; import java.util.Collection; @@ -85,6 +88,17 @@ public static void doImport(PipelineJob job, StudyImportContext ctx, BindExcepti if (importer.getTiming() == Timing.Late) importer.process(ctx, vf, errors); } + + StudyDocument.Study studyXml = ctx.getXml(); + StudyImpl study = StudyManager.getInstance().getStudy(ctx.getContainer()); + + // after the data has been imported, configure the new study setting for undefined timepoints + if (studyXml.isSetFailForUndefinedTimepoints() && !study.isFailForUndefinedTimepoints()) + { + StudyImpl mutableStudy = study.createMutable(); + mutableStudy.setFailForUndefinedTimepoints(true); + StudyManager.getInstance().updateStudy(ctx.getUser(), mutableStudy); + } } catch (Exception e) { diff --git a/study/src/org/labkey/study/importer/StudyImporterFactory.java b/study/src/org/labkey/study/importer/StudyImporterFactory.java index e29c2d95215..8519052b6d5 100644 --- a/study/src/org/labkey/study/importer/StudyImporterFactory.java +++ b/study/src/org/labkey/study/importer/StudyImporterFactory.java @@ -27,6 +27,7 @@ import org.labkey.api.cloud.CloudArchiveImporterSupport; import org.labkey.api.data.Container; import org.labkey.api.pipeline.PipelineJob; +import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.specimen.SpecimenMigrationService; import org.labkey.api.study.SpecimenService; @@ -161,7 +162,12 @@ public void process(@Nullable PipelineJob job, FolderImportContext ctx, VirtualF } ctx.getLogger().info("Updating study-wide subject/visit information..."); - StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(user, datasets, null, null, true, ctx.getLogger()); + ValidationException validationException = StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(user, datasets, null, null, true, + ctx.isFailForUndefinedVisits() || study.isFailForUndefinedTimepoints(), + ctx.getLogger()); + if (validationException.hasErrors()) + validationException.getErrors().forEach(ve -> ctx.getLogger().error(ve.getMessage())); + ctx.getLogger().info("Subject/visit update complete."); // the final study import task handles registered study importers like: cohorts, participant comments, categories, etc. diff --git a/study/src/org/labkey/study/importer/TopLevelStudyPropertiesImporter.java b/study/src/org/labkey/study/importer/TopLevelStudyPropertiesImporter.java index 9c5ebe6f11a..d830e806608 100644 --- a/study/src/org/labkey/study/importer/TopLevelStudyPropertiesImporter.java +++ b/study/src/org/labkey/study/importer/TopLevelStudyPropertiesImporter.java @@ -16,6 +16,7 @@ package org.labkey.study.importer; import org.apache.commons.lang3.StringUtils; +import org.labkey.api.query.ValidationException; import org.labkey.api.writer.VirtualFile; import org.labkey.study.model.SecurityType; import org.labkey.study.model.StudyImpl; @@ -117,9 +118,16 @@ else if (studyXml.isSetStudyDescription() && studyXml.getStudyDescription().isSe StudyManager.getInstance().updateStudy(ctx.getUser(), study); - // Issue 39822: update participant visits after importing study details like start date - StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(ctx.getUser(), study.getDatasets()); - + if (!errors.hasErrors()) + { + // Issue 39822: update participant visits after importing study details like start date + ValidationException validationException = StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(ctx.getUser(), study.getDatasets(), + null, null, true, + ctx.isFailForUndefinedVisits() || study.isFailForUndefinedTimepoints(), + null); + if (validationException.hasErrors()) + validationException.getErrors().forEach(ve -> errors.reject(null, ve.getMessage())); + } ctx.getLogger().info("Done importing " + getDescription()); } } diff --git a/study/src/org/labkey/study/model/StudyImpl.java b/study/src/org/labkey/study/model/StudyImpl.java index 131ee458d87..a0ed7efde15 100644 --- a/study/src/org/labkey/study/model/StudyImpl.java +++ b/study/src/org/labkey/study/model/StudyImpl.java @@ -148,6 +148,7 @@ public class StudyImpl extends ExtensibleStudyEntity implements Study private boolean _allowReqLocClinic = true; private boolean _allowReqLocSal = true; private boolean _allowReqLocEndpoint = true; + private boolean _failForUndefinedTimepoints; private Integer _participantAliasDatasetId; private String _participantAliasSourceProperty; @@ -927,6 +928,18 @@ public void setAllowReqLocEndpoint(boolean allowReqLocEndpoint) _allowReqLocEndpoint = allowReqLocEndpoint; } + @Override + public boolean isFailForUndefinedTimepoints() + { + return _failForUndefinedTimepoints; + } + + public void setFailForUndefinedTimepoints(boolean failForUndefinedTimepoints) + { + verifyMutability(); + _failForUndefinedTimepoints = failForUndefinedTimepoints; + } + public Integer getLastSpecimenRequest() { return _lastSpecimenRequest; @@ -1230,7 +1243,7 @@ public Visit getVisit(String participantID, Double visitID, Date date, boolean r VisitImpl result = StudyManager.getInstance().getVisitForSequence(this, sequenceNumBD); if (result == null && returnPotentialTimepoints) { - result = StudyManager.getInstance().ensureVisit(this, null, sequenceNumBD, null, false); + result = StudyManager.getInstance().getVisit(this, null, sequenceNumBD, null); } return result; } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index fdc4d323c54..d78c0893992 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -41,11 +41,34 @@ import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.compliance.ComplianceService; -import org.labkey.api.data.*; +import org.labkey.api.data.Activity; +import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.CompareType; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.CoreSchema; +import org.labkey.api.data.DbSchema; +import org.labkey.api.data.DbScope; import org.labkey.api.data.DbScope.CommitTaskOption; import org.labkey.api.data.DbScope.Transaction; +import org.labkey.api.data.Filter; +import org.labkey.api.data.JdbcType; +import org.labkey.api.data.PHI; +import org.labkey.api.data.PropertyManager; +import org.labkey.api.data.RuntimeSQLException; +import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SimpleFilter.OrClause; import org.labkey.api.data.SimpleFilter.SQLClause; +import org.labkey.api.data.Sort; +import org.labkey.api.data.SqlExecutor; +import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.Table; +import org.labkey.api.data.TableInfo; +import org.labkey.api.data.TableSelector; +import org.labkey.api.data.UpdateableTableInfo; +import org.labkey.api.data.UpgradeCode; import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.dataiterator.BeanDataIterator; import org.labkey.api.dataiterator.DataIteratorBuilder; @@ -88,6 +111,7 @@ import org.labkey.api.query.QueryService; import org.labkey.api.query.QueryUpdateService; import org.labkey.api.query.SchemaKey; +import org.labkey.api.query.SimpleValidationError; import org.labkey.api.query.UserSchema; import org.labkey.api.query.ValidationError; import org.labkey.api.query.ValidationException; @@ -623,17 +647,18 @@ public StudyImpl createStudy(User user, StudyImpl study) return study; } - public void updateStudy(@Nullable User user, StudyImpl study) + public ValidationException updateStudy(@Nullable User user, StudyImpl study) { StudyImpl oldStudy = getStudy(study.getContainer()); Date oldStartDate = oldStudy.getStartDate(); _studyHelper.update(user, study, study.getContainer()); + ValidationException errors = new ValidationException(); if (oldStudy.getTimepointType() == TimepointType.DATE && !Objects.equals(study.getStartDate(), oldStartDate)) { // start date has changed, and datasets may use that value. Uncache. RelativeDateVisitManager visitManager = (RelativeDateVisitManager) getVisitManager(study); - visitManager.recomputeDates(oldStartDate, user); + errors = visitManager.recomputeDates(oldStartDate, user); clearCaches(study.getContainer(), true); } else @@ -648,6 +673,7 @@ public void updateStudy(@Nullable User user, StudyImpl study) StudyService.get().addStudyAuditEvent(study.getContainer(), user, comment); } QueryService.get().updateLastModified(); + return errors; } public void createDatasetDefinition(User user, Container container, int datasetId) @@ -1094,34 +1120,52 @@ public VisitImpl createVisit(Study study, User user, VisitImpl visit, @Nullable return visit; } - public VisitImpl ensureVisit(Study study, User user, BigDecimal sequenceNum, Visit.Type type, boolean saveIfNew) + /** + * Return a visit object regardless of whether it exists in the database but will not insert a new + * record into the database. + */ + public VisitImpl getVisit(Study study, User user, BigDecimal sequenceNum, Visit.Type type) { List visits = getVisits(study, Visit.Order.SEQUENCE_NUM); - VisitImpl result = ensureVisitWithoutSaving(study, sequenceNum, type, visits); - if (saveIfNew && result.getRowId() == 0) - { - // Insert it into the database if it's new - return createVisit(study, user, result, visits); - } - return result; + return ensureVisitWithoutSaving(study, sequenceNum, type, visits); } - public boolean ensureVisits(Study study, User user, Set sequencenums, @Nullable Visit.Type type) + /** + * Ensures the existence of a visit for the specified sequence numbers and will insert into the database + * if the visit does not yet exist. + * + * @param failForUndefinedVisits If true, new visits will not be created and an error will be added to the returned + * ValidationException object. + * @return ValidationException which will contain any relevant errors. Callers should check hasErrors on the object. + */ + public @NotNull ValidationException ensureVisits(Study study, User user, Set sequencenums, @Nullable Visit.Type type, + boolean failForUndefinedVisits) { List visits = getVisits(study, Visit.Order.SEQUENCE_NUM); - boolean created = false; + ValidationException errors = new ValidationException(); + List seqNumFailures = new ArrayList<>(); + for (BigDecimal sequencenum : sequencenums) { VisitImpl result = ensureVisitWithoutSaving(study, sequencenum, type, visits); - if (result.getRowId() == 0) + if (result.getRowId() == 0 && !failForUndefinedVisits) { createVisit(study, user, result, visits); // Refresh existing visits to avoid constraint violation, see #44425 visits = getVisits(study, Visit.Order.SEQUENCE_NUM); - created = true; } + else + seqNumFailures.add(String.valueOf(sequencenum)); + } + + if (!seqNumFailures.isEmpty()) + { + String timepointNoun = study.getTimepointType().isVisitBased() ? "visit" : "timepoint"; + errors.addError(new SimpleValidationError(String.format("Creating new %ss is not allowed for this study. The following %s not currently exist : (%s)", + timepointNoun, timepointNoun + (seqNumFailures.size() > 1 ? "s do" : " does"), + String.join(", ", seqNumFailures)))); } - return created; + return errors; } private VisitImpl ensureVisitWithoutSaving(Study study, double seqNumDouble, @Nullable Visit.Type type, List existingVisits) @@ -1923,25 +1967,6 @@ public List getVisitsForDataset(Container container, int datasetId) return visits; } - public List getUndefinedSequenceNumsForDataset(Container container, int datasetId) - { - DatasetDefinition def = getDatasetDefinition(getStudy(container), datasetId); - TableInfo ds = def.getDatasetSchemaTableInfo(null, false); - Study visitStudy = getStudyForVisits(def.getStudy()); - - SQLFragment sql = new SQLFragment(); - sql.append("SELECT DISTINCT sd.SequenceNum FROM ").append(ds.getFromSQL("sd")).append("\n" + - "LEFT JOIN study.Visit v ON\n" + - "\tsd.SequenceNum >= v.SequenceNumMin AND sd.SequenceNum <=v.SequenceNumMax AND v.Container = ?\n" + - "WHERE v.RowId IS NULL" - ); - // shared visit container - sql.add(visitStudy.getContainer().getId()); - - SqlSelector selector = new SqlSelector(StudySchema.getInstance().getSchema(), sql); - return selector.getArrayList(Double.class); - } - public void updateDataQCState(Container container, User user, int datasetId, Collection lsids, DataState newState, String comments) { DbScope scope = StudySchema.getInstance().getSchema().getScope(); diff --git a/study/src/org/labkey/study/pipeline/AbstractDatasetImportTask.java b/study/src/org/labkey/study/pipeline/AbstractDatasetImportTask.java index a01f565f70a..13941c1ff3a 100644 --- a/study/src/org/labkey/study/pipeline/AbstractDatasetImportTask.java +++ b/study/src/org/labkey/study/pipeline/AbstractDatasetImportTask.java @@ -24,6 +24,7 @@ import org.labkey.api.pipeline.PipelineJobException; import org.labkey.api.pipeline.RecordedActionSet; import org.labkey.api.pipeline.TaskFactory; +import org.labkey.api.query.ValidationException; import org.labkey.api.query.snapshot.QuerySnapshotService; import org.labkey.api.util.Path; import org.labkey.api.util.SafeToRenderEnum; @@ -178,7 +179,12 @@ public static List doImport(VirtualFile datasetsDirectory, St if (job != null) job.setStatus("UPDATE participants"); ctx.getLogger().info("Updating participant visits"); - StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(ctx.getUser(), datasets, ctx.getLogger()); + ValidationException validationException = StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(ctx.getUser(), datasets, null, null, true, + ctx.isFailForUndefinedVisits() || study.isFailForUndefinedTimepoints(), + ctx.getLogger()); + if (validationException.hasErrors()) + ctx.getLogger().error(validationException); + ctx.getLogger().info("Finished updating participants"); } diff --git a/study/src/org/labkey/study/pipeline/DatasetImportRunnable.java b/study/src/org/labkey/study/pipeline/DatasetImportRunnable.java index ebb01ab1118..5067cabb45c 100644 --- a/study/src/org/labkey/study/pipeline/DatasetImportRunnable.java +++ b/study/src/org/labkey/study/pipeline/DatasetImportRunnable.java @@ -17,7 +17,6 @@ package org.labkey.study.pipeline; import org.apache.commons.io.IOUtils; -import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnInfo; @@ -40,9 +39,7 @@ import org.labkey.api.reader.TabLoader; import org.labkey.api.security.User; import org.labkey.api.study.Dataset; -import org.labkey.api.study.TimepointType; import org.labkey.api.util.CPUTimer; -import org.labkey.api.util.Filter; import org.labkey.api.util.UnexpectedException; import org.labkey.api.writer.VirtualFile; import org.labkey.study.StudySchema; @@ -293,7 +290,6 @@ public void run() assert cpuDelete.stop(); } - assert cpuImport.start(); _logger.info(_datasetDefinition.getLabel() + ": Starting import from " + _fileName); @@ -307,20 +303,27 @@ public void run() if (!batchErrors.hasErrors()) { - // optional check if new visits exist before committing, visit based timepoint studies only - boolean shouldCommit = true; - if (_studyImportContext.isFailForUndefinedVisits() && _study.getTimepointType() == TimepointType.VISIT) + boolean commit = true; + if (_studyImportContext.isFailForUndefinedVisits() || _study.isFailForUndefinedTimepoints()) { - List undefinedSequenceNums = StudyManager.getInstance().getUndefinedSequenceNumsForDataset(_datasetDefinition.getContainer(), _datasetDefinition.getDatasetId()); - if (!undefinedSequenceNums.isEmpty()) + // check for undefined visits and don't commit the data if there are errors + ValidationException validationException = StudyManager.getInstance().getVisitManager(_study).updateParticipantVisits( + _studyImportContext.getUser(), + Collections.singletonList(_datasetDefinition), + null, + null, + true, + _studyImportContext.isFailForUndefinedVisits() || _study.isFailForUndefinedTimepoints(), + _logger); + + if (validationException.hasErrors()) { - Collections.sort(undefinedSequenceNums); - _logger.error("The following undefined visits exist in the dataset data: " + StringUtils.join(undefinedSequenceNums, ", ")); - shouldCommit = false; + commit = false; + batchErrors.addRowError(validationException); } } - if (shouldCommit) + if (commit) { assert cpuCommit.start(); transaction.commit(); diff --git a/study/src/org/labkey/study/query/DatasetUpdateService.java b/study/src/org/labkey/study/query/DatasetUpdateService.java index 0e63c46d117..bc69a57e1fd 100644 --- a/study/src/org/labkey/study/query/DatasetUpdateService.java +++ b/study/src/org/labkey/study/query/DatasetUpdateService.java @@ -57,6 +57,7 @@ import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.roles.Role; import org.labkey.api.study.Dataset; +import org.labkey.api.study.Study; import org.labkey.api.study.StudyService; import org.labkey.api.study.security.StudySecurityEscalator; import org.labkey.study.model.DatasetDefinition; @@ -218,8 +219,15 @@ public int mergeRows(User user, Container container, DataIteratorBuilder rows, B int count = _importRowsUsingDIB(user, container, rows, null, getDataIteratorContext(errors, InsertOption.MERGE, configParameters), extraScriptContext); if (count > 0) { - StudyManager.datasetModified(_dataset, true); - resyncStudy(user, container, null, null, true); + try + { + StudyManager.datasetModified(_dataset, true); + resyncStudy(user, container, null, null, true); + } + catch (ValidationException e) + { + errors.addRowError(e); + } } return count; } @@ -230,8 +238,15 @@ public int loadRows(User user, Container container, DataIteratorBuilder rows, Da int count = _importRowsUsingDIB(user, container, rows, null, context, extraScriptContext); if (count > 0 && !Boolean.TRUE.equals(context.getConfigParameterBoolean(Config.SkipResyncStudy))) { - StudyManager.datasetModified(_dataset, true); - resyncStudy(user, container, null, null, true); + try + { + StudyManager.datasetModified(_dataset, true); + resyncStudy(user, container, null, null, true); + } + catch (ValidationException e) + { + context.getErrors().addRowError(e); + } } return count; } @@ -283,8 +298,15 @@ else if (!isBulkLoad()) _participantVisitResyncRequired = true; // 13717 : Study failing to resync() on dataset insert if (configParameters == null || !Boolean.TRUE.equals(configParameters.get(DatasetUpdateService.Config.SkipResyncStudy))) { - StudyManager.datasetModified(_dataset, true); - resyncStudy(user, container); + try + { + StudyManager.datasetModified(_dataset, true); + resyncStudy(user, container); + } + catch (ValidationException e) + { + errors.addRowError(e); + } } } return result; @@ -483,19 +505,26 @@ public int hashCode() public List> updateRows(User user, final Container container, List> rows, List> oldKeys, BatchValidationException errors, @Nullable Map configParameters, Map extraScriptContext) throws InvalidKeyException, BatchValidationException, QueryUpdateServiceException, SQLException { - List> result = super.updateRows(user, container, rows, oldKeys, errors, configParameters, extraScriptContext); - if (null != extraScriptContext && Boolean.TRUE.equals(extraScriptContext.get("synchronousParticipantPurge"))) - { - PurgeParticipantCommitTask addObj = new PurgeParticipantCommitTask(container, _potentiallyDeletedParticipants); - PurgeParticipantCommitTask setObj = getQueryTable().getSchema().getScope().addCommitTask(addObj, DbScope.CommitTaskOption.POSTCOMMIT); - setObj._potentiallyDeletedParticipants.addAll(addObj._potentiallyDeletedParticipants); - } + List> result = super.updateRows(user, container, rows, oldKeys, errors, configParameters, extraScriptContext); + if (null != extraScriptContext && Boolean.TRUE.equals(extraScriptContext.get("synchronousParticipantPurge"))) + { + PurgeParticipantCommitTask addObj = new PurgeParticipantCommitTask(container, _potentiallyDeletedParticipants); + PurgeParticipantCommitTask setObj = getQueryTable().getSchema().getScope().addCommitTask(addObj, DbScope.CommitTaskOption.POSTCOMMIT); + setObj._potentiallyDeletedParticipants.addAll(addObj._potentiallyDeletedParticipants); + } + try + { resyncStudy(user, container); - return result; } + catch (ValidationException e) + { + errors.addRowError(e); + } + return result; + } - private void resyncStudy(User user, Container container) + private void resyncStudy(User user, Container container) throws ValidationException { resyncStudy(user, container, _potentiallyNewParticipants, _potentiallyDeletedParticipants, _participantVisitResyncRequired); @@ -508,15 +537,24 @@ private void resyncStudy(User user, Container container) * Resyncs the study : updates the participant, visit, and (optionally) participant visit tables. Also updates automatic cohort assignments. * * @param potentiallyAddedParticipants optionally, the specific participants that may have been added to the study. - * If null, all of the changedDatasets and specimens will be checked to see if they contain new participants + * If null, all the changedDatasets and specimens will be checked to see if they contain new participants * @param potentiallyDeletedParticipants optionally, the specific participants that may have been removed from the * study. If null, all participants will be checked to see if they are still in the study. * @param participantVisitResyncRequired If true, will force an update of the ParticipantVisit mapping for this study */ - private void resyncStudy(User user, Container container, @Nullable Set potentiallyAddedParticipants, @Nullable Set potentiallyDeletedParticipants, boolean participantVisitResyncRequired) + private void resyncStudy(User user, Container container, @Nullable Set potentiallyAddedParticipants, + @Nullable Set potentiallyDeletedParticipants, + boolean participantVisitResyncRequired) throws ValidationException { StudyImpl study = StudyManager.getInstance().getStudy(container); - StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(user, Collections.singletonList(_dataset), potentiallyAddedParticipants, potentiallyDeletedParticipants, participantVisitResyncRequired, null); + Study sharedStudy = StudyManager.getInstance().getSharedStudy(study); + + ValidationException errors = StudyManager.getInstance().getVisitManager(study).updateParticipantVisits(user, Collections.singletonList(_dataset), + potentiallyAddedParticipants, potentiallyDeletedParticipants, participantVisitResyncRequired, + sharedStudy != null ? sharedStudy.isFailForUndefinedTimepoints() : study.isFailForUndefinedTimepoints(), null); + + if (errors.hasErrors()) + throw errors; } @Override @@ -658,7 +696,14 @@ public List> deleteRows(User user, Container container, List throws InvalidKeyException, BatchValidationException, QueryUpdateServiceException, SQLException { List> result = super.deleteRows(user, container, keys, configParameters, extraScriptContext); - resyncStudy(user, container); + try + { + resyncStudy(user, container); + } + catch (ValidationException e) + { + throw new BatchValidationException(e); + } return result; } diff --git a/study/src/org/labkey/study/view/manageTimepoints.jsp b/study/src/org/labkey/study/view/manageTimepoints.jsp index 928daa2be79..a1dac9c7fee 100644 --- a/study/src/org/labkey/study/view/manageTimepoints.jsp +++ b/study/src/org/labkey/study/view/manageTimepoints.jsp @@ -88,6 +88,17 @@ value="<%=DateUtil.formatDate(getContainer(), form.getStartDate())%>" /> +
+
+

Automatic Timepoint Creation

+
+
By default, new timepoint rows will be created in the study during the insert or update of any dataset or + specimen rows which have a new, undefined timepoint. If, instead you would like + for the operation to fail when it encounters a visit that is not already defined in the study, check the box + below. +

+ <%= button("Update").submit(true) %> <%= generateBackButton() %> diff --git a/study/src/org/labkey/study/view/manageVisits.jsp b/study/src/org/labkey/study/view/manageVisits.jsp index 887454e900b..b628bd74302 100644 --- a/study/src/org/labkey/study/view/manageVisits.jsp +++ b/study/src/org/labkey/study/view/manageVisits.jsp @@ -29,11 +29,13 @@ <%@ page import="org.labkey.study.controllers.StudyController.VisitVisibilityAction" %> <%@ page import="org.labkey.study.model.VisitImpl" %> <%@ page import="java.util.List" %> +<%@ page import="org.labkey.study.controllers.StudyController" %> <%@ page extends="org.labkey.study.view.BaseStudyPage" %> <%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> <% List allVisits = getVisits(Visit.Order.DISPLAY); %> +
@@ -79,6 +81,19 @@
View study schedule<%= link("Create New Visit", CreateVisitAction.class)%>
+

+ + +

By default, new visit rows will be created in the study during the insert or update of any dataset or + specimen rows which have a new, undefined visit. If, instead you would like + for the operation to fail when it encounters a visit that is not already defined in the study, check the box + below. +

+ + <%= button("Update").submit(true) %> + + <% if (allVisits.size() > 0) diff --git a/study/src/org/labkey/study/visitmanager/AbsoluteDateVisitManager.java b/study/src/org/labkey/study/visitmanager/AbsoluteDateVisitManager.java index 2dd228bae46..88ea5a307e1 100644 --- a/study/src/org/labkey/study/visitmanager/AbsoluteDateVisitManager.java +++ b/study/src/org/labkey/study/visitmanager/AbsoluteDateVisitManager.java @@ -17,11 +17,13 @@ package org.labkey.study.visitmanager; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.TableInfo; import org.labkey.api.query.FilteredTable; +import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.study.CohortFilter; import org.labkey.api.study.DataspaceContainerFilter; @@ -147,8 +149,9 @@ protected void updateParticipantVisitTable(@Nullable User user, @Nullable Logger } @Override - protected void updateVisitTable(User user, @Nullable Logger logger) + protected @NotNull ValidationException updateVisitTable(User user, @Nullable Logger logger, boolean failForUndefinedVisits) { // no-op + return new ValidationException(); } } diff --git a/study/src/org/labkey/study/visitmanager/RelativeDateVisitManager.java b/study/src/org/labkey/study/visitmanager/RelativeDateVisitManager.java index 344b9105403..7de09e8b1a3 100644 --- a/study/src/org/labkey/study/visitmanager/RelativeDateVisitManager.java +++ b/study/src/org/labkey/study/visitmanager/RelativeDateVisitManager.java @@ -17,6 +17,7 @@ package org.labkey.study.visitmanager; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; @@ -29,6 +30,7 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.query.FieldKey; import org.labkey.api.query.FilteredTable; +import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.study.CohortFilter; import org.labkey.api.study.Study; @@ -299,7 +301,7 @@ private void _updateVisitRowId() /** Make sure there is a Visit for each row in StudyData otherwise rows will be orphaned */ @Override - protected void updateVisitTable(final User user, @Nullable Logger logger) + protected @NotNull ValidationException updateVisitTable(final User user, @Nullable Logger logger, boolean failForUndefinedVisits) { DbSchema schema = StudySchema.getInstance().getSchema(); TableInfo tableParticipantVisit = StudySchema.getInstance().getTableInfoParticipantVisit(); @@ -317,13 +319,15 @@ protected void updateVisitTable(final User user, @Nullable Logger logger) daysToEnsure.add(seqNum); }); - StudyManager.getInstance().ensureVisits(getStudy(), user, daysToEnsure, null); + ValidationException errors = StudyManager.getInstance().ensureVisits(getStudy(), user, daysToEnsure, null, failForUndefinedVisits); if (daysToEnsure.size() > 0) _updateVisitRowId(); + + return errors; } - public void recomputeDates(Date oldStartDate, User user) + public @Nullable ValidationException recomputeDates(Date oldStartDate, User user) { if (null != oldStartDate) { @@ -344,9 +348,10 @@ public void recomputeDates(Date oldStartDate, User user) executor.execute("DELETE FROM " + tableVisit + " WHERE Container=?", c); //Now recompute everything - updateParticipantVisits(user, getStudy().getDatasets()); + return updateParticipantVisits(user, getStudy().getDatasets()); } } + return null; } // Return sql for fetching all datasets and their visit sequence numbers, given a container diff --git a/study/src/org/labkey/study/visitmanager/SequenceVisitManager.java b/study/src/org/labkey/study/visitmanager/SequenceVisitManager.java index 25f6f118e4e..7250a46e8b5 100644 --- a/study/src/org/labkey/study/visitmanager/SequenceVisitManager.java +++ b/study/src/org/labkey/study/visitmanager/SequenceVisitManager.java @@ -18,6 +18,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; @@ -30,6 +31,7 @@ import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.query.FieldKey; import org.labkey.api.query.FilteredTable; +import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.study.CohortFilter; import org.labkey.api.study.Study; @@ -37,7 +39,6 @@ import org.labkey.api.study.Visit; import org.labkey.api.study.model.ParticipantGroup; import org.labkey.api.util.DateUtil; -import org.labkey.api.util.GUID; import org.labkey.study.StudySchema; import org.labkey.study.StudyUnionTableInfo; import org.labkey.study.model.DatasetDefinition; @@ -495,10 +496,11 @@ private void _updateVisitRowId(boolean updateAll, @Nullable Logger logger) /** Make sure there is a Visit for each row in StudyData otherwise rows will be orphaned */ @Override - protected void updateVisitTable(User user, @Nullable Logger logger) + protected @NotNull ValidationException updateVisitTable(User user, @Nullable Logger logger, boolean failForUndefinedVisits) { DbSchema schema = StudySchema.getInstance().getSchema(); TableInfo tableParticipantVisit = StudySchema.getInstance().getTableInfoParticipantVisit(); + ValidationException errors = new ValidationException(); SQLFragment sql = new SQLFragment("SELECT DISTINCT SequenceNum\n" + "FROM " + tableParticipantVisit + "\n"+ @@ -513,9 +515,10 @@ protected void updateVisitTable(User user, @Nullable Logger logger) if (sequenceNums.size() > 0) { info(logger, "Ensure visits for " + sequenceNums.size() + " distinct sequence numbers"); - StudyManager.getInstance().ensureVisits(getStudy(), user, sequenceNums, null); + errors = StudyManager.getInstance().ensureVisits(getStudy(), user, sequenceNums, null, failForUndefinedVisits); _updateVisitRowId(true, logger); } + return errors; } diff --git a/study/src/org/labkey/study/visitmanager/VisitManager.java b/study/src/org/labkey/study/visitmanager/VisitManager.java index 3649071ed9f..893a2468b40 100644 --- a/study/src/org/labkey/study/visitmanager/VisitManager.java +++ b/study/src/org/labkey/study/visitmanager/VisitManager.java @@ -40,6 +40,7 @@ import org.labkey.api.data.TableSelector; import org.labkey.api.exceptions.TableNotFoundException; import org.labkey.api.query.FieldKey; +import org.labkey.api.query.ValidationException; import org.labkey.api.search.SearchService; import org.labkey.api.security.User; import org.labkey.api.specimen.SpecimenSchema; @@ -97,27 +98,25 @@ protected VisitManager(StudyImpl study) * @param user the current user * @param changedDatasets the datasets that may have one or more rows modified */ - public void updateParticipantVisits(User user, @NotNull Collection changedDatasets) + public ValidationException updateParticipantVisits(User user, @NotNull Collection changedDatasets) { - updateParticipantVisits(user, changedDatasets, null, null, true, null); + return updateParticipantVisits(user, changedDatasets, null, null, true, _study.isFailForUndefinedTimepoints(), null); } - public void updateParticipantVisits(User user, @NotNull Collection changedDatasets, @Nullable Logger logger) + public ValidationException updateParticipantVisitsWithCohortUpdate(User user, boolean failForUndefinedVisits, @Nullable Logger logger) { - updateParticipantVisits(user, changedDatasets, null, null, true, logger); + return updateParticipantVisits(user, Collections.emptyList(), null, null, true, failForUndefinedVisits, logger); } - public void updateParticipantVisitsWithCohortUpdate(User user, @Nullable Logger logger) - { - updateParticipantVisits(user, Collections.emptyList(), null, null, true, true, logger); - } - - public void updateParticipantVisits(User user, @NotNull Collection changedDatasets, @Nullable Set potentiallyAddedParticipants, - @Nullable Set potentiallyDeletedParticipants, boolean participantVisitResyncRequired, + public ValidationException updateParticipantVisits(User user, @NotNull Collection changedDatasets, + @Nullable Set potentiallyAddedParticipants, + @Nullable Set potentiallyDeletedParticipants, + boolean participantVisitResyncRequired, + boolean failForUndefinedVisits, @Nullable Logger logger) { - updateParticipantVisits(user, changedDatasets, potentiallyAddedParticipants, potentiallyDeletedParticipants, - participantVisitResyncRequired, false, logger); + return _updateParticipantVisits(user, changedDatasets, potentiallyAddedParticipants, potentiallyDeletedParticipants, + participantVisitResyncRequired, failForUndefinedVisits, logger); } protected void info(@Nullable Logger logger, String message) @@ -131,16 +130,19 @@ protected void info(@Nullable Logger logger, String message) * @param user the current user * @param changedDatasets the datasets that may have one or more rows modified * @param potentiallyAddedParticipants optionally, the specific participants that may have been added to the study. - * If null, all of the changedDatasets and specimens will be checked to see if they contain new participants + * If null, all the changedDatasets and specimens will be checked to see if they contain new participants * @param potentiallyDeletedParticipants optionally, the specific participants that may have been removed from the * study. If null, all participants will be checked to see if they are still in the study. * @param participantVisitResyncRequired If true, will force an update of the ParticipantVisit mapping for this study - * @param forceUpdateCohorts If true, forces updateParticipantCohorts() (specimen import case) + * @param failForUndefinedVisits If true, will return an error if visits do not currently exist in the study * @param logger Log4j logger to use for detailed performance information */ - public void updateParticipantVisits(User user, @NotNull Collection changedDatasets, @Nullable Set potentiallyAddedParticipants, - @Nullable Set potentiallyDeletedParticipants, boolean participantVisitResyncRequired, - boolean forceUpdateCohorts, @Nullable Logger logger) + private ValidationException _updateParticipantVisits(User user, @NotNull Collection changedDatasets, + @Nullable Set potentiallyAddedParticipants, + @Nullable Set potentiallyDeletedParticipants, + boolean participantVisitResyncRequired, + boolean failForUndefinedVisits, + @Nullable Logger logger) { info(logger, "Updating participants"); updateParticipants(changedDatasets, potentiallyAddedParticipants, potentiallyDeletedParticipants); @@ -163,22 +165,21 @@ public void updateParticipantVisits(User user, @NotNull Collection