Yair Zaslavsky has posted comments on this change.

Change subject: aaa: reactivate user
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/29974/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java:

Line 109:         // First check if the user is already in the database, if it 
is we need to update, if not we need to insert:
Line 110:         DbUser dbUser = 
dao.getByExternalId(getParameters().getDirectory(), principal.<String> 
get(PrincipalRecord.ID));
Line 111:         DbUser mappedDbUser = 
DirectoryUtils.mapPrincipalRecordToDbUser(getParameters().getDirectory(), 
principal);
Line 112:         mappedDbUser.setId(dbUser != null ? dbUser.getId() : 
Guid.newGuid());
Line 113:         mappedDbUser.setActive(dbUser != null ? dbUser.isActive() : 
true);
> but:
the command should not care if it was executed from rest-api or gui, the 
comment is 1 IMHO is not that valid (REST-API also uses canDoActions 
indirectly, as when a command is run in backend, the canDoAction is the first 
stage to be run.
2. your idea for separation is valid,but IMHO should not be in the scope of 
this patch (maybe a patch before this one).
Line 114: 
Line 115:         DirectoryUtils.syncAndReactivatePrincipal(principal, 
mappedDbUser, "the user is rectivated as it was added again to the system");
Line 116:         setActionReturnValue(mappedDbUser.getId());
Line 117:         setSucceeded(true);


-- 
To view, visit http://gerrit.ovirt.org/29974
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I556d8da48a858ce193865e84fb6c7cb4043a8e5b
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[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

Reply via email to