Arik Hadas has posted comments on this change. Change subject: core, webadmin: remove name from template version ......................................................................
Patch Set 7: (4 comments) http://gerrit.ovirt.org/#/c/26270/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java: Line 369: Line 370: } Line 371: // template version name should be the same as the base template name Line 372: else { Line 373: setVmTemplateName(userSelectedBaseTemplate.getName()); I think it is better that this setting won't be in the canDoAction method for 2 reasons: 1. it is not a validation 2. it might be needed in the end-action phase, for audit log for example (did you check that the audit log at the end of the action uses the right name?) How about setting it like we set the template id in this command? Line 374: } Line 375: } Line 376: Line 377: return imagesRelatedChecks() && AddVmCommand.checkCpuSockets(getParameters().getMasterVm().getNumOfSockets(), http://gerrit.ovirt.org/#/c/26270/7/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 139: VMT_CANNOT_REMOVE_BLANK_TEMPLATE=Cannot ${action} ${type}. Removing Blank Template is not allowed. Line 140: VMT_CANNOT_EDIT_BLANK_TEMPLATE=Cannot ${action} ${type}. Editing Blank Template is not allowed. Line 141: VMT_CANNOT_EXPORT_BLANK_TEMPLATE=Cannot export Blank Template. Line 142: VMT_CANNOT_UPDATE_ILLEGAL_FIELD=Failed updating the properties of the VM template. Line 143: VMT_CANNOT_UPDATE_VERSION_NAME=Cannot update the name of Template Version, Only the Version name can be updated. 1. I understand why you wrote it as you did, but I'm not sure it is easy to understand from user's perspective. Maybe "Cannot update the name of sub-Template, only the Version name can be changed" 2. s/Only/only Line 144: DIRECTORY_GROUP_CANNOT_REMOVE_DIRECTORY_GROUP_ATTACHED_TO_VM=Cannot remove Directory Group. Detach Directory Group from VM first. Line 145: VM_NOT_FOUND=VM not found Line 146: ACTION_TYPE_FAILED_CLUSTER_UNDEFINED_ARCHITECTURE=Cannot ${action} ${type}. The cluster does not have a defined architecture. Line 147: ACTION_TYPE_FAILED_MISSED_STORAGES_FOR_SOME_DISKS=Cannot ${action} ${type}. One or more provided storage domains are in maintenance/non-operational status. http://gerrit.ovirt.org/#/c/26270/7/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewTemplateVmModelBehavior.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewTemplateVmModelBehavior.java: Line 466: if (!getModel().getIsSubTemplate().getEntity()) { Line 467: getModel().getName().setEntity(""); //$NON-NLS-1$ Line 468: } else { Line 469: // by default select the template of the vm Line 470: getModel().getBaseTemplate().setEntity(getModel().getTemplate().getEntity()); don't you need to make the version name unchangeable here as well/clear its previous value? Line 471: Line 472: // copy any entered name to be the template-version name Line 473: getModel().getTemplateVersionName().setEntity(getModel().getName().getEntity()); Line 474: getModel().getName().setEntity(getModel().getBaseTemplate().getSelectedItem().getName()); http://gerrit.ovirt.org/#/c/26270/7/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java: Line 187: getModel().getTemplate().setSelectedItem(subVersions.get(1)); Line 188: } Line 189: } Line 190: Line 191: protected void isSubTemplateEntityChanged() { 1. can it be abstract method? 2. I think subTemplateEntityChanged it more appropriate name as this is not a question, it's a callback which is supposed to be called after the entity changed Line 192: } Line 193: Line 194: public boolean validate() Line 195: { -- To view, visit http://gerrit.ovirt.org/26270 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I27f26e7d196d1d2eebfe86e8aac886f3e6a8fc5e Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@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
