Martin Mucha has posted comments on this change. Change subject: webadmin: gui for adding macPool permissions. ......................................................................
Patch Set 14: (3 comments) http://gerrit.ovirt.org/#/c/32211/14/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 84: private SystemPermissionListModel systemPermissionListModel; Line 85: private ClusterPolicyListModel clusterPolicyListModel; Line 86: private InstanceTypeListModel instanceTypeListModel; Line 87: private SharedMacPoolListModel sharedMacPoolListModel; Line 88: private MacPoolPermissionsListModel macPoolPermissionsListModel; > Since SharedMacPoolListModel extends ListWithDetailsModel (i.e. is a main t Done Line 89: Line 90: // NOTE: when adding a new ListModel here, be sure to add it to the list in initItems() Line 91: private ListWithDetailsAndReportsModel dataCenterList; Line 92: private ClusterListModel clusterList; http://gerrit.ovirt.org/#/c/32211/14/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/macpool/MacPoolPermissionsListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/macpool/MacPoolPermissionsListModel.java: Line 3: import org.ovirt.engine.ui.uicommonweb.models.configure.PermissionListModel; Line 4: import org.ovirt.engine.ui.uicommonweb.models.users.AdElementListModel; Line 5: import org.ovirt.engine.ui.uicommonweb.models.users.AdElementListModelForMacPoolRoles; Line 6: Line 7: public class MacPoolPermissionsListModel extends PermissionListModel { > 1. It can be parametrized to a type of entity, and fetch permissions relate ad 1. that's too speculative design and such thing would be some sort of prophecy (you'll be able to return all roles based on some class) ad 2. ins't important; also sane gui isn't something user/customer should explicitely ask about. —— The question is simpler: do we *really* want to present unrelated roles in given dialog or not? For me it's nonsense and serves only for LOC optimizations. But you make decisions, my responsibility is to point out flaws and then met your decisions. Line 8: Line 9: @Override Line 10: protected AdElementListModel createAdElementListModel() { Line 11: return new AdElementListModelForMacPoolRoles(); http://gerrit.ovirt.org/#/c/32211/14/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/macpool/SharedMacPoolListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/macpool/SharedMacPoolListModel.java: Line 122: setConfirmWindow(null); Line 123: } Line 124: Line 125: private void onRemove() { Line 126: setConfirmWindow(null); > Just because you don't agree with something doesn't mean it's wrong. Please Done Line 127: ArrayList<VdcActionParametersBase> params = new ArrayList<VdcActionParametersBase>(); Line 128: for (MacPool macPool : (Iterable<MacPool>) getSelectedItems()) { Line 129: params.add(new RemoveMacPoolByIdParameters(macPool.getId())); Line 130: } -- 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: 14 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
