Lior Vernia has posted comments on this change.

Change subject: frontend: Sorting columns in VM subtabs
......................................................................


Patch Set 4:

(12 comments)

Work has been made much simpler due to the implementation of default sorting 
methods on the different column type.

http://gerrit.ovirt.org/#/c/28360/4/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/vm/VmInterfaceListModelTable.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/vm/VmInterfaceListModelTable.java:

Line 75:     public void initTable(final CommonApplicationConstants constants) {
Line 76:         getTable().enableColumnResizing();
Line 77: 
Line 78:         NicActivateStatusColumn<VmNetworkInterface> statusColumn = new 
NicActivateStatusColumn<VmNetworkInterface>();
Line 79:         
statusColumn.makeSortable(VmNetworkInterfaceComparator.BY_PLUGGED_AND_LINKED);
Please use this class's default sorting method, i.e. 
statusColumn.makeSortable() (already used by network/VMs subtab, and sorting 
should be the same).
Line 80:         getTable().addColumn(statusColumn, constants.empty(), "30px"); 
//$NON-NLS-1$
Line 81: 
Line 82:         TextColumnWithTooltip<VmNetworkInterface> nameColumn = new 
TextColumnWithTooltip<VmNetworkInterface>() {
Line 83:             @Override


Line 84:             public String getValue(VmNetworkInterface object) {
Line 85:                 return object.getName();
Line 86:             }
Line 87:         };
Line 88:         nameColumn.makeSortable(VmNetworkInterfaceComparator.BY_NAME);
Please call nameColumn.makeSortable() instead - as should be done with all text 
columns.
Line 89:         getTable().addColumn(nameColumn, constants.nameInterface(), 
"150px"); //$NON-NLS-1$
Line 90: 
Line 91:         CheckboxColumn<VmNetworkInterface> pluggedColumn = new 
CheckboxColumn<VmNetworkInterface>() {
Line 92:             @Override


Line 98:             protected boolean canEdit(VmNetworkInterface object) {
Line 99:                 return false;
Line 100:             }
Line 101:         };
Line 102:         
pluggedColumn.makeSortable(VmNetworkInterfaceComparator.BY_PLUGGED);
Please call pluggedColumn,makeSortable() - same for all checkbox columns.
Line 103:         getTable().addColumnWithHtmlHeader(pluggedColumn, 
constants.plugged(), "60px"); //$NON-NLS-1$
Line 104: 
Line 105:         TextColumnWithTooltip<VmNetworkInterface> networkNameColumn = 
new TextColumnWithTooltip<VmNetworkInterface>() {
Line 106:             @Override


Line 107:             public String getValue(VmNetworkInterface object) {
Line 108:                 return object.getNetworkName();
Line 109:             }
Line 110:         };
Line 111:         
networkNameColumn.makeSortable(VmNetworkInterfaceComparator.BY_NETWORK_NAME);
Same comment as nameColumn.
Line 112:         getTable().addColumn(networkNameColumn, 
constants.networkNameInterface(), "150px"); //$NON-NLS-1$
Line 113: 
Line 114:         TextColumnWithTooltip<VmNetworkInterface> profileNameColumn = 
new TextColumnWithTooltip<VmNetworkInterface>() {
Line 115:             @Override


Line 116:             public String getValue(VmNetworkInterface object) {
Line 117:                 return object.getVnicProfileName();
Line 118:             }
Line 119:         };
Line 120:         
profileNameColumn.makeSortable(VmNetworkInterfaceComparator.BY_PROFILE_NAME);
Same comment as nameColumn.
Line 121:         getTable().addColumn(profileNameColumn, 
constants.profileNameInterface(), "150px"); //$NON-NLS-1$
Line 122: 
Line 123:         BooleanColumn<VmNetworkInterface> linkStateColumn =
Line 124:                 new 
BooleanColumn<VmNetworkInterface>(constants.linkedNetworkInterface(),


Line 127:                     protected Boolean getRawValue(VmNetworkInterface 
object) {
Line 128:                         return object.isLinked();
Line 129:                     }
Line 130:                 };
Line 131:         
linkStateColumn.makeSortable(VmNetworkInterfaceComparator.BY_LINK_STATE);
Same comment as nameColumn.
Line 132:         getTable().addColumnWithHtmlHeader(linkStateColumn, 
constants.linkStateNetworkInterface(), "65px"); //$NON-NLS-1$
Line 133: 
Line 134:         TextColumnWithTooltip<VmNetworkInterface> typeColumn = new 
EnumColumn<VmNetworkInterface, VmInterfaceType>() {
Line 135:             @Override


Line 136:             protected VmInterfaceType getRawValue(VmNetworkInterface 
object) {
Line 137:                 return VmInterfaceType.forValue(object.getType());
Line 138:             }
Line 139:         };
Line 140:         typeColumn.makeSortable(VmNetworkInterfaceComparator.BY_TYPE);
Same comment as nameColumn.
Line 141:         getTable().addColumn(typeColumn, constants.typeInterface(), 
"100px"); //$NON-NLS-1$
Line 142: 
Line 143:         TextColumnWithTooltip<VmNetworkInterface> macColumn = new 
TextColumnWithTooltip<VmNetworkInterface>() {
Line 144:             @Override


Line 145:             public String getValue(VmNetworkInterface object) {
Line 146:                 return object.getMacAddress();
Line 147:             }
Line 148:         };
Line 149:         
macColumn.makeSortable(VmNetworkInterfaceComparator.BY_MAC_ADDRESS);
Same comment as nameColumn.
Line 150:         getTable().addColumn(macColumn, constants.macInterface(), 
"150px"); //$NON-NLS-1$
Line 151: 
Line 152:         TextColumnWithTooltip<VmNetworkInterface> speedColumn = new 
TextColumnWithTooltip<VmNetworkInterface>() {
Line 153:             @Override


Line 158:                     return null;
Line 159:                 }
Line 160:             }
Line 161:         };
Line 162:         
speedColumn.makeSortable(VmNetworkInterfaceComparator.BY_SPEED);
Same comment as nameColumn.
Line 163:         getTable().addColumnWithHtmlHeader(speedColumn,
Line 164:                 templates.sub(constants.speedInterface(), 
constants.mbps()).asString(), "150px"); //$NON-NLS-1$
Line 165: 
Line 166:         getTable().addActionButton(new 
UiCommandButtonDefinition<VmNetworkInterface>(getEventBus(),


http://gerrit.ovirt.org/#/c/28360/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/comparators/VmNetworkInterfaceComparator.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/comparators/VmNetworkInterfaceComparator.java:

None of this is required if default column sorting is used.
Line 1: package org.ovirt.engine.ui.uicommonweb.comparators;
Line 2: 
Line 3: import java.util.Comparator;
Line 4: import 
org.ovirt.engine.core.common.businessentities.comparators.LexoNumericComparator;


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

Line 275:                     
snapshotModel.setApps(Arrays.asList(snapshot.getAppList() != null ?
Line 276:                             snapshot.getAppList().split(",") : new 
String[]{})); //$NON-NLS-1$
Line 277: 
Line 278:                     Collections.sort(snapshotModel.getDisks(), new 
Linq.DiskByAliasComparer());
Line 279:                     Collections.sort(snapshotModel.getNics(), 
VmNetworkInterfaceComparator.BY_NAME);
This should probably stay VmInterfaceComparer, as the 
VmNetworkInterfaceComparator class shouldn't be added anymore.
Line 280:                 }
Line 281: 
Line 282:                 onUpdateAsyncCallback.onSuccess(snapshotModel, null);
Line 283:             }


http://gerrit.ovirt.org/#/c/28360/4/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/virtualMachine/SubTabVirtualMachineNetworkInterfaceView.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/virtualMachine/SubTabVirtualMachineNetworkInterfaceView.java:

Line 48:                             public Boolean 
getRawValue(VmNetworkInterface object) {
Line 49:                                 return object.isPortMirroring();
Line 50:                             }
Line 51:                         };
Line 52:                 
portMirroringColumn.makeSortable(VmNetworkInterfaceComparator.BY_PORT_MIRRORING);
Again, probably better to use the default portMirroringColumn.makeSortable().
Line 53:                 
getTable().addColumnWithHtmlHeader(portMirroringColumn, 
constants.portMirroring(), "85px"); //$NON-NLS-1$
Line 54:             }
Line 55:         });
Line 56:         ViewIdHandler.idHandler.generateAndSetIds(this);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b55d40f4978f994cc2c84f3a99d46294b7928fc
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Lior Vernia <[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