Skip to content

Commit

Permalink
#688 | Create SubjectMigration (if required) if Subject is updated vi…
Browse files Browse the repository at this point in the history 
…a csv upload
  • Loading branch information
@1t5j0y @petmongrels
1t5j0y authored and petmongrels committed May 6, 2024
1 parent 0116227 commit 9ed772c
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 17 deletions.
9 changes: 8 additions & 1 deletion avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/SubjectWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public class SubjectWriter extends EntityWriter implements ItemWriter<Row>, Seri
private final S3Service s3Service;
private final EntityApprovalStatusWriter entityApprovalStatusWriter;
private AddressLevelCreator addressLevelCreator;
private final SubjectMigrationService subjectMigrationService;

private static final Logger logger = LoggerFactory.getLogger(SubjectWriter.class);

Expand All @@ -63,7 +64,7 @@ public SubjectWriter(AddressLevelTypeRepository addressLevelTypeRepository,
ObservationCreator observationCreator, IndividualService individualService, EntityApprovalStatusWriter entityApprovalStatusWriter,
S3Service s3Service,
OrganisationConfigService organisationConfigService,
AddressLevelCreator addressLevelCreator) {
AddressLevelCreator addressLevelCreator, SubjectMigrationService subjectMigrationService) {
super(organisationConfigService);
this.addressLevelTypeRepository = addressLevelTypeRepository;
this.locationRepository = locationRepository;
Expand All @@ -79,6 +80,7 @@ public SubjectWriter(AddressLevelTypeRepository addressLevelTypeRepository,
this.individualService = individualService;
this.entityApprovalStatusWriter = entityApprovalStatusWriter;
this.addressLevelCreator = addressLevelCreator;
this.subjectMigrationService = subjectMigrationService;
this.locationCreator = new LocationCreator();
this.s3Service = s3Service;
}
Expand All @@ -94,6 +96,8 @@ private void write(Row row) throws Exception {
locationTypes.sort(Comparator.comparingDouble(AddressLevelType::getLevel).reversed());

Individual individual = getOrCreateIndividual(row);
AddressLevel oldAddressLevel = individual.getAddressLevel();
ObservationCollection oldObservations = individual.getObservations();
List<String> allErrorMsgs = new ArrayList<>();

SubjectType subjectType = subjectTypeCreator.getSubjectType(row.get(SubjectHeaders.subjectTypeHeader), SubjectHeaders.subjectTypeHeader);
Expand Down Expand Up @@ -127,6 +131,9 @@ private void write(Row row) throws Exception {
savedIndividual = individualService.save(individual);
visitCreator.saveScheduledVisits(formMapping.getType(), savedIndividual.getUuid(), null, ruleResponse.getVisitSchedules(), null);
}
if (oldAddressLevel != null) { // existing subject is being updated
subjectMigrationService.markSubjectMigrationIfRequired(savedIndividual.getUuid(), oldAddressLevel, savedIndividual.getAddressLevel(), oldObservations, savedIndividual.getObservations(), false);
}
entityApprovalStatusWriter.saveStatus(formMapping, savedIndividual.getId(), EntityApprovalStatus.EntityType.Subject, savedIndividual.getSubjectType().getUuid());
} catch (Exception e) {
logger.warn("Error in writing row", e);
Expand Down
11 changes: 6 additions & 5 deletions avni-server-api/src/main/java/org/avni/server/service/SubjectMigrationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,28 +75,29 @@ public boolean isScopeEntityChanged(DateTime lastModifiedDateTime, String subjec
This problem is illustrated in this test - org.avni.server.dao.SubjectMigrationIntegrationTest.migrations_created_by_one_user_is_returned_for_another_user_even_when_concept_attributes_dont_match
At this moment the performance of this seems small as other filters help in reducing the number of records. Functionally this is not an issue because mobile app does the checks before applying subject migration. */
@Transactional
public void markSubjectMigrationIfRequired(String individualUuid, AddressLevel newAddressLevel, ObservationCollection newObservations, boolean executingInBulk) {
public void markSubjectMigrationIfRequired(String individualUuid, AddressLevel oldAddressLevel, AddressLevel newAddressLevel, ObservationCollection oldObservations, ObservationCollection newObservations, boolean executingInBulk) {
Individual individual = individualRepository.findByUuid(individualUuid);
if (individual == null || newAddressLevel == null) {
return;
}
SubjectType subjectType = individual.getSubjectType();
String syncConcept1 = subjectType.getSyncRegistrationConcept1();
String syncConcept2 = subjectType.getSyncRegistrationConcept2();
ObservationCollection oldObservations = individual.getObservations();
if (oldObservations == null) oldObservations = individual.getObservations();
String oldObsSingleStringValueSyncConcept1 = oldObservations.getObjectAsSingleStringValue(syncConcept1);
String newObsSingleStringValueSyncConcept1 = newObservations.getObjectAsSingleStringValue(syncConcept1);
String oldObsSingleStringValueSyncConcept2 = oldObservations.getObjectAsSingleStringValue(syncConcept2);
String newObsSingleStringValueSyncConcept2 = newObservations.getObjectAsSingleStringValue(syncConcept2);
if (!Objects.equals(individual.getAddressLevel().getId(), newAddressLevel.getId()) ||
if (oldAddressLevel == null) oldAddressLevel = individual.getAddressLevel();
if (!Objects.equals(oldAddressLevel.getId(), newAddressLevel.getId()) ||
!Objects.equals(oldObsSingleStringValueSyncConcept1, newObsSingleStringValueSyncConcept1) ||
!Objects.equals(oldObsSingleStringValueSyncConcept2, newObsSingleStringValueSyncConcept2)) {
logger.info(String.format("Migrating subject with UUID %s from %s to %s", individualUuid, addressLevelService.getTitleLineage(individual.getAddressLevel()), addressLevelService.getTitleLineage(newAddressLevel)));
SubjectMigration subjectMigration = new SubjectMigration();
subjectMigration.assignUUID();
subjectMigration.setIndividual(individual);
subjectMigration.setSubjectType(individual.getSubjectType());
subjectMigration.setOldAddressLevel(individual.getAddressLevel());
subjectMigration.setOldAddressLevel(oldAddressLevel);
subjectMigration.setNewAddressLevel(newAddressLevel);
if (!Objects.equals(oldObsSingleStringValueSyncConcept1, newObsSingleStringValueSyncConcept1)) {
subjectMigration.setOldSyncConcept1Value(oldObsSingleStringValueSyncConcept1);
Expand Down Expand Up @@ -124,7 +125,7 @@ public void markSubjectMigrationIfRequired(String individualUuid, AddressLevel n
@Transactional
public void changeSubjectsAddressLevel(List<Individual> subjects, AddressLevel destAddressLevel) {
subjects.forEach(individual -> {
this.markSubjectMigrationIfRequired(individual.getUuid(), destAddressLevel, individual.getObservations(), true);
this.markSubjectMigrationIfRequired(individual.getUuid(), null, destAddressLevel, null, individual.getObservations(), true);
individual.setAddressLevel(destAddressLevel);
individualRepository.save(individual);
});
Expand Down
2 changes: 1 addition & 1 deletion avni-server-api/src/main/java/org/avni/server/web/IndividualController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public AvniEntityResponse save(@RequestBody IndividualRequest individualRequest)
}

private void markSubjectMigrationIfRequired(IndividualRequest individualRequest, ObservationCollection newObservations) {
subjectMigrationService.markSubjectMigrationIfRequired(individualRequest.getUuid(), getAddressLevel(individualRequest), newObservations, false);
subjectMigrationService.markSubjectMigrationIfRequired(individualRequest.getUuid(), null, getAddressLevel(individualRequest), null, newObservations, false);
}

private void addObservationsFromDecisions(ObservationCollection observations, Decisions decisions) {
Expand Down
4 changes: 2 additions & 2 deletions avni-server-api/src/main/java/org/avni/server/web/api/SubjectApiController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ private void updateSubjectDetails(Individual subject, ApiSubjectRequest request)
subject.setRegistrationDate(request.getRegistrationDate());
ObservationCollection observations = RequestUtils.createObservations(request.getObservations(), conceptRepository);
AddressLevel newAddressLevel = addressLevel.orElse(null);
subjectMigrationService.markSubjectMigrationIfRequired(subject.getUuid(), newAddressLevel, observations, false);
subjectMigrationService.markSubjectMigrationIfRequired(subject.getUuid(), null, newAddressLevel, null, observations, false);
subject.setAddressLevel(newAddressLevel);
if (subjectType.isPerson()) {
subject.setDateOfBirth(request.getDateOfBirth());
Expand Down Expand Up @@ -268,7 +268,7 @@ private void patchSubject(Individual subject, Map<String, Object> request) throw
Optional<AddressLevel> addressLevel = locationRepository.findByTitleLineageIgnoreCase(locationTitleLineage);
AddressLevel newAddressLevel = addressLevel.orElseThrow(() -> new IllegalArgumentException(String.format("Address '%s' not found", locationTitleLineage)));
subject.setAddressLevel(newAddressLevel);
subjectMigrationService.markSubjectMigrationIfRequired(subject.getUuid(), newAddressLevel, subject.getObservations(), false);
subjectMigrationService.markSubjectMigrationIfRequired(subject.getUuid(), null, newAddressLevel, null, subject.getObservations(), false);
}

if (subject.getSubjectType().isPerson()) {
Expand Down
16 changes: 8 additions & 8 deletions avni-server-api/src/test/java/org/avni/server/dao/SubjectMigrationIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,23 +105,23 @@ public void checkSyncStrategy() {
// Subject with one concept attribute, location migrated
ObservationCollection observations = ObservationCollectionBuilder.withOneObservation(concept1, concept1.getAnswerConcept("Answer 11").getUuid());
Individual s1 = testSubjectService.save(new SubjectBuilder().withMandatoryFieldsForNewEntity().withSubjectType(subjectType).withLocation(catchmentData.getAddressLevel1()).withObservations(observations).build());
subjectMigrationService.markSubjectMigrationIfRequired(s1.getUuid(), catchmentData.getAddressLevel2(), observations, false);
subjectMigrationService.markSubjectMigrationIfRequired(s1.getUuid(), null, catchmentData.getAddressLevel2(), null, observations, false);
List syncDetails = getSyncDetails();
assertTrue(syncDetails.contains(EntitySyncStatusContract.createForComparison(SyncEntityName.SubjectMigration.name(), subjectType.getUuid())));
assertEquals(1, getMigrations(subjectType, DateTime.now().minusDays(1), DateTime.now()).size());
assertTrue(hasMigrationFor(subjectType, DateTime.now().minusDays(1), DateTime.now(), s1));

// Subject with one concept attribute, attribute migrated
Individual s3 = testSubjectService.save(new SubjectBuilder().withMandatoryFieldsForNewEntity().withSubjectType(subjectType).withLocation(catchmentData.getAddressLevel1()).withObservations(ObservationCollectionBuilder.withOneObservation(concept1, concept1.getAnswerConcept("Answer 11").getUuid())).build());
subjectMigrationService.markSubjectMigrationIfRequired(s3.getUuid(), catchmentData.getAddressLevel1(), ObservationCollectionBuilder.withOneObservation(concept1, concept1.getAnswerConcept("Answer 12").getUuid()), false);
subjectMigrationService.markSubjectMigrationIfRequired(s3.getUuid(), null, catchmentData.getAddressLevel1(), null, ObservationCollectionBuilder.withOneObservation(concept1, concept1.getAnswerConcept("Answer 12").getUuid()), false);
assertTrue(hasMigrationFor(subjectType, DateTime.now().minusDays(1), DateTime.now(), s3));
assertEquals(2, getMigrations(subjectType, DateTime.now().minusDays(1), DateTime.now()).size());

// Subject migrated but its old and new attributes are not assigned to user
observations = ObservationCollectionBuilder.withOneObservation(concept1, concept1.getAnswerConcept("Answer 12").getUuid());
ObservationCollection newObservations = ObservationCollectionBuilder.withOneObservation(concept1, concept1.getAnswerConcept("Answer 13").getUuid());
Individual s7 = testSubjectService.save(new SubjectBuilder().withMandatoryFieldsForNewEntity().withSubjectType(subjectType).withLocation(catchmentData.getAddressLevel1()).withObservations(observations).build());
subjectMigrationService.markSubjectMigrationIfRequired(s7.getUuid(), catchmentData.getAddressLevel1(), newObservations, false);
subjectMigrationService.markSubjectMigrationIfRequired(s7.getUuid(), null, catchmentData.getAddressLevel1(), null, newObservations, false);
assertFalse(hasMigrationFor(subjectType, DateTime.now().minusDays(1), DateTime.now(), s7));
assertEquals(2, getMigrations(subjectType, DateTime.now().minusDays(1), DateTime.now()).size());

Expand All @@ -140,23 +140,23 @@ public void checkSyncStrategy() {
// Subject with two concept attributes, location migrated
observations = new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 12")).addObservation(concept2, concept2.getAnswerConcept("Answer 22")).build();
Individual s2 = testSubjectService.save(new SubjectBuilder().withMandatoryFieldsForNewEntity().withSubjectType(subjectType).withLocation(catchmentData.getAddressLevel1()).withObservations(observations).build());
subjectMigrationService.markSubjectMigrationIfRequired(s2.getUuid(), catchmentData.getAddressLevel2(), observations, false);
subjectMigrationService.markSubjectMigrationIfRequired(s2.getUuid(), null, catchmentData.getAddressLevel2(), null, observations, false);
assertTrue(hasMigrationFor(subjectType, DateTime.now().minusDays(1), DateTime.now(), s2));

// Subject with two concept attributes, first attribute migrated
Individual s4 = testSubjectService.save(new SubjectBuilder().withMandatoryFieldsForNewEntity().withSubjectType(subjectType).withLocation(catchmentData.getAddressLevel1()).withObservations(new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 11")).addObservation(concept2, concept2.getAnswerConcept("Answer 21")).build()).build());
subjectMigrationService.markSubjectMigrationIfRequired(s4.getUuid(), catchmentData.getAddressLevel1(), new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 12")).addObservation(concept2, concept2.getAnswerConcept("Answer 21")).build(), false);
subjectMigrationService.markSubjectMigrationIfRequired(s4.getUuid(), null, catchmentData.getAddressLevel1(), null, new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 12")).addObservation(concept2, concept2.getAnswerConcept("Answer 21")).build(), false);
assertTrue(hasMigrationFor(subjectType, DateTime.now().minusDays(1), DateTime.now(), s4));

// Subject with two concept attributes, second attribute migrated
Individual s5 = testSubjectService.save(new SubjectBuilder().withMandatoryFieldsForNewEntity().withSubjectType(subjectType).withLocation(catchmentData.getAddressLevel1()).withObservations(new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 11")).addObservation(concept2, concept2.getAnswerConcept("Answer 21")).build()).build());
subjectMigrationService.markSubjectMigrationIfRequired(s5.getUuid(), catchmentData.getAddressLevel1(), new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 11")).addObservation(concept2, concept2.getAnswerConcept("Answer 22")).build(), false);
subjectMigrationService.markSubjectMigrationIfRequired(s5.getUuid(), null, catchmentData.getAddressLevel1(), null, new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 11")).addObservation(concept2, concept2.getAnswerConcept("Answer 22")).build(), false);
boolean hasMigrationFor = hasMigrationFor(subjectType, DateTime.now().minusDays(1), DateTime.now(), s5);
assertTrue(hasMigrationFor);

// Subject with two concept attributes, both attributes migrated
Individual s6 = testSubjectService.save(new SubjectBuilder().withMandatoryFieldsForNewEntity().withSubjectType(subjectType).withLocation(catchmentData.getAddressLevel1()).withObservations(new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 11")).addObservation(concept2, concept2.getAnswerConcept("Answer 21")).build()).build());
subjectMigrationService.markSubjectMigrationIfRequired(s6.getUuid(), catchmentData.getAddressLevel1(), new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 12")).addObservation(concept2, concept2.getAnswerConcept("Answer 22")).build(), false);
subjectMigrationService.markSubjectMigrationIfRequired(s6.getUuid(), null, catchmentData.getAddressLevel1(), null, new ObservationCollectionBuilder().addObservation(concept1, concept1.getAnswerConcept("Answer 12")).addObservation(concept2, concept2.getAnswerConcept("Answer 22")).build(), false);
assertTrue(hasMigrationFor(subjectType, DateTime.now().minusDays(1), DateTime.now(), s6));

// User without sync attributes setup will not get any migration as that is more performance optimised. Setting
Expand All @@ -178,7 +178,7 @@ public void migrations_created_by_one_user_is_returned_for_another_user_even_whe
ObservationCollection observations = ObservationCollectionBuilder.withOneObservation(concept1, concept1.getAnswerConcept("Answer 11").getUuid());
ObservationCollection newObservations = ObservationCollectionBuilder.withOneObservation(concept1, concept1.getAnswerConcept("Answer 11").getUuid());
Individual s = testSubjectService.save(new SubjectBuilder().withMandatoryFieldsForNewEntity().withSubjectType(subjectType).withLocation(catchmentData.getAddressLevel1()).withObservations(observations).build());
subjectMigrationService.markSubjectMigrationIfRequired(s.getUuid(), catchmentData.getAddressLevel2(), newObservations, false);
subjectMigrationService.markSubjectMigrationIfRequired(s.getUuid(), null, catchmentData.getAddressLevel2(), null, newObservations, false);
assertTrue(getSyncDetails().contains(EntitySyncStatusContract.createForComparison(SyncEntityName.SubjectMigration.name(), subjectType.getUuid())));
assertEquals(1, getMigrations(subjectType, DateTime.now().minusDays(1), DateTime.now()).size());

Expand Down

0 comments on commit 9ed772c

Please sign in to comment.