From 67ee674d412da216644813fe4ddcd7b81c95b8af Mon Sep 17 00:00:00 2001 From: Melanie Retter Date: Thu, 30 Jun 2016 17:48:58 +0200 Subject: [PATCH] Fix Update bug of softwareModuleType, refactored based on git comments Signed-off-by: Melanie Retter --- .../repository/jpa/JpaSoftwareManagement.java | 2 +- .../CreateUpdateSoftwareTypeLayout.java | 15 +------ .../hawkbit/ui/common/CommonDialogWindow.java | 40 +++++++++++++++++-- .../CreateUpdateDistSetTypeLayout.java | 29 +++++++------- .../AbstractCreateUpdateTagLayout.java | 4 -- .../TargetAddUpdateWindowLayout.java | 10 ++++- .../management/targettable/TargetDetails.java | 3 +- .../rollout/AddUpdateRolloutWindowLayout.java | 30 ++++++++------ .../ui/rollout/rollout/RolloutListHeader.java | 2 +- 9 files changed, 83 insertions(+), 52 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaSoftwareManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaSoftwareManagement.java index 50127e227..b561667a9 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaSoftwareManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaSoftwareManagement.java @@ -132,7 +132,7 @@ public class JpaSoftwareManagement implements SoftwareManagement { final JpaSoftwareModuleType type = softwareModuleTypeRepository.findOne(sm.getId()); boolean updated = false; - if (sm.getDescription() != null && !sm.getDescription().equals(type.getDescription())) { + if (sm.getDescription() == null || !sm.getDescription().equals(type.getDescription())) { type.setDescription(sm.getDescription()); updated = true; } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtype/CreateUpdateSoftwareTypeLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtype/CreateUpdateSoftwareTypeLayout.java index 18d6b159c..ce7631c26 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtype/CreateUpdateSoftwareTypeLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/smtype/CreateUpdateSoftwareTypeLayout.java @@ -220,21 +220,14 @@ public class CreateUpdateSoftwareTypeLayout extends CreateUpdateTypeLayout { SoftwareModuleType newSWType = entityFactory.generateSoftwareModuleType(typeKeyValue, typeNameValue, typeDescValue, assignNumber); newSWType.setColour(colorPicked); - - if (null != typeDescValue) { - newSWType.setDescription(typeDescValue); - } - + newSWType.setDescription(typeDescValue); newSWType.setColour(colorPicked); - newSWType = swTypeManagementService.createSoftwareModuleType(newSWType); uiNotification.displaySuccess(i18n.get("message.save.success", new Object[] { newSWType.getName() })); eventBus.publish(this, new SoftwareModuleTypeEvent(SoftwareModuleTypeEnum.ADD_SOFTWARE_MODULE_TYPE, newSWType)); - } else { uiNotification.displayValidationError(i18n.get("message.error.missing.typenameorkey")); - } } @@ -244,19 +237,15 @@ public class CreateUpdateSoftwareTypeLayout extends CreateUpdateTypeLayout { final String typeDescValue = HawkbitCommonUtil.trimAndNullIfEmpty(tagDesc.getValue()); if (null != typeNameValue) { existingType.setName(typeNameValue); - - existingType.setDescription(null != typeDescValue ? typeDescValue : null); - + existingType.setDescription(typeDescValue); existingType.setColour(ColorPickerHelper.getColorPickedString(getColorPickerLayout().getSelPreview())); swTypeManagementService.updateSoftwareModuleType(existingType); uiNotification.displaySuccess(i18n.get("message.update.success", new Object[] { existingType.getName() })); eventBus.publish(this, new SoftwareModuleTypeEvent(SoftwareModuleTypeEnum.UPDATE_SOFTWARE_MODULE_TYPE, existingType)); - } else { uiNotification.displayValidationError(i18n.get("message.tag.update.mandatory")); } - } @Override diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/CommonDialogWindow.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/CommonDialogWindow.java index 1d2f3b2d4..725ff4b9b 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/CommonDialogWindow.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/CommonDialogWindow.java @@ -52,6 +52,7 @@ import com.vaadin.ui.AbstractOrderedLayout; import com.vaadin.ui.Alignment; import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.CheckBox; import com.vaadin.ui.Component; import com.vaadin.ui.Field; import com.vaadin.ui.GridLayout; @@ -207,6 +208,10 @@ public class CommonDialogWindow extends Window implements Serializable { addListeners(); } + /** + * saves the original values in a Map so we can use them for detecting + * changes + */ public final void setOrginaleValues() { for (final AbstractField field : allComponents) { Object value = field.getValue(); @@ -244,10 +249,13 @@ public class CommonDialogWindow extends Window implements Serializable { private boolean isValuesChanged(final Component currentChangedComponent, final Object newValue) { for (final AbstractField field : allComponents) { - final Object orginalValue = orginalValues.get(field); + Object originalValue = orginalValues.get(field); + if (field instanceof CheckBox && originalValue == null) { + originalValue = Boolean.FALSE; + } final Object currentValue = getCurrentVaue(currentChangedComponent, newValue, field); - if (!isValueEquals(field, orginalValue, currentValue)) { + if (!isValueEquals(field, originalValue, currentValue)) { return true; } } @@ -300,7 +308,15 @@ public class CommonDialogWindow extends Window implements Serializable { requiredComponents.addAll(allComponents.stream().filter(this::hasNullValidator).collect(Collectors.toList())); for (final AbstractField field : requiredComponents) { - final Object value = getCurrentVaue(currentChangedComponent, newValue, field); + Object value = getCurrentVaue(currentChangedComponent, newValue, field); + + if (String.class.equals(field.getType())) { + value = Strings.emptyToNull((String) value); + } + + if (Set.class.equals(field.getType())) { + value = emptyToNull((Collection) value); + } if (value == null) { return false; @@ -319,6 +335,10 @@ public class CommonDialogWindow extends Window implements Serializable { return valid; } + private static Object emptyToNull(final Collection c) { + return (c == null || c.isEmpty()) ? null : c; + } + private boolean hasNullValidator(final Component component) { if (component instanceof AbstractField) { @@ -459,4 +479,18 @@ public class CommonDialogWindow extends Window implements Serializable { } } + /** + * Adds the component manually to the allComponents-List and adds a + * ValueChangeListener to it. Necessary in Update Distribution Type as the + * CheckBox concerned is an ItemProperty... + * + * @param component + * AbstractField + */ + public void updateAllComponents(final AbstractField component) { + + allComponents.add(component); + component.addValueChangeListener(new ChangeListener(component)); + } + } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/disttype/CreateUpdateDistSetTypeLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/disttype/CreateUpdateDistSetTypeLayout.java index 122599520..2a12bc859 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/disttype/CreateUpdateDistSetTypeLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/disttype/CreateUpdateDistSetTypeLayout.java @@ -9,6 +9,7 @@ package org.eclipse.hawkbit.ui.distributions.disttype; import java.util.List; +import java.util.Map; import java.util.Set; import org.eclipse.hawkbit.repository.DistributionSetManagement; @@ -83,6 +84,8 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout { private IndexedContainer originalSelectedTableContainer; + private Map mandatoryCheckboxMap; + @Override protected void createRequiredComponents() { @@ -315,7 +318,9 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout { saveTblitem = selectedTableContainer.addItem(id); saveTblitem.getItemProperty(DIST_TYPE_NAME).setValue( sourceTable.getContainerDataSource().getItem(id).getItemProperty(DIST_TYPE_NAME).getValue()); - saveTblitem.getItemProperty(DIST_TYPE_MANDATORY).setValue(new CheckBox()); + final CheckBox mandatoryCheckBox = new CheckBox(); + window.updateAllComponents(mandatoryCheckBox); + saveTblitem.getItemProperty(DIST_TYPE_MANDATORY).setValue(mandatoryCheckBox); saveTblitem.getItemProperty(DIST_TYPE_DESCRIPTION).setValue( sourceTable.getContainerDataSource().getItem(id).getItemProperty(DIST_TYPE_DESCRIPTION).getValue()); } @@ -327,7 +332,6 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout { if (sourceTableContainer != null) { Item saveTblitem; saveTblitem = sourceTableContainer.addItem(selectedId); - selectedTable.getContainerDataSource().getItem(selectedId).getItemProperty(DIST_TYPE_NAME); saveTblitem.getItemProperty(DIST_TYPE_NAME).setValue(selectedTable.getContainerDataSource() .getItem(selectedId).getItemProperty(DIST_TYPE_NAME).getValue()); saveTblitem.getItemProperty(DIST_TYPE_DESCRIPTION).setValue(selectedTable.getContainerDataSource() @@ -357,10 +361,7 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout { final SoftwareModuleType swModuleType = softwareManagement.findSoftwareModuleTypeByName(distTypeName); checkMandatoryAndAddMandatoryModuleType(newDistType, isMandatory, swModuleType); } - if (null != typeDescValue) { - newDistType.setDescription(typeDescValue); - } - + newDistType.setDescription(typeDescValue); newDistType.setColour(colorPicked); newDistType = distributionSetManagement.createDistributionSetType(newDistType); uiNotification.displaySuccess(i18n.get("message.save.success", new Object[] { newDistType.getName() })); @@ -387,7 +388,7 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout { if (null != typeNameValue) { updateDistSetType.setName(typeNameValue); updateDistSetType.setKey(typeKeyValue); - updateDistSetType.setDescription(null != typeDescValue ? typeDescValue : null); + updateDistSetType.setDescription(typeDescValue); if (distributionSetManagement.countDistributionSetsByType(existingType) <= 0 && null != itemIds && !itemIds.isEmpty()) { @@ -407,7 +408,6 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout { .displaySuccess(i18n.get("message.update.success", new Object[] { updateDistSetType.getName() })); eventBus.publish(this, new DistributionSetTypeEvent(DistributionSetTypeEnum.UPDATE_DIST_SET_TYPE, updateDistSetType)); - } else { uiNotification.displayValidationError(i18n.get("message.tag.update.mandatory")); } @@ -417,7 +417,6 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout { final Boolean isMandatory, final SoftwareModuleType swModuleType) { if (isMandatory) { updateDistSetType.addMandatoryModuleType(swModuleType); - } else { updateDistSetType.addOptionalModuleType(swModuleType); } @@ -561,15 +560,15 @@ public class CreateUpdateDistSetTypeLayout extends CreateUpdateTypeLayout { final Item saveTblitem = selectedTableContainer.addItem(swModuleType.getId()); sourceTable.removeItem(swModuleType.getId()); saveTblitem.getItemProperty(DIST_TYPE_NAME).setValue(swModuleType.getName()); - saveTblitem.getItemProperty(DIST_TYPE_MANDATORY).setValue(new CheckBox("", mandatory)); - - // TODO CHECKBOX - final CheckBox mandatoryCheckbox = (CheckBox) selectedTableContainer - .getContainerProperty(swModuleType.getId(), DIST_TYPE_MANDATORY).getValue(); + final CheckBox mandatoryCheckbox = new CheckBox("", mandatory); + mandatoryCheckbox.setId(swModuleType.getName()); + saveTblitem.getItemProperty(DIST_TYPE_MANDATORY).setValue(mandatoryCheckbox); final Item originalItem = originalSelectedTableContainer.addItem(swModuleType.getId()); originalItem.getItemProperty(DIST_TYPE_NAME).setValue(swModuleType.getName()); - originalItem.getItemProperty(DIST_TYPE_MANDATORY).setValue(new CheckBox("", mandatory)); + originalItem.getItemProperty(DIST_TYPE_MANDATORY).setValue(mandatoryCheckbox); + + window.updateAllComponents(mandatoryCheckbox); } @Override diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/layouts/AbstractCreateUpdateTagLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/layouts/AbstractCreateUpdateTagLayout.java index 6527d5db5..8269d4e89 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/layouts/AbstractCreateUpdateTagLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/layouts/AbstractCreateUpdateTagLayout.java @@ -602,14 +602,10 @@ public abstract class AbstractCreateUpdateTagLayout extends CustomComponent @Override public void addColorChangeListener(final ColorChangeListener listener) { - // TODO Auto-generated method stub - } @Override public void removeColorChangeListener(final ColorChangeListener listener) { - // TODO Auto-generated method stub - } } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetAddUpdateWindowLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetAddUpdateWindowLayout.java index 93c5a4514..2b08c4dbf 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetAddUpdateWindowLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetAddUpdateWindowLayout.java @@ -171,6 +171,13 @@ public class TargetAddUpdateWindowLayout extends CustomComponent { return window; } + public Window getWindow(final String entityId) { + populateValuesOfTarget(entityId); + getWindow(); + window.addStyleName("target-update-window"); + return window; + } + /** * clear all fields of Target Edit Window. */ @@ -203,7 +210,7 @@ public class TargetAddUpdateWindowLayout extends CustomComponent { /** * @param controllerId */ - public void populateValuesOfTarget(final String controllerId) { + private void populateValuesOfTarget(final String controllerId) { resetComponents(); this.controllerId = controllerId; editTarget = Boolean.TRUE; @@ -214,7 +221,6 @@ public class TargetAddUpdateWindowLayout extends CustomComponent { if (target.getDescription() != null) { descTextArea.setValue(target.getDescription()); } - window.addStyleName("target-update-window"); } public FormLayout getFormLayout() { diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetDetails.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetDetails.java index ffaa39411..020b29372 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetDetails.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/targettable/TargetDetails.java @@ -106,7 +106,8 @@ public class TargetDetails extends AbstractTableDetailsLayout { if (getSelectedBaseEntity() == null) { return; } - targetAddUpdateWindowLayout.populateValuesOfTarget(getSelectedBaseEntity().getControllerId()); + targetAddUpdateWindowLayout.getWindow(getSelectedBaseEntity().getControllerId()); + // targetAddUpdateWindowLayout.populateValuesOfTarget(getSelectedBaseEntity().getControllerId()); openWindow(); } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/AddUpdateRolloutWindowLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/AddUpdateRolloutWindowLayout.java index f97f8e43f..05eabf249 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/AddUpdateRolloutWindowLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/AddUpdateRolloutWindowLayout.java @@ -155,14 +155,18 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { } public CommonDialogWindow getWindow(final Long rolloutId) { - resetComponents(); - window = SPUIWindowDecorator.getWindow(i18n.get("caption.configure.rollout"), null, - SPUIDefinitions.CREATE_UPDATE_WINDOW, this, event -> onRolloutSave(), null, - uiProperties.getLinks().getDocumentation().getRolloutView(), this, i18n); + window = getWindow(); populateData(rolloutId); return window; } + public CommonDialogWindow getWindow() { + resetComponents(); + return SPUIWindowDecorator.getWindow(i18n.get("caption.configure.rollout"), null, + SPUIDefinitions.CREATE_UPDATE_WINDOW, this, event -> onRolloutSave(), null, + uiProperties.getLinks().getDocumentation().getRolloutView(), this, i18n); + } + /** * Reset the field values. */ @@ -610,9 +614,7 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { @Override public void validate(final Object value) { try { - if (HawkbitCommonUtil.trimAndNullIfEmpty(noOfGroups.getValue()) == null - || (HawkbitCommonUtil.trimAndNullIfEmpty((String) targetFilterQueryCombo.getValue()) == null - && targetFilterQuery.getValue() == null)) { + if (isNoOfGroupsOrTargetFilterEmpty()) { uiNotification .displayValidationError(i18n.get("message.rollout.noofgroups.or.targetfilter.missing")); } else { @@ -626,14 +628,18 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { // suppress the need of preserve original exception, will blow // up the // log and not necessary here - catch ( - - @SuppressWarnings("squid:S1166") final InvalidValueException ex) { + catch (final InvalidValueException ex) { // we have to throw the exception here, otherwise the UI won't // show the vaadin validation error! throw ex; } } + + private boolean isNoOfGroupsOrTargetFilterEmpty() { + return HawkbitCommonUtil.trimAndNullIfEmpty(noOfGroups.getValue()) == null + || (HawkbitCommonUtil.trimAndNullIfEmpty((String) targetFilterQueryCombo.getValue()) == null + && targetFilterQuery.getValue() == null); + } } private int getGroupSize() { @@ -654,7 +660,7 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { // suppress the need of preserve original exception, will blow // up the // log and not necessary here - catch (@SuppressWarnings("squid:S1166") final InvalidValueException ex) { + catch (final InvalidValueException ex) { // we have to throw the exception here, otherwise the UI won't // show the vaadin validation error! throw ex; @@ -676,7 +682,7 @@ public class AddUpdateRolloutWindowLayout extends GridLayout { // suppress the need of preserve original exception, will blow // up the // log and not necessary here - catch (@SuppressWarnings("squid:S1166") final InvalidValueException ex) { + catch (final InvalidValueException ex) { // we have to throw the exception here, otherwise the UI won't // show the vaadin validation error! throw ex; diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/RolloutListHeader.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/RolloutListHeader.java index 49bb90100..81608d42d 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/RolloutListHeader.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/rollout/rollout/RolloutListHeader.java @@ -91,7 +91,7 @@ public class RolloutListHeader extends AbstractGridHeader { @Override protected void addNewItem(final ClickEvent event) { - final Window addTargetWindow = addUpdateRolloutWindow.getWindow(null); + final Window addTargetWindow = addUpdateRolloutWindow.getWindow(); UI.getCurrent().addWindow(addTargetWindow); addTargetWindow.setVisible(Boolean.TRUE);