Lior Vernia has posted comments on this change. Change subject: webadmin: gui for adding macPool permissions. ......................................................................
Patch Set 19: (5 comments) http://gerrit.ovirt.org/#/c/32211/19/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java: Line 223: GetAllServerCpuList, Line 224: Line 225: // Multi Level Administration queries Line 226: GetAllRoles(VdcQueryAuthType.User), Line 227: GetAllMacPoolRoles(VdcQueryAuthType.Admin), I think this and the rest of the backend code can be removed, now that you're using the original GetAllRoles. Line 228: GetRoleById(VdcQueryAuthType.User), Line 229: GetRoleByName, Line 230: GetPermissionById(VdcQueryAuthType.User), Line 231: GetPermissionByRoleId, http://gerrit.ovirt.org/#/c/32211/19/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/CommonModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/CommonModel.java: Line 545: setWindow(model); Line 546: model.setTitle(ConstantsManager.getInstance().getConstants().ConfigureTitle()); Line 547: model.setHelpTag(HelpTag.configure); Line 548: model.setHashName("configure"); //$NON-NLS-1$ Line 549: //TODO MM: what is this for? This should probably be removed :) Line 550: model.setEntity(new Model[] { roleListModel, systemPermissionListModel, clusterPolicyListModel, sharedMacPoolListModel }); Line 551: Line 552: UICommand tempVar = new UICommand("Cancel", this); //$NON-NLS-1$ Line 553: tempVar.setTitle(ConstantsManager.getInstance().getConstants().close()); http://gerrit.ovirt.org/#/c/32211/19/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/macpool/SharedMacPoolView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/macpool/SharedMacPoolView.java: Line 72: initWidget(rootPanel); Line 73: } Line 74: Line 75: private void setupAuthorizationTableVisibility() { Line 76: final boolean isExactlyOnePoolSelected = macPoolTable.getSelectionModel().getSelectedList().size() == 1; Since this is called from a method where you already fetch the currently selected items, and already check if their number equals one... You can pass here the boolean of whether to show the permissions table from there instead of fetching them and doing that check here again. Line 77: authorizationTable.setVisible(isExactlyOnePoolSelected); Line 78: } Line 79: Line 80: private SplitLayoutPanel createRootPanel() { Line 73: } Line 74: Line 75: private void setupAuthorizationTableVisibility() { Line 76: final boolean isExactlyOnePoolSelected = macPoolTable.getSelectionModel().getSelectedList().size() == 1; Line 77: authorizationTable.setVisible(isExactlyOnePoolSelected); It might be better to clear the panel contents, and only addSouth() if isExactlyOnePoolSelected instead. Currently, the panel will be split even if nothing is selected in the top part, and have a clear panel in the bottom part, won't it? Line 78: } Line 79: Line 80: private SplitLayoutPanel createRootPanel() { Line 81: SplitLayoutPanel rootPanel = new SplitLayoutPanel(); Line 146: Line 147: final List<MacPool> selectedItems = macPoolTable.getSelectedItems(); Line 148: sharedMacPoolModelProvider.setSelectedItems(selectedItems); Line 149: Line 150: final PermissionListModel model = permissionModelProvider.getModel(); Maybe you could fetch the model here as well by calling sharedMacPoolModelProvider.getModel().getDetailModels() etc.? This will obviate the entire PermissionModelProvider class, the binding in UiCommonModule, the injection here and the member. You'll just need to add some code to SharedMacPoolModelProvider instead. Line 151: model.setEntity(selectedItems.size() == 1 ? selectedItems.get(0) : null); Line 152: } Line 153: }); Line 154: -- To view, visit http://gerrit.ovirt.org/32211 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb6b442e7eebce367b9008f4b9c1df9ade84745f Gerrit-PatchSet: 19 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Martin Mucha <[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
