Omer Frenkel has posted comments on this change.
Change subject: engine: Add/Edit quota commands
......................................................................
Patch Set 11: (6 inline comments)
missing audit log for the commands, but IIRC, it will be in a future patch?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddQuotaCommand.java
Line 28: if (getParameters() == null) {
i think this check is not needed, don't think the factory will create the
command with null params, and if you decide to keep this check, you have to add
error message.
Line 34: return true;
assuming you accept my previous comment, you can change this method to be:
return
QuotaHelper.getInstance().checkQuotaValidationForAddEdit(getParameters().getQuota(),
getReturnValue().getCanDoActionMessages()));
Line 45: public Map<Guid, VdcObjectType> getPermissionCheckSubjects() {
this will always fail, what you ask here is that the user will have permissions
on the quota he did not create yet. add quota should ask for permission on
parent, which is storage pool
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateQuotaCommand.java
Line 13: public class UpdateQuotaCommand<T extends QuotaCRUDParameters> extends
CommandBase<T> {
IIRC, the VdcActionType enum has EditQuota, so this will not work, if im
right, its better to change the enum, and not the command, as other commands
are Update and not Edit
Line 28: if (getParameters() == null) {
same comment about this being null, but you can keep it, as you have a msg for
it
Line 34: } else if (getParameters().getQuota().getId() == null) {
this is not enough, you need to check that the quota really exist in the db
(what if i send update with new quota object?)
--
To view, visit http://gerrit.ovirt.org/2084
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4abf8bab03b8044e11f1e0a07dbb01e3b0fd8e32
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
