Fix ui save button autoassignment (#620)

* Save button state (enabled or disabled) depends on data change but data
change wasn't recognized. Fixed by setting values to view elements
before window builder is called. Because the CommonDialogWindow stores
internally the original values for later comparing them against the new
values.
DistributionSet Table stays now visible independently from the
assignment checkbox, only the enabled/disabled state of the table
changes. This keeps the window from changing its size when toggling the
checkbox.

Signed-off-by: Markus Block <markus.block@bosch-si.com>

* Fixed sonar issues, and incorporated code review remarks.

Signed-off-by: Markus Block <markus.block@bosch-si.com>

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/CommonDialogWindow.java

@@ -326,8 +326,8 @@ public class CommonDialogWindow extends Window {
     private boolean isMandatoryFieldNotEmptyAndValid(final Component currentChangedComponent, final Object newValue) {
 
         boolean valid = true;
-        final List<AbstractField<?>> requiredComponents = allComponents.stream().filter(field -> field.isRequired())
+        final List<AbstractField<?>> requiredComponents = allComponents.stream().filter(AbstractField::isRequired)
-                .collect(Collectors.toList());
+                .filter(AbstractField::isEnabled).collect(Collectors.toList());
 
         requiredComponents.addAll(allComponents.stream().filter(this::hasNullValidator).collect(Collectors.toList()));
 
@@ -396,12 +396,18 @@ public class CommonDialogWindow extends Window {
 
             if (c instanceof TabSheet) {
                 final TabSheet tabSheet = (TabSheet) c;
-                for (final Iterator<Component> i = tabSheet.iterator(); i.hasNext();) {
+                components.addAll(getAllComponentsFromTabSheet(tabSheet));
-                    final Component component = i.next();
+            }
-                    if (component instanceof AbstractLayout) {
+        }
-                        components.addAll(getAllComponents((AbstractLayout) component));
+        return components;
-                    }
+    }
-                }
+
+    private static List<AbstractField<?>> getAllComponentsFromTabSheet(final TabSheet tabSheet) {
+        final List<AbstractField<?>> components = new ArrayList<>();
+        for (final Iterator<Component> i = tabSheet.iterator(); i.hasNext();) {
+            final Component component = i.next();
+            if (component instanceof AbstractLayout) {
+                components.addAll(getAllComponents((AbstractLayout) component));
             }
         }
         return components;

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/filtermanagement/DistributionSetSelectWindow.java

@@ -58,7 +58,6 @@ public class DistributionSetSelectWindow
 
     private final transient TargetFilterQueryManagement targetFilterQueryManagement;
 
-    private CommonDialogWindow window;
     private CheckBox checkBox;
     private Long tfqId;
 
@@ -71,7 +70,7 @@ public class DistributionSetSelectWindow
         this.targetFilterQueryManagement = targetFilterQueryManagement;
     }
 
-    private void initLocal() {
+    private VerticalLayout initView() {
         final Label label = new Label(i18n.getMessage("label.auto.assign.description"));
 
         checkBox = new CheckBox(i18n.getMessage("label.auto.assign.enable"));
@@ -79,22 +78,21 @@ public class DistributionSetSelectWindow
         checkBox.setImmediate(true);
         checkBox.addValueChangeListener(this);
 
+        setTableEnabled(false);
+
         final VerticalLayout verticalLayout = new VerticalLayout();
         verticalLayout.addComponent(label);
         verticalLayout.addComponent(checkBox);
         verticalLayout.addComponent(dsTable);
 
-        window = new WindowBuilder(SPUIDefinitions.CREATE_UPDATE_WINDOW)
+        return verticalLayout;
-                .caption(i18n.getMessage("caption.select.auto.assign.dist")).content(verticalLayout).layout(verticalLayout)
-                .i18n(i18n).saveDialogCloseListener(this).buildCommonDialogWindow();
-        window.setId(UIComponentIdProvider.DIST_SET_SELECT_WINDOW_ID);
     }
 
-    public void setValue(final Long distSet) {
+    private void setValue(final Long distSet) {
-        dsTable.setVisible(distSet != null);
         checkBox.setValue(distSet != null);
         dsTable.setValue(distSet);
         dsTable.setCurrentPageFirstItemId(distSet);
+        dsTable.setNullSelectionAllowed(false);
     }
 
     /**
@@ -108,7 +106,7 @@ public class DistributionSetSelectWindow
         final TargetFilterQuery tfq = targetFilterQueryManagement.get(tfqId)
                 .orElseThrow(() -> new EntityNotFoundException(TargetFilterQuery.class, tfqId));
 
-        initLocal();
+        final VerticalLayout verticalLayout = initView();
 
         final DistributionSet distributionSet = tfq.getAutoAssignDistributionSet();
         if (distributionSet != null) {
@@ -117,6 +115,12 @@ public class DistributionSetSelectWindow
             setValue(null);
         }
 
+        // build window after values are set to view elements
+        final CommonDialogWindow window = new WindowBuilder(SPUIDefinitions.CREATE_UPDATE_WINDOW)
+                .caption(i18n.getMessage("caption.select.auto.assign.dist")).content(verticalLayout)
+                .layout(verticalLayout).i18n(i18n).saveDialogCloseListener(this).buildCommonDialogWindow();
+        window.setId(UIComponentIdProvider.DIST_SET_SELECT_WINDOW_ID);
+
         window.setWidth(40.0F, Sizeable.Unit.PERCENTAGE);
         UI.getCurrent().addWindow(window);
         window.setVisible(true);
@@ -130,13 +134,19 @@ public class DistributionSetSelectWindow
      */
     @Override
     public void valueChange(final Property.ValueChangeEvent event) {
-        dsTable.setVisible(checkBox.getValue());
+        if (checkBox.getValue()) {
-        if (window != null) {
+            setTableEnabled(true);
-            window.center();
+        } else {
-
+            dsTable.select(null);
+            setTableEnabled(false);
         }
     }
 
+    private void setTableEnabled(final boolean enabled) {
+        dsTable.setEnabled(enabled);
+        dsTable.setRequired(enabled);
+    }
+
     /**
      * Is triggered when the save button is clicked
      *
@@ -144,7 +154,19 @@ public class DistributionSetSelectWindow
      */
     @Override
     public boolean canWindowSaveOrUpdate() {
-        return !checkBox.getValue() || dsTable.getValue() != null;
+        return isFormularValid();
+    }
+
+    private boolean isFormularValid() {
+        return isAutoAssignmentDisabled() || isAutoAssignmentEnabledAndDistributionSetSelected();
+    }
+
+    private boolean isAutoAssignmentEnabledAndDistributionSetSelected() {
+        return checkBox.getValue() && dsTable.getValue() != null;
+    }
+
+    private boolean isAutoAssignmentDisabled() {
+        return !checkBox.getValue();
     }
 
     /**
@@ -239,7 +261,7 @@ public class DistributionSetSelectWindow
                 mainTextLabel = new Label(i18n.getMessage("message.confirm.assign.consequences.none"));
             } else {
                 mainTextLabel = new Label(
-                        i18n.getMessage("message.confirm.assign.consequences.text", new Object[] { targetsCount }));
+                        i18n.getMessage("message.confirm.assign.consequences.text", targetsCount));
             }
 
             layout.addComponent(mainTextLabel);