From 69da5c13c0f68c3aec6f555eab907cecd47fdcf2 Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Fri, 22 Dec 2023 16:18:32 +0100 Subject: [PATCH] [kbss-cvut/record-manager-ui#38] Set imported record phase to open unless specified otherwise. When current user is admin, imported record phase is preserved. --- .../study/service/PatientRecordService.java | 3 +- .../RepositoryPatientRecordService.java | 21 +++++-- .../RepositoryPatientRecordServiceTest.java | 60 +++++++++---------- 3 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java b/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java index aa5dd528..6e57840f 100644 --- a/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java +++ b/src/main/java/cz/cvut/kbss/study/service/PatientRecordService.java @@ -64,7 +64,8 @@ public interface PatientRecordService extends BaseService { * records. *

* If the current user is an admin, the import procedure retains provenance data of the record. Otherwise, the - * current user is set as the record's author. + * current user is set as the record's author. Also, if the current user is not an admin, the phase of all + * the imported records is set to {@link RecordPhase#open}, for admin, the phase of the records is retained. * * @param records Records to import * @return Instance representing the import result diff --git a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java index 49e8477f..cc20e7ba 100644 --- a/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java +++ b/src/main/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordService.java @@ -22,6 +22,7 @@ import java.util.Date; import java.util.List; import java.util.Objects; +import java.util.Optional; @Service public class RepositoryPatientRecordService extends KeySupportingRepositoryService @@ -92,11 +93,15 @@ protected void preUpdate(PatientRecord instance) { public RecordImportResult importRecords(List records) { Objects.requireNonNull(records); LOG.debug("Importing records."); + return importRecordsImpl(records, Optional.empty()); + } + + private RecordImportResult importRecordsImpl(List records, Optional targetPhase) { final User author = securityUtils.getCurrentUser(); final Date created = new Date(); final RecordImportResult result = new RecordImportResult(records.size()); records.forEach(r -> { - setImportedRecordProvenance(author, created, r); + setImportedRecordProvenance(author, created, targetPhase, r); if (recordDao.exists(r.getUri())) { LOG.warn("Record {} already exists. Skipping it.", Utils.uriToString(r.getUri())); result.addError("Record " + Utils.uriToString(r.getUri()) + " already exists."); @@ -108,13 +113,18 @@ public RecordImportResult importRecords(List records) { return result; } - private void setImportedRecordProvenance(User currentUser, Date now, PatientRecord record) { + private void setImportedRecordProvenance(User currentUser, Date now, Optional targetPhase, + PatientRecord record) { if (!currentUser.isAdmin()) { record.setAuthor(currentUser); record.setInstitution(currentUser.getInstitution()); record.setDateCreated(now); - } else if (!userService.exists(record.getAuthor().getUri())) { - throw new RecordAuthorNotFoundException("Author of record " + record + "not found during import."); + targetPhase.ifPresentOrElse(record::setPhase, () -> record.setPhase(RecordPhase.open)); + } else { + targetPhase.ifPresent(record::setPhase); + if (!userService.exists(record.getAuthor().getUri())) { + throw new RecordAuthorNotFoundException("Author of record " + record + "not found during import."); + } } } @@ -124,7 +134,6 @@ private void setImportedRecordProvenance(User currentUser, Date now, PatientReco public RecordImportResult importRecords(List records, RecordPhase targetPhase) { Objects.requireNonNull(records); LOG.debug("Importing records to target phase '{}'.", targetPhase); - records.forEach(r -> r.setPhase(targetPhase)); - return importRecords(records); + return importRecordsImpl(records, Optional.ofNullable(targetPhase)); } } diff --git a/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java b/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java index d49c0a27..a337ff52 100644 --- a/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java +++ b/src/test/java/cz/cvut/kbss/study/service/repository/RepositoryPatientRecordServiceTest.java @@ -20,6 +20,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import java.net.URI; import java.util.Date; import java.util.List; @@ -62,13 +63,7 @@ void setUp() { @Test void importRecordsSetsCurrentUserAsAuthorWhenTheyAreRegularUserAndImportsSpecifiedRecords() { final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); - final List toImport = - List.of(Generator.generatePatientRecord(originalAuthor), Generator.generatePatientRecord(originalAuthor)); - toImport.forEach(r -> { - // Simulate that the records existed in another deployment from which they are imported - r.setKey(IdentificationUtils.generateKey()); - r.setDateCreated(new Date(System.currentTimeMillis() - 10000L)); - }); + final List toImport = generateRecordsToImport(originalAuthor); when(recordDao.exists(any())).thenReturn(false); final RecordImportResult result = sut.importRecords(toImport); @@ -87,9 +82,7 @@ void importRecordsSetsCurrentUserAsAuthorWhenTheyAreRegularUserAndImportsSpecifi } } - @Test - void importRecordsRetainsRecordProvenanceDataWhenCurrentUserIsAdmin() { - final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + private List generateRecordsToImport(User originalAuthor) { final List toImport = List.of(Generator.generatePatientRecord(originalAuthor), Generator.generatePatientRecord(originalAuthor)); toImport.forEach(r -> { @@ -97,6 +90,13 @@ void importRecordsRetainsRecordProvenanceDataWhenCurrentUserIsAdmin() { r.setKey(IdentificationUtils.generateKey()); r.setDateCreated(new Date(System.currentTimeMillis() - 10000L)); }); + return toImport; + } + + @Test + void importRecordsRetainsRecordProvenanceDataWhenCurrentUserIsAdmin() { + final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final List toImport = generateRecordsToImport(originalAuthor); user.addType(Vocabulary.s_c_administrator); Environment.setCurrentUser(user); when(userService.exists(originalAuthor.getUri())).thenReturn(true); @@ -112,19 +112,14 @@ void importRecordsRetainsRecordProvenanceDataWhenCurrentUserIsAdmin() { assertEquals(toImport.get(i).getKey(), imported.get(i).getKey()); assertEquals(originalAuthor, imported.get(i).getAuthor()); assertEquals(toImport.get(i).getDateCreated(), imported.get(i).getDateCreated()); + assertEquals(toImport.get(i).getPhase(), imported.get(i).getPhase()); } } @Test void importRecordsThrowsRecordAuthorNotFoundExceptionWhenAdminImportsRecordsAndRecordAuthorIsNotFound() { final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); - final List toImport = - List.of(Generator.generatePatientRecord(originalAuthor), Generator.generatePatientRecord(originalAuthor)); - toImport.forEach(r -> { - // Simulate that the records existed in another deployment from which they are imported - r.setKey(IdentificationUtils.generateKey()); - r.setDateCreated(new Date(System.currentTimeMillis() - 10000L)); - }); + final List toImport = generateRecordsToImport(originalAuthor); user.addType(Vocabulary.s_c_administrator); Environment.setCurrentUser(user); @@ -133,17 +128,11 @@ void importRecordsThrowsRecordAuthorNotFoundExceptionWhenAdminImportsRecordsAndR @Test void importRecordsSkipsImportingRecordsThatAlreadyExist() { - final List toImport = - List.of(Generator.generatePatientRecord(user), Generator.generatePatientRecord(user)); final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final List toImport = generateRecordsToImport(originalAuthor); final PatientRecord existing = toImport.get(Generator.randomIndex(toImport)); + when(recordDao.exists(any(URI.class))).thenReturn(false); when(recordDao.exists(existing.getUri())).thenReturn(true); - toImport.forEach(r -> { - // Simulate that the records existed in another deployment from which they are imported - r.setKey(IdentificationUtils.generateKey()); - r.setDateCreated(new Date(System.currentTimeMillis() - 10000L)); - r.setAuthor(originalAuthor); - }); final RecordImportResult result = sut.importRecords(toImport); assertEquals(toImport.size(), result.getTotalCount()); @@ -154,16 +143,9 @@ void importRecordsSkipsImportingRecordsThatAlreadyExist() { @Test void importRecordsWithPhaseSetsSpecifiedPhaseToAllRecords() { - final List toImport = - List.of(Generator.generatePatientRecord(user), Generator.generatePatientRecord(user)); final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final List toImport = generateRecordsToImport(originalAuthor); final RecordPhase targetPhase = RecordPhase.values()[Generator.randomInt(0, RecordPhase.values().length)]; - toImport.forEach(r -> { - // Simulate that the records existed in another deployment from which they are imported - r.setKey(IdentificationUtils.generateKey()); - r.setDateCreated(new Date(System.currentTimeMillis() - 10000L)); - r.setAuthor(originalAuthor); - }); when(recordDao.exists(any())).thenReturn(false); sut.importRecords(toImport, targetPhase); @@ -171,4 +153,16 @@ void importRecordsWithPhaseSetsSpecifiedPhaseToAllRecords() { verify(recordDao, times(toImport.size())).persist(captor.capture()); captor.getAllValues().forEach(r -> assertEquals(targetPhase, r.getPhase())); } + + @Test + void importRecordsSetsRecordPhaseToOpenOnAllImportedRecordsWhenCurrentUserIsRegularUser() { + final User originalAuthor = Generator.generateUser(Generator.generateInstitution()); + final List toImport = generateRecordsToImport(originalAuthor); + when(recordDao.exists(any())).thenReturn(false); + + sut.importRecords(toImport); + final ArgumentCaptor captor = ArgumentCaptor.forClass(PatientRecord.class); + verify(recordDao, times(toImport.size())).persist(captor.capture()); + captor.getAllValues().forEach(r -> assertEquals(RecordPhase.open, r.getPhase())); + } } \ No newline at end of file