Tomas Jelinek has posted comments on this change.

Change subject: frontend: support custom properties for templates
......................................................................


Patch Set 4:

(2 comments)

- please add the 
model.setCustomPropertiesKeysList(AsyncDataProvider.getInstance().getCustomPropertiesList());
 also to UserPortalListModel.newTemplate() - it is the list model behind the 
list of VMs in the user portal

- pools don't look quite right... First of all you need to add the 
getModel().getCustomPropertySheet().deserialize(template.getCustomProperties());
 to NewPoolModelBehavior.template_SelectedItemChanged()

- than please verify if the edit pool has the custom properties correct, but 
I'd suspect that not, because I don't see them parsed. If not, please add 
getModel().getCustomPropertySheet().deserialize(pool.getCustomProperties()); to 
ExistingPoolModelBehavior.postDataCenterWithClusterSelectedItemChanged()

http://gerrit.ovirt.org/#/c/35119/4/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 51:         super.initialize(systemTreeSelectedItem);
Line 52:         getModel().getVmInitEnabled().setEntity(vm.getVmInit() != 
null);
Line 53:         getModel().getVmInitModel().init(vm.getStaticData());
Line 54:         getModel().getTemplate().setIsChangable(false);
Line 55:         
getModel().getCustomPropertySheet().deserialize(vm.getCustomProperties());
after thinking a bit more this call should be after the 
updateCustomPropertySheet(); is called, because the 
updateCustomPropertySheet(); sets the keys up and than the 
getModel().getCustomPropertySheet().deserialize(vm.getCustomProperties()); 
fills to the available keys the values from vm.getCustomProperties(); It would 
start making problems in cases when changing the cluster to a one with a 
different version making a different set of keys available but not updating the 
properySheet accordingly...

So please just move this line to the 
postDataCenterWithClusterSelectedItemChanged() right after the 
updateCustomPropertySheet();
Line 56: 
Line 57:         getModel().getVmType().setIsChangable(true);
Line 58:         getModel().getCopyPermissions().setIsAvailable(true);
Line 59: 


http://gerrit.ovirt.org/#/c/35119/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/TemplateVmModelBehavior.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/TemplateVmModelBehavior.java:

Line 113: 
Line 114:         updateRngDevice(template.getId());
Line 115: 
Line 116:         
getModel().getMigrationMode().setSelectedItem(template.getMigrationSupport());
Line 117:         
getModel().getCustomPropertySheet().deserialize(template.getCustomProperties());
here it should be safe but anyway please move it under the 
updateCustomPropertySheet(); - it is more safe there
Line 118: 
Line 119:         setupBaseTemplate(template.getBaseTemplateId());
Line 120:     }
Line 121: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If5d982307a9ffcee758dfec712916dc47ce3aab5
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to