avniproject/avni-webapp#1306 - Location create csv check for no locat…

…ion provided.

UserCatchment CSV - minor changes to error message. Fixed check for missing headers. Only add one error for a field. check wrong bool values in track location and beneficiary mode. Check for empty string instead of null value. Do not send regex in error message instead send in valid chars.
General/All - ignore additional cells in data row. more test scenarios.

avni-server-api/src/main/java/org/avni/server/domain/JsonObject.java

@@ -21,8 +21,8 @@ public JsonObject with(String key, Object value) {
         return this;
     }
 
-    public JsonObject withEmptyCheckAndTrim(String key, String value){
-        if(!S.isEmpty(value)){
+    public JsonObject withEmptyCheckAndTrim(String key, String value) {
+        if (!S.isEmpty(value)) {
             super.put(key, value.trim());
         }
         return this;

avni-server-api/src/main/java/org/avni/server/domain/User.java

@@ -394,7 +394,7 @@ public static void validateUsername(String username, String userSuffix) {
             throw new ValidationException(String.format("Invalid username '%s'. Include correct userSuffix %s at the end", username, userSuffix));
         }
         if (ValidationUtil.checkNullOrEmptyOrContainsDisallowedCharacters(username.trim(), ValidationUtil.COMMON_INVALID_CHARS_PATTERN)) {
-            throw new ValidationException(String.format("Invalid username '%s', contains atleast one disallowed character %s", username, ValidationUtil.COMMON_INVALID_CHARS_PATTERN));
+            throw new ValidationException(String.format("Invalid username '%s', contains at least one disallowed character %s", username, ValidationUtil.COMMON_INVALID_CHARS));
         }
     }
 
@@ -403,7 +403,7 @@ public static void validateUsername(String username, String userSuffix) {
      */
     public static void validateName(String name) {
         if (ValidationUtil.checkNullOrEmptyOrContainsDisallowedCharacters(name, ValidationUtil.NAME_INVALID_CHARS_PATTERN)) {
-            throw new ValidationException(String.format("Invalid name '%s', contains atleast one disallowed character %s", name, ValidationUtil.NAME_INVALID_CHARS_PATTERN));
+            throw new ValidationException(String.format("Invalid name '%s', contains at least one disallowed character %s", name, ValidationUtil.NAME_INVALID_CHARS));
         }
     }
 }

avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/BulkLocationCreator.java

@@ -1,5 +1,6 @@
 package org.avni.server.importer.batch.csv.writer;
 
+import com.google.common.collect.Sets;
 import org.avni.server.application.FormElement;
 import org.avni.server.builder.BuilderException;
 import org.avni.server.dao.AddressLevelTypeRepository;
@@ -20,10 +21,7 @@
 import org.springframework.util.StringUtils;
 
 import javax.transaction.Transactional;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Comparator;
-import java.util.List;
+import java.util.*;
 import java.util.stream.Collectors;
 
 // This class is need so that the logic can be instantiated in integration tests. Spring batch configuration is not working in integration tests.
@@ -68,23 +66,29 @@ public void createLocation(Row row, List<String> allErrorMsgs, List<String> loca
         }
     }
 
-    private List<String> validateCreateModeHeaders(String[] headers, List<String> allErrorMsgs, String locationHierarchy) {
+    private List<String> validateHeaders(String[] headers, List<String> allErrorMsgs, String locationHierarchy) {
         List<String> headerList = Arrays.asList(headers);
-        List<String> locationTypeHeaders = checkIfHeaderHasLocationTypesInOrderForHierarchy(locationHierarchy, headerList, allErrorMsgs);
-        List<String> additionalHeaders = new ArrayList<>(headerList.subList(locationTypeHeaders.size(), headerList.size()));
+        List<String> locationTypeHeaders = checkIfHeaderHasLocationTypesAndInOrderForHierarchy(locationHierarchy, headerList, allErrorMsgs);
+        List<String> additionalHeaders = headerList.size() > locationTypeHeaders.size() ? new ArrayList<>(headerList.subList(locationTypeHeaders.size(), headerList.size())) : new ArrayList<>();
         checkIfHeaderRowHasUnknownHeaders(additionalHeaders, allErrorMsgs);
         return locationTypeHeaders;
     }
 
-    private List<String> checkIfHeaderHasLocationTypesInOrderForHierarchy(String locationHierarchy, List<String> headerList, List<String> allErrorMsgs) {
-        List<String> locationTypeNamesForHierachy = importService.getAddressLevelTypesForCreateModeSingleHierarchy(locationHierarchy)
+    private List<String> checkIfHeaderHasLocationTypesAndInOrderForHierarchy(String locationHierarchy, List<String> headerList, List<String> allErrorMsgs) {
+        List<String> locationTypeNamesForHierarchy = importService.getAddressLevelTypesForCreateModeSingleHierarchy(locationHierarchy)
                 .stream().map(AddressLevelType::getName).collect(Collectors.toList());
 
-        if (headerList.size() >= locationTypeNamesForHierachy.size() && !headerList.subList(0, locationTypeNamesForHierachy.size()).equals(locationTypeNamesForHierachy)) {
+        HashSet<String> expectedHeaders = new HashSet<>(locationTypeNamesForHierarchy);
+        if (Sets.difference(new HashSet<>(expectedHeaders), new HashSet<>(headerList)).size() == locationTypeNamesForHierarchy.size()) {
             allErrorMsgs.add(LocationTypesHeaderError);
             throw new RuntimeException(String.join(", ", allErrorMsgs));
         }
-        return locationTypeNamesForHierachy;
+
+        if (headerList.size() >= locationTypeNamesForHierarchy.size() && !headerList.subList(0, locationTypeNamesForHierarchy.size()).equals(locationTypeNamesForHierarchy)) {
+            allErrorMsgs.add(LocationTypesHeaderError);
+            throw new RuntimeException(String.join(", ", allErrorMsgs));
+        }
+        return locationTypeNamesForHierarchy;
     }
 
     private void checkIfHeaderRowHasUnknownHeaders(List<String> additionalHeaders, List<String> allErrorMsgs) {
@@ -136,7 +140,7 @@ private void validateRow(Row row, List<String> hierarchicalLocationTypeNames, Li
     @Transactional(Transactional.TxType.REQUIRES_NEW)
     public void write(List<? extends Row> rows, String idBasedLocationHierarchy) {
         List<String> allErrorMsgs = new ArrayList<>();
-        List<String> hierarchicalLocationTypeNames = validateCreateModeHeaders(rows.get(0).getHeaders(), allErrorMsgs, idBasedLocationHierarchy);
+        List<String> hierarchicalLocationTypeNames = validateHeaders(rows.get(0).getHeaders(), allErrorMsgs, idBasedLocationHierarchy);
         for (Row row : rows) {
             if (skipRow(row, hierarchicalLocationTypeNames)) {
                 continue;

avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/UserAndCatchmentWriter.java

@@ -1,6 +1,5 @@
 package org.avni.server.importer.batch.csv.writer;
 
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import org.avni.server.dao.LocationRepository;
 import org.avni.server.dao.UserRepository;
@@ -10,6 +9,7 @@
 import org.avni.server.importer.batch.csv.writer.header.UsersAndCatchmentsHeaders;
 import org.avni.server.importer.batch.model.Row;
 import org.avni.server.service.*;
+import org.avni.server.util.PhoneNumberUtil;
 import org.avni.server.util.RegionUtil;
 import org.avni.server.util.S;
 import org.avni.server.web.request.syncAttribute.UserSyncSettings;
@@ -32,7 +32,6 @@
 
 @Component
 public class UserAndCatchmentWriter implements ItemWriter<Row>, Serializable {
-    public static final String ERR_MSG_DELIMITER = "\",\"";
     private final UserService userService;
     private final CatchmentService catchmentService;
     private final LocationRepository locationRepository;
@@ -44,14 +43,15 @@ public class UserAndCatchmentWriter implements ItemWriter<Row>, Serializable {
     private final Pattern compoundHeaderPattern;
     private final ResetSyncService resetSyncService;
 
-    private static final String ERR_MSG_MANDATORY_FIELD = "Invalid or Empty value specified for mandatory field %s";
-    private static final String ERR_MSG_LOCATION_FIELD = "Provided Location does not exist in Avni. Please add it or check for spelling mistakes '%s'";
-    private static final String ERR_MSG_LOCALE_FIELD = "Provided value '%s' for Preferred Language is invalid";
-    private static final String ERR_MSG_DATE_PICKER_FIELD = "Provided value '%s' for Date picker mode is invalid";
-    private static final String ERR_MSG_UNKNOWN_HEADERS = "Unknown headers - %s included in file. Please refer to sample file for valid list of headers";
+    private static final String ERR_MSG_MANDATORY_OR_INVALID_FIELD = "Invalid or Empty value specified for mandatory field %s.";
+    private static final String ERR_MSG_LOCATION_FIELD = "Provided Location does not exist in Avni. Please add it or check for spelling mistakes '%s'.";
+    private static final String ERR_MSG_LOCALE_FIELD = "Provided value '%s' for Preferred Language is invalid.";
+    private static final String ERR_MSG_DATE_PICKER_FIELD = "Provided value '%s' for Date picker mode is invalid.";
+    private static final String ERR_MSG_INVALID_PHONE_NUMBER = "Provided value '%s' for phone number is invalid.";
+    private static final String ERR_MSG_UNKNOWN_HEADERS = "Unknown headers - %s included in file. Please refer to sample file for valid list of headers.";
     private static final String ERR_MSG_MISSING_MANDATORY_FIELDS = "Mandatory columns are missing from uploaded file - %s. Please refer to sample file for the list of mandatory headers.";
-    private static final String ERR_MSG_INVALID_CONCEPT_ANSWER = "'%s' is not a valid value for the concept '%s'" +
-            "To input this value, add this as an answer to the coded concept '%s'";
+    private static final String ERR_MSG_INVALID_CONCEPT_ANSWER = "'%s' is not a valid value for the concept '%s'. " +
+            "To input this value, add this as an answer to the coded concept '%s'.";
     private static final String METADATA_ROW_START_STRING = "Mandatory field.";
     private static final List<String> DATE_PICKER_MODE_OPTIONS = Arrays.asList("calendar", "spinner");
 
@@ -93,7 +93,7 @@ private void validateHeaders(String[] headers) {
         checkForMissingHeaders(headerList, allErrorMsgs, expectedStandardHeaders, syncAttributeHeadersForSubjectTypes);
         checkForUnknownHeaders(headerList, allErrorMsgs, expectedStandardHeaders, syncAttributeHeadersForSubjectTypes);
         if (!allErrorMsgs.isEmpty()) {
-            throw new RuntimeException(String.join(ERR_MSG_DELIMITER, allErrorMsgs));
+            throw new RuntimeException(createMultiErrorMessage(allErrorMsgs));
         }
     }
 
@@ -107,11 +107,10 @@ private void checkForUnknownHeaders(List<String> headerList, List<String> allErr
         }
     }
 
-    private void checkForMissingHeaders(List<String> headerList, List<String> allErrorMsgs, List<String> expectedStandardHeaders, List<String> syncAttributeHeadersForSubjectTypes) {
+    private void checkForMissingHeaders(List<String> headerList, List<String> allErrorMsgs, List<String> expectedStandardHeaders, List<String> expectedSyncAttributeHeadersForSubjectTypes) {
         HashSet<String> expectedHeaders = new HashSet<>(expectedStandardHeaders);
-        expectedHeaders.addAll(syncAttributeHeadersForSubjectTypes);
+        expectedHeaders.addAll(expectedSyncAttributeHeadersForSubjectTypes);
         HashSet<String> presentHeaders = new HashSet<>(headerList);
-        presentHeaders.addAll(syncAttributeHeadersForSubjectTypes);
         Sets.SetView<String> missingHeaders = Sets.difference(expectedHeaders, presentHeaders);
         if (!missingHeaders.isEmpty()) {
             allErrorMsgs.add(String.format(ERR_MSG_MISSING_MANDATORY_FIELDS, String.join(", ", missingHeaders)));
@@ -138,7 +137,7 @@ private void write(Row row) throws IDPException {
         Organisation organisation = UserContextHolder.getUserContext().getOrganisation();
         String userSuffix = "@".concat(organisation.getEffectiveUsernameSuffix());
         JsonObject syncSettings = constructSyncSettings(row, rowValidationErrorMsgs);
-        validateRowAndAssimilateErrors(rowValidationErrorMsgs, fullAddress, catchmentName, nameOfUser, username, email, phoneNumber, language, datePickerMode, location, locale, userSuffix);
+        validateRowAndAssimilateErrors(rowValidationErrorMsgs, fullAddress, catchmentName, nameOfUser, username, email, phoneNumber, language, datePickerMode, location, locale, userSuffix, trackLocation, beneficiaryMode);
         Catchment catchment = catchmentService.createOrUpdate(catchmentName, location);
         User user = userRepository.findByUsername(username.trim());
         User currentUser = userService.getCurrentUser();
@@ -175,26 +174,34 @@ private void write(Row row) throws IDPException {
         }
     }
 
-    private void validateRowAndAssimilateErrors(List<String> rowValidationErrorMsgs, String fullAddress, String catchmentName, String nameOfUser, String username, String email, String phoneNumber, String language, String datePickerMode, AddressLevel location, Locale locale, String userSuffix) {
-        addErrMsgIfValidationFails(!StringUtils.hasLength(catchmentName), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, CATCHMENT_NAME));
-        addErrMsgIfValidationFails(!StringUtils.hasLength(username), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, USERNAME));
-        addErrMsgIfValidationFails(!StringUtils.hasLength(nameOfUser), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, FULL_NAME_OF_USER));
-        addErrMsgIfValidationFails(!StringUtils.hasLength(email), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, EMAIL_ADDRESS));
-        addErrMsgIfValidationFails(!StringUtils.hasLength(phoneNumber), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, MOBILE_NUMBER));
+    private void validateRowAndAssimilateErrors(List<String> rowValidationErrorMsgs, String fullAddress, String catchmentName, String nameOfUser, String username, String email, String phoneNumber, String language, String datePickerMode, AddressLevel location, Locale locale, String userSuffix, Boolean trackLocation, Boolean beneficiaryMode) {
+        addErrMsgIfValidationFails(!StringUtils.hasLength(catchmentName), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, CATCHMENT_NAME));
+        if (!addErrMsgIfValidationFails(!StringUtils.hasLength(username), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, USERNAME)))
+            extractUserUsernameValidationErrMsg(rowValidationErrorMsgs, username, userSuffix);
+        if (!addErrMsgIfValidationFails(!StringUtils.hasLength(nameOfUser), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, FULL_NAME_OF_USER)))
+            extractUserNameValidationErrMsg(rowValidationErrorMsgs, nameOfUser);
+        if (!addErrMsgIfValidationFails(!StringUtils.hasLength(email), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, EMAIL_ADDRESS)))
+            extractUserEmailValidationErrMsg(rowValidationErrorMsgs, email);
+        if (!addErrMsgIfValidationFails(!StringUtils.hasLength(phoneNumber), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, MOBILE_NUMBER)))
+            addErrMsgIfValidationFails(!PhoneNumberUtil.isValidPhoneNumber(phoneNumber, RegionUtil.getCurrentUserRegion()), rowValidationErrorMsgs, format(ERR_MSG_INVALID_PHONE_NUMBER, MOBILE_NUMBER));
 
-        addErrMsgIfValidationFails(Objects.isNull(location), rowValidationErrorMsgs, format(ERR_MSG_LOCATION_FIELD, fullAddress));
-        addErrMsgIfValidationFails(Objects.isNull(locale), rowValidationErrorMsgs, format(ERR_MSG_LOCALE_FIELD, language));
-        addErrMsgIfValidationFails(Objects.isNull(datePickerMode) || !DATE_PICKER_MODE_OPTIONS.contains(datePickerMode),
+        addErrMsgIfValidationFails(StringUtils.isEmpty(location), rowValidationErrorMsgs, format(ERR_MSG_LOCATION_FIELD, fullAddress));
+        addErrMsgIfValidationFails(StringUtils.isEmpty(locale), rowValidationErrorMsgs, format(ERR_MSG_LOCALE_FIELD, language));
+        addErrMsgIfValidationFails(StringUtils.isEmpty(datePickerMode) || !DATE_PICKER_MODE_OPTIONS.contains(datePickerMode),
                 rowValidationErrorMsgs, format(ERR_MSG_DATE_PICKER_FIELD, datePickerMode));
 
-        extractUserUsernameValidationErrMsg(rowValidationErrorMsgs, username, userSuffix);
-        extractUserNameValidationErrMsg(rowValidationErrorMsgs, nameOfUser);
-        extractUserEmailValidationErrMsg(rowValidationErrorMsgs, email);
+        addErrMsgIfValidationFails(trackLocation == null, rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, TRACK_LOCATION));
+        addErrMsgIfValidationFails(beneficiaryMode == null, rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, ENABLE_BENEFICIARY_MODE));
+
         if (!rowValidationErrorMsgs.isEmpty()) {
-            throw new RuntimeException(String.join(ERR_MSG_DELIMITER, rowValidationErrorMsgs));
+            throw new RuntimeException(createMultiErrorMessage(rowValidationErrorMsgs));
         }
     }
 
+    private static String createMultiErrorMessage(List<String> errorMsgs) {
+        return errorMsgs.stream().map(s -> "\"" + s + "\"").collect(Collectors.joining("; "));
+    }
+
     private void extractUserNameValidationErrMsg(List<String> rowValidationErrorMsgs, String nameOfUser) {
         try {
             User.validateName(nameOfUser);
@@ -219,10 +226,11 @@ private void extractUserUsernameValidationErrMsg(List<String> rowValidationError
         }
     }
 
-    private void addErrMsgIfValidationFails(boolean validationCheckResult, List<String> rowValidationErrorMsgs, String validationErrorMessage) {
+    private boolean addErrMsgIfValidationFails(boolean validationCheckResult, List<String> rowValidationErrorMsgs, String validationErrorMessage) {
         if (validationCheckResult) {
             rowValidationErrorMsgs.add(validationErrorMessage);
         }
+        return validationCheckResult;
     }
 
     private JsonObject constructSyncSettings(Row row, List<String> rowValidationErrorMsgs) {
@@ -245,7 +253,8 @@ private void updateSyncSettingsFor(String saHeader, Row row, Map<String, UserSyn
         if (headerPatternMatcher.matches()) {
             String conceptName = headerPatternMatcher.group("conceptName");
             String conceptValues = row.get(saHeader);
-            addErrMsgIfValidationFails(StringUtils.isEmpty(conceptValues), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, saHeader));
+            if (addErrMsgIfValidationFails(StringUtils.isEmpty(conceptValues), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, saHeader))) return;
+
             String subjectTypeName = headerPatternMatcher.group("subjectTypeName");
             SubjectType subjectType = subjectTypeService.getByName(subjectTypeName);
 

avni-server-api/src/main/java/org/avni/server/importer/batch/model/Row.java

@@ -11,13 +11,16 @@
 
 public class Row extends HashMap<String, String> {
     public static final Pattern TRUE_VALUE = Pattern.compile("y|yes|true|1", Pattern.CASE_INSENSITIVE);
+    public static final Pattern FALSE_VALUE = Pattern.compile("n|no|false|0", Pattern.CASE_INSENSITIVE);
     private final String[] headers;
     private final String[] values;
 
     public Row(String[] headers, String[] values) {
         this.headers = headers;
         this.values = values;
-        IntStream.range(0, values.length).forEach(index -> this.put(headers[index].trim(), values[index].trim()));
+        IntStream.range(0, headers.length).forEach(index -> {
+            this.put(headers[index].trim(), values.length > index ? values[index].trim() : "");
+        });
     }
 
     private String nullSafeTrim(String s) {
@@ -52,12 +55,17 @@ public String getOrDefault(Object key, String defaultValue) {
     @Override
     public String toString() {
         return IntStream.range(0, headers.length)
-                .mapToObj(index -> index < values.length? format("\"%s\"", values[index]): "\"\"")
+                .mapToObj(index -> index < values.length ? format("\"%s\"", values[index]) : "\"\"")
                 .reduce((c1, c2) -> format("%s,%s", c1, c2))
                 .get();
     }
 
     public Boolean getBool(String header) {
-        return TRUE_VALUE.matcher(String.valueOf(get(header))).matches();
+        if (TRUE_VALUE.matcher(String.valueOf(get(header))).matches()) {
+            return true;
+        } else if (FALSE_VALUE.matcher(String.valueOf(get(header))).matches()) {
+            return false;
+        }
+        return null;
     }
 }