Allon Mureinik has posted comments on this change.

Change subject: webadmin: Enabling ISO & Export path editing.
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

http://gerrit.ovirt.org/#/c/37336/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java:

Line 475: 
Line 476:     private boolean isDomainTypeValidForPathEditing(StorageDomain 
storage) {
Line 477:         StorageDomainType storageDomainType = 
storage.getStorageDomainType();
Line 478:         return (storageDomainType == StorageDomainType.Data || 
storageDomainType == StorageDomainType.ImportExport
Line 479:                 || storageDomainType == StorageDomainType.ISO || 
storageDomainType == StorageDomainType.Master);
This logic will probably be repeated all over the show - let's move it to the 
StorageDomainType
Line 480:     }
Line 481: 
Line 482:     private boolean isStorageStatusValidForPathEditing(StorageDomain 
storage) {
Line 483:         return (storage.getStatus() == StorageDomainStatus.Maintenance


Line 480:     }
Line 481: 
Line 482:     private boolean isStorageStatusValidForPathEditing(StorageDomain 
storage) {
Line 483:         return (storage.getStatus() == StorageDomainStatus.Maintenance
Line 484:                 || storage.getStorageDomainSharedStatus() == 
StorageDomainSharedStatus.Unattached);
Is this reliable? Does it take ALL the datacenters into account?
Line 485:     }
Line 486: 
Line 487:     private IStorageModel prepareLocalStorageForEdit(StorageDomain 
storage)
Line 488:     {


Line 1162:                 || storageDomain.getStatus() == 
StorageDomainStatus.PreparingForMaintenance);
Line 1163:         boolean isUnattached = 
(storageDomain.getStorageDomainSharedStatus() == 
StorageDomainSharedStatus.Unattached
Line 1164:                 || storageDomain.getStatus() == 
StorageDomainStatus.Detaching);
Line 1165:         boolean isDataISOorExportDomain = 
storageDomain.getStorageDomainType().isDataDomain()
Line 1166:                 || 
storageDomain.getStorageDomainType().isIsoOrImportExportDomain();
name suggestion: isInternalDomain
Line 1167:         boolean isBlockStorage = 
storageDomain.getStorageType().isBlockDomain();
Line 1168: 
Line 1169:         isEditAvailable = isActive || isBlockStorage || 
((isInMaintenance || isUnattached) && isDataISOorExportDomain);
Line 1170:         return isEditAvailable;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0a57fa74c4bbd41272f119925fa7fef9a67b267d
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Amit Aviram <[email protected]>
Gerrit-Reviewer: Ala Hino <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Amit Aviram <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Idan Shaby <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to