Daniel Erez has posted comments on this change.
Change subject: userportal, webadmin: Show warning for non-exportable disks
......................................................................
Patch Set 2: (12 inline comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmMakeTemplatePopupWidget.java
Line 72: EntityModelCheckBoxEditor isTemplatePublicEditor;
Line 73:
Line 74: @UiField
Line 75: @Ignore
Line 76: HtmlLabel messageHTML;
Won't it be simpler/nicer to use a FlowPanel and add Labels to it?
I.e. a new Label for every message - something like itemPanel in
RemoveConfirmationPopupView (or extract it to some new widget e.g. LabelPanel).
Or, handle it on the model for keeping the model/view synchronization.
Line 77:
Line 78: @UiField
Line 79: @Ignore
Line 80: Label disksAllocationLabel;
Line 154: @Override
Line 155: public void eventRaised(Event ev, Object sender,
EventArgs args) {
Line 156: String propName = ((PropertyChangedEventArgs)
args).PropertyName;
Line 157: if ("Message".equals(propName)) { //$NON-NLS-1$
Line 158:
VmMakeTemplatePopupWidget.this.setMessage(model.getMessage());
'VmMakeTemplatePopupWidget.this' can be removed.
Rename 'setMessage' to something like 'appendMessage'.
Line 159: }
Line 160: }
Line 161: });
Line 162: }
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmSnapshotCreatePopupWidget.java
Line 38: EntityModelTextBoxEditor descriptionEditor;
Line 39:
Line 40: @UiField
Line 41: @Ignore
Line 42: HtmlLabel messageHTML;
same here
Line 43:
Line 44: public VmSnapshotCreatePopupWidget(CommonApplicationConstants
constants) {
Line 45: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this));
Line 46: localize(constants);
Line 60: @Override
Line 61: public void eventRaised(Event ev, Object sender, EventArgs
args) {
Line 62: String propName = ((PropertyChangedEventArgs)
args).PropertyName;
Line 63: if ("Message".equals(propName)) { //$NON-NLS-1$
Line 64:
VmSnapshotCreatePopupWidget.this.setMessage(model.getMessage());
same here
Line 65: }
Line 66: }
Line 67: });
Line 68: }
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/VmBaseListModel.java
Line 200: }
Line 201: }
Line 202:
Line 203: private String composeExistingVmsWarningMessage(List<T>
existingVms) {
Line 204: final StringBuffer s = new StringBuffer();
Please use engine code format (missing whitespace after if/for/etc...)
Line 205: for (T t : existingVms) {
Line 206: if(s.length() > 0) {
Line 207: s.append(", "); //$NON-NLS-1$
Line 208: }
Line 202:
Line 203: private String composeExistingVmsWarningMessage(List<T>
existingVms) {
Line 204: final StringBuffer s = new StringBuffer();
Line 205: for (T t : existingVms) {
Line 206: if(s.length() > 0) {
This concatenation logic repeats a few times - try extracting it to a common
method. Preferably, create a list of string an use uicompat's StringUtils.join
(i.e. StringUtils.join(stringList, ", ")).
Line 207: s.append(", "); //$NON-NLS-1$
Line 208: }
Line 209: s.append(extractNameFromEntity(t));
Line 210: }
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewTemplateVmModelBehavior.java
Line 117: Disk disk = (Disk) disksIterator.next();
Line 118:
Line 119: if (disk.getDiskStorageType() ==
DiskStorageType.IMAGE && !disk.isShareable()) {
Line 120: disks.add(disk);
Line 121: } else if(!disk.isAllowSnapshot()) {
code format
Line 122: nonExportableDisks.add(disk);
Line 123: }
Line 124: }
Line 125:
Line 137: getModel().getQuota().setIsAvailable(false);
Line 138: }
Line 139: }
Line 140:
Line 141: private void sendWarningForNonExportableDisks(List<Disk>
nonExportableDisks) {
Please use engine code format (missing whitespace after if/for/etc...)
Line 142: if(!nonExportableDisks.isEmpty()) {
Line 143: final StringBuilder s = new StringBuilder();
Line 144:
Line 145: for(Disk disk : nonExportableDisks) {
Line 141: private void sendWarningForNonExportableDisks(List<Disk>
nonExportableDisks) {
Line 142: if(!nonExportableDisks.isEmpty()) {
Line 143: final StringBuilder s = new StringBuilder();
Line 144:
Line 145: for(Disk disk : nonExportableDisks) {
This concatenation logic repeats a few times - try extracting it to a common
method. Preferably, create a list of string an use uicompat's StringUtils.join
(i.e. StringUtils.join(stringList, ", ")).
Line 146: if(s.length() > 0) {
Line 147: s.append(", "); //$NON-NLS-1$
Line 148: }
Line 149:
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/SnapshotModel.java
Line 176: s.append(disk.getDiskAlias());
Line 177: }
Line 178:
Line 179: // append warning message
Line 180:
setMessage(ConstantsManager.getInstance().getMessages().disksWillNotBePartOfTheExportedVMSnapshot(s.toString()));
It's preferable to keep the model and view synchronized (iiuc, according to the
current solution - model's message value could be different than the view.
Try handling the messages concatenation in the model or maintain a list of
messages for view's usage.
Line 181: }
Line 182: }
Line 183:
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
Line 1329:
Line 1330: @Override
Line 1331: protected void sendWarningForNonExportableDisks(VM entity) {
Line 1332: // load VM disks and check if there is one which doesn't
allow snapshot
Line 1333: AsyncDataProvider.GetVmDiskList(new
AsyncQuery((ExportVmModel) getWindow(),
same here
Line 1334: new INewAsyncCallback() {
Line 1335: @Override
Line 1336: public void OnSuccess(Object target, Object
returnValue) {
Line 1337: final ExportVmModel model = (ExportVmModel)
target;
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/vm/VmExportPopupView.java
Line 37: EntityModelCheckBoxEditor collapseSnapshots;
Line 38:
Line 39: @UiField
Line 40: @Ignore
Line 41: HtmlLabel messageHTML;
same here...
Line 42:
Line 43: @Inject
Line 44: public VmExportPopupView(EventBus eventBus, ApplicationResources
resources, ApplicationConstants constants) {
Line 45: super(eventBus, resources);
--
To view, visit http://gerrit.ovirt.org/11144
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a183931e36ccd4668225ad206edc4a5e58795e3
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Libor Spevak <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Libor Spevak <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
