x1559: Allow several sections to be put into one slot in sectioning

src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/ConfirmSectionServiceImp.java

@@ -183,20 +183,19 @@ public ConfirmLabwareResult confirmLabware(User user, ConfirmSectionLabware csl,
         Set<Slot> slotsToSave = new HashSet<>();
         List<Measurement> measurements = new ArrayList<>();
         List<OperationComment> opComs = new ArrayList<>();
-
         var planActionMap = getPlanActionMap(plan.getPlanActions(), lwId);
         List<Action> actions = new ArrayList<>(secs.size());
         for (ConfirmSection sec : secs) {
             for (Address ad : sec.getDestinationAddresses()) {
-                PlanAction pa = planActionMap.get(ad);
-                if (pa == null) {
-                    throw new IllegalArgumentException(String.format("No plan action found into %s slot %s.",
-                            lw.getBarcode(), ad));
+                PlanAction pa = planActionMap.get(new ActionKey(ad, sec.getSampleId()));
+                if (pa==null) {
+                    throw new IllegalArgumentException(String.format("No plan action found into %s slot %s from sample %s.",
+                            lw.getBarcode(), ad, sec.getSampleId()));
                 }
                 Slot slot = lw.getSlot(ad);
                 final Sample sample = getSection(sectionMap, sec, pa, slot);
                 Action action = makeAction(sample, pa, slot);
-                slot.getSamples().add(sample);
+                slot.addSample(sample);
                 slotsToSave.add(slot);
                 String thickness = thickness(sec, pa);
                 if (!nullOrEmpty(thickness)) {
@@ -255,7 +254,7 @@ public Action makeAction(Sample newSample, PlanAction pa, Slot slot) {
      * The sectionMap uses tissue (id), section number and bio state in its keys.
      * @param sectionMap a cache of existing sections to prevent dupes
      * @param sec the request pertaining to a single section
-     * @param pa the plan action for the source sample going into this slot
+     * @param pa the plan actions for the sample id going into this slot
      * @param slot the destination slot
      * @return the sample as specified
      */
@@ -345,18 +344,24 @@ private Stream<OperationComment> streamOpComments(AddressCommentId ac, Map<Integ
         return sampleIdStream.map(sampleId -> new OperationComment(null, comment, opId, sampleId, slotId, null));
     }
 
+    record ActionKey(Address address, int sampleId) {
+        ActionKey(PlanAction pa) {
+            this(pa.getDestination().getAddress(), pa.getSample().getId());
+        }
+    }
+
     /**
-     * Puts plan actions into a map from destination address.
+     * Puts plan actions into a map from destination address and source sample id.
      * Only those linked to the indicated labware will be included.
      * @param planActions the plan actions to put into a map
      * @param lwId the id of the labware whose plan actions we want to include
-     * @return a map from address to {@code PlanAction}
+     * @return a map from address/sample id to {@code PlanAction}s
      */
-    public Map<Address, PlanAction> getPlanActionMap(Collection<PlanAction> planActions, final int lwId) {
-        Map<Address, PlanAction> planActionMap = new HashMap<>(planActions.size());
+    Map<ActionKey, PlanAction> getPlanActionMap(Collection<PlanAction> planActions, final int lwId) {
+        Map<ActionKey, PlanAction> planActionMap = new HashMap<>(planActions.size());
         for (PlanAction pa : planActions) {
             if (pa.getDestination().getLabwareId()==lwId) {
-                planActionMap.putIfAbsent(pa.getDestination().getAddress(), pa);
+                planActionMap.put(new ActionKey(pa), pa);
             }
         }
         return planActionMap;

src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/ConfirmSectionValidationServiceImp.java

@@ -1,5 +1,6 @@
 package uk.ac.sanger.sccp.stan.service.operation.confirm;
 
+import org.jetbrains.annotations.NotNull;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.stereotype.Service;
@@ -52,7 +53,7 @@ public ConfirmSectionValidationServiceImp(LabwareRepo labwareRepo, PlanOperation
     public ConfirmSectionValidation validate(ConfirmSectionRequest request) {
         requireNonNull(request, "Request is null");
         final Set<String> problems = new LinkedHashSet<>();
-        if (request.getLabware()==null || request.getLabware().isEmpty()) {
+        if (nullOrEmpty(request.getLabware())) {
             problems.add("No labware specified in request.");
             return new ConfirmSectionValidation(problems);
         }
@@ -74,30 +75,33 @@ public ConfirmSectionValidation validate(ConfirmSectionRequest request) {
     }
 
     /**
-     * Checks that slots aren't assigned multiple sections
+     * Checks that slots aren't assigned duplicate actions
      * @param problems receptacle for problems found
      * @param csls the specification of each labware
      */
     public void checkRepeatedDestSlots(Collection<String> problems, Collection<ConfirmSectionLabware> csls) {
+        record ActionKey(Address address, int sampleId, String section) {
+            @NotNull
+            @Override
+            public String toString() {
+                return String.format("(slot=%s, sampleId=%d, section=%s)", address, sampleId, section);
+            }
+        }
         for (ConfirmSectionLabware csl : csls) {
             if (nullOrEmpty(csl.getBarcode())) {
                 continue; // Request is already broken, and we can't give meaningful problem messages
             }
-            Map<Address, Integer> sectionCount = new HashMap<>();
-
+            Set<ActionKey> seenActions = new HashSet<>();
             for (ConfirmSection cs : csl.getConfirmSections()) {
                 for (Address address : cs.getDestinationAddresses()) {
-                    if (address != null) {
-                        sectionCount.merge(address, 1, Integer::sum);
+                    if (address != null && cs.getSampleId() != null && !nullOrEmpty(cs.getNewSection())) {
+                        final ActionKey key = new ActionKey(address, cs.getSampleId(), cs.getNewSection());
+                        if (!seenActions.add(key)) {
+                            addProblem(problems, "Repeated action for destination %s: %s", csl.getBarcode(), key);
+                        }
                     }
                 }
             }
-            for (Map.Entry<Address, Integer> entry : sectionCount.entrySet()) {
-                if (entry.getValue() > 1) {
-                    problems.add(String.format("Multiple actions linked to destination %s %s.",
-                            csl.getBarcode(), entry.getKey()));
-                }
-            }
         }
     }
 

src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/SampleDescriber.java

@@ -10,7 +10,7 @@
 /**
  * Util to provide descriptions of samples used in ops.
  * <p>Usage: <code>"You have misused sample "+describer.describe(sampleId)+"."</code>
- * <br><code>You have misused sample 40 (section 8 of EXT12) from STAN-12 (A3).</code>
+ * <br><code>You have misused sample id 40 (section 8 of EXT12) from STAN-12 (A3).</code>
  * <p>Descriptions are cached.
  * @author dr6
  */

src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanServiceImp.java

@@ -258,6 +258,7 @@ public List<Labware> getSources(PlanOperation plan) {
     public void createSlotGroups(int planId, Labware lw, PlanRequestLabware prl) {
         List<List<Address>> addressGroups = prl.getActions().stream()
                 .map(PlanRequestAction::getAddresses)
+                .distinct()
                 .toList();
         slotGroupService.saveGroups(lw, planId, addressGroups);
     }

src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanValidationImp.java

@@ -205,6 +205,7 @@ public void validateDestinations(UCMap<Labware> sourceLabwareMap) {
             }
             validatePrebarcode(plw.getBarcode(), lt);
             checkActions(plw, lt);
+            checkSlotGroups(plw);
             if (gotBarcode && !alreadySeen && labwareRepo.existsByBarcode(plw.getBarcode())) {
                 addProblem("Labware with the barcode "+plw.getBarcode()+" already exists in the database.");
             } else if (gotBarcode && !alreadySeen && labwareRepo.existsByExternalBarcode(plw.getBarcode())) {
@@ -310,12 +311,22 @@ public OperationType validateOperation() {
         return opType;
     }
 
+    // We are going to disallow the same sample being transferred into the same slot
+    //  in several ways, because it is too complicated to handle
+    record SectionKey(int sourceSampleId, Address destAddress) {
+        static Stream<SectionKey> from(PlanRequestAction action) {
+            final int sourceSampleId = action.getSampleId();
+            return action.getAddresses().stream()
+                    .map(ad -> new SectionKey(sourceSampleId, ad));
+        }
+    }
+
     public void checkActions(PlanRequestLabware lw, LabwareType lt) {
         if (lw.getActions().isEmpty()) {
             addProblem("No actions specified for labware %s.", lwErrorDesc(lw));
             return;
         }
-        Set<Address> seenAddresses = new HashSet<>();
+        final Set<SectionKey> seenSectionKeys = new HashSet<>();
         for (PlanRequestAction ac : lw.getActions()) {
             if (nullOrEmpty(ac.getAddresses())) {
                 addProblem("Missing destination address.");
@@ -331,12 +342,40 @@ public void checkActions(PlanRequestLabware lw, LabwareType lt) {
                             invalidAddresses);
                 }
             }
-            for (Address ad : ac.getAddresses()) {
-                if (!seenAddresses.add(ad)) {
-                    addProblem("Actions for labware %s contains duplicate address: %s", lwErrorDesc(lw), ad);
+            SectionKey.from(ac).forEach(sk -> {
+                if (!seenSectionKeys.add(sk)) {
+                    addProblem("Duplicate actions transfer sample ID %s into slot %s of labware %s.",
+                            sk.sourceSampleId(), sk.destAddress(), lwErrorDesc(lw));
                 }
+            });
+        }
+    }
+
+    /** Looks for cases where slot groups are specified that overlap but do not match. */
+    void checkSlotGroups(PlanRequestLabware prl) {
+        if (nullOrEmpty(prl.getActions())) {
+            return;
+        }
+        final Set<Set<Address>> addressGroups = new HashSet<>();
+        boolean anyOverlaps = false;
+        for (PlanRequestAction ac : prl.getActions()) {
+            if (nullOrEmpty(ac.getAddresses())) {
+                continue;
+            }
+            Set<Address> addressGroup = new HashSet<>(ac.getAddresses());
+            Optional<Set<Address>> matchingGroup = addressGroups.stream()
+                    .filter(g -> !Collections.disjoint(addressGroup, g))
+                    .findAny();
+            if (matchingGroup.isEmpty()) {
+                addressGroups.add(addressGroup);
+            } else if (!matchingGroup.get().equals(addressGroup)) {
+                anyOverlaps = true;
+                break;
             }
         }
+        if (anyOverlaps) {
+            addProblem("There are overlapping slot groups given for labware "+lwErrorDesc(prl)+".");
+        }
     }
 
     /**
@@ -389,10 +428,10 @@ public boolean hasDividedLayout(UCMap<Labware> sourceLwMap, PlanRequestLabware l
     }
 
     private static String lwErrorDesc(PlanRequestLabware lw) {
-        if (lw.getBarcode()!=null && !lw.getBarcode().isEmpty()) {
+        if (!nullOrEmpty(lw.getBarcode())) {
             return lw.getBarcode();
         }
-        if (lw.getLabwareType()!=null && !lw.getLabwareType().isEmpty()) {
+        if (!nullOrEmpty(lw.getLabwareType())) {
             return "of type "+lw.getLabwareType();
         }
         return "of unspecified type";

src/test/java/uk/ac/sanger/sccp/stan/integrationtest/TestPlanAndRecordSectionMutations.java

@@ -9,8 +9,7 @@
 import uk.ac.sanger.sccp.stan.EntityCreator;
 import uk.ac.sanger.sccp.stan.GraphQLTester;
 import uk.ac.sanger.sccp.stan.model.*;
-import uk.ac.sanger.sccp.stan.repo.LabwareNoteRepo;
-import uk.ac.sanger.sccp.stan.repo.OperationCommentRepo;
+import uk.ac.sanger.sccp.stan.repo.*;
 
 import javax.persistence.EntityManager;
 import javax.transaction.Transactional;
@@ -42,6 +41,102 @@ public class TestPlanAndRecordSectionMutations {
     @Autowired
     private LabwareNoteRepo lwNoteRepo;
 
+    @Autowired
+    private LabwareTypeRepo ltRepo;
+
+    /** Section sectioning several block-samples into the same slot */
+    @Test
+    @Transactional
+    public void testPlanAndRecordSection_mult() throws Exception {
+        tester.setUser(entityCreator.createUser("dr6"));
+        LabwareType provLt = ltRepo.getByName("Proviasette");
+        Sample[] blockSamples = {
+                entityCreator.createBlockSample(entityCreator.createTissue(entityCreator.createDonor("DONOR1"), "TISSUE1")),
+                entityCreator.createBlockSample(entityCreator.createTissue(entityCreator.createDonor("DONOR2"), "TISSUE2")),
+        };
+        final int[] blockSampleIds = Arrays.stream(blockSamples).mapToInt(Sample::getId).toArray();
+        Labware sourceProv = entityCreator.createLabware("STAN-0001", provLt, new Sample[][] { blockSamples });
+        String mutation = tester.readGraphQL("plan_mult.graphql");
+        mutation = mutation.replace("55555", String.valueOf(blockSampleIds[0]));
+        mutation = mutation.replace("55556", String.valueOf(blockSampleIds[1]));
+        Map<String, ?> result = tester.post(mutation);
+        assertNoErrors(result);
+        Object resultPlan = chainGet(result, "data", "plan");
+        List<?> planResultLabware = chainGet(resultPlan, "labware");
+        assertEquals(1, planResultLabware.size());
+        String barcode = chainGet(planResultLabware, 0, "barcode");
+        testRetrievePlanData_mult(barcode, blockSampleIds);
+        testConfirm_mult(blockSampleIds, barcode, sourceProv);
+    }
+
+    private void testRetrievePlanData_mult(String barcode, int[] blockSampleIds) throws Exception {
+        String planQuery = tester.readGraphQL("plandata.graphql");
+        Map<String, ?> result = tester.post(planQuery.replace("$BARCODE", barcode));
+
+        Map<String, ?> planData = chainGet(result, "data", "planData");
+        List<List<String>> groups = chainGet(planData, "groups");
+        assertThat(groups).hasSize(1);
+        assertThat(groups.getFirst()).containsExactly("A1");
+        List<Map<String, ?>> planActionsData = chainGet(planData, "plan", "planActions");
+        assertThat(planActionsData).hasSize(2);
+        for (var paData : planActionsData) {
+            assertEquals("A1", chainGet(paData, "destination", "address"));
+            assertNotNull(chainGet(paData, "destination", "labwareId"));
+            assertEquals("A1", chainGet(paData, "source", "address"));
+        }
+        assertThat(planActionsData.stream()
+                .mapToInt(paData -> chainGet(paData, "sample", "id")))
+                .containsExactlyInAnyOrder(Arrays.stream(blockSampleIds).boxed().toArray(Integer[]::new));
+    }
+
+    void testConfirm_mult(int[] blockSampleIds, String barcode, Labware source) throws Exception {
+        Work work = entityCreator.createWork(null, null, null, null, null);
+        String mutation = tester.readGraphQL("confirmmult.graphql")
+                .replace("$BARCODE", barcode)
+                .replace("55555", String.valueOf(blockSampleIds[0]))
+                .replace("55556", String.valueOf(blockSampleIds[1]))
+                .replace("SGP4000", work.getWorkNumber())
+                ;
+        Object response = tester.post(mutation);
+        assertNoErrors(response);
+
+        List<Map<String, ?>> lwList = chainGet(response, "data", "confirmSection", "labware");
+        assertThat(lwList).hasSize(1);
+        Map<String, ?> lwData = lwList.getFirst();
+        assertEquals(barcode, lwData.get("barcode"));
+        List<Map<String, ?>> slotList = chainGetList(lwData, "slots");
+        assertThat(slotList).hasSize(1);
+        Map<String, ?> slotData = slotList.getFirst();
+        assertEquals("A1", slotData.get("address"));
+        List<Map<String, ?>> sampleList = chainGetList(slotData, "samples");
+        assertThat(sampleList).hasSize(2);
+        Map<String, ?> sampleData = sampleList.getFirst();
+        assertEquals("14", sampleData.get("section"));
+        assertEquals("TISSUE1", chainGet(sampleData, "tissue", "externalName"));
+        sampleData = sampleList.getLast();
+        assertEquals("17", sampleData.get("section"));
+        assertEquals("TISSUE2", chainGet(sampleData, "tissue", "externalName"));
+        int lwId = (int) lwData.get("id");
+
+        List<Map<String, ?>> opList = chainGetList(response, "data", "confirmSection", "operations");
+        assertThat(opList).hasSize(1);
+        Map<String, ?> opData = opList.getFirst();
+        List<Map<String, ?>> actionList = chainGetList(opData, "actions");
+        assertThat(actionList).hasSize(2);
+        for (var actionData : actionList) {
+            assertEquals("A1", chainGet(actionData, "source", "address"));
+            assertEquals("A1", chainGet(actionData, "destination", "address"));
+            assertEquals(source.getId(), chainGet(actionData, "source", "labwareId"));
+            assertEquals(lwId, (Integer) chainGet(actionData, "destination", "labwareId"));
+        }
+        sampleData = chainGet(actionList, 0, "sample");
+        assertEquals("14", sampleData.get("section"));
+        assertEquals("TISSUE1", chainGet(sampleData, "tissue", "externalName"));
+        sampleData = chainGet(actionList, 1, "sample");
+        assertEquals("17", sampleData.get("section"));
+        assertEquals("TISSUE2", chainGet(sampleData, "tissue", "externalName"));
+    }
+
     @Test
     @Transactional
     public void testPlanAndRecordSection() throws Exception {