Tomas Jelinek has uploaded a new change for review.

Change subject: webadmin: changeing vm_type and os_type did reset the dialog
......................................................................

webadmin: changeing vm_type and os_type did reset the dialog

For change of os type:
The problem was that the change of the vm_type in the vm dialog triggered the
InstanceTypeManager.updateFields().

It was not correct since this caused the whole dialog to be re-inited from the
selected instance type/template (depending on the actual selection) and that
should happen only after "big" changes like cluster change which changes
availability of some fields.

Since the vm type only provides some defaults for the fields and not the
availability of the fields, there is no need to go through the instance type
manager - just simply set the fields up.

For the os type:
Similar but a bit more tricky. There are some fields which are actually
provided from instance type. So if the os type changes, in case the selected
instance type is not custom, it should set the "IsChangable" flag and let the
instance type manager to copy the corresponding fields. But not all to avoid
the dialog will be reset again - only the ones related to the os change.

Change-Id: I5859ca02ddcbeb60583a0e79b94f7b9f2647596a
Bug-Url: https://bugzilla.redhat.com/1125344
Signed-off-by: Tomas Jelinek <[email protected]>
---
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmModelBehavior.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/InstanceTypeManager.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewPoolInstanceTypeManager.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewVmInstanceTypeManager.java
6 files changed, 94 insertions(+), 37 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/14/32514/1

diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmModelBehavior.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmModelBehavior.java
index f40fe16..a0fca05 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmModelBehavior.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmModelBehavior.java
@@ -346,8 +346,6 @@
 
     @Override
     public void vmTypeChanged(VmType vmType) {
-        deactivateInstanceTypeManager();
-
         // provisioning thin -> false
         // provisioning clone -> true
         if (getModel().getProvisioning().getIsAvailable()) {
@@ -355,7 +353,6 @@
         }
 
         super.vmTypeChanged(vmType);
-        activateInstanceTypeManager();
     }
 
     @Override
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
index 769213d..9178de6 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
@@ -1735,12 +1735,7 @@
         else if 
(ev.matchesDefinition(ListModel.selectedItemChangedEventDefinition))
         {
             if (sender == getVmType()) {
-                deactivateInstanceTypeManagerAndUpdateFields();
-
                 vmTypeChanged();
-
-                getBehavior().activateInstanceTypeManager();
-
             } else if (sender == getDataCenterWithClustersList())
             {
                 dataCenterWithClusterSelectedItemChanged(sender, args);
@@ -1761,7 +1756,14 @@
                 defaultHost_SelectedItemChanged(sender, args);
             }
             else if (sender == getOSType()) {
-                deactivateInstanceTypeManagerAndUpdateFields();
+                getBehavior().deactivateInstanceTypeManager(new 
InstanceTypeManager.ActivatedListener() {
+                    @Override
+                    public void activated() {
+                        if (getBehavior().getInstanceTypeManager() != null && 
!getBehavior().basedOnCustomInstanceType()) {
+                            
getBehavior().getInstanceTypeManager().updateFildsAfterOsChanged();
+                        }
+                    }
+                });
 
                 oSType_SelectedItemChanged(sender, args);
                 getBehavior().oSType_SelectedItemChanged();
@@ -1849,17 +1851,6 @@
             }
 
         }
-    }
-
-    private void deactivateInstanceTypeManagerAndUpdateFields() {
-        getBehavior().deactivateInstanceTypeManager(new 
InstanceTypeManager.ActivatedListener() {
-            @Override
-            public void activated() {
-                if (getBehavior().getInstanceTypeManager() != null) {
-                    getBehavior().getInstanceTypeManager().updateFields();
-                }
-            }
-        });
     }
 
     private void vmInitEnabledChanged() {
@@ -1980,11 +1971,13 @@
             getUsbPolicy().setIsChangable(false);
         }
 
-        Collection<UsbPolicy> policies = getUsbPolicy().getItems();
-        if (policies.contains(prevSelectedUsbPolicy)) {
-            getUsbPolicy().setSelectedItem(prevSelectedUsbPolicy);
-        } else if (policies.size() > 0) {
-            getUsbPolicy().setSelectedItem(policies.iterator().next());
+        if (getBehavior().basedOnCustomInstanceType()) {
+            Collection<UsbPolicy> policies = getUsbPolicy().getItems();
+            if (policies.contains(prevSelectedUsbPolicy)) {
+                getUsbPolicy().setSelectedItem(prevSelectedUsbPolicy);
+            } else if (policies.size() > 0) {
+                getUsbPolicy().setSelectedItem(policies.iterator().next());
+            }
         }
     }
 
@@ -2076,12 +2069,10 @@
 
         getMemoryBalloonDeviceEnabled().setIsChangable(isBalloonEnabled);
 
-        getMemoryBalloonDeviceEnabled().setEntity(isBalloonEnabled);
-        if (!isBalloonEnabled) {
-            getBehavior().deactivateInstanceTypeManager();
+        if (getBehavior().basedOnCustomInstanceType()) {
             getMemoryBalloonDeviceEnabled().setEntity(isBalloonEnabled);
-            getBehavior().activateInstanceTypeManager();
         }
+
     }
 
     private void initFirstBootDevice()
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java
index 111049d..668ac93 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java
@@ -1111,7 +1111,12 @@
     }
 
     public void vmTypeChanged(VmType vmType) {
-        getModel().getIsSoundcardEnabled().setEntity(vmType == VmType.Desktop);
+        if (basedOnCustomInstanceType()) {
+            // this field is normally taken from instance type. If the 
"custom" is selected, then it is supposed to use the default
+            // determined by vm type
+            getModel().getIsSoundcardEnabled().setEntity(vmType == 
VmType.Desktop);
+        }
+
         getModel().getAllowConsoleReconnect().setEntity(vmType == 
VmType.Server);
     }
 
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/InstanceTypeManager.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/InstanceTypeManager.java
index 342f0e3..b2a3e91 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/InstanceTypeManager.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/InstanceTypeManager.java
@@ -8,6 +8,7 @@
 import org.ovirt.engine.core.common.businessentities.MigrationSupport;
 import org.ovirt.engine.core.common.businessentities.VmBase;
 import org.ovirt.engine.core.common.businessentities.VmDevice;
+import org.ovirt.engine.core.common.businessentities.VmEntityType;
 import org.ovirt.engine.core.common.businessentities.VmRngDevice;
 import org.ovirt.engine.core.common.businessentities.VmTemplate;
 import org.ovirt.engine.core.common.businessentities.VmWatchdog;
@@ -142,6 +143,32 @@
     public void updateFields() {
         getModel().startProgress(null);
         doUpdateManagedFieldsFrom(getSource());
+    }
+
+    public void updateFildsAfterOsChanged() {
+        deactivateAndStartProgress();
+        VmBase vmBase = getSource();
+
+        maybeSetSingleQxlPci(vmBase);
+        updateWatchdog(vmBase, false);
+        updateBalloon(vmBase, false);
+        maybeSetSelectedItem(model.getUsbPolicy(), vmBase.getUsbPolicy());
+
+        activate();
+    }
+
+    /**
+     * Starts progress, deactivates the manager and waits until it gets 
activated again.
+     * On activation it stops the progress
+     */
+    public void deactivateAndStartProgress() {
+        model.startProgress(null);
+        deactivate(new ActivatedListener() {
+            @Override
+            public void activated() {
+                model.stopProgress();
+            }
+        });
     }
 
     private void registerListeners(UnitVmModel model) {
@@ -297,10 +324,10 @@
             activate();
         }
 
-        updateWatchdog(vmBase);
+        updateWatchdog(vmBase, true);
     }
 
-    private void updateWatchdog(final VmBase vmBase) {
+    private void updateWatchdog(final VmBase vmBase, final boolean 
continueWithNext) {
         AsyncDataProvider.getWatchdogByVmId(new AsyncQuery(this.getModel(), 
new INewAsyncCallback() {
             @Override
             public void onSuccess(Object target, Object returnValue) {
@@ -325,12 +352,15 @@
                 }
                 activate();
 
-                updateBalloon(vmBase);
+                if (continueWithNext) {
+                    updateBalloon(vmBase, true);
+                }
+
             }
         }), vmBase.getId());
     }
 
-    private void updateBalloon(final VmBase vmBase) {
+    protected void updateBalloon(final VmBase vmBase, final boolean 
continueWithNext) {
         if (model.getMemoryBalloonDeviceEnabled().getIsChangable() && 
model.getMemoryBalloonDeviceEnabled().getIsAvailable()) {
             Frontend.getInstance().runQuery(VdcQueryType.IsBalloonEnabled, new 
IdQueryParameters(vmBase.getId()), new AsyncQuery(this,
                     new INewAsyncCallback() {
@@ -339,17 +369,19 @@
                             deactivate();
                             
getModel().getMemoryBalloonDeviceEnabled().setEntity((Boolean) 
((VdcQueryReturnValue)returnValue).getReturnValue());
                             activate();
-                            updateRngDevice(vmBase);
+                            if (continueWithNext) {
+                                updateRngDevice(vmBase);
+                            }
                         }
                     }
             ));
-        } else {
+        } else if (continueWithNext) {
             updateRngDevice(vmBase);
         }
 
     }
 
-    private void updateRngDevice(final VmBase vmBase) {
+    protected void updateRngDevice(final VmBase vmBase) {
         if (model.getIsRngEnabled().getIsChangable() && 
model.getIsRngEnabled().getIsAvailable()) {
             Frontend.getInstance().runQuery(VdcQueryType.GetRngDevice, new 
IdQueryParameters(vmBase.getId()), new AsyncQuery(
                     this,
@@ -473,4 +505,19 @@
 
     protected boolean isNextRunConfigurationExists() { return false; }
 
+    protected boolean isSourceCustomInstanceTypeOrBlankTemplate() {
+        if (getSource() instanceof VmTemplate) {
+            VmTemplate source = (VmTemplate) getSource();
+            if (source.getTemplateType() == VmEntityType.INSTANCE_TYPE) {
+                // only the custom instance type has null id.
+                // not using instanceof check because findbugs can not handle 
it properly here
+                return source.getId() == null;
+            }
+
+            return source.getId().equals(Guid.Empty);
+        }
+
+        return false;
+
+    }
 }
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewPoolInstanceTypeManager.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewPoolInstanceTypeManager.java
index 9eadbee..0596f24 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewPoolInstanceTypeManager.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewPoolInstanceTypeManager.java
@@ -20,6 +20,14 @@
         }
     }
 
+    @Override
+    protected void updateBalloon(VmBase vmBase, boolean continueWithNext) {
+        if (!isSourceCustomInstanceTypeOrBlankTemplate()) {
+            super.updateBalloon(vmBase, continueWithNext);
+        } else if (continueWithNext) {
+            updateRngDevice(vmBase);
+        }
+    }
 
     @Override
     protected void maybeSetSingleQxlPci(VmBase vmBase) {
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewVmInstanceTypeManager.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewVmInstanceTypeManager.java
index 5c1a5d5..5de63ef 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewVmInstanceTypeManager.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/instancetypes/NewVmInstanceTypeManager.java
@@ -25,6 +25,15 @@
     }
 
     @Override
+    protected void updateBalloon(VmBase vmBase, boolean continueWithNext) {
+        if (!isSourceCustomInstanceTypeOrBlankTemplate()) {
+            super.updateBalloon(vmBase, continueWithNext);
+        } else if (continueWithNext) {
+            updateRngDevice(vmBase);
+        }
+    }
+
+    @Override
     protected ProfileBehavior getNetworkProfileBehavior() {
         return networkBehavior;
     }


-- 
To view, visit http://gerrit.ovirt.org/32514
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5859ca02ddcbeb60583a0e79b94f7b9f2647596a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Tomas Jelinek <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to