Skip to content

Commit

Permalink
MODDATAIMP-1052 Error appears when edit via quickMARC MARC Instance s…
Browse files Browse the repository at this point in the history 
…hared from Member tenant (#723)

* MODDATAIMP-1052: removed IsNotEmpty check for externalHolder InstanceHrid
  • Loading branch information
@Maksat-Galymzhan
Maksat-Galymzhan authored May 27, 2024
1 parent 0ecb85a commit 2b91a0b
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 7 deletions.
1 change: 1 addition & 0 deletions NEWS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Invalid values (as it is) created in JSON when value is not matching accepted options provided in Instance field mapping for Nature of Content term [MODINV-1012](https://folio-org.atlassian.net/browse/MODINV-1012)
* Remove null values from electronicAccess object before returning item and instance [MODINV-1006](https://folio-org.atlassian.net/browse/MODINV-1006)
* 422 Unprocessable Content Error while updating Instances and Items with electronic access without URI field populated. [MODINV-1024](https://folio-org.atlassian.net/browse/MODINV-1024)
* Error appears when edit via quickMARC MARC Instance shared from Member tenant [MODDATAIMP-1052](https://folio-org.atlassian.net/browse/MODDATAIMP-1052)

## 20.2.0 2023-03-20
* Inventory cannot process Holdings with virtual fields ([MODINV-941](https://issues.folio.org/browse/MODINV-941))
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ public static boolean isFieldsFillingNeeded(Record srcRecord, Instance instance)
}

private static boolean isValidIdAndHrid(String id, String hrid, String externalId, String externalHrid) {
return (isNotEmpty(externalId) && isNotEmpty(externalHrid)) && (id.equals(externalId) && !hrid.equals(externalHrid));
return (isNotEmpty(externalId)) && (id.equals(externalId) && !hrid.equals(externalHrid));
}

/**
Expand Down
24 changes: 22 additions & 2 deletions .../java/org/folio/inventory/dataimport/handlers/actions/CreateInstanceEventHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.folio.processing.value.StringValue;
import org.folio.rest.client.SourceStorageRecordsClient;
import org.folio.rest.jaxrs.model.EntityType;
import org.folio.rest.jaxrs.model.ExternalIdsHolder;
import org.folio.rest.jaxrs.model.MappingDetail;
import org.folio.rest.jaxrs.model.MappingRule;
import org.folio.rest.jaxrs.model.ParsedRecord;
Expand Down Expand Up @@ -423,6 +424,7 @@ public void shouldProcessConsortiumEvent() throws InterruptedException, Executio
String recordId = "567859ad-505a-400d-a699-0028a1fdbf84";
String instanceId = "957985c6-97e3-4038-b0e7-343ecd0b8120";
String title = "titleValue";
String instanceHrid = "in00000000028";
RecordToEntity recordToInstance = RecordToEntity.builder().recordId(recordId).entityId(instanceId).build();

when(fakeReader.read(any(MappingRule.class))).thenReturn(StringValue.of(instanceTypeId), StringValue.of(title));
Expand All @@ -437,12 +439,28 @@ public void shouldProcessConsortiumEvent() throws InterruptedException, Executio
MappingManager.registerWriterFactory(new InstanceWriterFactory());

HashMap<String, String> context = new HashMap<>();
Record record = new Record().withParsedRecord(new ParsedRecord().withContent(PARSED_CONTENT_999ffi));
Record record = new Record().withParsedRecord(new ParsedRecord().withContent(PARSED_CONTENT_999ffi))
.withExternalIdsHolder(new ExternalIdsHolder().withInstanceId(instanceId));
record.setId(recordId);

doAnswer(invocationOnMock -> {
Instance instanceRecord = invocationOnMock.getArgument(0);
JsonObject instanceJson = instanceRecord.getJsonForStorage();
instanceJson.put("hrid", instanceHrid);
Instance instanceRecordToSubstitute = Instance.fromJson(instanceJson);

instanceRecordToSubstitute.setPrecedingTitles(instanceRecord.getPrecedingTitles());
instanceRecordToSubstitute.setSucceedingTitles(instanceRecord.getSucceedingTitles());
instanceRecordToSubstitute.setNotes(instanceRecord.getNotes());

Consumer<Success<Instance>> successHandler = invocationOnMock.getArgument(1);
successHandler.accept(new Success<>(instanceRecordToSubstitute));
return null;
}).when(instanceRecordCollection).add(any(), any(Consumer.class), any(Consumer.class));

context.put(MARC_BIBLIOGRAPHIC.value(), Json.encode(record));

Buffer buffer = BufferImpl.buffer("{\"id\":\"567859ad-505a-400d-a699-0028a1fdbf84\",\"parsedRecord\":{\"content\":\"{\\\"leader\\\":\\\"00567nam 22001211a 4500\\\",\\\"fields\\\":[{\\\"035\\\":{\\\"subfields\\\":[{\\\"a\\\":\\\"ybp7406411\\\"}],\\\"ind1\\\":\\\" \\\",\\\"ind2\\\":\\\" \\\"}},{\\\"245\\\":{\\\"subfields\\\":[{\\\"a\\\":\\\"titleValue\\\"}],\\\"ind1\\\":\\\"1\\\",\\\"ind2\\\":\\\"0\\\"}},{\\\"336\\\":{\\\"subfields\\\":[{\\\"b\\\":\\\"b6698d38-149f-11ec-82a8-0242ac130003\\\"}],\\\"ind1\\\":\\\"1\\\",\\\"ind2\\\":\\\"0\\\"}},{\\\"780\\\":{\\\"subfields\\\":[{\\\"t\\\":\\\"Houston oil directory\\\"}],\\\"ind1\\\":\\\"0\\\",\\\"ind2\\\":\\\"0\\\"}},{\\\"785\\\":{\\\"subfields\\\":[{\\\"t\\\":\\\"SAIS review of international affairs\\\"},{\\\"x\\\":\\\"1945-4724\\\"}],\\\"ind1\\\":\\\"0\\\",\\\"ind2\\\":\\\"0\\\"}},{\\\"500\\\":{\\\"subfields\\\":[{\\\"a\\\":\\\"Adaptation of Xi xiang ji by Wang Shifu.\\\"}],\\\"ind1\\\":\\\" \\\",\\\"ind2\\\":\\\" \\\"}},{\\\"520\\\":{\\\"subfields\\\":[{\\\"a\\\":\\\"Ben shu miao shu le cui ying ying he zhang sheng wei zheng qu hun yin zi you li jin qu zhe jian xin zhi hou, zhong cheng juan shu de ai qing gu shi. jie lu le bao ban hun yin he feng jian li jiao de zui e.\\\"}],\\\"ind1\\\":\\\" \\\",\\\"ind2\\\":\\\" \\\"}},{\\\"999\\\":{\\\"subfields\\\":[{\\\"i\\\":\\\"957985c6-97e3-4038-b0e7-343ecd0b8120\\\"}],\\\"ind1\\\":\\\"f\\\",\\\"ind2\\\":\\\"f\\\"}}]}\"},\"deleted\":false,\"state\":\"ACTUAL\"}");
Buffer buffer = BufferImpl.buffer("{\"id\": \"567859ad-505a-400d-a699-0028a1fdbf84\",\"parsedRecord\": {\"content\": \"{\\\"leader\\\":\\\"00567nam 22001211a 4500\\\",\\\"fields\\\":[{\\\"035\\\":{\\\"subfields\\\":[{\\\"a\\\":\\\"ybp7406411\\\"}],\\\"ind1\\\":\\\" \\\",\\\"ind2\\\":\\\" \\\"}},{\\\"245\\\":{\\\"subfields\\\":[{\\\"a\\\":\\\"titleValue\\\"}],\\\"ind1\\\":\\\"1\\\",\\\"ind2\\\":\\\"0\\\"}},{\\\"336\\\":{\\\"subfields\\\":[{\\\"b\\\":\\\"b6698d38-149f-11ec-82a8-0242ac130003\\\"}],\\\"ind1\\\":\\\"1\\\",\\\"ind2\\\":\\\"0\\\"}},{\\\"780\\\":{\\\"subfields\\\":[{\\\"t\\\":\\\"Houston oil directory\\\"}],\\\"ind1\\\":\\\"0\\\",\\\"ind2\\\":\\\"0\\\"}},{\\\"785\\\":{\\\"subfields\\\":[{\\\"t\\\":\\\"SAIS review of international affairs\\\"},{\\\"x\\\":\\\"1945-4724\\\"}],\\\"ind1\\\":\\\"0\\\",\\\"ind2\\\":\\\"0\\\"}},{\\\"500\\\":{\\\"subfields\\\":[{\\\"a\\\":\\\"Adaptation of Xi xiang ji by Wang Shifu.\\\"}],\\\"ind1\\\":\\\" \\\",\\\"ind2\\\":\\\" \\\"}},{\\\"520\\\":{\\\"subfields\\\":[{\\\"a\\\":\\\"Ben shu miao shu le cui ying ying he zhang sheng wei zheng qu hun yin zi you li jin qu zhe jian xin zhi hou, zhong cheng juan shu de ai qing gu shi. jie lu le bao ban hun yin he feng jian li jiao de zui e.\\\"}],\\\"ind1\\\":\\\" \\\",\\\"ind2\\\":\\\" \\\"}},{\\\"999\\\":{\\\"subfields\\\":[{\\\"i\\\":\\\"957985c6-97e3-4038-b0e7-343ecd0b8120\\\"}],\\\"ind1\\\":\\\"f\\\",\\\"ind2\\\":\\\"f\\\"}}]}\"},\"deleted\": false,\"order\": 0,\"externalIdsHolder\": {\"instanceId\": \"b5e25bc3-a5a5-474a-8333-4a728d2f3485\",\"instanceHrid\": \"in00000000028\"},\"state\": \"ACTUAL\"}");
HttpResponse<Buffer> resp = buildHttpResponseWithBuffer(buffer);
when(sourceStorageClient.postSourceStorageRecords(any())).thenReturn(Future.succeededFuture(resp));

Expand All @@ -465,6 +483,8 @@ public void shouldProcessConsortiumEvent() throws InterruptedException, Executio
String actualInstanceId = createdInstance.getString("id");
assertNotNull(actualInstanceId);
assertEquals(instanceId, actualInstanceId);
assertNotNull(createdInstance.getString("hrid"));
assertEquals(instanceHrid, createdInstance.getString("hrid"));
assertEquals(title, createdInstance.getString("title"));
assertEquals(instanceTypeId, createdInstance.getString("instanceTypeId"));
assertEquals("MARC", createdInstance.getString("source"));
Expand Down
29 changes: 25 additions & 4 deletions src/test/java/org/folio/inventory/dataimport/util/AdditionalFieldsUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static org.folio.inventory.dataimport.util.AdditionalFieldsUtil.removeField;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

Expand Down Expand Up @@ -250,18 +251,38 @@ public void shouldReplaceControlledFieldInMarcRecord() throws IOException {
public void isFieldsFillingNeededTrue() {
String instanceId = UUID.randomUUID().toString();
String instanceHrId = UUID.randomUUID().toString();
Record record = new Record().withExternalIdsHolder(new ExternalIdsHolder().withInstanceId(instanceId).withInstanceHrid(UUID.randomUUID().toString()));
Record srcRecord = new Record().withExternalIdsHolder(new ExternalIdsHolder().withInstanceId(instanceId).withInstanceHrid(UUID.randomUUID().toString()));
Instance instance = new Instance(instanceId, "0", instanceHrId, "", "", "");
Assert.assertTrue(AdditionalFieldsUtil.isFieldsFillingNeeded(record, instance));
Assert.assertTrue(AdditionalFieldsUtil.isFieldsFillingNeeded(srcRecord, instance));

srcRecord.getExternalIdsHolder().setInstanceHrid(null);
Assert.assertTrue(AdditionalFieldsUtil.isFieldsFillingNeeded(srcRecord, instance));
}

@Test
public void isFieldsFillingNeededFalse() {
String instanceId = UUID.randomUUID().toString();
String instanceHrId = UUID.randomUUID().toString();
Record record = new Record().withExternalIdsHolder(new ExternalIdsHolder().withInstanceId(instanceId).withInstanceHrid(instanceHrId));
Record srcRecord = new Record().withExternalIdsHolder(new ExternalIdsHolder().withInstanceId(instanceId).withInstanceHrid(instanceHrId));
Instance instance = new Instance(instanceId, "0", instanceHrId, "", "", "");
Assert.assertFalse(AdditionalFieldsUtil.isFieldsFillingNeeded(record, instance));
assertFalse(AdditionalFieldsUtil.isFieldsFillingNeeded(srcRecord, instance));

srcRecord.getExternalIdsHolder().withInstanceId(instanceId);
instance = new Instance(UUID.randomUUID().toString(), "0", instanceHrId, "", "", "");
assertFalse(AdditionalFieldsUtil.isFieldsFillingNeeded(srcRecord, instance));

srcRecord.getExternalIdsHolder().withInstanceId(null).withInstanceHrid(null);
instance = new Instance(UUID.randomUUID().toString(), "0", instanceHrId, "", "", "");
assertFalse(AdditionalFieldsUtil.isFieldsFillingNeeded(srcRecord, instance));
}

@Test(expected = Exception.class)
public void isFieldsFillingNeededForExternalHolderInstanceShouldThrowException() {
String instanceId = UUID.randomUUID().toString();
String instanceHrId = UUID.randomUUID().toString();
Record srcRecord = new Record().withExternalIdsHolder(new ExternalIdsHolder().withInstanceId(instanceId).withInstanceHrid(instanceHrId));
Instance instance = new Instance(null, "0", instanceHrId, "", "", "");
AdditionalFieldsUtil.isFieldsFillingNeeded(srcRecord, instance);
}

@Test
Expand Down

0 comments on commit 2b91a0b

Please sign in to comment.