Allow updates to projects via project load mechanism #3466

Author: chrisala

grails-app/services/au/org/ala/merit/ImportService.groovy

@@ -307,8 +307,7 @@ class ImportService {
             mu?.managementUnitId
         }
         refreshOrganisationList()
-        def mapper = new GmsMapper(metadataService.activitiesModel(), metadataService.programsModel(), organisations, abnLookupService, metadataService.getOutputTargetScores(), programs, managementUnits)
-
+        def mapper = new GmsMapper(metadataService.activitiesModel(), metadataService.programsModel(), organisations, abnLookupService, metadataService.getOutputTargetScores(), programs, managementUnits, false, update)
         def action = preview?{rows -> mapProjectRows(rows, status, mapper, update)}:{rows -> importAll(rows, status, mapper, update)}
 
         Map result = [:]
@@ -360,7 +359,7 @@ class ImportService {
 
     def mapProjectRows(projectRows, List status, GmsMapper mapper, Boolean update) {
 
-        Map mappingResults = mapper.mapProject(projectRows)
+        Map mappingResults = mapper.mapProject(projectRows, update)
 
         String grantId = mappingResults.project.grantId
         String externalId = mappingResults.project.externalId
@@ -394,9 +393,6 @@ class ImportService {
             def editorEmail = projectDetails.project.remove('editorEmail')
             def editorEmail2 = projectDetails.project.remove('editorEmail2')
 
-            //When projects are loaded into MERIT via CSV upload, they are given a status of "Application".
-            projectDetails.project.status ?: 'application'
-
             // Create the organisation first so we can link it to the project.
             if (projectDetails.organisation) {
                 Map orgCreationResult = organisationService.update(null, projectDetails.organisation)

grails-app/views/admin/import.gsp

@@ -34,9 +34,7 @@
         </p>
         <div data-bind="if:update">
         <p class="alert alert-warning" >
-            Please note this function will replace all project information for each project. <br/>
-            It is designed for use with grants hub data and to fix newly loaded projects.  Please test changes
-            to existing MERIT projects in the staging system before running the update in production.
+            Please test changes to existing MERIT projects in the staging system before running the update in production.
         </p>
         </div>
         <div class="form-check">

src/main/groovy/au/org/ala/merit/GmsMapper.groovy

@@ -2,7 +2,7 @@ package au.org.ala.merit
 
 import au.com.bytecode.opencsv.CSVWriter
 import org.apache.commons.lang.WordUtils
-import org.apache.commons.validator.EmailValidator
+import org.apache.commons.validator.routines.EmailValidator
 
 import java.text.DecimalFormat
 import java.text.SimpleDateFormat
@@ -95,6 +95,7 @@ class GmsMapper {
 
     def geographicInfoMapping = [
             NATIONWIDE:[name:'nationwide', type:'boolean', description:'If true, this project does not have a primary state'],
+            STATEWIDE:[name:'statewide', type:'boolean', description:'If true, this project does not have a primary electorate'],
             PRIMARY_STATE:[name:'primaryState', type:'string', description:'The primary state to be manually assigned to this project'],
             PRIMARY_ELECTORATE:[name:'primaryElectorate', type:'string', description:'The primary electorate to be manually assigned to this project'],
             OTHER_ELECTORATES:[name:'otherElectorates', type:'list', description:'Other electorates to be manually assigned to this project.  Enter as a comma separated list'],
@@ -135,6 +136,8 @@ class GmsMapper {
     ]
 
     private boolean includeProgress
+    /** If the mapping is being done to update a project, mandatory fields aren't required as they won't be updated if absent */
+    private boolean mapForUpdate = false
 
     public GmsMapper() {
         this.activitiesModel = []
@@ -144,9 +147,10 @@ class GmsMapper {
         this.programs = [:]
         this.managementUnits = [:]
         includeProgress = false
+        mapForUpdate = false
     }
 
-    GmsMapper(activitiesModel, programModel, organisations, abnLookup, List<Map> scores, Map programs = [:], Map managementUnits = [:], includeProgress = false) {
+    GmsMapper(activitiesModel, programModel, organisations, abnLookup, List<Map> scores, Map programs = [:], Map managementUnits = [:], includeProgress = false, mapForUpdate = false) {
         this.activitiesModel = activitiesModel
         this.programModel = programModel
         this.includeProgress = includeProgress
@@ -155,6 +159,7 @@ class GmsMapper {
         this.programs = programs
         this.managementUnits = managementUnits
         this.abnLookupService = abnLookup
+        this.mapForUpdate = mapForUpdate
     }
 
     /** Creates a CSV file in the format required to import projects into MERIT with additional instructions */
@@ -222,29 +227,21 @@ class GmsMapper {
         return errors
     }
 
-    def mapProject(projectRows) {
+    def mapProject(projectRows, boolean update = false) {
 
         List errors = []
         List messages = []
-        Map result = gmsToMerit(projectRows[0], projectMapping) // All project rows have the project details.
+        Map result = csvToProjectProperties(projectRows[0], projectMapping, update) // All project rows have the project details.
         def project = result.mappedData
-        project.projectType = 'works'
-        project.isMERIT = true
+        if (!update) {
+            // Set some default project properties for the MERIT hub
+            project.projectType = 'works'
+            project.isMERIT = true
+        }
 
-        mapGeographicInfo(projectRows[0], project, errors)
+        mapGeographicInfo(projectRows[0], project, update, errors)
 
-
-        String programName = project.associatedSubProgram ?: project.associatedProgram
-        Map program = programs[programName]
-        String programId = program?.programId
-        if (programId) {
-            project.remove('associatedProgram')
-            project.remove('associatedSubProgram')
-            project.programId = programId
-        }
-        else {
-            errors << "Program ${programName} doesn't match an existing MERIT programme"
-        }
+        mapProgram(project, update, errors)
 
         if (project.managementUnitName) {
             String managementUnitName = project.remove('managementUnitName')
@@ -254,10 +251,9 @@ class GmsMapper {
             }
         }
 
-        Map organisation = mapOrganisation(project, program, errors, messages)
+        Map organisation = mapOrganisation(project, update, errors, messages)
 
         errors.addAll(result.errors)
-        project.planStatus = project.planStatus ?: 'not approved'
 
         mapRisks(projectRows, project, errors)
 
@@ -270,18 +266,44 @@ class GmsMapper {
             project.custom = [details:meriPlan]
         }
 
+        if (update) {
+            Set propertiesToUpdate = new HashSet(project.keySet())
+            propertiesToUpdate.remove('grantId') // This identifies the project and won't be updated.
+            messages << "Update: ${propertiesToUpdate}"
+        }
+
         [project:project, sites:sites, activities:activities, errors:errors, messages:messages, organisation:organisation]
 
     }
 
+    private Map mapProgram(LinkedHashMap<Object, Object> project, boolean update, ArrayList errors) {
+        String programName = project.associatedSubProgram ?: project.associatedProgram
+        if (!programName) {
+            if (!update) {
+                errors << "Please supply a program name (PROGRAM_NM) or sub-program name (ROUND_NM) for the project"
+            }
+            return
+        }
+        Map program = programs[programName]
+        String programId = program?.programId
+        if (programId) {
+            project.remove('associatedProgram')
+            project.remove('associatedSubProgram')
+            project.programId = programId
+        } else {
+            errors << "Program ${programName} doesn't match an existing MERIT programme"
+        }
+        program
+    }
+
     private Map findExistingOrganisation(String organisationId, String abn) {
         organisations.find{
             (abn && (it.abn == abn)) ||
                     (organisationId && (it.organisationId == organisationId)) }
 
     }
 
-    private Map mapOrganisation(Map project, Map program, List errors, List messages) {
+    private Map mapOrganisation(Map project, boolean update, List errors, List messages) {
         Map organisation
         Map abnLookup
         String error = null
@@ -339,7 +361,7 @@ class GmsMapper {
                 project.associatedOrgs = [
                         [organisationId:organisation.organisationId, name: contractName ?: organisation.name, description:description]]
             }
-        } else {
+        } else if (!update) {
             error = "Please supply an organisationId (ORG_ID) or ABN (ABN) for the project"
         }
         if (error) {
@@ -395,7 +417,7 @@ class GmsMapper {
         def count = siteRows.size()
         siteRows.eachWithIndex {siteRow, i ->
 
-            def siteResult = gmsToMerit(siteRow, siteMapping)
+            def siteResult = csvToProjectProperties(siteRow, siteMapping)
             def site = siteResult.mappedData
             errors.addAll(siteResult.errors)
 
@@ -432,7 +454,7 @@ class GmsMapper {
         def activityRows = projectRows.findAll{it[DATA_TYPE_COLUMN] == ACTIVITY_DATA_TYPE && it[DATA_SUB_TYPE_COLUMN] == ACTIVITY_DATA_SUB_TYPE}
         def activities = []
         activityRows.eachWithIndex { activityRow, i ->
-            def activityResult = gmsToMerit(activityRow, activityMapping)
+            def activityResult = csvToProjectProperties(activityRow, activityMapping)
             def mappedActivity = activityResult.mappedData
             errors.addAll(activityResult.errors)
 
@@ -526,7 +548,7 @@ class GmsMapper {
         def risks = []
         riskRows.eachWithIndex { riskRow, i ->
 
-            def result = gmsToMerit(riskRow, riskMapping)
+            def result = csvToProjectProperties(riskRow, riskMapping)
             errors.addAll(result.errors)
             def risk = [
                     "threat"  : "",
@@ -546,19 +568,19 @@ class GmsMapper {
         }
     }
 
-    private void mapGeographicInfo(Map rowData, Map project, List errors) {
+    private void mapGeographicInfo(Map rowData, Map project, boolean update, List errors) {
 
-        Map result = gmsToMerit(rowData, geographicInfoMapping)
+        Map result = csvToProjectProperties(rowData, geographicInfoMapping, update)
         if (result.mappedData) {
             project.geographicInfo = result.mappedData
         }
-        errors.addAll(errors)
+        errors.addAll(result.errors)
     }
 
     private def mapTarget(rowMap) {
 
         def errors = []
-        def map = gmsToMerit(rowMap, outputTargetColumnMapping)
+        def map = csvToProjectProperties(rowMap, outputTargetColumnMapping)
         def target = map.mappedData
         errors.addAll(map.errors)
 
@@ -594,7 +616,7 @@ class GmsMapper {
         [mappedData:result, errors: errors]
     }
 
-    private def gmsToMerit(Map rowMap, Map mapping) {
+    private def csvToProjectProperties(Map rowMap, Map mapping, boolean update = false) {
         def result = [:]
         def errors = []
         mapping.each { Map.Entry e ->
@@ -605,16 +627,20 @@ class GmsMapper {
                 String propertyName = columnMapping.name
                 if (columnMapping.multipleColumnsSupported) {
                     List values = findMultiValue(mappingColumnName, rowMap)
-                    if (!result[propertyName]) {
+                    if (!result[propertyName] && !update) {
                         result[propertyName] = []
                     }
                     values = values.findAll{it}
-                    result[propertyName].addAll(values.collect{convertByType(it, columnMapping)}.findAll{it})
+                    if (values || !update) {
+                        result[propertyName].addAll(values.collect{convertByType(it, columnMapping, update)}.findAll{it})
+                    }
                 }
                 else {
-                    result[propertyName] = convertByType(rowMap[mappingColumnName], columnMapping)
+                    def value = rowMap[mappingColumnName]?.trim()
+                    if (value || !update) { // If we are doing an update, ignore empty values.
+                        result[propertyName] = convertByType(rowMap[mappingColumnName], columnMapping, update)
+                    }
                 }
-
             }
             catch (Exception ex) {
                 errors << "Error converting value: ${rowMap[mappingColumnName]} from row ${rowMap.index} column: ${mappingColumnName}, ${ex.getMessage()}"
@@ -627,22 +653,25 @@ class GmsMapper {
     private List findMultiValue(String mappingColumnName, Map rowMap) {
         rowMap.findAll { String key, def value ->
             key.startsWith(mappingColumnName)
-        }.collect{it.value}
+        }.collect{it.value?.trim()}
     }
 
-    private def convertByType(String value, Map mapping) {
+    private def convertByType(String value, Map mapping, boolean update) {
         String type = mapping.type
         value = value?value.trim():''
         def result = null
         switch (type) {
             case 'date':
-                result = convertDate(value, mapping.mandatory)
+                result = convertDate(value, mapping.mandatory && !update)
                 break
             case 'decimal':
                 result = convertDecimal(value)
                 break
             case 'string':
-                result = value ?: (mapping['default']?:value)
+                result = value
+                if (!result && !update) {
+                    result = mapping['default']?:value
+                }
                 break
             case 'url':
                 URI.create(value) // validation purposes only
@@ -669,6 +698,9 @@ class GmsMapper {
             case 'list':
                 result = value?.split(',').collect{it.trim()}.findAll{it}
                 break
+            case 'boolean':
+                result = value?.toBoolean() // accept "y"/"true"/"1" as true
+                break
             default:
                 throw new IllegalArgumentException("Unsupported type: ${type}")
         }

src/test/groovy/au/org/ala/merit/GmsMapperSpec.groovy

@@ -30,7 +30,7 @@ class GmsMapperSpec extends Specification{
         activitiesModel = JSON.parse(new InputStreamReader(getClass().getResourceAsStream('/activities-model.json')))
         Map programModel = [programs:[[name:'Green Army']]]
         List organisations = [[ organisationId: "123", name:'Test org 1', abn:'12345678901'], [organisationId:'2', name:"Org 2", abn:""]]
-        gmsMapper = new GmsMapper(activitiesModel, programModel, organisations, abnLookupService, scores, programs)
+        gmsMapper = new GmsMapper(activitiesModel, programModel, organisations, abnLookupService, scores, programs, [:], false, false)
     }
 
     /**

src/test/groovy/au/org/ala/merit/ImportServiceSpec.groovy

@@ -114,7 +114,7 @@ class ImportServiceSpec extends Specification implements ServiceUnitTest<ImportS
 
     def "The import service can create projects that have been loaded and mapped via CSV"() {
         setup:
-        GmsMapper mapper = new GmsMapper(activitiesModel, [:], [[organisationId:"123", name:"Test organisation"]],abnLookupService, scores, ["Test program":["programId":"p1"]], ["Test MU":"m1"])
+        GmsMapper mapper = new GmsMapper(activitiesModel, [:], [[organisationId:"123", name:"Test organisation"]],abnLookupService, scores, ["Test program":["programId":"p1"]], ["Test MU":"m1"], false, false)
         List projectRows = projectRowData()
         List status = []
         String projectId = 'p1'
@@ -230,7 +230,7 @@ class ImportServiceSpec extends Specification implements ServiceUnitTest<ImportS
 
     def "A project won't be imported if update=false and MERIT already has a project with the same grantId/externalId"() {
         setup:
-        GmsMapper mapper = new GmsMapper(activitiesModel, [:], [[organisationId:"123", name:"Test organisation"]],abnLookupService, scores, ["Test program":[programId:"p1"]], ["Test MU":"m1"])
+        GmsMapper mapper = new GmsMapper(activitiesModel, [:], [[organisationId:"123", name:"Test organisation"]],abnLookupService, scores, ["Test program":[programId:"p1"]], ["Test MU":"m1"], false, false)
         List projectRows = projectRowData()
         List status = []
         String projectId = 'p1'
@@ -251,7 +251,7 @@ class ImportServiceSpec extends Specification implements ServiceUnitTest<ImportS
 
     def "A project won't be updated if update=true and MERIT does not have a project with the same grantId/externalId"() {
         setup:
-        GmsMapper mapper = new GmsMapper(activitiesModel, [:], [[organisationId:"123", name:"Test organisation"]],abnLookupService, scores, ["Test program":[programId:"p1"]], ["Test MU":"m1"])
+        GmsMapper mapper = new GmsMapper(activitiesModel, [:], [[organisationId:"123", name:"Test organisation"]],abnLookupService, scores, ["Test program":[programId:"p1"]], ["Test MU":"m1"], false, false)
         List projectRows = projectRowData()
         List status = []
         String projectId = 'p1'