Lior Vernia 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; > I don't know role of this class. But getter of this field is accessed (from Since SharedMacPoolListModel extends ListWithDetailsModel (i.e. is a main tab), you should override initDetailsModel() in it and add a PermissionListModel to its collection of detail models. Then from the provider you can access the correct (and only) detail model via the getter for sharedMacPoolListModel, then call getDetailModels() etc. 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 { > I don't see how we can parameterized GetAllRolesQuery to obtain this. Query 1. It can be parametrized to a type of entity, and fetch permissions related to that entity. This entity can be MAC pool or anything else, the type of entity shouldn't affect the logic but only the specific filter used for fetching. 2. Existence of RFEs doesn't depend on complexity of solution - if an RFE doesn't exist, it means no user/customer cared enough. 3. Because this should be handled in an infrastructural fashion as I wrote earlier. 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); > I can, of course, but are you aware, that this is utterly wrong? Just because you don't agree with something doesn't mean it's wrong. Please spend less time thinking of ridiculous "analogies". I agree that the *naming* of the method could be better. Other than that, its semantics are the same all over the frontend and are well-understood. A proper analogy would be that both haveAbortion() and giveBirth() call endPregnancy(), where endPregnancy() might be named cancelPregnancy() instead. But this is just a naming issue. Please do not see this as an invitation to either continue discussing this or to try and come up with similar analogies in the future. 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
