Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEPPER-923 Restrict onchistory upload for exited participants #2948

Merged
merged 18 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.broadinstitute.dsm.exception.DSMBadRequestException;
import org.broadinstitute.dsm.exception.DsmInternalError;
import org.broadinstitute.dsm.model.elastic.Dsm;
import org.broadinstitute.dsm.model.elastic.search.ElasticSearchParticipantDto;
import org.broadinstitute.dsm.service.elastic.ElasticSearchService;

@Slf4j
Expand All @@ -27,6 +28,18 @@ public ESParticipantIdProvider(String realm, String participantIndex) {
* @throws DSMBadRequestException when no participant ID is found for short ID
*/
public int getParticipantIdForShortId(String shortId) {
//getParticipantDataForShortId does all the validations to make sure participant ID exists
return getParticipantDataForShortId(shortId).getDsm().get().getParticipant().get().getParticipantId().intValue();
ssettipalli marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Given a participant short ID return a participant ID
ssettipalli marked this conversation as resolved.
Show resolved Hide resolved
* @throws DsmInternalError for bad ES behavior
* @throws DSMBadRequestException when no participant ID is found for short ID
*/
public ElasticSearchParticipantDto getParticipantDataForShortId(String shortId) {
ElasticSearchParticipantDto ptpData = elasticSearchService.getParticipantDocumentByShortId(shortId, participantIndex)
.orElseThrow(() -> new DSMBadRequestException("No participant found for shortId " + shortId));
Dsm dsm = elasticSearchService.getParticipantDsmByShortId(shortId, participantIndex);
Optional<Participant> dsmParticipant = dsm.getParticipant();
if (dsmParticipant.isEmpty()) {
Expand All @@ -37,9 +50,10 @@ public int getParticipantIdForShortId(String shortId) {
throw new DsmInternalError("ES returned empty dsm.participant.participantId object for shortId " + shortId);
}
try {
ssettipalli marked this conversation as resolved.
Show resolved Hide resolved
return participantID.intValue();
return ptpData;
} catch (Exception e) {
throw new DsmInternalError("Invalid dsm.participant.participantId for shortId " + shortId);
}
}

ssettipalli marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.broadinstitute.dsm.exception.DsmInternalError;
import org.broadinstitute.dsm.files.parser.onchistory.OncHistoryParser;
import org.broadinstitute.dsm.model.elastic.converters.camelcase.CamelCaseConverter;
import org.broadinstitute.dsm.model.elastic.search.ElasticSearchParticipantDto;
import org.broadinstitute.lddp.db.SimpleResult;

@Slf4j
Expand Down Expand Up @@ -125,23 +126,35 @@ public void upload(String fileContent) {
/**
* Given participant short IDs in uploaded rows, verify the short ID, and get and record associated
* participant IDs and medical record IDs
* verify if any exited participant(s) exists in passed onc history records.
*
* @throws OncHistoryValidationException for failed verification
* @throws OncHistoryValidationException for failed verifications
*/
protected Map<Integer, Integer> getParticipantIds(List<OncHistoryRecord> oncHistoryRecords,
ParticipantIdProvider participantIdProvider, boolean updateElastic) {
protected Map<Integer, Integer> getParticipantIds(List<OncHistoryRecord> oncHistoryRecords, ParticipantIdProvider participantIdProvider,
boolean updateElastic) {
Map<Integer, Integer> medIds = new HashMap<>();
List<String> exitedParticipants = new ArrayList<>();

ParticipantDao participantDao = ParticipantDao.of();

for (OncHistoryRecord rec : oncHistoryRecords) {
int participantId = participantIdProvider.getParticipantIdForShortId(rec.getParticipantTextId());
ssettipalli marked this conversation as resolved.
Show resolved Hide resolved
ElasticSearchParticipantDto ptpData = participantIdProvider.getParticipantDataForShortId(rec.getParticipantTextId());
if (ptpData.getStatus().isPresent() && ptpData.getStatus().get().startsWith("EXITED")) {
exitedParticipants.add(rec.getParticipantTextId());
}

if (!exitedParticipants.isEmpty()) {
//skip MR verify/creation. continuing to collect any other exited participant hruids.
continue;
}

//since participantIdProvider.getParticipantDataForShortId already made sure participant ID exists.. its ok to get it directly
int participantId = ptpData.getDsm().get().getParticipant().get().getParticipantId().intValue();
ssettipalli marked this conversation as resolved.
Show resolved Hide resolved
try {
ParticipantDto participant = participantDao.get(participantId).orElseThrow();
rec.setDdpParticipantId(participant.getDdpParticipantId().orElseThrow());
} catch (Exception e) {
throw new DsmInternalError("Participant not found for id " + participantId, e);
throw new DsmInternalError("Failed to get Participant/Participant Id for " + rec.getParticipantTextId(), e);
ssettipalli marked this conversation as resolved.
Show resolved Hide resolved
}

rec.setParticipantId(participantId);
Expand All @@ -152,6 +165,13 @@ protected Map<Integer, Integer> getParticipantIds(List<OncHistoryRecord> oncHist
this.realm, updateElastic);
medIds.put(participantId, medId);
}

if (!exitedParticipants.isEmpty()) {
log.error("Found {} exited participants: {} in Onc History Upload", exitedParticipants.size(), exitedParticipants);
throw new OncHistoryValidationException("One or more of the uploaded onc histories is associated with a withdrawn participant. "
+ "Please remove onc histories for these withdrawn participants from the file and upload it again: " + exitedParticipants);
}

return medIds;
}

Expand Down Expand Up @@ -383,4 +403,6 @@ public Map<String, OncHistoryUploadColumn> initColumnsForStudy() {
protected Map<String, OncHistoryUploadColumn> getStudyColumns() {
return studyColumns;
}

ssettipalli marked this conversation as resolved.
Show resolved Hide resolved

}
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
package org.broadinstitute.dsm.service.onchistory;

import org.broadinstitute.dsm.model.elastic.search.ElasticSearchParticipantDto;

import java.util.Optional;

public interface ParticipantIdProvider {

/**
* Given a participant short ID return a participant ID
*/
int getParticipantIdForShortId(String shortId);

ElasticSearchParticipantDto getParticipantDataForShortId(String shortId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.broadinstitute.dsm.DbTxnBaseTest;
import org.broadinstitute.dsm.db.MedicalRecord;
import org.broadinstitute.dsm.db.OncHistoryDetail;
import org.broadinstitute.dsm.db.Participant;
import org.broadinstitute.dsm.db.dao.ddp.instance.DDPInstanceDao;
import org.broadinstitute.dsm.db.dao.ddp.institution.DDPInstitutionDao;
import org.broadinstitute.dsm.db.dao.ddp.medical.records.MedicalRecordDao;
Expand All @@ -33,6 +34,8 @@
import org.broadinstitute.dsm.db.dto.onchistory.OncHistoryDto;
import org.broadinstitute.dsm.files.parser.onchistory.OncHistoryParser;
import org.broadinstitute.dsm.files.parser.onchistory.OncHistoryParserTest;
import org.broadinstitute.dsm.model.elastic.Dsm;
import org.broadinstitute.dsm.model.elastic.search.ElasticSearchParticipantDto;
import org.broadinstitute.dsm.util.TestUtil;
import org.junit.AfterClass;
import org.junit.Assert;
Expand Down Expand Up @@ -101,6 +104,11 @@ public void testLmsWriteToDb() {
writeToDb(LMS_REALM, "onchistory/lmsOncHistory.txt");
}

@Test
public void testLmsWriteToDbExit() {
writeToDb(LMS_REALM, "onchistory/lmsOncHistoryExited.txt");
}

@Test
public void testCreateOncHistoryRecords() {
setupInstance(DEFAULT_REALM);
Expand Down Expand Up @@ -176,6 +184,10 @@ private void writeToDb(String realm, String testFile) {

// verify each participant ID for the study and get an associated medical record ID
Map<Integer, Integer> participantMedIds = getParticipantIds(rows, shortIdToId, realm);
if (rows.get(0).getParticipantTextId().startsWith("xyz-exit")) {
// expecting an exit record and exception which was checked in getParticipantIds
return;
}
Assert.assertEquals(2, participantMedIds.size());

Map<String, OncHistoryUploadColumn> studyColumns = uploadService.getStudyColumns();
Expand Down Expand Up @@ -335,8 +347,14 @@ private static Map<Integer, Integer> getParticipantIds(List<OncHistoryRecord> re
try {
return uploadService.getParticipantIds(records, participantIdProvider, false);
} catch (Exception e) {
Assert.fail("Exception from OncHistoryUploadService.getParticipantIds: " + e.toString());
return null;
if (records.get(0).getParticipantTextId().startsWith("xyz-exit")) {
// expecting an exit record and exception
Assert.assertTrue(e.getMessage().contains("One or more of the uploaded onc histories is associated with a withdrawn participant"));
return null;
} else {
Assert.fail("Exception from OncHistoryUploadService.getParticipantIds: " + e.toString());
return null;
}
}
}

Expand Down Expand Up @@ -411,6 +429,24 @@ public TestParticipantIdProvider(Map<String, Integer> shortIdToId) {
public int getParticipantIdForShortId(String shortId) {
return shortIdToId.get(shortId);
}

@Override
public ElasticSearchParticipantDto getParticipantDataForShortId(String shortId) {
Dsm dsm = new Dsm();
Participant participant = new Participant();
participant.setParticipantId(shortIdToId.get(shortId).longValue());
dsm.setParticipant(participant);

String participantStatus = "ENROLLED";
if (shortId.startsWith("xyz-exit")) {
participantStatus = "EXITED_AFTER_ENROLLMENT";
}
ElasticSearchParticipantDto dto = new ElasticSearchParticipantDto.Builder()
.withDsm(dsm)
.withStatus(participantStatus).build();
return dto;
}

}

private static class ParticipantInfo {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
RECORD_ID FACILITY_PATH_REVIEW FACILITY_PX ACCESSION DATE_PX TYPE_PX LOCATION_PX HISTOLOGY TUMOR_SIZE VIABLE_TUMOR NECROSIS TX_EFFECT BLOCKS_TO_REQUEST SLIDES_TO_REQUEST SLIDES_TOTAL REQUEST_STATUS
xyz-exit my place 4 3/15/2005 Office 5cm x 6cm x 1cm 100 50 abc "a,b" review
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used this ptp as EXITED ptp and mocked data accordingly for testing

lmn Office 5 2/12/2019 Unknown None 94 None abc a 3 sent
lmn Clinic your place 12/12/2022 Hospital 5cm x 6cm x 1cm 50 abc "c,d" 1
Loading