AMP-31133 : Activity form not saving disaggregated values

amp/TEMPLATE/reamp/package.json

@@ -23,7 +23,7 @@
   "author": "Alexei Savca",
   "license": "inherit",
   "devDependencies": {
-    "amp-ui": "github:devgateway/amp-ui#add-missing-me-fields-to-preview-and-api",
+    "amp-ui": "github:devgateway/amp-ui#fix/AMP-31133/Indicator-values-display-fixes",
     "babel-core": "^6.26.3",
     "babel-jest": "^6.0.1",
     "babel-loader": "^6.3.2",

amp/src/main/java/org/dgfoundation/amp/onepager/components/ListEditor.java

@@ -72,6 +72,12 @@ protected final void onBeforeRender(){
 
     @Override
     public void updateModel(){
+        // items is transient — it is null after page deserialization (e.g. on form submit).
+        // In that case re-populate from the model so we don't wipe the backing collection.
+        if (items == null) {
+            Set<T> current = model.getObject();
+            items = current != null ? new ArrayList<T>(current) : new ArrayList<T>();
+        }
         Set<T> set = model.getObject();
         if (set == null)
             set = new LinkedHashSet<T>();

amp/src/main/java/org/dgfoundation/amp/onepager/components/ListItem.java

@@ -11,8 +11,15 @@ public ListItem(String id, int index){
 
     private class ListItemModel extends AbstractReadOnlyModel<T>{
         public T getObject(){
-            return ((ListEditor<T>)ListItem.this.getParent())
-            .items.get(getIndex());
+            ListEditor<T> editor = (ListEditor<T>) ListItem.this.getParent();
+            if (editor == null || editor.items == null) {
+                return null;
+            }
+            int idx = getIndex();
+            if (idx < 0 || idx >= editor.items.size()) {
+                return null;
+            }
+            return editor.items.get(idx);
         }
     }
 }

amp/src/main/java/org/dgfoundation/amp/onepager/components/features/AmpActivityFormFeature.java

@@ -1384,13 +1384,15 @@ public void component(Component component, IVisit<Void> visit) {
                                 if ("commitments".equals(id)) {
 
                                     AmpFunding funding = (AmpFunding) component.getDefaultModel().getObject();
+                                    if (funding == null) { visit.dontGoDeeper(); return; }
                                     boolean commitmentsRequired = FMUtil.isFmVisible(findComponentById(form, "requireCommitments"));
                                     logger.info("Commitments required: " + commitmentsRequired);
                                     setErrorWHenItemMissing(component, funding, commitmentsRequired, Constants.COMMITMENT, errors, target);
 
                                 }
                                 if ("disbursements".equals(id)) {
                                     AmpFunding funding = (AmpFunding) component.getDefaultModel().getObject();
+                                    if (funding == null) { visit.dontGoDeeper(); return; }
                                     boolean disbursementsRequired = FMUtil.isFmVisible(findComponentById(form, "requireDisbursements"));
                                     logger.info("Disbursements required: " + disbursementsRequired);
                                     setErrorWHenItemMissing(component, funding, disbursementsRequired, Constants.DISBURSEMENT, errors, target);

amp/src/main/java/org/dgfoundation/amp/onepager/components/features/items/AmpMEDisaggregationActualValuesPanel.java

@@ -1,83 +1,94 @@
 package org.dgfoundation.amp.onepager.components.features.items;
 
 import org.apache.wicket.ajax.AjaxRequestTarget;
-import org.apache.wicket.markup.html.list.ListItem;
-import org.apache.wicket.markup.html.list.ListView;
 import org.apache.wicket.model.IModel;
-import org.apache.wicket.model.LoadableDetachableModel;
 import org.apache.wicket.model.PropertyModel;
+import org.apache.wicket.util.convert.IConverter;
 import org.dgfoundation.amp.onepager.OnePagerUtil;
+import org.dgfoundation.amp.onepager.components.ListEditor;
 import org.dgfoundation.amp.onepager.components.ListEditorRemoveButton;
+import org.dgfoundation.amp.onepager.components.ListItem;
 import org.dgfoundation.amp.onepager.components.features.AmpFeaturePanel;
-import org.dgfoundation.amp.onepager.components.features.me.singlecountry.AmpMEActualValuesFormTableFeaturePanel;
 import org.dgfoundation.amp.onepager.components.fields.AmpAjaxLinkField;
 import org.dgfoundation.amp.onepager.components.fields.AmpDatePickerFieldPanel;
 import org.dgfoundation.amp.onepager.components.fields.AmpTextFieldPanel;
+import org.dgfoundation.amp.onepager.converters.CustomDoubleConverter;
+import org.dgfoundation.amp.onepager.models.AbstractMixedSetModel;
+import org.digijava.module.aim.dbentity.AmpActivityLocation;
 import org.digijava.module.aim.dbentity.AmpIndicatorDisaggregationValue;
 import org.digijava.module.aim.dbentity.AmpIndicatorGlobalValue;
-import org.dgfoundation.amp.onepager.converters.CustomDoubleConverter;
 
-import java.util.*;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
 
 /**
  * Panel rendering and editing ACTUAL values (AmpIndicatorGlobalValue entries with type ACTUAL)
  * for a specific AmpIndicatorDisaggregationValue child category combination.
  */
 public class AmpMEDisaggregationActualValuesPanel extends AmpFeaturePanel<AmpIndicatorDisaggregationValue> {
 
-    private final ListView<AmpIndicatorGlobalValue> listView;
+    private final ListEditor<AmpIndicatorGlobalValue> listEditor;
 
-    public AmpMEDisaggregationActualValuesPanel(String id, IModel<AmpIndicatorDisaggregationValue> model) {
+    public AmpMEDisaggregationActualValuesPanel(String id, IModel<AmpIndicatorDisaggregationValue> model,
+                                                IModel<AmpActivityLocation> location) {
         super(id, model, "Disaggregation Actual Values", true);
         setOutputMarkupId(true);
 
-        // Use a List model for ListView (was Set, causing type mismatch)
-        IModel<List<AmpIndicatorGlobalValue>> listModel = new LoadableDetachableModel<List<AmpIndicatorGlobalValue>>() {
+        // ListEditor.updateModel() writes submitted rows back into this backing set.
+        IModel<Set<AmpIndicatorGlobalValue>> parentModel = new PropertyModel<>(model, "actualValues");
+        IModel<Set<AmpIndicatorGlobalValue>> setModel = new AbstractMixedSetModel<AmpIndicatorGlobalValue>(parentModel) {
             @Override
-            protected List<AmpIndicatorGlobalValue> load() {
-                AmpIndicatorDisaggregationValue disagg = AmpMEDisaggregationActualValuesPanel.this.getModel().getObject();
-                if (disagg.getActualValues() == null) {
-                    disagg.setActualValues(new java.util.HashSet<>());
+            public boolean condition(AmpIndicatorGlobalValue item) {
+                AmpActivityLocation currentLocation = location != null ? location.getObject() : null;
+                if (currentLocation == null) {
+                    return item.getActivityLocation() == null;
                 }
-                return new java.util.ArrayList<>(disagg.getActualValues());
+                return item.getActivityLocation() != null
+                        && Objects.equals(item.getActivityLocation().getId(), currentLocation.getId());
             }
         };
 
-        listView = new ListView<AmpIndicatorGlobalValue>("rows", listModel) {
+        listEditor = new ListEditor<AmpIndicatorGlobalValue>("rows", setModel) {
             @Override
-            protected void populateItem(ListItem<AmpIndicatorGlobalValue> item) {
-                AmpIndicatorGlobalValue val = item.getModelObject();
+            protected void onPopulateItem(ListItem<AmpIndicatorGlobalValue> item) {
                 item.setOutputMarkupId(true);
                 item.add(new AmpTextFieldPanel<Double>("actualValue", new PropertyModel<>(item.getModel(), "originalValue"), "Actual Value") {
-                    public org.apache.wicket.util.convert.IConverter getInternalConverter(java.lang.Class<?> type) {
+                    public IConverter<Double> getInternalConverter(Class<?> type) {
                         return CustomDoubleConverter.INSTANCE;
                     }
                 });
                 item.add(new AmpDatePickerFieldPanel("actualDate", new PropertyModel<>(item.getModel(), "originalValueDate"), "Actual Date"));
                 item.add(new ListEditorRemoveButton("delActualValue", "Delete", "Delete") {
                     @Override
                     public void onClick(AjaxRequestTarget target) {
-                        AmpMEDisaggregationActualValuesPanel.this.getModel().getObject().getActualValues().remove(val);
+                        super.onClick(target);
                         target.appendJavaScript(OnePagerUtil.getToggleChildrenJS(AmpMEDisaggregationActualValuesPanel.this));
                         target.add(AmpMEDisaggregationActualValuesPanel.this);
                     }
                 });
             }
         };
-        listView.setOutputMarkupId(true);
-        add(listView);
+        listEditor.setOutputMarkupId(true);
+        add(listEditor);
 
         AmpAjaxLinkField addActual = new AmpAjaxLinkField("addDisaggActualValue", "Add Actual Value", "Add Actual Value") {
             @Override
             public void onClick(AjaxRequestTarget target) {
                 AmpIndicatorDisaggregationValue disaggVal = AmpMEDisaggregationActualValuesPanel.this.getModel().getObject();
+                logger.info("Adding new ACTUAL value for disaggregation value: " + disaggVal);
+                logger.info("Current actual values: " + disaggVal.getActualValues());
                 if (disaggVal.getActualValues() == null) {
-                    disaggVal.setActualValues(new java.util.HashSet<>());
+                    disaggVal.setActualValues(new HashSet<>());
                 }
                 AmpIndicatorGlobalValue val = new AmpIndicatorGlobalValue(AmpIndicatorGlobalValue.ACTUAL);
                 val.setIndicator(disaggVal.getIndicator());
+                if (location != null) {
+                    val.setActivityLocation(location.getObject());
+                }
                 val.setOriginalValueDate(new Date());
-                disaggVal.getActualValues().add(val);
+                listEditor.addItem(val);
                 target.add(AmpMEDisaggregationActualValuesPanel.this);
             }
         };

amp/src/main/java/org/dgfoundation/amp/onepager/components/features/items/AmpMEDisaggregationValuesFeaturePanel.java

@@ -10,9 +10,9 @@
 import org.apache.wicket.model.IModel;
 import org.apache.wicket.model.LoadableDetachableModel;
 import org.apache.wicket.model.Model;
-import org.apache.wicket.model.PropertyModel;
 import org.dgfoundation.amp.onepager.components.features.AmpFeaturePanel;
 import org.digijava.kernel.persistence.PersistenceManager;
+import org.digijava.module.aim.dbentity.AmpActivityLocation;
 import org.digijava.module.aim.dbentity.AmpIndicator;
 import org.digijava.module.aim.dbentity.AmpIndicatorDisaggregationValue;
 import org.digijava.module.aim.dbentity.AmpIndicatorGlobalValue;
@@ -35,12 +35,14 @@ public class AmpMEDisaggregationValuesFeaturePanel extends AmpFeaturePanel<AmpIn
     protected void onConfigure() {
         super.onConfigure();
         AmpIndicator indicator = indicatorModel.getObject();
-        if (indicator == null || indicator.getDisaggregation() == null || indicator.getDisaggregation().isEmpty()) {
-            setVisible(false);
-        }
+        boolean hasDisaggregationValues = indicator != null
+                && indicator.getDisaggregationValues() != null
+                && !indicator.getDisaggregationValues().isEmpty();
+        setVisible(isVisible() && hasDisaggregationValues);
     }
 
-    public AmpMEDisaggregationValuesFeaturePanel(String id, String fmName, IModel<AmpIndicator> indicatorModel) {
+    public AmpMEDisaggregationValuesFeaturePanel(String id, String fmName, IModel<AmpIndicator> indicatorModel,
+                                                IModel<AmpActivityLocation> locationModel) {
         super(id, fmName, true);
         this.indicatorModel = indicatorModel;
         logger.info("Initializing AmpMEDisaggregationValuesFeaturePanel for indicator: " + (indicatorModel.getObject() != null ? indicatorModel.getObject().getName() : "null"));
@@ -150,7 +152,7 @@ protected void populateItem(ListItem<AmpIndicatorDisaggregationValue> childItem)
                         actualValuesContainer.setOutputMarkupId(true);
                         childItem.add(actualValuesContainer);
 
-                        AmpMEDisaggregationActualValuesPanel actualPanel = new AmpMEDisaggregationActualValuesPanel("actualValuesPanel", Model.of(disaggVal));
+                        AmpMEDisaggregationActualValuesPanel actualPanel = new AmpMEDisaggregationActualValuesPanel("actualValuesPanel", Model.of(disaggVal), locationModel);
                         actualPanel.setOutputMarkupId(true);
                         actualPanel.setOutputMarkupPlaceholderTag(true);
                         actualValuesContainer.add(actualPanel);
@@ -170,4 +172,8 @@ protected void onEvent(AjaxRequestTarget target) {
         parentList.setOutputMarkupId(true);
         add(parentList);
     }
+
+    public AmpMEDisaggregationValuesFeaturePanel(String id, String fmName, IModel<AmpIndicator> indicatorModel) {
+        this(id, fmName, indicatorModel, null);
+    }
 }

amp/src/main/java/org/dgfoundation/amp/onepager/components/features/items/AmpMEIndicatorFeaturePanel.java

@@ -10,7 +10,6 @@
 import org.dgfoundation.amp.onepager.components.QuarterInformationPanel;
 import org.dgfoundation.amp.onepager.components.features.AmpFeaturePanel;
 import org.dgfoundation.amp.onepager.components.features.tables.AmpMEActualValuesFormTableFeaturePanel;
-import org.dgfoundation.amp.onepager.components.features.items.AmpMEDisaggregationValuesFeaturePanel;
 import org.dgfoundation.amp.onepager.components.fields.AmpAjaxLinkField;
 import org.dgfoundation.amp.onepager.components.fields.AmpCategorySelectFieldPanel;
 import org.dgfoundation.amp.onepager.components.fields.AmpSelectFieldPanel;
@@ -49,8 +48,10 @@ public AmpMEIndicatorFeaturePanel(String id, String fmName, final IModel<Indicat
 
         final Label indicatorNameLabel = new Label("indicatorName", new PropertyModel<String>(indicator, "name"));
         add(indicatorNameLabel);
-        final boolean hasDisaggregation = indicator.getObject().getDisaggregation() != null
-                && !indicator.getObject().getDisaggregation().isEmpty();
+        logger.info("Indicator disagg: " + indicator.getObject().getDisaggregationValues());
+        final boolean hasDisaggregation = indicator.getObject().getDisaggregationValues() != null
+                && !indicator.getObject().getDisaggregationValues().isEmpty();
+        logger.info("Has disaggregation: " + hasDisaggregation);
 
         String indCodeString = "";
         if (indicator.getObject().getCode() != null && indicator.getObject().getCode().trim().compareTo("") != 0) {
@@ -70,7 +71,7 @@ public AmpMEIndicatorFeaturePanel(String id, String fmName, final IModel<Indicat
 
         }
 
-        final AmpSelectFieldPanel<AmpIndicatorRiskRatings> riskSelect = new AmpSelectFieldPanel("risk",
+        final AmpSelectFieldPanel<AmpIndicatorRiskRatings> riskSelect = new AmpSelectFieldPanel<AmpIndicatorRiskRatings>("risk",
                 new PropertyModel<>(conn, "risk"), MEIndicatorsUtil.getAllIndicatorRisks(), "Risk", false, false,
                 new TranslatedChoiceRenderer<AmpIndicatorRiskRatings>(), false);
         add(riskSelect);
@@ -137,15 +138,19 @@ protected String load() {
         });
         add(indicatorTargetDateLabel);
 
-        AmpMEActualValuesFormTableFeaturePanel valuesTable = new AmpMEActualValuesFormTableFeaturePanel("valuesSubsection", indicator, conn, location,"Actual Values", false, 7);
+        AmpMEActualValuesFormTableFeaturePanel valuesTable = new AmpMEActualValuesFormTableFeaturePanel("valuesSubsection", indicator, conn, location,"Actual Values", false, 7) {
+            @Override
+            protected void onConfigure() {
+                super.onConfigure();
+                setVisible(isVisible() && !hasDisaggregation);
+            }
+        };
         valuesTable.setOutputMarkupId(true);
         valuesTable.setOutputMarkupPlaceholderTag(true);
         add(valuesTable);
 
 
 
-        logger.info("Table" + valuesTable.getMarkupId());
-        logger.info("Id " + valuesTable.getId());
         AmpAjaxLinkField addActualValue = new AmpAjaxLinkField("addActualValue", "Add Actual Value", "Add Actual Value") {
             @Override
             public void onClick(AjaxRequestTarget target) {
@@ -158,32 +163,41 @@ public void onClick(AjaxRequestTarget target) {
                 target.add(valuesTable);
                 target.appendJavaScript(QuarterInformationPanel.getJSUpdate(getSession()));
             }
+
+            @Override
+            protected void onConfigure() {
+                super.onConfigure();
+                setVisible(isVisible() && !hasDisaggregation);
+            }
         };
 
 
-        logger.info("Button" + addActualValue.getMarkupId());
-        logger.info("Id " + addActualValue.getId());
+
         addActualValue.setOutputMarkupId(true);
         addActualValue.setOutputMarkupPlaceholderTag(true);
 
         add(addActualValue);
 
+
+
         AmpMEIndicatorBaseFeaturePanel baseValues = null;
 
         try {
-            baseValues = new AmpMEIndicatorBaseFeaturePanel("addBaseTargetValue", "Add Base Target Values", conn, indicator, values, location);
+            baseValues = new AmpMEIndicatorBaseFeaturePanel("addBaseTargetValue", "Add Base Target Values", conn, indicator, values, location) {
+                @Override
+                protected void onConfigure() {
+                    super.onConfigure();
+                    setVisible(isVisible() && !hasDisaggregation);
+                }
+            };
         } catch (Exception e) {
             throw new RuntimeException(e);
         }
         add(baseValues);
 
-        AmpMEDisaggregationValuesFeaturePanel disaggPanel = new AmpMEDisaggregationValuesFeaturePanel("disaggregationValuesSubsection", "Disaggregation Values", indicator);
+        AmpMEDisaggregationValuesFeaturePanel disaggPanel = new AmpMEDisaggregationValuesFeaturePanel("disaggregationValuesSubsection", "Disaggregation Values", indicator, location);
         disaggPanel.setOutputMarkupId(true);
         disaggPanel.setVisible(hasDisaggregation);
-        // Add disaggregation values subsection
-        if (indicator.getObject().getDisaggregation()== null || indicator.getObject().getDisaggregation().isEmpty()) {
-            disaggPanel.setVisible(false);
-        }
         add(disaggPanel);