Skip to content

Commit

Permalink
avniproject/avni-webapp#1214 - if optional fields are not present in …
Browse files Browse the repository at this point in the history
…the header then do not fail. do not join multiple error messages in a multiple columns.
  • Loading branch information
petmongrels committed Sep 16, 2024
1 parent 396ccca commit edf82e3
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public void afterJob(JobExecution jobExecution) {
logger.debug(format("Bulk upload '%s'! %s", jobExecution.getStatus(), jobInfo));
try {
ObjectInfo metadata = bulkUploadS3Service.uploadErrorFile(errorFile, uuid);
logger.debug(format("Bulk upload '%s'! Check for errors at '%s'", jobExecution.getStatus(), metadata.getKey()));
logger.debug(format("Bulk upload '%s'! Check for errors at '%s'. Local file: %s", jobExecution.getStatus(), metadata.getKey(), errorFile.getAbsolutePath()));
} catch (IOException e) {
logger.error(String.format("Unable to create error files in S3 %s", e.getMessage()), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.avni.server.util.CollectionUtil;
import org.avni.server.util.S;
import org.avni.server.web.request.LocationContract;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import org.springframework.util.StringUtils;

Expand All @@ -37,6 +38,7 @@ public class BulkLocationCreator extends BulkLocationModifier {
public static final String ParentMissingOfLocation = "Parent missing for location provided";
public static final String NoLocationProvided = "No location provided";

@Autowired
public BulkLocationCreator(LocationService locationService, LocationRepository locationRepository, AddressLevelTypeRepository addressLevelTypeRepository, ObservationCreator observationCreator, ImportService importService, FormService formService) {
super(locationRepository, observationCreator);
this.locationService = locationService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,9 @@ private void validateHeaders(String[] headers) {
List<String> headerList = new ArrayList<>(Arrays.asList(headers));
List<String> allErrorMsgs = new ArrayList<>();
UsersAndCatchmentsHeaders usersAndCatchmentsHeaders = new UsersAndCatchmentsHeaders();
List<String> expectedStandardHeaders = Arrays.asList(usersAndCatchmentsHeaders.getAllHeaders());
List<String> syncAttributeHeadersForSubjectTypes = subjectTypeService.constructSyncAttributeHeadersForSubjectTypes();
checkForMissingHeaders(headerList, allErrorMsgs, expectedStandardHeaders, syncAttributeHeadersForSubjectTypes);
checkForUnknownHeaders(headerList, allErrorMsgs, expectedStandardHeaders, syncAttributeHeadersForSubjectTypes);
checkForMissingHeaders(headerList, allErrorMsgs, Arrays.asList(usersAndCatchmentsHeaders.getMandatoryHeaders()), syncAttributeHeadersForSubjectTypes);
checkForUnknownHeaders(headerList, allErrorMsgs, Arrays.asList(usersAndCatchmentsHeaders.getAllHeaders()), syncAttributeHeadersForSubjectTypes);
if (!allErrorMsgs.isEmpty()) {
throw new RuntimeException(createMultiErrorMessage(allErrorMsgs));
}
Expand Down Expand Up @@ -201,7 +200,7 @@ private void validateRowAndAssimilateErrors(List<String> rowValidationErrorMsgs,
}

private static String createMultiErrorMessage(List<String> errorMsgs) {
return errorMsgs.stream().map(s -> "\"" + s + "\"").collect(Collectors.joining("; "));
return String.join(" ", errorMsgs);
}

private void extractUserNameValidationErrMsg(List<String> rowValidationErrorMsgs, String nameOfUser) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import java.util.Arrays;
import java.util.List;

public class UsersAndCatchmentsHeaders implements Headers{
public class UsersAndCatchmentsHeaders implements Headers {
public static final String LOCATION_WITH_FULL_HIERARCHY = "Location with full hierarchy";
public static final String CATCHMENT_NAME = "Catchment Name";
public static final String FULL_NAME_OF_USER = "Full Name of User";
Expand All @@ -18,9 +18,15 @@ public class UsersAndCatchmentsHeaders implements Headers{
public static final String IDENTIFIER_PREFIX = "Identifier Prefix";
public static final String USER_GROUPS = "User Groups";

private static final List<String> OPTIONAL_HEADERS = Arrays.asList(ENABLE_BENEFICIARY_MODE, TRACK_LOCATION);

@Override
public String[] getAllHeaders() {
List<String> headers = new ArrayList<>(Arrays.asList(LOCATION_WITH_FULL_HIERARCHY, CATCHMENT_NAME, USERNAME, FULL_NAME_OF_USER, EMAIL_ADDRESS, MOBILE_NUMBER, PREFERRED_LANGUAGE, TRACK_LOCATION, DATE_PICKER_MODE, ENABLE_BENEFICIARY_MODE, IDENTIFIER_PREFIX, USER_GROUPS));
return headers.toArray(new String[0]);
}

public String[] getMandatoryHeaders() {
return Arrays.stream(this.getAllHeaders()).filter(header -> !OPTIONAL_HEADERS.contains(header)).toArray(String[]::new);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import org.avni.server.dao.CatchmentRepository;
import org.avni.server.dao.UserRepository;
import org.avni.server.dao.application.FormRepository;
import org.avni.server.domain.AddressLevel;
import org.avni.server.domain.AddressLevelType;
import org.avni.server.domain.Concept;
Expand Down Expand Up @@ -35,8 +34,6 @@ public class UserAndCatchmentWriterIntegrationTest extends BaseCSVImportTest {
@Autowired
private TestConceptService testConceptService;
@Autowired
private FormRepository formRepository;
@Autowired
private TestLocationService testLocationService;
@Autowired
private UserAndCatchmentWriter userAndCatchmentWriter;
Expand All @@ -49,13 +46,13 @@ public class UserAndCatchmentWriterIntegrationTest extends BaseCSVImportTest {

@Override
public void setUp() {
TestDataSetupService.TestOrganisationData organisationData = testDataSetupService.setupOrganisation("example", "User Group 1");
testDataSetupService.setupOrganisation("example", "User Group 1");
AddressLevelType block = new AddressLevelTypeBuilder().name("Block").level(2d).withUuid(UUID.randomUUID()).build();
AddressLevelType district = new AddressLevelTypeBuilder().name("District").level(3d).withUuid(UUID.randomUUID()).build();
AddressLevelType state = new AddressLevelTypeBuilder().name("State").level(4d).withUuid(UUID.randomUUID()).build();
testDataSetupService.saveLocationTypes(Arrays.asList(block, district, state));
Concept codedConcept = testConceptService.createCodedConcept("Sync Concept", "Answer 1", "Answer 2");
Concept textConcept = testConceptService.createConcept("Text Concept", ConceptDataType.Text);
testConceptService.createConcept("Text Concept", ConceptDataType.Text);

testSubjectTypeService.createWithDefaults(
new SubjectTypeBuilder()
Expand Down Expand Up @@ -85,6 +82,10 @@ private boolean userCreated(boolean b) {
return b;
}

private String[] has(String... errors) {
return errors;
}

private void success(String[] headers, String[] cells, boolean catchmentCreated, boolean userCreated) throws IDPException {
long numberOfUsers = userRepository.count();
long numberOfCatchments = catchmentRepository.count();
Expand All @@ -99,7 +100,7 @@ private void success(String[] headers, String[] cells, boolean catchmentCreated,
assertEquals(userRepository.count(), numberOfUsers);
}

private void failure(String[] headers, String[] cells, String... errorMessages) throws IDPException {
private void failure(String[] headers, String[] cells, String[] errorMessages) throws IDPException {
try {
userAndCatchmentWriter.write(Collections.singletonList(new Row(headers, cells)));
fail();
Expand All @@ -119,7 +120,7 @@ private void failure(String[] headers, String[] cells, String... errorMessages)
}
}

private void failsOnMissingHeader(String[] headers, String... errorMessages) throws IDPException {
private void failsOnMissingHeader(String[] headers, String[] errorMessages, String... nonExistentErrorMessages) throws IDPException {
try {
userAndCatchmentWriter.write(Collections.singletonList(new Row(headers, new String[]{})));
fail();
Expand All @@ -135,6 +136,12 @@ private void failsOnMissingHeader(String[] headers, String... errorMessages) thr
fail("Expected error message: " + s + " not present in: " + message);
}
});
Arrays.stream(nonExistentErrorMessages).forEach(s -> {
if (message.contains(s)) {
e.printStackTrace();
fail("Unexpected error message: " + s + " present in: " + message);
}
});
}
}
}
Expand Down Expand Up @@ -181,36 +188,45 @@ public void shouldCreateUpdate() throws IDPException {
dataRow(" Bihar, District1, Block11", " Catchment 5", " username6@example", " User 6", " [email protected] ", " 9455509147 ", "English ", "", " spinner", " ", "", " User Group 1", " Answer 1"),
catchmentCreated(true),
userCreated(true));
// without mandatory fields and including in header
success(
header(" Location with full hierarchy", " Catchment Name", "Username ", " Full Name of User", "Email Address", "Mobile Number", " Preferred Language", "Date picker mode", " Identifier Prefix", " User Groups", " SubjectTypeWithSyncAttributeBasedSync->Sync Concept"),
dataRow(" Bihar, District1, Block11", " Catchment 6", " username7@example", " User 7", " [email protected] ", " 9455509147 ", "English ", " spinner", "", " User Group 1", " Answer 1"),
catchmentCreated(true),
userCreated(true));

// wrong - username, email, phone number, language, track location, date picker mode, enable beneficiary mode
failure(
header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"),
dataRow("Bihar, District1, Block11", "Catchment 1", "username1@exmplee", "User 1", "username1@examplecom", "9455047", "Irish", "truee", "spinnerr", "falsee", "", "User Group 1", "Answer 1"),
hasError("Invalid username 'username1@exmplee'. Include correct userSuffix @example at the end"),
hasError("Invalid email address username1@examplecom"),
hasError("Provided value 'Mobile Number' for phone number is invalid."),
hasError("Provided value 'Irish' for Preferred Language is invalid."),
hasError("Provided value 'spinnerr' for Date picker mode is invalid."),
hasError("Provided value 'truee' for track location is invalid."),
hasError("Provided value 'falsee' for enable beneficiary mode is invalid.")
has(
error("Invalid username 'username1@exmplee'. Include correct userSuffix @example at the end"),
error("Invalid email address username1@examplecom"),
error("Provided value 'Mobile Number' for phone number is invalid."),
error("Provided value 'Irish' for Preferred Language is invalid."),
error("Provided value 'spinnerr' for Date picker mode is invalid."),
error("Provided value 'truee' for track location is invalid."),
error("Provided value 'falsee' for enable beneficiary mode is invalid.")
)
);
// empty - catchment name, username, Full Name of User, email, phone number, track location, date picker mode, enable beneficiary mode
failure(
header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"),
dataRow("Bihar, District1, Block11", " ", "", " ", " ", " ", " ", " ", "", "", "", "User Group 1", "Answer 1"),
hasError("Invalid or Empty value specified for mandatory field Catchment Name."),
hasError("Invalid or Empty value specified for mandatory field Username."),
hasError("Invalid or Empty value specified for mandatory field Full Name of User."),
hasError("Invalid or Empty value specified for mandatory field Email Address."),
hasError("Invalid or Empty value specified for mandatory field Mobile Number."),
hasError("Provided value '' for Date picker mode is invalid.")
has(
error("Invalid or Empty value specified for mandatory field Catchment Name."),
error("Invalid or Empty value specified for mandatory field Username."),
error("Invalid or Empty value specified for mandatory field Full Name of User."),
error("Invalid or Empty value specified for mandatory field Email Address."),
error("Invalid or Empty value specified for mandatory field Mobile Number."),
error("Provided value '' for Date picker mode is invalid."))
);

// invalid User Group Name
failure(
header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"),
dataRow("Bihar, District1, Block11", "Catchment 3", "username2@example", "User 2", "[email protected]", "9455509147", "English", "true", "spinner", "false", "", "User Group 1345", "Answer 1"),
hasError("Group 'User Group 1345' not found")
has(error("Group 'User Group 1345' not found"))
);
// same user group twice
success(
Expand All @@ -224,25 +240,26 @@ public void shouldCreateUpdate() throws IDPException {
failure(
header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"),
dataRow("Bihar, District1, Block11", "Catchment 3", "username2@example", "User 2", "[email protected]", "9455509147", "English", "true", "spinner", "false", "", "User Group 1", "Answer 1223"),
hasError("'Answer 1223' is not a valid value for the concept 'Sync Concept'.")
has(error("'Answer 1223' is not a valid value for the concept 'Sync Concept'."))
);

// Wrong location hierarchy
failure(
header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"),
dataRow("Bihar, District1, NoBlock11", "Catchment 3", "username2@example", "User 2", "[email protected]", "9455509147", "English", "true", "spinner", "false", "", "User Group 1", "Answer 1"),
hasError("Provided Location does not exist in Avni. Please add it or check for spelling mistakes 'Bihar, District1, NoBlock11'.")
has(error("Provided Location does not exist in Avni. Please add it or check for spelling mistakes 'Bihar, District1, NoBlock11'."))
);

// Missing headers - sync attributes
failsOnMissingHeader(
header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups"),
hasError("Mandatory columns are missing from uploaded file - SubjectTypeWithSyncAttributeBasedSync->Sync Concept. Please refer to sample file for the list of mandatory headers.")
has(error("Mandatory columns are missing from uploaded file - SubjectTypeWithSyncAttributeBasedSync->Sync Concept. Please refer to sample file for the list of mandatory headers."))
);
// Missing headers - all
failsOnMissingHeader(
header(),
hasError("Mandatory columns are missing from uploaded file - Track Location, Identifier Prefix, Catchment Name, Full Name of User, Mobile Number, Enable Beneficiary mode, User Groups, Username, SubjectTypeWithSyncAttributeBasedSync->Sync Concept, Preferred Language, Location with full hierarchy, Date picker mode, Email Address. Please refer to sample file for the list of mandatory headers.")
has(error("Mandatory columns are missing from uploaded file - Identifier Prefix, Mobile Number, User Groups, Username, SubjectTypeWithSyncAttributeBasedSync->Sync Concept, Preferred Language, Catchment Name, Location with full hierarchy, Date picker mode, Full Name of User, Email Address. Please refer to sample file for the list of mandatory headers.")),
doesntHaveError("\"Mandatory columns are missing from uploaded file - Identifier Prefix, Mobile Number, User Groups, Username, SubjectTypeWithSyncAttributeBasedSync->Sync Concept, Preferred Language, Catchment Name, Location with full hierarchy, Date picker mode, Full Name of User, Email Address. Please refer to sample file for the list of mandatory headers.")
);

// allow additional cells in data row
Expand Down
10 changes: 5 additions & 5 deletions makefiles/externalDB.mk
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,9 @@ endif
--exclude-table-data=audit \
--exclude-table-data='public.sync_telemetry' \
--exclude-table-data='rule_failure_log' \
--exclude-table-data='scheduled_job_run' \
--exclude-table-data='qrtz_*' \
--exclude-table-data='batch_*' \
--exclude-table='qrtz_*' \
--exclude-table='scheduled_job_run' \
--exclude-table='public.individual_copy' \
--exclude-table='public.program_enrolment_copy' \
--exclude-table='public.encounter_copy' \
Expand Down Expand Up @@ -206,9 +206,9 @@ else
endif

backup-org-db:
ifndef orgName
@echo "Provde the orgName variable"
ifndef orgDbUser
@echo "Provde the orgDbUser variable"
exit 1
else
sudo -u $(su) pg_dump avni_org > ../avni-db-dumps/local-$(orgName).sql
sudo -u $(su) pg_dump avni_org > ../avni-db-dumps/local-$(orgDbUser).sql
endif

0 comments on commit edf82e3

Please sign in to comment.