diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/ConfirmSectionServiceImp.java b/src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/ConfirmSectionServiceImp.java index 9e7fe5ea..6f76e810 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/ConfirmSectionServiceImp.java +++ b/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 slotsToSave = new HashSet<>(); List measurements = new ArrayList<>(); List opComs = new ArrayList<>(); - var planActionMap = getPlanActionMap(plan.getPlanActions(), lwId); List 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 streamOpComments(AddressCommentId ac, Map 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 getPlanActionMap(Collection planActions, final int lwId) { - Map planActionMap = new HashMap<>(planActions.size()); + Map getPlanActionMap(Collection planActions, final int lwId) { + Map 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; diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/ConfirmSectionValidationServiceImp.java b/src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/ConfirmSectionValidationServiceImp.java index 3e86a71b..bc7d586e 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/ConfirmSectionValidationServiceImp.java +++ b/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 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 problems, Collection 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 sectionCount = new HashMap<>(); - + Set 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 entry : sectionCount.entrySet()) { - if (entry.getValue() > 1) { - problems.add(String.format("Multiple actions linked to destination %s %s.", - csl.getBarcode(), entry.getKey())); - } - } } } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/SampleDescriber.java b/src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/SampleDescriber.java index 6d9e700e..5d8aafab 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/confirm/SampleDescriber.java +++ b/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. *

Usage: "You have misused sample "+describer.describe(sampleId)+"." - *
You have misused sample 40 (section 8 of EXT12) from STAN-12 (A3). + *
You have misused sample id 40 (section 8 of EXT12) from STAN-12 (A3). *

Descriptions are cached. * @author dr6 */ diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanServiceImp.java b/src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanServiceImp.java index f674f44e..97271384 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanServiceImp.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanServiceImp.java @@ -258,6 +258,7 @@ public List getSources(PlanOperation plan) { public void createSlotGroups(int planId, Labware lw, PlanRequestLabware prl) { List> addressGroups = prl.getActions().stream() .map(PlanRequestAction::getAddresses) + .distinct() .toList(); slotGroupService.saveGroups(lw, planId, addressGroups); } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanValidationImp.java b/src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanValidationImp.java index 065c2852..f5d7885c 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanValidationImp.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/operation/plan/PlanValidationImp.java @@ -205,6 +205,7 @@ public void validateDestinations(UCMap 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 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

seenAddresses = new HashSet<>(); + final Set 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> addressGroups = new HashSet<>(); + boolean anyOverlaps = false; + for (PlanRequestAction ac : prl.getActions()) { + if (nullOrEmpty(ac.getAddresses())) { + continue; + } + Set
addressGroup = new HashSet<>(ac.getAddresses()); + Optional> 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 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"; diff --git a/src/test/java/uk/ac/sanger/sccp/stan/integrationtest/TestPlanAndRecordSectionMutations.java b/src/test/java/uk/ac/sanger/sccp/stan/integrationtest/TestPlanAndRecordSectionMutations.java index 92d0772b..5e1a4fe4 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/integrationtest/TestPlanAndRecordSectionMutations.java +++ b/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 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 result = tester.post(planQuery.replace("$BARCODE", barcode)); + + Map planData = chainGet(result, "data", "planData"); + List> groups = chainGet(planData, "groups"); + assertThat(groups).hasSize(1); + assertThat(groups.getFirst()).containsExactly("A1"); + List> 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> lwList = chainGet(response, "data", "confirmSection", "labware"); + assertThat(lwList).hasSize(1); + Map lwData = lwList.getFirst(); + assertEquals(barcode, lwData.get("barcode")); + List> slotList = chainGetList(lwData, "slots"); + assertThat(slotList).hasSize(1); + Map slotData = slotList.getFirst(); + assertEquals("A1", slotData.get("address")); + List> sampleList = chainGetList(slotData, "samples"); + assertThat(sampleList).hasSize(2); + Map 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> opList = chainGetList(response, "data", "confirmSection", "operations"); + assertThat(opList).hasSize(1); + Map opData = opList.getFirst(); + List> 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 { diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/operation/confirm/TestConfirmSectionService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/operation/confirm/TestConfirmSectionService.java index 949bea5e..684b9985 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/operation/confirm/TestConfirmSectionService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/operation/confirm/TestConfirmSectionService.java @@ -12,8 +12,7 @@ import uk.ac.sanger.sccp.stan.request.confirm.*; import uk.ac.sanger.sccp.stan.request.confirm.ConfirmSectionLabware.AddressCommentId; import uk.ac.sanger.sccp.stan.service.*; -import uk.ac.sanger.sccp.stan.service.operation.confirm.ConfirmSectionServiceImp.ConfirmLabwareResult; -import uk.ac.sanger.sccp.stan.service.operation.confirm.ConfirmSectionServiceImp.SectionKey; +import uk.ac.sanger.sccp.stan.service.operation.confirm.ConfirmSectionServiceImp.*; import uk.ac.sanger.sccp.stan.service.work.WorkService; import uk.ac.sanger.sccp.utils.BasicUtils; import uk.ac.sanger.sccp.utils.UCMap; @@ -253,7 +252,7 @@ public void testConfirmLabwareMissingPlanAction() { Sample sample = EntityFactory.getSample(); lw.setBarcode("STAN-01"); PlanOperation plan = new PlanOperation(); - Map planActionMap = Map.of(); + Map planActionMap = Map.of(); doReturn(planActionMap).when(service).getPlanActionMap(any(), anyInt()); final Address A1 = new Address(1,1); Map commentMap = Map.of(1, new Comment(1, "com", "cat")); @@ -262,7 +261,7 @@ public void testConfirmLabwareMissingPlanAction() { assertThat(assertThrows(IllegalArgumentException.class, () -> service.confirmLabware(EntityFactory.getUser(), csl, lw, plan, commentMap))) - .hasMessage("No plan action found into STAN-01 slot A1."); + .hasMessage("No plan action found into STAN-01 slot A1 from sample "+sample.getId()+"."); verifyNoInteractions(mockSlotRepo); verifyNoInteractions(mockMeasurementRepo); } @@ -288,10 +287,10 @@ public void testConfirmLabware() { ); csecs.getFirst().setThickness("10.5"); ConfirmSectionLabware csl = new ConfirmSectionLabware(lw1.getBarcode(), false, csecs, List.of(), null); - Map planActionMap = Stream.of( + Map planActionMap = Stream.of( new PlanAction(1, 1, source, lw1.getSlot(A1), sample), new PlanAction(2, 1, source, lw1.getSlot(B3), sample, "12", "50", null) - ).collect(BasicUtils.inMap(pa -> pa.getDestination().getAddress(), HashMap::new)); + ).collect(BasicUtils.inMap(ActionKey::new, HashMap::new)); plan.setPlanActions(new ArrayList<>(planActionMap.values())); doReturn(planActionMap).when(service).getPlanActionMap(any(), anyInt()); @@ -304,7 +303,7 @@ public void testConfirmLabware() { doReturn(section).when(service).createSection(eq(sample.getTissue()), eq(csec.getNewSection()), eq(sample.getBioState())); for (Address ad : csec.getDestinationAddresses()) { sections.add(section); - PlanAction pa = planActionMap.get(ad); + PlanAction pa = planActionMap.get(new ActionKey(ad, csec.getSampleId())); Action action = new Action(null, null, pa.getSource(), pa.getDestination(), section, sample); doReturn(action).when(service).makeAction(any(), same(pa), same(pa.getDestination())); actions.add(action); @@ -331,7 +330,7 @@ public void testConfirmLabware() { verify(service).getPlanActionMap(plan.getPlanActions(), lw1.getId()); for (ConfirmSection csec : csecs) { for (Address ad : csec.getDestinationAddresses()) { - verify(service).makeAction(any(), eq(planActionMap.get(ad)), eq(lw1.getSlot(ad))); + verify(service).makeAction(any(), eq(planActionMap.get(new ActionKey(ad, csec.getSampleId()))), eq(lw1.getSlot(ad))); } } verify(mockSlotRepo).saveAll(Matchers.sameElements(List.of(lw1.getSlot(A1), lw1.getSlot(B3)), true)); @@ -346,7 +345,7 @@ public void testConfirmLabware() { new OperationComment(null, com1, opId, sections.get(0).getId(), lw1.getFirstSlot().getId(), null))); for (ConfirmSection csec : csecs) { for (Address ad : csec.getDestinationAddresses()) { - verify(service).thickness(csec, planActionMap.get(ad)); + verify(service).thickness(csec, planActionMap.get(new ActionKey(ad, csec.getSampleId()))); } } } @@ -529,10 +528,10 @@ public void testGetPlanActionMap() { new PlanAction(54, 1, source, slots[2], samples[0]), }; - Map map = service.getPlanActionMap(Arrays.asList(pas), 10); + var map = service.getPlanActionMap(Arrays.asList(pas), 10); assertEquals(2, map.size()); - assertEquals(pas[0], map.get(A1)); - assertEquals(pas[1], map.get(A2)); + assertEquals(pas[0], map.get(new ActionKey(A1, samples[0].getId()))); + assertEquals(pas[1], map.get(new ActionKey(A2, samples[1].getId()))); } /** diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/operation/confirm/TestConfirmSectionValidationService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/operation/confirm/TestConfirmSectionValidationService.java index a4c37b4c..c5595e7c 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/operation/confirm/TestConfirmSectionValidationService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/operation/confirm/TestConfirmSectionValidationService.java @@ -131,17 +131,19 @@ public void testCheckRepeatedDestSlots() { List csls = List.of( new ConfirmSectionLabware(null), new ConfirmSectionLabware("STAN-1", false, List.of( - new ConfirmSection(List.of(A1, A2), null, null, null), - new ConfirmSection(A3, null, null, null) + new ConfirmSection(List.of(A1, A2), 10, "14b", null), + new ConfirmSection(List.of(A1, A2), 10, "14c", null), + new ConfirmSection(List.of(A1, A2), 11, "14c", null), + new ConfirmSection(A3, 10, "14b", null) ), null, null), new ConfirmSectionLabware("STAN-2", false, List.of( - new ConfirmSection(List.of(A1, A2), null, null, null), - new ConfirmSection(List.of(A2, A3), null, null, null) + new ConfirmSection(List.of(A1, A2), 10, "14b", null), + new ConfirmSection(List.of(A2, A3), 10, "14b", null) ), null, null) ); List problems = new ArrayList<>(1); service.checkRepeatedDestSlots(problems, csls); - assertProblem(problems, "Multiple actions linked to destination STAN-2 A2."); + assertProblem(problems, "Repeated action for destination STAN-2: (slot=A2, sampleId=10, section=14b)"); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/operation/plan/TestPlanValidation.java b/src/test/java/uk/ac/sanger/sccp/stan/service/operation/plan/TestPlanValidation.java index 5efde13f..cc1d4466 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/operation/plan/TestPlanValidation.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/operation/plan/TestPlanValidation.java @@ -211,6 +211,27 @@ public void testCheckActions(String barcode, Object planRequestActions, LabwareT assertProblems(expectedProblems, validation.problems); } + @ParameterizedTest + @ValueSource(booleans={false,true}) + void testCheckSlotGroups(boolean ok) { + final Address A1 = new Address(1,1), A2 = new Address(1,2), A3 = new Address(1,3); + PlanRequestSource prs = new PlanRequestSource("STAN-0", A1); + List pras = List.of( + new PlanRequestAction(A1, 1, prs, null), + new PlanRequestAction(A1, 2, prs, null), + new PlanRequestAction(A1, 3, prs, null) + ); + pras.get(0).setAddresses(List.of(A1,A2)); + pras.get(1).setAddresses(List.of(A1,A2)); + pras.get(2).setAddresses(ok ? List.of(A3) : List.of(A1, A3)); + PlanRequestLabware prlw = new PlanRequestLabware("Tube", "STAN-1", pras); + final String problem = ok ? null : "There are overlapping slot groups given for labware STAN-1."; + PlanRequest request = new PlanRequest("opname", List.of(prlw)); + PlanValidationImp validation = makeValidation(request); + validation.checkSlotGroups(prlw); + assertProblems(problem, validation.problems); + } + @ParameterizedTest @MethodSource("destinationData") public void testValidateDestinations(Object planRequestLabware, @@ -220,6 +241,7 @@ public void testValidateDestinations(Object planRequestLabware, PlanValidationImp validation = makeValidation(request); doNothing().when(validation).checkActions(any(), any()); + doNothing().when(validation).checkSlotGroups(any()); doNothing().when(validation).validateLotAndCostings(any()); doNothing().when(validation).validatePrebarcode(any(), any()); @@ -268,6 +290,7 @@ public void testValidateDestinations(Object planRequestLabware, if (expectedProblems==null) { verify(validation, times(request.getLabware().size())).checkActions(any(), any()); + verify(validation, times(request.getLabware().size())).checkSlotGroups(any()); verify(validation, times(request.getLabware().size())) .validatePrebarcode(any(), isNotNull()); for (PlanRequestLabware prlw : request.getLabware()) { @@ -472,12 +495,16 @@ static Stream actionsData() { Arguments.of("STAN-100", List.of(new PlanRequestAction(A1, 4, src, null), new PlanRequestAction(A1, 4, src, null), new PlanRequestAction(A2, 4, srcAlt, null)), - lt, "Actions for labware STAN-100 contains duplicate address: A1"), + lt, "Duplicate actions transfer sample ID 4 into slot A1 of labware STAN-100."), //Duplicate actions from a non-block source without a barcode Arguments.of(null, List.of(new PlanRequestAction(A1, 4, src, null), new PlanRequestAction(A1, 4, src, null), new PlanRequestAction(A2, 4, src, null)), - lt, "Actions for labware of type "+lt.getName()+" contains duplicate address: A1"), + lt, "Duplicate actions transfer sample ID 4 into slot A1 of labware of type "+lt.getName()+"."), + //Non-duplicate actions into the same slot + Arguments.of("STAN-100", List.of(new PlanRequestAction(A1, 4, src, null), + new PlanRequestAction(A1, 5, src, null)), + lt, null), Arguments.of("STAN-100", new PlanRequestAction(null, 4, src, null), lt, "Missing destination address."), Arguments.of(null, new PlanRequestAction(null, 4, src, null), lt, diff --git a/src/test/resources/graphql/confirmmult.graphql b/src/test/resources/graphql/confirmmult.graphql new file mode 100644 index 00000000..48e42654 --- /dev/null +++ b/src/test/resources/graphql/confirmmult.graphql @@ -0,0 +1,64 @@ +mutation { + confirmSection(request: { + labware: [ + { + barcode: "$BARCODE", + workNumber: "SGP4000" + confirmSections: [ + { + destinationAddresses: ["A1"] + newSection: "14" + sampleId: 55555 + commentIds: [2] + } + { + destinationAddresses: ["A1"] + newSection: "17" + sampleId: 55556 + } + ] + addressComments: [ + { + address: "A1" + commentId: 1 + } + ] + } + ] + }) { + labware { + id + barcode + slots { + address + samples { + id + tissue { externalName } + section + bioState { name } + } + } + } + operations { + id + performed + operationType { name } + actions { + source { + address + labwareId + } + destination { + address + labwareId + } + sample { + id + tissue { externalName } + section + bioState { name } + } + } + } + } +} \ No newline at end of file diff --git a/src/test/resources/graphql/plan_mult.graphql b/src/test/resources/graphql/plan_mult.graphql new file mode 100644 index 00000000..f40b663e --- /dev/null +++ b/src/test/resources/graphql/plan_mult.graphql @@ -0,0 +1,46 @@ +mutation { + plan(request:{ + operationType: "Section" + labware:[{ + labwareType:"Tube" + actions:[ + { + source:{ barcode:"STAN-0001" } + addresses: ["A1"] + sampleId: 55555 + sectioningOrder: 1 + } + { + source:{ barcode:"STAN-0001" } + addresses: ["A1"] + sampleId: 55556 + sectioningOrder: 1 + } + ] + }] + }) { + labware { + barcode + labwareType { name } + } + operations { + operationType { + name + } + planActions { + source { + address + labwareId + } + destination { + address + labwareId + } + sample { id } + newSection + sampleThickness + sectioningOrder + } + } + } +} \ No newline at end of file diff --git a/src/test/resources/graphql/plandata.graphql b/src/test/resources/graphql/plandata.graphql index 4b7c8195..0bde1386 100644 --- a/src/test/resources/graphql/plandata.graphql +++ b/src/test/resources/graphql/plandata.graphql @@ -8,6 +8,8 @@ query { address } sectioningOrder + source { address } + sample { id } } } destination {