Merge 26.7 to develop

labkey-teamcity · labkey-teamcity · commit 866ccde89a56 · 2026-07-08T05:50:59.000Z
diff --git a/onprc_billing/src/org/labkey/onprc_billing/ONPRC_BillingController.java b/onprc_billing/src/org/labkey/onprc_billing/ONPRC_BillingController.java
@@ -162,7 +162,7 @@ public ModelAndView getConfirmView(QueryForm form, BindException errors) throws
             Set<String> ids = DataRegionSelection.getSelected(form.getViewContext(), true);
 
             StringBuilder msg = new StringBuilder("You have selected " + ids.size() + " billing runs to delete.  This will also delete: <p>");
-            for (String m : ONPRC_BillingManager.get().deleteBillingRuns(getUser(), ids, true))
+            for (String m : ONPRC_BillingManager.get().deleteBillingRuns(getUser(), getContainer(), ids, true))
             {
                 msg.append(m).append("<br>");
             }
@@ -176,7 +176,7 @@ public ModelAndView getConfirmView(QueryForm form, BindException errors) throws
         public boolean handlePost(QueryForm form, BindException errors) throws Exception
         {
             Set<String> ids = DataRegionSelection.getSelected(form.getViewContext(), true);
-            ONPRC_BillingManager.get().deleteBillingRuns(getUser(), ids, false);
+            ONPRC_BillingManager.get().deleteBillingRuns(getUser(), getContainer(), ids, false);
 
             return true;
         }
diff --git a/onprc_billing/src/org/labkey/onprc_billing/ONPRC_BillingManager.java b/onprc_billing/src/org/labkey/onprc_billing/ONPRC_BillingManager.java
@@ -15,6 +15,10 @@
  */
 package org.labkey.onprc_billing;
 
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 import org.labkey.api.collections.CaseInsensitiveHashMap;
 import org.labkey.api.data.CompareType;
 import org.labkey.api.data.Container;
@@ -31,12 +35,19 @@
 import org.labkey.api.query.FieldKey;
 import org.labkey.api.query.Queryable;
 import org.labkey.api.security.User;
+import org.labkey.api.util.GUID;
+import org.labkey.api.util.JunitUtil;
+import org.labkey.api.util.TestContext;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * User: bimber
@@ -71,16 +82,28 @@ public static ONPRC_BillingManager get()
         return _instance;
     }
 
-    public List<String> deleteBillingRuns(User user, Collection<String> pks, boolean testOnly)
+    public List<String> deleteBillingRuns(User user, Container container, Collection<String> pks, boolean testOnly)
     {
         TableInfo invoiceRuns = ONPRC_BillingSchema.getInstance().getSchema().getTable(ONPRC_BillingSchema.TABLE_INVOICE_RUNS);
         TableInfo invoicedItems = ONPRC_BillingSchema.getInstance().getSchema().getTable(ONPRC_BillingSchema.TABLE_INVOICED_ITEMS);
         TableInfo miscCharges = ONPRC_BillingSchema.getInstance().getSchema().getTable(ONPRC_BillingSchema.TABLE_MISC_CHARGES);
 
         //create filters
-        SimpleFilter invoiceRunFilter = new SimpleFilter(FieldKey.fromString("invoiceId"), pks, CompareType.IN);
+        SimpleFilter invoiceRunsFilter = createContainerScopedInFilter(container, "objectid", pks);
+        Set<String> invoiceRunIds = getInvoiceRunIds(invoiceRuns, invoiceRunsFilter);
+        if (invoiceRunIds.isEmpty())
+        {
+            List<String> ret = new ArrayList<>();
+            if (testOnly)
+            {
+                ret.add("0 records from invoiced items");
+                ret.add("0 records from misc charges will be removed from the deleted invoice, which means they will be picked up by the next billing period.  They are not deleted.");
+            }
+            return ret;
+        }
 
-        SimpleFilter miscChargesFilter = new SimpleFilter(FieldKey.fromString("invoiceId"), pks, CompareType.IN);
+        SimpleFilter invoiceRunFilter = createContainerScopedInFilter(container, "invoiceId", invoiceRunIds);
+        SimpleFilter miscChargesFilter = createMiscChargesFilter(invoiceRunIds);
 
         //perform the work
         List<String> ret = new ArrayList<>();
@@ -96,7 +119,7 @@ public List<String> deleteBillingRuns(User user, Collection<String> pks, boolean
         {
             try (DbScope.Transaction transaction = ExperimentService.get().ensureTransaction())
             {
-                long deleted1 = Table.delete(invoicedItems, invoiceRunFilter);
+                Table.delete(invoicedItems, invoiceRunFilter);
 
                 TableSelector tsMiscCharges2 = new TableSelector(miscCharges, Collections.singleton("objectid"), miscChargesFilter, null);
                 String[] miscChargesIds = tsMiscCharges2.getArray(String.class);
@@ -107,7 +130,7 @@ public List<String> deleteBillingRuns(User user, Collection<String> pks, boolean
                     Table.update(user, miscCharges, map, objectid);
                 }
 
-                long deleted3 = Table.delete(invoiceRuns, new SimpleFilter(FieldKey.fromString("objectid"), pks, CompareType.IN));
+                Table.delete(invoiceRuns, invoiceRunsFilter);
 
                 transaction.commit();
             }
@@ -116,6 +139,33 @@ public List<String> deleteBillingRuns(User user, Collection<String> pks, boolean
         return ret;
     }
 
+    private SimpleFilter createContainerScopedInFilter(Container container, String columnName, Collection<String> values)
+    {
+        return SimpleFilter.createContainerFilter(container).addInClause(FieldKey.fromString(columnName), values);
+    }
+
+    private Set<String> getInvoiceRunIds(TableInfo invoiceRuns, SimpleFilter objectIdFilter)
+    {
+        TableSelector tsInvoiceRuns = new TableSelector(invoiceRuns, Collections.singleton("objectid"), objectIdFilter, null);
+        String[] invoiceRunIds = tsInvoiceRuns.getArray(String.class);
+        return new HashSet<>(Arrays.asList(invoiceRunIds));
+    }
+
+    private SimpleFilter createMiscChargesFilter(Collection<String> invoiceRunIds)
+    {
+        // Intentionally NOT container-scoped. Source miscCharges records can live in different containers
+        // (the billing/finance container, the configured EHR study container, or other satellite containers that
+        // feed charges into a billing run), so there is no reliable, complete set of "source" containers to filter on.
+        //
+        // This is not a cross-container security issue: invoiceRunIds has already been narrowed by getInvoiceRunIds()
+        // to the run ids that actually exist in the requesting container (see the container-scoped objectIdFilter).
+        // A forged or out-of-container run id never reaches this filter, so matching miscCharges solely by
+        // invoiceId only ever touches charges belonging to runs the caller is already authorized to delete.
+        SimpleFilter filter = new SimpleFilter();
+        filter.addInClause(FieldKey.fromString("invoiceId"), invoiceRunIds);
+        return filter;
+    }
+
     public Container getBillingContainer(Container c)
     {
         Module billing = ModuleLoader.getInstance().getModule(ONPRC_BillingModule.NAME);
@@ -139,4 +189,145 @@ public Container getSLADataFolder(Container c)
         return ContainerManager.getForPath(path);
 
     }
+
+    public static class TestCase extends Assert
+    {
+        private static final String FOLDER_A = "ONPRCBillingDeleteTestA";
+        private static final String FOLDER_B = "ONPRCBillingDeleteTestB";
+        private static final String FOLDER_SATELLITE = "ONPRCBillingDeleteTestSatellite";
+
+        private User _user;
+        private Container _containerA;
+        private Container _containerB;
+        private Container _containerSatellite;
+        private String _runIdA;
+        private String _runIdB;
+
+        @Before
+        public void setUp()
+        {
+            _user = TestContext.get().getUser();
+            deleteTestFolders();
+
+            Container junit = JunitUtil.getTestContainer();
+            _containerA = createBillingFolder(junit, FOLDER_A);
+            _containerB = createBillingFolder(junit, FOLDER_B);
+            _containerSatellite = createBillingFolder(junit, FOLDER_SATELLITE);
+
+            _runIdA = insertBillingRun(_containerA);
+            _runIdB = insertBillingRun(_containerB);
+        }
+
+        @After
+        public void tearDown()
+        {
+            deleteTestFolders();
+        }
+
+        @Test
+        public void testDeleteBillingRunsIsContainerScoped()
+        {
+            ONPRC_BillingManager manager = ONPRC_BillingManager.get();
+            ONPRC_BillingSchema schema = ONPRC_BillingSchema.getInstance();
+
+            // A testOnly preview issued from container A targeting container B's run must not see container B's rows.
+            for (String summary : manager.deleteBillingRuns(_user, _containerA, List.of(_runIdB), true))
+                assertTrue("Preview from another container should count 0 rows, but got: " + summary, summary.startsWith("0 "));
+
+            // An actual delete issued from container A targeting container B's run must leave container B untouched.
+            manager.deleteBillingRuns(_user, _containerA, List.of(_runIdB), false);
+            assertEquals("invoiceRuns row in container B should survive a delete issued from container A", 1, containerRowCount(getTable(schema, ONPRC_BillingSchema.TABLE_INVOICE_RUNS), _containerB));
+            assertEquals("invoicedItems row in container B should survive a delete issued from container A", 1, containerRowCount(getTable(schema, ONPRC_BillingSchema.TABLE_INVOICED_ITEMS), _containerB));
+            assertEquals("miscCharges row in container B should still reference its invoice", 1, miscChargesWithInvoiceCount(_containerB));
+
+            // Satellite source data: miscCharges can live in a container that is neither the requesting/finance
+            // container nor the run's own container. The delete is keyed off the authorized run id, so these rows are
+            // still previewed and detached.
+            String runIdWithSatelliteMiscCharge = insertBillingRun(_containerA, _containerSatellite);
+            List<String> satellitePreview = manager.deleteBillingRuns(_user, _containerA, List.of(runIdWithSatelliteMiscCharge), true);
+            assertTrue("Preview should count miscCharges rows in an unrelated source container: " + satellitePreview,
+                    satellitePreview.stream().anyMatch(summary -> summary.startsWith("1 records from misc charges")));
+
+            manager.deleteBillingRuns(_user, _containerA, List.of(runIdWithSatelliteMiscCharge), false);
+            assertEquals("miscCharges row in the satellite source container should be detached from the deleted invoice", 0, miscChargesWithInvoiceCount(_containerSatellite));
+            assertEquals("miscCharges row in the satellite source container should not be deleted", 1, containerRowCount(getTable(schema, ONPRC_BillingSchema.TABLE_MISC_CHARGES), _containerSatellite));
+
+            // Positive control: deleting a run from its own container removes its rows.
+            manager.deleteBillingRuns(_user, _containerA, List.of(_runIdA), false);
+            assertEquals("invoiceRuns row in container A should be deleted", 0, containerRowCount(getTable(schema, ONPRC_BillingSchema.TABLE_INVOICE_RUNS), _containerA));
+            assertEquals("invoicedItems row in container A should be deleted", 0, containerRowCount(getTable(schema, ONPRC_BillingSchema.TABLE_INVOICED_ITEMS), _containerA));
+            assertEquals("miscCharges row in container A should be detached from the deleted invoice", 0, miscChargesWithInvoiceCount(_containerA));
+            assertEquals("miscCharges row in container A should not be deleted", 1, containerRowCount(getTable(schema, ONPRC_BillingSchema.TABLE_MISC_CHARGES), _containerA));
+        }
+
+        private Container createBillingFolder(Container parent, String name)
+        {
+            Container c = ContainerManager.createContainer(parent, name, _user);
+            Set<Module> active = new HashSet<>(c.getActiveModules());
+            active.add(ModuleLoader.getInstance().getModule(ONPRC_BillingModule.NAME));
+            c.setActiveModules(active, _user);
+            return c;
+        }
+
+        private String insertBillingRun(Container c)
+        {
+            return insertBillingRun(c, c);
+        }
+
+        private String insertBillingRun(Container billingContainer, Container miscChargesContainer)
+        {
+            ONPRC_BillingSchema schema = ONPRC_BillingSchema.getInstance();
+            String runId = GUID.makeGUID();
+
+            Map<String, Object> run = new CaseInsensitiveHashMap<>();
+            run.put("objectid", runId);
+            run.put("runDate", new Date());
+            run.put("billingPeriodStart", new Date());
+            run.put("billingPeriodEnd", new Date());
+            run.put("container", billingContainer.getId());
+            Table.insert(_user, getTable(schema, ONPRC_BillingSchema.TABLE_INVOICE_RUNS), run);
+
+            Map<String, Object> invoicedItem = new CaseInsensitiveHashMap<>();
+            invoicedItem.put("objectid", GUID.makeGUID());
+            invoicedItem.put("invoiceId", runId);
+            invoicedItem.put("container", billingContainer.getId());
+            Table.insert(_user, getTable(schema, ONPRC_BillingSchema.TABLE_INVOICED_ITEMS), invoicedItem);
+
+            Map<String, Object> miscCharge = new CaseInsensitiveHashMap<>();
+            miscCharge.put("objectid", GUID.makeGUID());
+            miscCharge.put("invoiceId", runId);
+            miscCharge.put("container", miscChargesContainer.getId());
+            Table.insert(_user, getTable(schema, ONPRC_BillingSchema.TABLE_MISC_CHARGES), miscCharge);
+
+            return runId;
+        }
+
+        private TableInfo getTable(ONPRC_BillingSchema schema, String tableName)
+        {
+            return schema.getSchema().getTable(tableName);
+        }
+
+        private long containerRowCount(TableInfo table, Container c)
+        {
+            return new TableSelector(table, SimpleFilter.createContainerFilter(c), null).getRowCount();
+        }
+
+        private long miscChargesWithInvoiceCount(Container c)
+        {
+            SimpleFilter filter = SimpleFilter.createContainerFilter(c);
+            filter.addCondition(FieldKey.fromParts("invoiceId"), null, CompareType.NONBLANK);
+            return new TableSelector(getTable(ONPRC_BillingSchema.getInstance(), ONPRC_BillingSchema.TABLE_MISC_CHARGES), filter, null).getRowCount();
+        }
+
+        private void deleteTestFolders()
+        {
+            Container junit = JunitUtil.getTestContainer();
+            for (String name : List.of(FOLDER_A, FOLDER_B, FOLDER_SATELLITE))
+            {
+                Container c = junit.getChild(name);
+                if (c != null)
+                    ContainerManager.delete(c, _user);
+            }
+        }
+    }
 }
diff --git a/onprc_billing/src/org/labkey/onprc_billing/ONPRC_BillingModule.java b/onprc_billing/src/org/labkey/onprc_billing/ONPRC_BillingModule.java
@@ -166,6 +166,13 @@ public Set<String> getSchemaNames()
         return Collections.singleton(ONPRC_BillingSchema.NAME);
     }
 
+    @Override
+    @NotNull
+    public Set<Class<?>> getIntegrationTests()
+    {
+        return Collections.singleton(ONPRC_BillingManager.TestCase.class);
+    }
+
     @Override
     protected void registerSchemas()
     {
diff --git a/onprc_ehr/resources/queries/study/ParentageDamMismatch.sql b/onprc_ehr/resources/queries/study/ParentageDamMismatch.sql
@@ -18,49 +18,64 @@
  * 6. Excludes old parentage entries, enddate IS BLANK
 */
 
+/*
+ Modifications by Kolli, June 2026
+ Refer tkt# 14891 for details
+
+ * 1. Remove cases in which the ID has a surrogate dam entered in prime. (There are several but 41043 is one example)
+ * 2. Remove cases in which the observed dam is "unknown"
+ * 3. Remove cases in which the observed dam ID is not an ONPRC. E.g. there are several NEPRC IDs of the format ###-####.
+ */
+
 SELECT
     d.Id,
     d.Id.curLocation.area AS Area,
-    coalesce(p2.parent, '') as geneticDam,
-    coalesce(b.dam, '') as observedDam
+    gd.geneticDam,
+    b.dam AS observedDam
+
 FROM study.demographics d
 
-    LEFT JOIN (
+LEFT JOIN (
     SELECT
-        p2.Id,
-        MAX(p2.parent) AS parent
-    FROM study.parentage p2
-    WHERE (p2.method = 'Genetic' OR p2.method = 'Provisional Genetic')
-      AND p2.relationship = 'Dam'
-      AND p2.enddate IS NULL
-    GROUP BY p2.Id
-) p2 ON d.Id = p2.Id
+        p.Id,
+        MAX(p.parent) AS geneticDam
+    FROM study.parentage p
+    WHERE p.relationship = 'Dam'
+      AND p.method IN ('Genetic', 'Provisional Genetic')
+      AND p.enddate IS NULL
+    GROUP BY p.Id
+) gd
+    ON gd.Id = d.Id
 
-    LEFT JOIN (
-    SELECT
-        p3.Id,
-        MAX(p3.parent) AS parent
-    FROM study.parentage p3
-    WHERE p3.relationship = 'Foster Dam'
-      AND p3.enddate IS NULL
-    GROUP BY p3.Id
-) p3 ON d.Id = p3.Id
+LEFT JOIN study.birth b
+    ON b.Id = d.Id
 
-    LEFT JOIN study.birth b
-        ON b.Id = d.Id
+WHERE d.calculated_status.code IN ('Alive', 'Dead')
+  AND d.qcstate = 18
 
-WHERE d.calculated_status.code IN ('Alive', 'Dead') AND d.qcstate = 18
-    /* exclude foster-dam cases (NULL or blank only) */
-  AND COALESCE(RTRIM(LTRIM(CAST(p3.parent AS VARCHAR(50)))), '') = ''
+    /* exclude animals with active Foster Dam  */
+    /* And, exclude animals with active Surrogate Dam */
+  AND NOT EXISTS (
+    SELECT 1
+    FROM study.parentage fd
+    WHERE fd.Id = d.Id
+      AND fd.relationship IN ('Foster Dam', 'Surrogate Dam')
+      AND fd.enddate IS NULL
+      AND COALESCE(RTRIM(LTRIM(CAST(fd.parent AS VARCHAR(50)))), '') <> ''
+  )
 
-    /* exclude blank observed dam */
+    /* observed dam is not blank */
   AND COALESCE(RTRIM(LTRIM(CAST(b.dam AS VARCHAR(50)))), '') <> ''
 
-    /* exclude blank genetic dam */
-  AND COALESCE(RTRIM(LTRIM(CAST(p2.parent AS VARCHAR(50)))), '') <> ''
+    /* observed dam is not unknown */
+  AND LOWER(COALESCE(RTRIM(LTRIM(CAST(b.dam AS VARCHAR(50)))), '')) <> 'unknown'
 
-    /* mismatch observed vs genetic */
-  AND COALESCE(RTRIM(LTRIM(CAST(b.dam AS VARCHAR(50)))), '') <>
-      COALESCE(RTRIM(LTRIM(CAST(p2.parent AS VARCHAR(50)))), '')
+    /* observed dam is an ONPRC ID, not external format like ###-#### */
+  AND COALESCE(RTRIM(LTRIM(CAST(b.dam AS VARCHAR(50)))), '') NOT LIKE '%-%'
 
+    /* genetic dam is not blank */
+  AND COALESCE(RTRIM(LTRIM(CAST(gd.geneticDam AS VARCHAR(50)))), '') <> ''
 
+    /* observed dam does not match genetic dam */
+  AND COALESCE(RTRIM(LTRIM(CAST(b.dam AS VARCHAR(50)))), '') <>
+      COALESCE(RTRIM(LTRIM(CAST(gd.geneticDam AS VARCHAR(50)))), '');
diff --git a/onprc_ehr/src/org/labkey/onprc_ehr/notification/ColonyAlertsNotification.java b/onprc_ehr/src/org/labkey/onprc_ehr/notification/ColonyAlertsNotification.java

Original file line number	Diff line number	Diff line change
`@@ -162,7 +162,7 @@ public ModelAndView getConfirmView(QueryForm form, BindException errors) throws`
`162`	`162`	`Set<String> ids = DataRegionSelection.getSelected(form.getViewContext(), true);`
`163`	`163`
`164`	`164`	`StringBuilder msg = new StringBuilder("You have selected " + ids.size() + " billing runs to delete. This will also delete: <p>");`
`165`		`- for (String m : ONPRC_BillingManager.get().deleteBillingRuns(getUser(), ids, true))`
	`165`	`+ for (String m : ONPRC_BillingManager.get().deleteBillingRuns(getUser(), getContainer(), ids, true))`
`166`	`166`	`{`
`167`	`167`	`msg.append(m).append("<br>");`
`168`	`168`	`}`
`@@ -176,7 +176,7 @@ public ModelAndView getConfirmView(QueryForm form, BindException errors) throws`
`176`	`176`	`public boolean handlePost(QueryForm form, BindException errors) throws Exception`
`177`	`177`	`{`
`178`	`178`	`Set<String> ids = DataRegionSelection.getSelected(form.getViewContext(), true);`
`179`		`- ONPRC_BillingManager.get().deleteBillingRuns(getUser(), ids, false);`
	`179`	`+ ONPRC_BillingManager.get().deleteBillingRuns(getUser(), getContainer(), ids, false);`
`180`	`180`
`181`	`181`	`return true;`
`182`	`182`	`}`
Original file line number	Diff line number	Diff line change
`@@ -166,6 +166,13 @@ public Set<String> getSchemaNames()`
`166`	`166`	`return Collections.singleton(ONPRC_BillingSchema.NAME);`
`167`	`167`	`}`
`168`	`168`
	`169`	`+ @Override`
	`170`	`+ @NotNull`
	`171`	`+ public Set<Class<?>> getIntegrationTests()`
	`172`	`+ {`
	`173`	`+ return Collections.singleton(ONPRC_BillingManager.TestCase.class);`
	`174`	`+ }`
	`175`	`+`
`169`	`176`	`@Override`
`170`	`177`	`protected void registerSchemas()`
`171`	`178`	`{`