Eldan Shachar has posted comments on this change. Change subject: userportal\webadmin: Editing of template version for pools ......................................................................
Patch Set 5: (8 comments) https://gerrit.ovirt.org/#/c/36510/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/pools/PoolListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/pools/PoolListModel.java: Line 227: Line 228: new AsyncQuery(this, new INewAsyncCallback() { Line 229: @Override Line 230: public void onSuccess(Object modell, Object result) { Line 231: final VM vm = (VM) ((VdcQueryReturnValue) result).getReturnValue(); > can the VM be null? The previous code expected it to be null and there are added the check because it can (technically) be null.. the previous code caused exception when the vm is null so the new code doesn't continue to InitExistingPoolModel() but just return instead. removed the null check(and any relevant code) from initExistingPoolModel::eventRaised(). Line 232: if (vm.isNextRunConfigurationExists()) { Line 233: Frontend.getInstance().runQuery(VdcQueryType.GetVmNextRunConfiguration, Line 234: new IdQueryParameters(vm.getId()), Line 235: new AsyncQuery(this, new INewAsyncCallback() { Line 228: new AsyncQuery(this, new INewAsyncCallback() { Line 229: @Override Line 230: public void onSuccess(Object modell, Object result) { Line 231: final VM vm = (VM) ((VdcQueryReturnValue) result).getReturnValue(); Line 232: if (vm.isNextRunConfigurationExists()) { > please break this logic down to specific methods - e.g. as a body of the if sure, it accumulated over time and became monstrous.. Line 233: Frontend.getInstance().runQuery(VdcQueryType.GetVmNextRunConfiguration, Line 234: new IdQueryParameters(vm.getId()), Line 235: new AsyncQuery(this, new INewAsyncCallback() { Line 236: @Override Line 236: @Override Line 237: public void onSuccess(Object a, Object result) { Line 238: final VM nextRunVm = Line 239: (VM) ((VdcQueryReturnValue) result).getReturnValue(); Line 240: if (nextRunVm!=null) { // template version was changed -> load data from template > formatter - please add spaces around "!=". Done Line 241: vm.setVmtGuid(nextRunVm.getVmtGuid()); Line 242: vm.setUseLatestVersion(nextRunVm.isUseLatestVersion()); Line 243: } Line 244: if (vm.isUseLatestVersion()) { // next run, latest template Line 246: new IdQueryParameters(vm.getVmtGuid()), Line 247: new AsyncQuery(this, new INewAsyncCallback() { Line 248: @Override Line 249: public void onSuccess(Object a, Object result) { Line 250: if (result != null && ((VdcQueryReturnValue)result).getReturnValue() != null) { > this is copy pasted from the "if (vm.isUseLatestVersion()) " - if it will b Done. tried to avoid from doing this untill now because the async part forces this method to also call initExistingPoolModel() which complicates the normal flow, did some more changes and now updatePoolTemplateDataAndInitModel() handle this part... Line 251: VmTemplate vmt = (((VdcQueryReturnValue)result).getReturnValue()); Line 252: vm.setVmtGuid(vmt.getId()); Line 253: initExistingPoolModel(vm, true, pool); Line 254: } https://gerrit.ovirt.org/#/c/36510/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingPoolModelBehavior.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingPoolModelBehavior.java: Line 32: private final VM pool; Line 33: Line 34: private InstanceTypeManager instanceTypeManager; Line 35: Line 36: boolean templateVersionWasChanged; > private Removed Line 37: boolean forceTemplateData; // indicates that the VM object passed is waiting for next-run upgrade and raw template data should be used instead Line 38: Line 39: public ExistingPoolModelBehavior(VM pool, boolean forceTemplateData) { Line 40: this.pool = pool; Line 33: Line 34: private InstanceTypeManager instanceTypeManager; Line 35: Line 36: boolean templateVersionWasChanged; Line 37: boolean forceTemplateData; // indicates that the VM object passed is waiting for next-run upgrade and raw template data should be used instead > - please make private Done Line 38: Line 39: public ExistingPoolModelBehavior(VM pool, boolean forceTemplateData) { Line 40: this.pool = pool; Line 41: this.forceTemplateData = forceTemplateData; Line 100: if (!forceTemplateData) { Line 101: instanceTypeManager.updateAll(); Line 102: } else { Line 103: List<InstanceType> instanceTypeOptions=new ArrayList<InstanceType>(); Line 104: instanceTypeOptions.add(CustomInstanceType.INSTANCE); > why are you adding the custom instance type like this? instanceTypeManager is in charge of setting this, but in this case it's loaded from the template directly which is always considered custom instance. if we don't set this explicitly the combobox will be null and fail the validations. Line 105: getModel().getInstanceTypes().setItems(instanceTypeOptions); Line 106: getModel().getInstanceTypes().setSelectedItem(instanceTypeOptions.get(0)); Line 107: getModel().stopProgress(); Line 108: } Line 124: updateRngDevice(template.getId()); Line 125: getModel().getCustomPropertySheet().deserialize(template.getCustomProperties()); Line 126: Line 127: // check if template-version selected requires to manually load the model instead of using the InstanceTypeManager Line 128: if ((!pool.getVmtGuid().equals(template.getId()) && !pool.isUseLatestVersion()) // template ID changed but not latest as it would cause false-positives > please extract this things to named boolean variables and if needed write t Done Line 129: || pool.isUseLatestVersion() != (template instanceof LatestVmTemplate) // latest property was changed Line 130: || (templateVersionWasChanged && forceTemplateData)) { // template data is used - always load manually (excluding initialization) Line 131: if (!templateVersionWasChanged && !forceTemplateData) { Line 132: deactivateInstanceTypeManager(new InstanceTypeManager.ActivatedListener() { -- To view, visit https://gerrit.ovirt.org/36510 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53686dae694f6ff826bdbeaada1e28fc55fd8d30 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eldan Shachar <eshac...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Eldan Shachar <eshac...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
