Tomas Jelinek has posted comments on this change.
Change subject: webadmin: Edit pool hangs when the pool has no VMs (#852297)
......................................................................
Patch Set 6: (2 inline comments)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/pools/PoolListModel.java
Line 752:
Line 753: private void UpdateActionAvailability()
Line 754: {
Line 755: getEditCommand().setIsExecutionAllowed(getSelectedItem() !=
null && getSelectedItems() != null
Line 756: && getSelectedItems().size() == 1 &&
hasVms(getSelectedItem()));
The method is not extracted because it is re-used but because it is a named
logic. I find it more readable. If you are against it, I can do it like this:
boolean hasVms = ((vm_pools) selectedItem).getvm_assigned_count() != 0;
... getSelectedItems().size() == 1 && hasVms ...
It is not that clean, but still readable and you have one less method.
What do you think?
Line 757:
Line 758: getRemoveCommand().setIsExecutionAllowed(getSelectedItems()
!= null && getSelectedItems().size() > 0
Line 759: && VdcActionUtils.CanExecute(getSelectedItems(),
vm_pools.class, VdcActionType.RemoveVmPool));
Line 760: }
....................................................
File
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java
Line 190:
Line 191: @DefaultStringValue("Detach Virtual Machine(s)")
Line 192: String detachVirtualMachinesTitle();
Line 193:
Line 194: @DefaultStringValue("You chose to detach all VMs from the pool -
this will remove the pool itself.")
Well, it looks like this:
There is "Are you sure you want to detach selected Virtual Machine(s)?"
Than list of VMs
Than "Approve operation" checkbox (like the force remove data center dialog)
Under this checkbox the message from above (in red) is shown.
Is this message OK for this case?
Line 195: String detachAllVmsWarning();
Line 196:
Line 197: @DefaultStringValue("Permissions")
Line 198: String permissionsTitle();
--
To view, visit http://gerrit.ovirt.org/7583
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b8f47e6dd5ce8149ba78c7c25455997b3bf62eb
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
