anmolbabu has posted comments on this change. Change subject: webadmin: add additional features support in cluster popup. ......................................................................
Patch Set 7: (6 comments) https://gerrit.ovirt.org/#/c/39757/7/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java: Line 4109: new GetClusterFeaturesByVersionAndCategoryParameters(version, category), Line 4110: aQuery); Line 4111: } Line 4112: Line 4113: public void getClusterFeaturesByClusterId(AsyncQuery aQuery, Guid clusterId) { > Here the parameters, Query and ReturnType all are different. So I had put i I understand, but since the kind of data being operated on and the way its operated on are same, this prompted me to think of it that way. But I would leave it to you Line 4114: aQuery.converterCallback = new IAsyncConverter() { Line 4115: @Override Line 4116: public Object Convert(Object source, AsyncQuery _asyncQuery) Line 4117: { https://gerrit.ovirt.org/#/c/39757/7/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterListModel.java: Line 754: cluster.setCompatibilityVersion(version); Line 755: cluster.setMigrateOnError(model.getMigrateOnErrorOption()); Line 756: cluster.setVirtService(model.getEnableOvirtService().getEntity()); Line 757: cluster.setGlusterService(model.getEnableGlusterService().getEntity()); Line 758: for (AdditionalClusterFeature feature : model.getAdditionalClusterFeatures().getSelectedItem()) { > Thats the trick with CheckBoxGroup. Entity model is defined as Hey yes you are right, I myself forgot it :( Line 759: cluster.getAddtionalFeaturesSupported().add(new SupportedAdditionalClusterFeature(cluster.getId(), Line 760: true, Line 761: feature)); Line 762: } https://gerrit.ovirt.org/#/c/39757/7/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterModel.java: Line 928: setAdditionalClusterFeatures(new ListModel<List<AdditionalClusterFeature>>()); Line 929: ArrayList<List<AdditionalClusterFeature>> newList = new ArrayList<List<AdditionalClusterFeature>>(); Line 930: newList.add(Collections.<AdditionalClusterFeature> emptyList()); Line 931: getAdditionalClusterFeatures().setItems(newList, Line 932: new ArrayList<AdditionalClusterFeature>()); > Yes. It doesn't work without the empty list. Let me fix the widget. Its not actually empty list may be you could do newList.get(0) I remember this used to be done by default in case of all widgets by framework if only 1 arg is passed. So yes a fix in widget would be nice to have. Line 933: setSpiceProxyEnabled(new EntityModel<Boolean>()); Line 934: getSpiceProxyEnabled().setEntity(false); Line 935: getSpiceProxyEnabled().getEntityChangedEvent().addListener(this); Line 936: Line 1738: List<AdditionalClusterFeature> features = new ArrayList<>(); Line 1739: List<AdditionalClusterFeature> selectedFeatures = new ArrayList<>(); Line 1740: for (AdditionalClusterFeature feature : featuresAvailable) { Line 1741: features.add(feature); Line 1742: if (featuresEnabled.contains(feature)) { > Yes featuredEnabled are the ones which are already selected for this cluste Done Line 1743: selectedFeatures.add(feature); Line 1744: } Line 1745: } Line 1746: List<List<AdditionalClusterFeature>> listOfFeatures = new ArrayList<>(); https://gerrit.ovirt.org/#/c/39757/7/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java: Line 4287: Line 4288: @DefaultStringValue("Warning : Recommendations for geo-replication not met -") Line 4289: String geoReplicationRecommendedConfigViolation(); Line 4290: Line 4291: @DefaultStringValue("Additional features supported in the Cluster") > This is the label for the collapsible widget not the title of the popup Oh yes :)) :D they are shown in Cluster dialog itself right. sorry for that please ignore it Line 4292: String addtionalClusterFeaturesTitle(); Line 4293: Line 4294: @DefaultStringValue("Additional Cluster Features") Line 4295: String addtionalClusterFeaturesLabel(); https://gerrit.ovirt.org/#/c/39757/7/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/cluster/ClusterPopupView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/cluster/ClusterPopupView.java: Line 220: ListModelCheckBoxGroup<AdditionalClusterFeature> additionalFeaturesEditor; Line 221: Line 222: @UiField Line 223: @Ignore Line 224: AdvancedParametersExpander additionalFeaturesExpander; > We can't select all of them by default. User has to select the feature whic you are right but I feel if its not expanded then there are more chances user misses this section. But yes it might not be hazardous as user can go and edit it. But I would still like it if it could be somehow clear and visible to user. But I would like you to take kanagaraj's opinion as there might be an alternative which I am not aware of. Line 225: Line 226: @UiField Line 227: @Ignore Line 228: FlowPanel additionalFeaturesExpanderContent; -- To view, visit https://gerrit.ovirt.org/39757 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic13dda67f3e9a7d7134030c3923470291c3c5aec Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ramesh N <rnach...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Ramesh N <rnach...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: anmolbabu <anb...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
