From df587d2bab347aaff0cef41a39726b080345aab5 Mon Sep 17 00:00:00 2001 From: bbimber Date: Sat, 15 Mar 2025 12:53:41 -0700 Subject: [PATCH 01/13] Improve handling of generated queries when dealing with a container-scoped list (#41) * Improve handling of generated queries when dealing with a container-scoped list * Also migrate from the deprecated WrappedColumn to WrappedColumnInfo --- .../api/ldk/table/ContainerScopedTable.java | 2 +- LDK/src/org/labkey/ldk/LDKController.java | 8 +- .../query/ContainerIncrementingTable.java | 2 +- .../query/LaboratoryTableCustomizer.java | 123 ++++++++++++------ 4 files changed, 90 insertions(+), 45 deletions(-) diff --git a/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java b/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java index 975f117e..3575eeca 100644 --- a/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java +++ b/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java @@ -122,7 +122,7 @@ public DataIteratorBuilder persistRows(DataIteratorBuilder data, DataIteratorCon protected class UpdateService extends SimpleQueryUpdateService { - private KeyManager _keyManager = new KeyManager(); + private final KeyManager _keyManager = new KeyManager(); public UpdateService(SimpleUserSchema.SimpleTable ti) { diff --git a/LDK/src/org/labkey/ldk/LDKController.java b/LDK/src/org/labkey/ldk/LDKController.java index 3f40d163..168ac090 100644 --- a/LDK/src/org/labkey/ldk/LDKController.java +++ b/LDK/src/org/labkey/ldk/LDKController.java @@ -847,14 +847,10 @@ public ModelAndView getView(QueryForm form, BindException errors) throws Excepti if (keyField != null) { + // Note: the ContainerContext will need to be set within QueryView DetailsURL importUrl = DetailsURL.fromString("/query/importData.view?schemaName=" + schemaName + "&query.queryName=" + queryName + "&keyField=" + keyField); - importUrl.setContainerContext(getContainer()); - DetailsURL updateUrl = DetailsURL.fromString("/ldk/manageRecord.view?schemaName=" + schemaName + "&query.queryName=" + queryName + "&keyField=" + keyField + "&key=${" + keyField + "}"); - updateUrl.setContainerContext(getContainer()); - DetailsURL deleteUrl = DetailsURL.fromString("/query/deleteQueryRows.view?schemaName=" + schemaName + "&query.queryName=" + queryName); - deleteUrl.setContainerContext(getContainer()); url.addParameter("importURL", importUrl.toString()); url.addParameter("updateURL", updateUrl.toString()); @@ -866,7 +862,7 @@ public ModelAndView getView(QueryForm form, BindException errors) throws Excepti url.addParameter("queryName", queryName); url.addParameter("allowChooseQuery", false); - WebPartFactory factory = Portal.getPortalPartCaseInsensitive("Query"); + WebPartFactory factory = Portal.getPortalPart("Query"); Portal.WebPart part = factory.createWebPart(); part.setProperties(url.getQueryString()); diff --git a/laboratory/api-src/org/labkey/api/laboratory/query/ContainerIncrementingTable.java b/laboratory/api-src/org/labkey/api/laboratory/query/ContainerIncrementingTable.java index bc3e7843..0845cbc6 100644 --- a/laboratory/api-src/org/labkey/api/laboratory/query/ContainerIncrementingTable.java +++ b/laboratory/api-src/org/labkey/api/laboratory/query/ContainerIncrementingTable.java @@ -59,7 +59,7 @@ public ContainerIncrementingTable(UserSchema us, TableInfo st, ContainerFilter c } @Override - public SimpleUserSchema.SimpleTable init() + public SimpleUserSchema.SimpleTable init() { super.init(); diff --git a/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java b/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java index af8a869d..5f0ae687 100644 --- a/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java +++ b/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java @@ -7,6 +7,7 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.AbstractTableInfo; +import org.labkey.api.data.BaseColumnInfo; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; @@ -18,7 +19,7 @@ import org.labkey.api.data.SQLFragment; import org.labkey.api.data.TableCustomizer; import org.labkey.api.data.TableInfo; -import org.labkey.api.data.WrappedColumn; +import org.labkey.api.data.WrappedColumnInfo; import org.labkey.api.gwt.client.FacetingBehaviorType; import org.labkey.api.laboratory.LaboratoryService; import org.labkey.api.ldk.LDKService; @@ -183,13 +184,16 @@ public void customizeColumns(AbstractTableInfo ti) { container.setHidden(true); - WrappedColumn wrappedContainer = new WrappedColumn(container, "workbook"); - wrappedContainer.setLabel("Workbook"); + BaseColumnInfo wrappedContainer = WrappedColumnInfo.wrapAsCopy(ti, FieldKey.fromString("workbook"), container, "Workbook", null); + wrappedContainer.setName("workbook"); + wrappedContainer.setCalculated(true); + wrappedContainer.setShownInInsertView(false); + wrappedContainer.setShownInUpdateView(false); wrappedContainer.setFk(QueryForeignKey .from(ti.getUserSchema(), ti.getContainerFilter()) .schema(us) .to("workbooks", LaboratoryWorkbooksTable.WORKBOOK_COL, "workbookId")); - wrappedContainer.setURL(DetailsURL.fromString("/project/start.view")); + wrappedContainer.setURL(DetailsURL.fromString("/project/begin.view")); wrappedContainer.setShownInDetailsView(true); wrappedContainer.setFacetingBehaviorType(FacetingBehaviorType.ALWAYS_OFF); wrappedContainer.setDisplayColumnFactory(new DisplayColumnFactory() @@ -290,11 +294,15 @@ private void appendDemographicsCols(final UserSchema us, AbstractTableInfo ti, C if (ti.getColumn(name) != null) continue; - WrappedColumn col = new WrappedColumn(subjectCol, name); - col.setLabel(qd.getLabel()); + BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ti, FieldKey.fromString(name), subjectCol, qd.getLabel(), null); + col.setName(name); + col.setCalculated(true); + col.setShownInInsertView(false); + col.setShownInUpdateView(false); col.setReadOnly(true); col.setIsUnselectable(true); col.setUserEditable(false); + col.setKeyField(false); UserSchema targetSchema = qd.getTableInfo(targetContainer, us.getUser()).getUserSchema(); col.setFk(new QueryForeignKey(us, ti.getContainerFilter(), targetSchema, null, qd.getQueryName(), qd.getTargetColumn(), qd.getTargetColumn()) @@ -366,12 +374,16 @@ private void appendMajorEventsCol(final UserSchema us, AbstractTableInfo ds, fin final String pkColSelectName = pk.getFieldKey().toSQLString(); final String pkColRawName = pk.getName(); - MutableColumnInfo col = new WrappedColumn(pk, name); - col.setLabel("Major Events"); + BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(name), pk, "Major Events", null); col.setDescription("This column shows all major events recorded in this subject's history and will calculate the time elapsed between the current sample and these dates."); + col.setName(name); + col.setCalculated(true); + col.setShownInInsertView(false); + col.setShownInUpdateView(false); col.setReadOnly(true); col.setIsUnselectable(true); col.setUserEditable(false); + col.setKeyField(false); final String schemaName = ds.getUserSchema().getSchemaPath().toSQLString(); final String subjectSelectName = ds.getSqlDialect().makeLegalIdentifier(subjectColName); @@ -384,11 +396,10 @@ private void appendMajorEventsCol(final UserSchema us, AbstractTableInfo ds, fin @Override public TableInfo getLookupTableInfo() { - Container target = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); - UserSchema effectiveUs = us.getContainer().isWorkbookOrTab() ? QueryService.get().getUserSchema(us.getUser(), target, us.getSchemaPath()) : us; - QueryDefinition qd = QueryService.get().createQueryDef(us.getUser(), target, effectiveUs, colName); + Container parentContainer = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); + QueryDefinition qd = createQueryDef(us, colName); - qd.setSql(getMajorEventsSql(target, schemaName, querySelectName, pkColSelectName, subjectSelectName, dateSelectName)); + qd.setSql(getMajorEventsSql(parentContainer, schemaName, querySelectName, pkColSelectName, subjectSelectName, dateSelectName)); qd.setIsTemporary(true); List errors = new ArrayList<>(); @@ -444,21 +455,24 @@ private void appendOverlapingProjectsCol(final UserSchema us, AbstractTableInfo final String subjectSelectName = ds.getSqlDialect().makeLegalIdentifier(subjectColName); final String dateSelectName = dateColName == null ? null : ds.getSqlDialect().makeLegalIdentifier(dateColName); - WrappedColumn col = new WrappedColumn(pk, name); - col.setLabel("Overlapping Groups"); + BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(name), pk, "Overlapping Groups", null); col.setDescription("This column shows all groups to which this subject belonged at the time of this sample."); + col.setName(name); + col.setCalculated(true); + col.setShownInInsertView(false); + col.setShownInUpdateView(false); col.setReadOnly(true); col.setIsUnselectable(true); col.setUserEditable(false); + col.setKeyField(false); col.setFk(new LookupForeignKey(){ @Override public TableInfo getLookupTableInfo() { - Container target = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); - UserSchema effectiveUs = us.getContainer().isWorkbookOrTab() ? QueryService.get().getUserSchema(us.getUser(), target, us.getSchemaPath()) : us; - QueryDefinition qd = QueryService.get().createQueryDef(us.getUser(), target, effectiveUs, colName); + Container parentContainer = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); + QueryDefinition qd = createQueryDef(us, colName); - qd.setSql(getOverlapSql(target, schemaName, querySelectName, pkColSelectName, subjectSelectName, dateSelectName)); + qd.setSql(getOverlapSql(parentContainer, schemaName, querySelectName, pkColSelectName, subjectSelectName, dateSelectName)); qd.setIsTemporary(true); List errors = new ArrayList<>(); @@ -490,20 +504,24 @@ public TableInfo getLookupTableInfo() //add pivot column String pivotColName = "overlappingProjectsPivot"; - WrappedColumn col2 = new WrappedColumn(pk, pivotColName); - col2.setLabel("Overlapping Group List"); + BaseColumnInfo col2 = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(pivotColName), pk, "Overlapping Group List", null); + col2.setName(pivotColName); + col2.setCalculated(true); + col2.setShownInInsertView(false); + col2.setShownInUpdateView(false); col2.setDescription("Shows groups to which this subject belonged at the time of this sample."); col2.setHidden(true); col2.setReadOnly(true); col2.setIsUnselectable(true); col2.setUserEditable(false); + col2.setKeyField(false); final String lookupColName = ds.getName() + "_overlappingProjectsPivot"; col2.setFk(new LookupForeignKey(){ @Override public TableInfo getLookupTableInfo() { Container target = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); - QueryDefinition qd = QueryService.get().createQueryDef(us.getUser(), target, us, lookupColName); + QueryDefinition qd = createQueryDef(us, lookupColName); qd.setSql(getOverlapPivotSql(target, schemaName, querySelectName, pkColSelectName, subjectColName, dateColName)); qd.setIsTemporary(true); @@ -555,21 +573,24 @@ public void appendProjectsCol(final UserSchema us, AbstractTableInfo ds, final S final String publicTableName = ds.getPublicName(); final String colName = ds.getName() + "_allProjects"; - WrappedColumn col = new WrappedColumn(pk, name); - col.setLabel("Groups"); + BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(name), pk, "Groups", null); + col.setName(name); + col.setCalculated(true); + col.setShownInInsertView(false); + col.setShownInUpdateView(false); col.setDescription("This column shows all groups to which this subject has ever been a member, regardless of whether that assignment overlaps with the current data point"); col.setReadOnly(true); col.setIsUnselectable(true); col.setUserEditable(false); + col.setKeyField(false); col.setFk(new LookupForeignKey(){ @Override public TableInfo getLookupTableInfo() { - Container target = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); - UserSchema effectiveUs = us.getContainer().isWorkbookOrTab() ? QueryService.get().getUserSchema(us.getUser(), target, us.getSchemaPath()) : us; - QueryDefinition qd = QueryService.get().createQueryDef(us.getUser(), target, effectiveUs, colName); + Container parentContainer = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); + QueryDefinition qd = createQueryDef(us, colName); - qd.setSql(getOverlapSql(target, schemaName, querySelectName, pkColSelectName, subjectSelectName, null)); + qd.setSql(getOverlapSql(parentContainer, schemaName, querySelectName, pkColSelectName, subjectSelectName, null)); qd.setIsTemporary(true); List errors = new ArrayList<>(); @@ -602,20 +623,23 @@ public TableInfo getLookupTableInfo() //add pivot column String pivotColName = "allProjectsPivot"; final String lookupName = ds.getName() + "_allProjectsPivot"; - WrappedColumn col2 = new WrappedColumn(pk, pivotColName); - col2.setLabel("Group Summary List"); + BaseColumnInfo col2 = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(pivotColName), pk, "Group Summary List", null); + col2.setName(pivotColName); + col2.setCalculated(true); + col2.setShownInInsertView(false); + col2.setShownInUpdateView(false); col2.setDescription("Shows groups to which this subject belonged at any point in time."); col2.setHidden(true); col2.setReadOnly(true); col2.setIsUnselectable(true); col2.setUserEditable(false); + col2.setKeyField(false); col2.setFk(new LookupForeignKey(){ @Override public TableInfo getLookupTableInfo() { Container target = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); - UserSchema effectiveUs = us.getContainer().isWorkbookOrTab() ? QueryService.get().getUserSchema(us.getUser(), target, us.getSchemaPath()) : us; - QueryDefinition qd = QueryService.get().createQueryDef(us.getUser(), target, effectiveUs, lookupName); + QueryDefinition qd = createQueryDef(us, lookupName); qd.setSql(getOverlapPivotSql(target, schemaName, querySelectName, pkColSelectName, subjectSelectName, null)); qd.setIsTemporary(true); @@ -749,11 +773,15 @@ private void appendRelativeDatesCol(final UserSchema us, AbstractTableInfo ds, f final String pkColSelectName = pk.getFieldKey().toSQLString(); final String pkColRawName = pk.getName(); - WrappedColumn col = new WrappedColumn(pk, name); - col.setLabel("Relative Dates"); + BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(name), pk, "Relative Dates", null); + col.setName(name); + col.setCalculated(true); + col.setShownInInsertView(false); + col.setShownInUpdateView(false); col.setReadOnly(true); col.setIsUnselectable(true); col.setUserEditable(false); + col.setKeyField(false); final String colName = ds.getName() + "_relativeDates"; final String schemaName = ds.getUserSchema().getSchemaPath().toSQLString(); @@ -765,9 +793,8 @@ private void appendRelativeDatesCol(final UserSchema us, AbstractTableInfo ds, f @Override public TableInfo getLookupTableInfo() { - Container target = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); - UserSchema effectiveUs = us.getContainer().isWorkbookOrTab() ? QueryService.get().getUserSchema(us.getUser(), target, us.getSchemaPath()) : us; - QueryDefinition qd = QueryService.get().createQueryDef(us.getUser(), target, effectiveUs, colName); + Container parentContainer = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); + QueryDefinition qd = createQueryDef(us, colName); qd.setSql("SELECT\n" + "t." + pkColSelectName + ",\n" + @@ -795,7 +822,7 @@ public TableInfo getLookupTableInfo() "ROUND(CONVERT(age_in_months(p.startdate, s." + dateSelectName + "), DOUBLE) / 12, 1) AS YearsPostStartDecimal,\n" + "\n" + "FROM " + schemaName + "." + publicTableName + " s\n" + - "JOIN \"" + target.getPath() + "\".laboratory.project_usage p\n" + + "JOIN \"" + parentContainer.getPath() + "\".laboratory.project_usage p\n" + "ON (s." + subjectSelectName + " = p.subjectId AND CONVERT(p.startdate, DATE) <= CONVERT(s." + dateSelectName + ", DATE) AND CONVERT(s." + dateSelectName + ", DATE) <= CONVERT(COALESCE(p.enddate, {fn curdate()}), DATE))\n" + "WHERE s." + dateSelectName + " IS NOT NULL and s." + subjectSelectName + " IS NOT NULL\n" + "\n" + @@ -830,6 +857,28 @@ public TableInfo getLookupTableInfo() ds.addColumn(col); } + private QueryDefinition createQueryDef(UserSchema us, String queryName) + { + if (!us.getContainer().isWorkbook()) + { + return QueryService.get().createQueryDef(us.getUser(), us.getContainer(), us, queryName); + } + + // The rationale is that if we are querying from a workbook, preferentially translate to the parent US + // However, there are situations like workbook-scoped lists, where that query might not exist on the parent + UserSchema parentUserSchema = QueryService.get().getUserSchema(us.getUser(), us.getContainer().getParent(), us.getSchemaPath()); + assert parentUserSchema != null; + + if (parentUserSchema.getTableNames().contains(queryName)) + { + return QueryService.get().createQueryDef(parentUserSchema.getUser(), parentUserSchema.getContainer(), parentUserSchema, queryName); + } + else + { + return QueryService.get().createQueryDef(us.getUser(), us.getContainer(), us, queryName); + } + } + public UserSchema getUserSchema(AbstractTableInfo ds, String name) { UserSchema us = ds.getUserSchema(); From 865087f235db239c0da36f4f6bb9c5b75d79d8e8 Mon Sep 17 00:00:00 2001 From: bbimber Date: Sun, 16 Mar 2025 12:04:13 -0700 Subject: [PATCH 02/13] Bugfix to DemographicsSources --- .../laboratory/query/LaboratoryTableCustomizer.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java b/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java index 5f0ae687..72fe0ef3 100644 --- a/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java +++ b/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java @@ -277,8 +277,8 @@ private void appendDemographicsCols(final UserSchema us, AbstractTableInfo ti, C { for (final DemographicsSource qd : qds) { - Container targetContainer = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); - TableInfo target = qd.getTableInfo(targetContainer, us.getUser()); + Container parentContainer = us.getContainer().isWorkbookOrTab() ? us.getContainer().getParent() : us.getContainer(); + TableInfo target = qd.getTableInfo(parentContainer, us.getUser()); //TODO: push this into LaboratoryService and also cache them? if (target != null) @@ -290,11 +290,10 @@ private void appendDemographicsCols(final UserSchema us, AbstractTableInfo ti, C } String name = ColumnInfo.legalNameFromName(qd.getLabel()); - if (ti.getColumn(name) != null) continue; - BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ti, FieldKey.fromString(name), subjectCol, qd.getLabel(), null); + ExprColumn col = new ExprColumn(ti, FieldKey.fromString(name), subjectCol.getValueSql(ExprColumn.STR_TABLE_ALIAS), JdbcType.INTEGER, subjectCol); col.setName(name); col.setCalculated(true); col.setShownInInsertView(false); @@ -304,7 +303,7 @@ private void appendDemographicsCols(final UserSchema us, AbstractTableInfo ti, C col.setUserEditable(false); col.setKeyField(false); - UserSchema targetSchema = qd.getTableInfo(targetContainer, us.getUser()).getUserSchema(); + final UserSchema targetSchema = target.getUserSchema(); col.setFk(new QueryForeignKey(us, ti.getContainerFilter(), targetSchema, null, qd.getQueryName(), qd.getTargetColumn(), qd.getTargetColumn()) { @Override From 646c8bdc40aa200bb0baf756887517eef9efab7c Mon Sep 17 00:00:00 2001 From: bbimber Date: Thu, 20 Mar 2025 05:58:15 -0700 Subject: [PATCH 03/13] Back out change to WrappedColumn in favor of ExprColumn --- .../labkey/test/tests/external/labModules/LabModulesTest.java | 3 ++- .../org/labkey/laboratory/query/LaboratoryTableCustomizer.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java index 23cdbdf6..cfa7e37b 100644 --- a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java +++ b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java @@ -1415,6 +1415,7 @@ private void samplesTableTest() throws Exception columnLabels.add(getColumnLabel(srr, name)); } + SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd"); List> rows = new ArrayList<>(); for (Map row : srr.getRows()) { @@ -1425,7 +1426,7 @@ private void samplesTableTest() throws Exception String val = row.get(name) == null ? "" : String.valueOf(row.get(name)); if (name.toLowerCase().contains("date")) { - val = StringUtils.isEmpty(val) ? "" : ExcelHelper.getDateTimeFormat().format(new Date(val)); + val = StringUtils.isEmpty(val) ? "" : dateFormat.format(val); } target.add(val); diff --git a/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java b/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java index 72fe0ef3..5994401f 100644 --- a/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java +++ b/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java @@ -184,7 +184,7 @@ public void customizeColumns(AbstractTableInfo ti) { container.setHidden(true); - BaseColumnInfo wrappedContainer = WrappedColumnInfo.wrapAsCopy(ti, FieldKey.fromString("workbook"), container, "Workbook", null); + ExprColumn wrappedContainer = new ExprColumn(ti, FieldKey.fromString("workbook"), container.getValueSql(ExprColumn.STR_TABLE_ALIAS), JdbcType.GUID, container); wrappedContainer.setName("workbook"); wrappedContainer.setCalculated(true); wrappedContainer.setShownInInsertView(false); From 3ca7188402f07fb023c515cdbe56da5ebb28b842 Mon Sep 17 00:00:00 2001 From: bbimber Date: Thu, 20 Mar 2025 06:16:25 -0700 Subject: [PATCH 04/13] Back out change to WrappedColumn in favor of ExprColumn --- .../api/ldk/table/ContainerScopedTable.java | 3 ++- .../query/LaboratoryTableCustomizer.java | 22 ++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java b/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java index 3575eeca..23a6dd6d 100644 --- a/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java +++ b/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java @@ -287,7 +287,8 @@ public Object call() Object pesudoPkVal = it.getInputColumnValue(inputColMap.get(_pseudoPk)); if (pesudoPkVal != null) { - if (_context.getInsertOption() != QueryUpdateService.InsertOption.MERGE && keyManager.rowExists(c, pesudoPkVal)) + // NOTE: this code is call for inserts and updates: + if (_context.getInsertOption() == QueryUpdateService.InsertOption.INSERT && keyManager.rowExists(c, pesudoPkVal)) { _context.getErrors().addRowError(new ValidationException("A record is already present with value: " + pesudoPkVal)); } diff --git a/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java b/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java index 5994401f..a1611849 100644 --- a/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java +++ b/laboratory/src/org/labkey/laboratory/query/LaboratoryTableCustomizer.java @@ -19,7 +19,6 @@ import org.labkey.api.data.SQLFragment; import org.labkey.api.data.TableCustomizer; import org.labkey.api.data.TableInfo; -import org.labkey.api.data.WrappedColumnInfo; import org.labkey.api.gwt.client.FacetingBehaviorType; import org.labkey.api.laboratory.LaboratoryService; import org.labkey.api.ldk.LDKService; @@ -184,7 +183,8 @@ public void customizeColumns(AbstractTableInfo ti) { container.setHidden(true); - ExprColumn wrappedContainer = new ExprColumn(ti, FieldKey.fromString("workbook"), container.getValueSql(ExprColumn.STR_TABLE_ALIAS), JdbcType.GUID, container); + ExprColumn wrappedContainer = new ExprColumn(ti, FieldKey.fromString("workbook"), container.getValueSql(ExprColumn.STR_TABLE_ALIAS), container.getJdbcType(), container); + wrappedContainer.setLabel("Workbook"); wrappedContainer.setName("workbook"); wrappedContainer.setCalculated(true); wrappedContainer.setShownInInsertView(false); @@ -373,7 +373,8 @@ private void appendMajorEventsCol(final UserSchema us, AbstractTableInfo ds, fin final String pkColSelectName = pk.getFieldKey().toSQLString(); final String pkColRawName = pk.getName(); - BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(name), pk, "Major Events", null); + BaseColumnInfo col = new ExprColumn(ds, FieldKey.fromString(name), pk.getValueSql(ExprColumn.STR_TABLE_ALIAS), pk.getJdbcType(), pk); + col.setLabel("Major Events"); col.setDescription("This column shows all major events recorded in this subject's history and will calculate the time elapsed between the current sample and these dates."); col.setName(name); col.setCalculated(true); @@ -454,7 +455,8 @@ private void appendOverlapingProjectsCol(final UserSchema us, AbstractTableInfo final String subjectSelectName = ds.getSqlDialect().makeLegalIdentifier(subjectColName); final String dateSelectName = dateColName == null ? null : ds.getSqlDialect().makeLegalIdentifier(dateColName); - BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(name), pk, "Overlapping Groups", null); + BaseColumnInfo col = new ExprColumn(ds, FieldKey.fromString(name), pk.getValueSql(ExprColumn.STR_TABLE_ALIAS), pk.getJdbcType(), pk); + col.setLabel("Overlapping Groups"); col.setDescription("This column shows all groups to which this subject belonged at the time of this sample."); col.setName(name); col.setCalculated(true); @@ -503,7 +505,8 @@ public TableInfo getLookupTableInfo() //add pivot column String pivotColName = "overlappingProjectsPivot"; - BaseColumnInfo col2 = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(pivotColName), pk, "Overlapping Group List", null); + BaseColumnInfo col2 = new ExprColumn(ds, FieldKey.fromString(pivotColName), pk.getValueSql(ExprColumn.STR_TABLE_ALIAS), pk.getJdbcType(), pk); + col2.setLabel("Overlapping Group List"); col2.setName(pivotColName); col2.setCalculated(true); col2.setShownInInsertView(false); @@ -572,7 +575,8 @@ public void appendProjectsCol(final UserSchema us, AbstractTableInfo ds, final S final String publicTableName = ds.getPublicName(); final String colName = ds.getName() + "_allProjects"; - BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(name), pk, "Groups", null); + BaseColumnInfo col = new ExprColumn(ds, FieldKey.fromString(name), pk.getValueSql(ExprColumn.STR_TABLE_ALIAS), pk.getJdbcType(), pk); + col.setLabel("Groups"); col.setName(name); col.setCalculated(true); col.setShownInInsertView(false); @@ -622,7 +626,8 @@ public TableInfo getLookupTableInfo() //add pivot column String pivotColName = "allProjectsPivot"; final String lookupName = ds.getName() + "_allProjectsPivot"; - BaseColumnInfo col2 = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(pivotColName), pk, "Group Summary List", null); + BaseColumnInfo col2 = new ExprColumn(ds, FieldKey.fromString(pivotColName), pk.getValueSql(ExprColumn.STR_TABLE_ALIAS), pk.getJdbcType(), pk); + col2.setLabel("Group Summary List"); col2.setName(pivotColName); col2.setCalculated(true); col2.setShownInInsertView(false); @@ -772,7 +777,8 @@ private void appendRelativeDatesCol(final UserSchema us, AbstractTableInfo ds, f final String pkColSelectName = pk.getFieldKey().toSQLString(); final String pkColRawName = pk.getName(); - BaseColumnInfo col = WrappedColumnInfo.wrapAsCopy(ds, FieldKey.fromString(name), pk, "Relative Dates", null); + BaseColumnInfo col = new ExprColumn(ds, FieldKey.fromString(name), pk.getValueSql(ExprColumn.STR_TABLE_ALIAS), pk.getJdbcType(), pk); + col.setLabel("Relative Dates"); col.setName(name); col.setCalculated(true); col.setShownInInsertView(false); From 0a8df16a0195093358af39cf43660a4dc88fbf1a Mon Sep 17 00:00:00 2001 From: bbimber Date: Thu, 20 Mar 2025 08:57:47 -0700 Subject: [PATCH 05/13] Fix date parsing --- .../labkey/test/tests/external/labModules/LabModulesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java index cfa7e37b..f8c29c36 100644 --- a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java +++ b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java @@ -1426,7 +1426,7 @@ private void samplesTableTest() throws Exception String val = row.get(name) == null ? "" : String.valueOf(row.get(name)); if (name.toLowerCase().contains("date")) { - val = StringUtils.isEmpty(val) ? "" : dateFormat.format(val); + val = StringUtils.isEmpty(val) ? "" : dateFormat.format(dateFormat.parse(val)); } target.add(val); From 0be7f18f006df169e783670ecfe281734b2f003c Mon Sep 17 00:00:00 2001 From: bbimber Date: Thu, 20 Mar 2025 09:01:59 -0700 Subject: [PATCH 06/13] Fix typo --- LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java b/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java index 23a6dd6d..40c6152b 100644 --- a/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java +++ b/LDK/api-src/org/labkey/api/ldk/table/ContainerScopedTable.java @@ -287,7 +287,7 @@ public Object call() Object pesudoPkVal = it.getInputColumnValue(inputColMap.get(_pseudoPk)); if (pesudoPkVal != null) { - // NOTE: this code is call for inserts and updates: + // NOTE: this code is called for both inserts and updates: if (_context.getInsertOption() == QueryUpdateService.InsertOption.INSERT && keyManager.rowExists(c, pesudoPkVal)) { _context.getErrors().addRowError(new ValidationException("A record is already present with value: " + pesudoPkVal)); From c6fa89031471d5b0118fc1023776943ed3514f5e Mon Sep 17 00:00:00 2001 From: bbimber Date: Thu, 20 Mar 2025 13:25:53 -0700 Subject: [PATCH 07/13] Restore lenient date parsing --- .../test/tests/external/labModules/LabModulesTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java index f8c29c36..2aa55a16 100644 --- a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java +++ b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java @@ -16,12 +16,15 @@ package org.labkey.test.tests.external.labModules; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.time.DateFormatUtils; +import org.apache.commons.lang3.time.DateUtils; import org.apache.commons.lang3.tuple.Pair; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.usermodel.Workbook; import org.junit.Assert; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.labkey.api.util.DateUtil; import org.labkey.remoteapi.CommandException; import org.labkey.remoteapi.Connection; import org.labkey.remoteapi.collections.CaseInsensitiveHashMap; @@ -1426,7 +1429,7 @@ private void samplesTableTest() throws Exception String val = row.get(name) == null ? "" : String.valueOf(row.get(name)); if (name.toLowerCase().contains("date")) { - val = StringUtils.isEmpty(val) ? "" : dateFormat.format(dateFormat.parse(val)); + val = StringUtils.isEmpty(val) ? "" : dateFormat.format(Date.parse(val)); } target.add(val); From 14b1618164ab695b85b600a4f0c2787e9df55d60 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Thu, 20 Mar 2025 13:12:31 -0700 Subject: [PATCH 08/13] Misc code cleanup - improve generics (#230) --- .../api/ldk/table/AbstractDataDefinedTable.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/LDK/api-src/org/labkey/api/ldk/table/AbstractDataDefinedTable.java b/LDK/api-src/org/labkey/api/ldk/table/AbstractDataDefinedTable.java index fe0cb3a8..a974a4d7 100644 --- a/LDK/api-src/org/labkey/api/ldk/table/AbstractDataDefinedTable.java +++ b/LDK/api-src/org/labkey/api/ldk/table/AbstractDataDefinedTable.java @@ -57,7 +57,7 @@ * Date: 4/2/13 * Time: 2:54 PM */ -abstract public class AbstractDataDefinedTable extends CustomPermissionsTable +abstract public class AbstractDataDefinedTable extends CustomPermissionsTable { protected String _pk; @@ -65,7 +65,7 @@ abstract public class AbstractDataDefinedTable extends CustomPermissionsTable protected String _filterValue; protected String _valueColumn; - public AbstractDataDefinedTable(UserSchema schema, SchemaTableInfo table, ContainerFilter cf, String filterColumn, String valueColumn, String tableName, String filterValue) + public AbstractDataDefinedTable(SchemaType schema, SchemaTableInfo table, ContainerFilter cf, String filterColumn, String valueColumn, String tableName, String filterValue) { super(schema, table, cf); _filterColumn = filterColumn; @@ -77,7 +77,7 @@ public AbstractDataDefinedTable(UserSchema schema, SchemaTableInfo table, Contai } @Override - public CustomPermissionsTable init() + public CustomPermissionsTable init() { super.init(); @@ -85,7 +85,7 @@ public CustomPermissionsTable init() addCondition(col, _filterValue); //enforce only showing rows from this category List pks = getRealTable().getPkColumnNames(); - assert pks.size() > 0; + assert !pks.isEmpty(); _pk = pks.get(0); var valueCol = getMutableColumn(_valueColumn); @@ -93,7 +93,7 @@ public CustomPermissionsTable init() valueCol.setKeyField(true); valueCol.setNullable(false); - getMutableColumn(_pk).setKeyField(false); + getMutableColumnOrThrow(_pk).setKeyField(false); ColumnInfo filterCol = getColumn(_filterColumn); assert filterCol != null; @@ -135,7 +135,7 @@ protected class UpdateService extends SimpleQueryUpdateService { private final ValuesManager _vm; - public UpdateService(SimpleUserSchema.SimpleTable ti) + public UpdateService(SimpleUserSchema.SimpleTable ti) { super(ti, ti.getRealTable()); @@ -217,8 +217,7 @@ public boolean testIfRowExists(String value) { boolean ret = _distinctValues.contains(value); - if (!_distinctValues.contains(value)) - _distinctValues.add(value); + _distinctValues.add(value); return ret; } From 46d4ca692b087b145bb1037ea7f3aa50cd0a131a Mon Sep 17 00:00:00 2001 From: bbimber Date: Thu, 20 Mar 2025 14:32:36 -0700 Subject: [PATCH 09/13] Add workflow_dispatch --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 461b2a0e..5816605b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,5 +1,6 @@ name: Build DISCVR on: + workflow_dispatch: push: pull_request: jobs: From cfaea0d635fdcb31a481c157437899204e7422f4 Mon Sep 17 00:00:00 2001 From: bbimber Date: Thu, 20 Mar 2025 15:03:16 -0700 Subject: [PATCH 10/13] Restore this for 24.11 --- .../labkey/test/tests/external/labModules/LabModulesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java index 2aa55a16..7503a70a 100644 --- a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java +++ b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java @@ -1429,7 +1429,7 @@ private void samplesTableTest() throws Exception String val = row.get(name) == null ? "" : String.valueOf(row.get(name)); if (name.toLowerCase().contains("date")) { - val = StringUtils.isEmpty(val) ? "" : dateFormat.format(Date.parse(val)); + val = StringUtils.isEmpty(val) ? "" : ExcelHelper.getDateTimeFormat().format(Date.parse(val)); } target.add(val); From 25616200a46be94bf83b26b7a2015900eae0fa43 Mon Sep 17 00:00:00 2001 From: bbimber Date: Thu, 20 Mar 2025 20:32:51 -0700 Subject: [PATCH 11/13] Debug test failure --- .../labkey/test/tests/external/labModules/LabModulesTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java index 7503a70a..42687997 100644 --- a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java +++ b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java @@ -1444,8 +1444,8 @@ private void samplesTableTest() throws Exception List> lines = ExcelHelper.getFirstNRows(sheet, 5); Assert.assertEquals(columnLabels, lines.get(0)); - Assert.assertEquals(rows.get(0), lines.get(1)); - Assert.assertEquals(rows.get(0), lines.get(2)); + Assert.assertEquals("Row did not match. ExcelHelper pattern: " + ExcelHelper.getDateTimeFormat().toPattern(), rows.get(0), lines.get(1)); + Assert.assertEquals("Row did not match. ExcelHelper pattern: " + ExcelHelper.getDateTimeFormat().toPattern(), rows.get(0), lines.get(2)); Assert.assertEquals(rows.get(1), lines.get(3)); Assert.assertEquals(rows.get(1), lines.get(4)); } From 826535db4e8c8bc0f07471538a0f59f1221a6c73 Mon Sep 17 00:00:00 2001 From: bbimber Date: Tue, 25 Mar 2025 15:04:45 -0700 Subject: [PATCH 12/13] Debug test failure --- .../labkey/test/tests/external/labModules/LabModulesTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java index 42687997..11474b14 100644 --- a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java +++ b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java @@ -1446,8 +1446,8 @@ private void samplesTableTest() throws Exception Assert.assertEquals(columnLabels, lines.get(0)); Assert.assertEquals("Row did not match. ExcelHelper pattern: " + ExcelHelper.getDateTimeFormat().toPattern(), rows.get(0), lines.get(1)); Assert.assertEquals("Row did not match. ExcelHelper pattern: " + ExcelHelper.getDateTimeFormat().toPattern(), rows.get(0), lines.get(2)); - Assert.assertEquals(rows.get(1), lines.get(3)); - Assert.assertEquals(rows.get(1), lines.get(4)); + Assert.assertEquals("Row did not match. ExcelHelper pattern: " + ExcelHelper.getDateTimeFormat().toPattern(), rows.get(1), lines.get(3)); + Assert.assertEquals("Row did not match. ExcelHelper pattern: " + ExcelHelper.getDateTimeFormat().toPattern(), rows.get(1), lines.get(4)); } refresh(); From 0956b83582a309b228e7f45f0df89a2cb9b238c0 Mon Sep 17 00:00:00 2001 From: bbimber Date: Wed, 26 Mar 2025 00:02:20 -0700 Subject: [PATCH 13/13] Debug test failure --- .../labkey/test/tests/external/labModules/LabModulesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java index 11474b14..59f673a4 100644 --- a/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java +++ b/LDK/test/src/org/labkey/test/tests/external/labModules/LabModulesTest.java @@ -1429,7 +1429,7 @@ private void samplesTableTest() throws Exception String val = row.get(name) == null ? "" : String.valueOf(row.get(name)); if (name.toLowerCase().contains("date")) { - val = StringUtils.isEmpty(val) ? "" : ExcelHelper.getDateTimeFormat().format(Date.parse(val)); + val = StringUtils.isEmpty(val) ? "" : dateFormat.format(Date.parse(val)); } target.add(val);