From d9d87786ed1a8d3741a212dde0ef1e49a7dd3002 Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 7 Apr 2026 13:36:28 -0500 Subject: [PATCH 1/4] GitHub Issue #688: use schema getTable() for materialIdForeignKey instead of custom createLookupTableInfo() --- .../org/labkey/api/exp/query/SamplesSchema.java | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/api/src/org/labkey/api/exp/query/SamplesSchema.java b/api/src/org/labkey/api/exp/query/SamplesSchema.java index c1e7412b208..004258612d2 100644 --- a/api/src/org/labkey/api/exp/query/SamplesSchema.java +++ b/api/src/org/labkey/api/exp/query/SamplesSchema.java @@ -255,21 +255,7 @@ public ForeignKey materialIdForeignKey(@Nullable final ExpSampleType st, @Nullab { ContainerFilter cf = getLookupContainerFilter(); String cacheKey = SamplesSchema.class.getName() + "/" + schemaName + "/" + tableName + "/" + (null==st ? "" : st.getMaterialLSIDPrefix()) + "/" + (null==domainProperty ? "" : domainProperty.getPropertyURI()) + cf.getCacheKey(); - return SamplesSchema.this.getCachedLookupTableInfo(cacheKey, this::createLookupTableInfo); - } - - private TableInfo createLookupTableInfo() - { - ExpMaterialTable ret = ExperimentService.get().createMaterialTable(SamplesSchema.this, getLookupContainerFilter(), st); - ret.populate(); - ret.overlayMetadata(ret.getPublicName(), SamplesSchema.this, new ArrayList<>()); - if (domainProperty != null && domainProperty.getPropertyType().getJdbcType().isText()) - { - // Hack to support lookup via RowId or Name - _columnName = "Name"; - } - ret.setLocked(true); - return ret; + return SamplesSchema.this.getCachedLookupTableInfo(cacheKey, () -> getTable(tableName, cf)); } @Override From f58b6213a9c27356a1082db41afdcfdd221877ce Mon Sep 17 00:00:00 2001 From: cnathe Date: Wed, 8 Apr 2026 09:46:27 -0500 Subject: [PATCH 2/4] Fix for failing selenium tests - revert setting of _columnName --- api/src/org/labkey/api/exp/query/SamplesSchema.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/exp/query/SamplesSchema.java b/api/src/org/labkey/api/exp/query/SamplesSchema.java index 004258612d2..a35546ec291 100644 --- a/api/src/org/labkey/api/exp/query/SamplesSchema.java +++ b/api/src/org/labkey/api/exp/query/SamplesSchema.java @@ -255,7 +255,16 @@ public ForeignKey materialIdForeignKey(@Nullable final ExpSampleType st, @Nullab { ContainerFilter cf = getLookupContainerFilter(); String cacheKey = SamplesSchema.class.getName() + "/" + schemaName + "/" + tableName + "/" + (null==st ? "" : st.getMaterialLSIDPrefix()) + "/" + (null==domainProperty ? "" : domainProperty.getPropertyURI()) + cf.getCacheKey(); - return SamplesSchema.this.getCachedLookupTableInfo(cacheKey, () -> getTable(tableName, cf)); + return SamplesSchema.this.getCachedLookupTableInfo(cacheKey, this::createLookupTableInfo); + } + + private TableInfo createLookupTableInfo() + { + // Hack to support lookup via RowId or Name + if (domainProperty != null && domainProperty.getPropertyType().getJdbcType().isText()) + _columnName = "Name"; + + return getTable(tableName, getLookupContainerFilter()); } @Override From 9b3a639bd18655910fd00b118bcb2b893da32a2a Mon Sep 17 00:00:00 2001 From: cnathe Date: Wed, 8 Apr 2026 14:24:27 -0500 Subject: [PATCH 3/4] AssayTest case for GitHub Issue #688 to verify sample lookup createdBy resolved value --- .../labkey/test/tests/study/AssayTest.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/study/test/src/org/labkey/test/tests/study/AssayTest.java b/study/test/src/org/labkey/test/tests/study/AssayTest.java index 2e177b59bd0..a1285d1436f 100644 --- a/study/test/src/org/labkey/test/tests/study/AssayTest.java +++ b/study/test/src/org/labkey/test/tests/study/AssayTest.java @@ -279,7 +279,6 @@ public void testSampleFieldUpdate() clickAndWait(Locator.linkWithText("view results")); assertElementPresent("Sample lookup failed for: OS_1", new Locator.LinkLocator("OS_1"), 1); - log("Edit assay design and change Sample field to point to created Sample Type"); goToManageAssays(); clickAndWait(Locator.LinkLocator.linkWithText(SAMPLE_FIELD_TEST_ASSAY)); @@ -294,10 +293,21 @@ public void testSampleFieldUpdate() importAssayData(SAMPLE_FIELD_TEST_ASSAY, TEST_RUN2, "SampleField\nS_1"); goToManageAssays().clickAndWait(Locator.linkWithText(SAMPLE_FIELD_TEST_ASSAY)); clickAndWait(Locator.linkWithText("view results")); -// assertElementPresent("Sample lookup failed for: OS_1", new Locator.LinkLocator("OS_1"), 1); //TODO this becomes the RowId "<123>" after change. Issue #40047 - + DataRegionTable table = new DataRegionTable("Data", getDriver()); + List sampleFieldValues = table.getColumnDataAsText("SampleField"); + assertTrue("First sample should not resolve to sample type", sampleFieldValues.get(0).startsWith("<")); + assertEquals("Second sample should resolve to sample type", "S_1", sampleFieldValues.get(1)); assertElementPresent("Sample lookup failed for: S_1", new Locator.LinkLocator("S_1"), 1); + log("GitHub Issue #688: verify sample lookup to createdBy"); + _customizeViewsHelper.openCustomizeViewPanel(); + _customizeViewsHelper.addColumn("SampleField/CreatedBy"); + _customizeViewsHelper.applyCustomView(); + table = new DataRegionTable("Data", getDriver()); + List createdByValues = table.getColumnDataAsText("SampleField/CreatedBy"); + assertEquals("First sample should not have a createdBy since it doesn't resolve", " ", createdByValues.get(0)); + assertEquals("Second sample should have a createdBy since it resolves to a sample type", getCurrentUserName(), createdByValues.get(1)); + log("Edit assay design and change Sample field to point back to 'All Samples'"); goToManageAssays(); clickAndWait(Locator.LinkLocator.linkWithText(SAMPLE_FIELD_TEST_ASSAY)); @@ -319,6 +329,10 @@ public void testSampleFieldUpdate() assertElementPresent("Sample lookup failed for: S_2", new Locator.LinkLocator("S_2"), 1); assertElementPresent("Sample lookup failed for: OS_2", new Locator.LinkLocator("OS_2"), 1); + log("GitHub Issue #688: verify sample lookup to createdBy"); + table = new DataRegionTable("Data", getDriver()); + for (int i = 0; i < table.getDataRowCount(); i++) + assertEquals("Row " + i + " should have current user as createdBy since they all resolve to samples", getCurrentUserName(), table.getDataAsText(i, "SampleField/CreatedBy")); } private void importAssayData(String assayName, String runName, String runDataStr) From f19993954d0c49e3067c30dcb3b4e0556e48b323 Mon Sep 17 00:00:00 2001 From: cnathe Date: Wed, 8 Apr 2026 15:11:07 -0500 Subject: [PATCH 4/4] claude code feedback - restore createMaterialTable() for st == null case --- api/src/org/labkey/api/exp/query/SamplesSchema.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/exp/query/SamplesSchema.java b/api/src/org/labkey/api/exp/query/SamplesSchema.java index a35546ec291..0f4149ce05d 100644 --- a/api/src/org/labkey/api/exp/query/SamplesSchema.java +++ b/api/src/org/labkey/api/exp/query/SamplesSchema.java @@ -264,7 +264,15 @@ private TableInfo createLookupTableInfo() if (domainProperty != null && domainProperty.getPropertyType().getJdbcType().isText()) _columnName = "Name"; - return getTable(tableName, getLookupContainerFilter()); + // GitHub Issue #688 + if (st != null) + return getTable(tableName, getLookupContainerFilter()); + + ExpMaterialTable ret = ExperimentService.get().createMaterialTable(SamplesSchema.this, getLookupContainerFilter(), st); + ret.populate(); + ret.overlayMetadata(ret.getPublicName(), SamplesSchema.this, new ArrayList<>()); + ret.setLocked(true); + return ret; } @Override