AMP-31128: Contact information is not saved in activity

amp/src/main/java/org/dgfoundation/amp/onepager/models/LocaleAwareProxyModel.java

@@ -1,6 +1,7 @@
 package org.dgfoundation.amp.onepager.models;
 
 import org.apache.wicket.model.IModel;
+import org.dgfoundation.amp.onepager.translation.TranslatorUtil;
 import org.digijava.kernel.request.TLSUtils;
 
 /**
@@ -30,7 +31,13 @@ public boolean forwardToModel(){
     protected String localeOfLangModel(){
         if (langModel == null || langModel.getObject() == null){
             //same language as page
-            return TLSUtils.getLangCode();
+            String pageLocale = TLSUtils.getLangCode();
+            if (pageLocale != null) {
+                return pageLocale;
+            }
+
+            String defaultLocale = TranslatorUtil.getDefaultLocaleCache();
+            return defaultLocale != null ? defaultLocale : "";
         }
         else
             return langModel.getObject();

amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java

@@ -212,13 +212,8 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a,
                 AmpActivityGroup tmpGroup = a.getAmpActivityGroup();
 
                 a = ActivityVersionUtil.cloneActivity(a);
-                // Always clear the session after cloning. When running in a batch context (e.g. Excel importer),
-                // validateAndImport executes queries with FlushMode.AUTO which can cascade-save new child entities
-                // (fundings, etc.) into the action queue. The subsequent session.evict(oldA) then cascade-evicts
-                // those children from the persistence context while they remain in the action queue.  The flush
-                // below would find them in the queue but not in the persistence context.
-                // Clearing the session here discards those stale queued actions; downstream session.save()/
-                // saveOrUpdate() re-registers everything cleanly before the real INSERTs are flushed.
+                a.setAmpActivityId(null);
+
                 session.clear();
                 a.setMember(new HashSet<>());
                 if (tmpGroup == null) {
@@ -271,18 +266,35 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a,
 
         if (!newActivity) {
             session.clear();
+            if (createNewVersion) {
+
+                group = session.get(AmpActivityGroup.class, group.getAmpActivityGroupId());
+                a.setAmpActivityGroup(group);
+            }
             //existing activity
             //previousVersion for current activity
-            if (group.getAmpActivityLastVersion().getAmpActivityId().equals(a.getAmpActivityId())) {
+            if (group.getAmpActivityLastVersion() != null
+                    && group.getAmpActivityLastVersion().getAmpActivityId() != null
+                    && group.getAmpActivityLastVersion().getAmpActivityId().equals(oldA.getAmpActivityId())) {
                 forceVersionIncrement(session, group);
             }
-            group.setAmpActivityLastVersion(a);
-            session.merge(group);
+            if (!createNewVersion) {
+                group.setAmpActivityLastVersion(a);
+                session.merge(group);
+            }
 
         }
 
         a.setAmpActivityGroup(group);
         updateMultiStakeholderField(a);
+
+        if (createNewVersion && a.getAmpActivityId() == null) {
+            a = (AmpActivityVersion) session.merge(a);
+            if (!newActivity) {
+                group.setAmpActivityLastVersion(a);
+            }
+        }
+
         if (isActivityForm) {
             saveActivityResources(a, session);
             saveActivityGPINiResources(a, session);
@@ -294,7 +306,9 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a,
         saveEditors(session, createNewVersion, editorStore, site);
 
         saveAgreements(a, session, isActivityForm);
-        saveContacts(a, session, (draft != draftChange), ampCurrentMember);
+    saveContacts(a, session, (draft != draftChange), ampCurrentMember,
+        createNewVersion ? oldA.getAmpActivityId() : a.getAmpActivityId(),
+        createNewVersion || a.getAmpActivityId() == null);
 
         updateComponentFunding(a, session);
         saveAnnualProjectBudgets(a, session);
@@ -305,10 +319,6 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a,
         if (createNewVersion) {
             if (a.getAmpActivityId() == null)
                 session.save(a);
-            else {
-                cleanObjectFromSession(session,AmpActivityVersion.class, a.getAmpActivityId());
-                session.saveOrUpdate(a);
-            }
         } else {
 //            session.saveOrUpdate(a);
             session.merge(a);
@@ -1291,9 +1301,15 @@ private static boolean shouldResponseToBeUpdated(AmpGPINiSurveyResponse surveyRe
 
     public static void saveContacts(AmpActivityVersion a, Session session, boolean checkForContactsRemoval,
                                     AmpTeamMember teamMember) throws Exception {
+        saveContacts(a, session, checkForContactsRemoval, teamMember, a.getAmpActivityId(),
+                a.getAmpActivityId() == null);
+    }
+
+    public static void saveContacts(AmpActivityVersion a, Session session, boolean checkForContactsRemoval,
+                                    AmpTeamMember teamMember, Long oldActivityId,
+                                    boolean newActivity) throws Exception {
         Set<AmpActivityContact> activityContacts = a.getActivityContacts();
         // if activity contains contact,which is not in contact list, we should remove it
-        Long oldActivityId = a.getAmpActivityId();
         if (oldActivityId != null) {
             if (checkForContactsRemoval || !ActivityVersionUtil.isVersioningEnabled()) {
                 //List<AmpActivityContact> activityDbContacts=ContactInfoUtil.getActivityContacts(oldActivityId);
@@ -1310,6 +1326,7 @@ public static void saveContacts(AmpActivityVersion a, Session session, boolean c
                             }
                         }
                         if (count == 0) { //if activity contains contact,which is not in contact list, we should remove it
+                            logger.info("Contact with id " + actContactId + " will be removed");
                             Query qry = session.createQuery("delete from " + AmpActivityContact.class.getName() + " a where a.id=" + actContactId);
                             qry.executeUpdate();
                         }
@@ -1319,8 +1336,6 @@ public static void saveContacts(AmpActivityVersion a, Session session, boolean c
             }
         }
 
-        boolean newActivity = a.getAmpActivityId() == null;
-
         //to avoid saving the same contact twice on the same session, we keep track of the
         //already saved ones.
         Map<Long, Boolean> savedContacts = new HashMap<Long, Boolean>();
@@ -1330,24 +1345,28 @@ public static void saveContacts(AmpActivityVersion a, Session session, boolean c
         }
         try {
             //add or edit activity contact and amp contact
-            if (activityContacts != null && !activityContacts.isEmpty()) {
+            if (activityContacts != null) {
                 for (AmpActivityContact activityContact : activityContacts) {
+                    activityContact.setActivity(a);
                     Long contactId = activityContact.getContact().getId();
+                    logger.info("Processing contact with id " + contactId);
                     // if the contact already exists on the DB, and was not saved
                     // already
                     if (contactId != null && savedContacts.get(contactId) == null) {
                         savedContacts.put(activityContact.getContact().getId(), false);
                     }
                     // save the contact first, if the contact is new or if it is not
                     // new but has not been saved already.
+                    logger.info("Contact with id " + contactId + " is new: " + (contactId == null) + " and has been saved: " + savedContacts.get(contactId));
                     if (contactId == null || (newActivity && !savedContacts.get(contactId))) {
                         activityContact.getContact().setCreator(creator);
                         session.saveOrUpdate(activityContact.getContact());
                         savedContacts.put(activityContact.getContact().getId(), true);
                     }
                     if (activityContact.getId() == null) {
                         session.saveOrUpdate(activityContact);
-                        if (!newActivity) {
+                        if (!newActivity && activityContact.getId() == null) {
+                            session.saveOrUpdate(activityContact);
                             session.merge(activityContact.getContact());
                         }
                     }

amp/src/main/java/org/digijava/module/aim/dbentity/AmpActivityFields.java

@@ -1350,12 +1350,16 @@ public Set<AmpActivityContact> getActivityContacts() {
         }
 
         public void setActivityContacts(Set<AmpActivityContact> activityContacts) {
-            if (this.activityContacts == null) {
+            if (activityContacts instanceof PersistentSet) {
                 this.activityContacts = activityContacts;
             } else {
+                if (this.activityContacts == null) {
+                    this.activityContacts = new HashSet<>();
+                }
                 this.activityContacts.clear();
-                if (activityContacts==null)activityContacts=new HashSet<>();
-                this.activityContacts.addAll(activityContacts);
+                if (activityContacts != null) {
+                    this.activityContacts.addAll(activityContacts);
+                }
             }
         }
 

amp/src/main/java/org/digijava/module/aim/dbentity/AmpAhsurvey.java

@@ -8,6 +8,7 @@
 
 import org.digijava.module.aim.annotations.interchange.Interchangeable;
 import org.digijava.module.aim.util.Output;
+import org.hibernate.collection.internal.PersistentSet;
 
 import java.io.Serializable;
 import java.util.*;
@@ -61,7 +62,21 @@ public Set<AmpAhsurveyResponse> getResponses() {
      * @param responses The responses to set.
      */
     public void setResponses(Set<AmpAhsurveyResponse> responses) {
-        this.responses = responses;
+        if (responses instanceof PersistentSet) {
+            this.responses = responses;
+        } else {
+            if (this.responses == null) {
+                if (responses == null) {
+                    responses = new HashSet<>();
+                }
+                this.responses = new HashSet<>(responses);
+            }
+            this.responses.clear();
+            if (responses == null) {
+                responses = new HashSet<>();
+            }
+            this.responses.addAll(responses);
+        }
     }
     /**
      * @return Returns the ampActivityId.
@@ -179,14 +194,18 @@ public Object prepareMerge(AmpActivityVersion newActivity) throws CloneNotSuppor
             Set<AmpAhsurveyResponse> responses = new HashSet<AmpAhsurveyResponse>();
             Iterator<AmpAhsurveyResponse> i = aux.responses.iterator();
             while (i.hasNext()) {
-                AmpAhsurveyResponse newResp = (AmpAhsurveyResponse) i.next().clone();
+                AmpAhsurveyResponse currentResponse = i.next();
+                if (currentResponse == null || currentResponse.getAmpQuestionId() == null) {
+                    continue;
+                }
+                AmpAhsurveyResponse newResp = (AmpAhsurveyResponse) currentResponse.clone();
                 newResp.setAmpAHSurveyId(aux);
                 newResp.setAmpReponseId(null);
                 responses.add(newResp);
             }
-            aux.responses = responses;
+            aux.setResponses(responses.isEmpty() ? null : responses);
         } else {
-            aux.responses = null;
+            aux.setResponses(null);
         }
         return aux;
     }

amp/src/main/java/org/digijava/module/aim/dbentity/AmpGPINiSurveyResponse.java

@@ -1,6 +1,7 @@
 package org.digijava.module.aim.dbentity;
 
 import org.apache.commons.lang3.StringUtils;
+import org.hibernate.collection.internal.PersistentSet;
 
 import java.io.Serializable;
 import java.util.HashSet;
@@ -110,7 +111,21 @@ public Set<AmpGPINiSurveyResponseDocument> getSupportingDocuments() {
     }
 
     public void setSupportingDocuments(Set<AmpGPINiSurveyResponseDocument> supportingDocuments) {
-        this.supportingDocuments = supportingDocuments;
+        if (supportingDocuments instanceof PersistentSet) {
+            this.supportingDocuments = supportingDocuments;
+        } else {
+            if (this.supportingDocuments == null) {
+                if (supportingDocuments == null) {
+                    supportingDocuments = new HashSet<>();
+                }
+                this.supportingDocuments = new HashSet<>(supportingDocuments);
+            }
+            this.supportingDocuments.clear();
+            if (supportingDocuments == null) {
+                supportingDocuments = new HashSet<>();
+            }
+            this.supportingDocuments.addAll(supportingDocuments);
+        }
     }
 
     public boolean isEmpty() {

amp/src/main/java/org/digijava/module/aim/dbentity/AmpOrgRole.java

@@ -10,6 +10,7 @@
 import org.digijava.module.aim.annotations.interchange.InterchangeableValidator;
 import org.digijava.module.aim.util.Output;
 import org.digijava.module.aim.util.SerializableComparator;
+import org.hibernate.collection.internal.PersistentSet;
 
 import java.io.Serializable;
 import java.util.*;
@@ -289,11 +290,19 @@ public Set<AmpGPINiSurvey> getGpiNiSurveys() {
         return gpiNiSurveys;
     }
     public void setGpiNiSurveys(Set<AmpGPINiSurvey> gpiNiSurveys) {
-        if (this.gpiNiSurveys == null) {
+        if (gpiNiSurveys instanceof PersistentSet) {
             this.gpiNiSurveys = gpiNiSurveys;
         } else {
+            if (this.gpiNiSurveys == null) {
+                if (gpiNiSurveys == null) {
+                    gpiNiSurveys = new HashSet<>();
+                }
+                this.gpiNiSurveys = new HashSet<>(gpiNiSurveys);
+            }
             this.gpiNiSurveys.clear();
-            if (gpiNiSurveys==null)gpiNiSurveys=new HashSet<>();
+            if (gpiNiSurveys == null) {
+                gpiNiSurveys = new HashSet<>();
+            }
             this.gpiNiSurveys.addAll(gpiNiSurveys);
         }
     }

amp/src/main/java/org/digijava/module/aim/util/ActivityVersionUtil.java

@@ -219,19 +219,20 @@ public static Long getLastVersionForVersion(Long oldActivity) throws CannotGetLa
      * @throws CloneNotSupportedException
      */
     public static AmpActivityVersion cloneActivity(AmpActivityVersion in) throws CloneNotSupportedException {
-        AmpActivityVersion out = (AmpActivityVersion) in.clone();
-
+        // Deep-copy first so all nested objects are independent instances.
+        AmpActivityVersion out = SerializationUtils.clone(in);
+
         Class clazz = AmpActivityFields.class;
-
-        Field[] fields = clazz.getDeclaredFields();//clazz.getFields();
+
+        // Re-initialize every Collection field: calls prepareMerge() on each
+        // Versionable child, which nulls its DB id and re-points its activity
+        Field[] fields = clazz.getDeclaredFields();
         for (Field field : fields) {
             if (Collection.class.isAssignableFrom(field.getType())) {
                 logger.debug("Init set: " + field.getName());
                 initSet(out, field);
             }
         }
-        out = SerializationUtils.clone(in);
-
 
 //        out.setAmpActivityGroup(null);
 

amp/src/main/resources/org/digijava/module/aim/dbentity/AmpAhsurvey.hbm.xml

@@ -25,7 +25,7 @@
 		<many-to-one name="pointOfDeliveryDonor" column="point_of_delivery_donor"
 			class="org.digijava.module.aim.dbentity.AmpOrganisation" cascade="none" />
 
-		<set name="responses" table="AMP_AHSURVEY_RESPONSE" inverse="true" cascade="save-update" lazy="false" fetch="join">
+		<set name="responses" table="AMP_AHSURVEY_RESPONSE" inverse="true" cascade="merge,save-update" lazy="false" fetch="join">
 			<cache usage="read-write" />
 			<key column="amp_ahsurvey_id" />
 			<one-to-many class="org.digijava.module.aim.dbentity.AmpAhsurveyResponse" />