From 4d580c041a878ed72e200bca6965f0beb22bc20d Mon Sep 17 00:00:00 2001 From: brianbrix Date: Mon, 18 May 2026 20:21:32 +0300 Subject: [PATCH 1/2] AMP-31128: Contact information is not saved in activity --- .../models/LocaleAwareProxyModel.java | 9 ++- .../amp/onepager/util/ActivityUtil.java | 62 +++++++++++++++---- .../aim/dbentity/AmpActivityFields.java | 10 ++- .../module/aim/dbentity/AmpAhsurvey.java | 27 ++++++-- .../aim/dbentity/AmpGPINiSurveyResponse.java | 17 ++++- .../module/aim/dbentity/AmpOrgRole.java | 13 +++- .../module/aim/util/ActivityVersionUtil.java | 16 +++-- .../module/aim/dbentity/AmpAhsurvey.hbm.xml | 2 +- 8 files changed, 125 insertions(+), 31 deletions(-) diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/models/LocaleAwareProxyModel.java b/amp/src/main/java/org/dgfoundation/amp/onepager/models/LocaleAwareProxyModel.java index d37bd3ca956..48bc8f72214 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/models/LocaleAwareProxyModel.java +++ b/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(); diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java index bf316094a25..556eb359bd1 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java @@ -212,6 +212,10 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, AmpActivityGroup tmpGroup = a.getAmpActivityGroup(); a = ActivityVersionUtil.cloneActivity(a); + // A cloned activity must become a new DB row. Keeping the original id would cause + // versioned children (contacts, structures, etc.) to remain linked to the current + // version row instead of the new one, so they appear lost on reopen. + a.setAmpActivityId(null); // 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 @@ -271,18 +275,41 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, if (!newActivity) { session.clear(); + if (createNewVersion) { + // After cloning, the group in 'a' is a stale deep-copy whose + // ampActivityLastVersion back-reference points to the clone + // (ampActivityId == null). Reload from DB so the comparison + // below uses the real stored last-version id. + 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); + + // When creating a new version, use the managed merged instance for the rest + // of the save flow so Hibernate owns the delete-orphan collections on that + // instance from the start. + 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 +321,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 +334,8 @@ 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: the managed merged instance above is still tracked in the session, + // so dirty-checking at flush() picks up all post-merge mutations. } else { // session.saveOrUpdate(a); session.merge(a); @@ -1291,9 +1318,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 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 activityDbContacts=ContactInfoUtil.getActivityContacts(oldActivityId); @@ -1310,6 +1343,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 +1353,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 savedContacts = new HashMap(); @@ -1330,9 +1362,11 @@ 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) { @@ -1340,6 +1374,7 @@ public static void saveContacts(AmpActivityVersion a, Session session, boolean c } // 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()); @@ -1347,7 +1382,8 @@ public static void saveContacts(AmpActivityVersion a, Session session, boolean c } if (activityContact.getId() == null) { session.saveOrUpdate(activityContact); - if (!newActivity) { + if (!newActivity && activityContact.getId() == null) { + session.saveOrUpdate(activityContact); session.merge(activityContact.getContact()); } } diff --git a/amp/src/main/java/org/digijava/module/aim/dbentity/AmpActivityFields.java b/amp/src/main/java/org/digijava/module/aim/dbentity/AmpActivityFields.java index b534d3abac0..667b49efd0a 100644 --- a/amp/src/main/java/org/digijava/module/aim/dbentity/AmpActivityFields.java +++ b/amp/src/main/java/org/digijava/module/aim/dbentity/AmpActivityFields.java @@ -1350,12 +1350,16 @@ public Set getActivityContacts() { } public void setActivityContacts(Set 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); + } } } diff --git a/amp/src/main/java/org/digijava/module/aim/dbentity/AmpAhsurvey.java b/amp/src/main/java/org/digijava/module/aim/dbentity/AmpAhsurvey.java index 97a3f95a8ae..bb7b75d61ef 100644 --- a/amp/src/main/java/org/digijava/module/aim/dbentity/AmpAhsurvey.java +++ b/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 getResponses() { * @param responses The responses to set. */ public void setResponses(Set 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 responses = new HashSet(); Iterator 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; } diff --git a/amp/src/main/java/org/digijava/module/aim/dbentity/AmpGPINiSurveyResponse.java b/amp/src/main/java/org/digijava/module/aim/dbentity/AmpGPINiSurveyResponse.java index f4aa30cd802..29ba620a7a4 100644 --- a/amp/src/main/java/org/digijava/module/aim/dbentity/AmpGPINiSurveyResponse.java +++ b/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 getSupportingDocuments() { } public void setSupportingDocuments(Set 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() { diff --git a/amp/src/main/java/org/digijava/module/aim/dbentity/AmpOrgRole.java b/amp/src/main/java/org/digijava/module/aim/dbentity/AmpOrgRole.java index 779f4597aaa..aa0b578d544 100644 --- a/amp/src/main/java/org/digijava/module/aim/dbentity/AmpOrgRole.java +++ b/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 getGpiNiSurveys() { return gpiNiSurveys; } public void setGpiNiSurveys(Set 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); } } diff --git a/amp/src/main/java/org/digijava/module/aim/util/ActivityVersionUtil.java b/amp/src/main/java/org/digijava/module/aim/util/ActivityVersionUtil.java index e30d9565ef9..67fb389b8a9 100644 --- a/amp/src/main/java/org/digijava/module/aim/util/ActivityVersionUtil.java +++ b/amp/src/main/java/org/digijava/module/aim/util/ActivityVersionUtil.java @@ -219,19 +219,23 @@ 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 + // reference to the new version. This MUST run after the deep-copy; + // doing it before was a no-op because SerializationUtils.clone(in) + // then overwrote all the prepared collections with the originals. + 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); diff --git a/amp/src/main/resources/org/digijava/module/aim/dbentity/AmpAhsurvey.hbm.xml b/amp/src/main/resources/org/digijava/module/aim/dbentity/AmpAhsurvey.hbm.xml index d5c5be128c1..54782ec09b6 100644 --- a/amp/src/main/resources/org/digijava/module/aim/dbentity/AmpAhsurvey.hbm.xml +++ b/amp/src/main/resources/org/digijava/module/aim/dbentity/AmpAhsurvey.hbm.xml @@ -25,7 +25,7 @@ - + From ac33651b270684dfd226e2053f0cc4b2c7a358ee Mon Sep 17 00:00:00 2001 From: brianbrix Date: Mon, 18 May 2026 20:22:07 +0300 Subject: [PATCH 2/2] AMP-31128: Contact information is not saved in activity --- .../amp/onepager/util/ActivityUtil.java | 21 ++----------------- .../module/aim/util/ActivityVersionUtil.java | 3 --- 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java index 556eb359bd1..4d747bb2583 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java @@ -212,17 +212,8 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, AmpActivityGroup tmpGroup = a.getAmpActivityGroup(); a = ActivityVersionUtil.cloneActivity(a); - // A cloned activity must become a new DB row. Keeping the original id would cause - // versioned children (contacts, structures, etc.) to remain linked to the current - // version row instead of the new one, so they appear lost on reopen. a.setAmpActivityId(null); - // 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. + session.clear(); a.setMember(new HashSet<>()); if (tmpGroup == null) { @@ -276,10 +267,7 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, if (!newActivity) { session.clear(); if (createNewVersion) { - // After cloning, the group in 'a' is a stale deep-copy whose - // ampActivityLastVersion back-reference points to the clone - // (ampActivityId == null). Reload from DB so the comparison - // below uses the real stored last-version id. + group = session.get(AmpActivityGroup.class, group.getAmpActivityGroupId()); a.setAmpActivityGroup(group); } @@ -300,9 +288,6 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, a.setAmpActivityGroup(group); updateMultiStakeholderField(a); - // When creating a new version, use the managed merged instance for the rest - // of the save flow so Hibernate owns the delete-orphan collections on that - // instance from the start. if (createNewVersion && a.getAmpActivityId() == null) { a = (AmpActivityVersion) session.merge(a); if (!newActivity) { @@ -334,8 +319,6 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, if (createNewVersion) { if (a.getAmpActivityId() == null) session.save(a); - // else: the managed merged instance above is still tracked in the session, - // so dirty-checking at flush() picks up all post-merge mutations. } else { // session.saveOrUpdate(a); session.merge(a); diff --git a/amp/src/main/java/org/digijava/module/aim/util/ActivityVersionUtil.java b/amp/src/main/java/org/digijava/module/aim/util/ActivityVersionUtil.java index 67fb389b8a9..b80d0a5ff79 100644 --- a/amp/src/main/java/org/digijava/module/aim/util/ActivityVersionUtil.java +++ b/amp/src/main/java/org/digijava/module/aim/util/ActivityVersionUtil.java @@ -226,9 +226,6 @@ public static AmpActivityVersion cloneActivity(AmpActivityVersion in) throws Clo // Re-initialize every Collection field: calls prepareMerge() on each // Versionable child, which nulls its DB id and re-points its activity - // reference to the new version. This MUST run after the deep-copy; - // doing it before was a no-op because SerializationUtils.clone(in) - // then overwrote all the prepared collections with the originals. Field[] fields = clazz.getDeclaredFields(); for (Field field : fields) { if (Collection.class.isAssignableFrom(field.getType())) {