On 04.02.2013 18:10, Francesco Chicchiriccò wrote:
> I agree with you that WorkflowController and the workflow methods in
>> UserController do not belong together. On the other hand I think it may
>> be a good idea to separate them from the UserService. So would it make
>> sense to have a UserWorkflowService? This is supported by the fact that
>> we already have a separate ApprovalRestClient in the client module.
>
> I am still not completely convinced of the rationale for creating such
> new UserWorkflowService (and RoleWorkflowService, please don't forget
> that also roles are workflow-enabled).
> We will end up with UserService / RoleService for read-only operations
> and UserWorkflowService / RoleWorkflowService for write-only
> operations: does it make sense?
UserService and RoleService contain the full set of crud operations. So
why would they be readonly?
I am not absolutely sure if separating the interfaces makes sense but I
think it could. One indicator is that the UserService always works with
the userId or userName while the workflow methods normally use taskId as
a key. So I think this indicates that the resource we are handling is
not the user but rather a workflow identified by a task. So naming it
UserWorkflowService sounds intuitive to me.
>
> Why make this distinction for SyncopeUser and not for any other
> entity? We "expose" internal ids for everything.
That is true. It would not make too much sense to make an exemption for
User.
>> One other thing we could consider is using a redirect from the user
>> based rest resource to the id based resource. So when you do a get on
>> users/readByUsername/{username} we could redirect to users/{username}.
>>
>> For the status changing methods like users/activate/{userid} we could
>> instead use: users/{userId}/status.
>> You could send put with body active to users/{userId}/status to make a
>> user active. Or you could get users/{userId}/status to retrieve the
>> status of a user.
>
> As explained in another e-mail thread, it does not make sense to set a
> "generic" status for users: "activate" an user is not the same as
> setting its status to "active" (and so on): the difference is given by
> the workflow definition that any single project could implement to
> suit its own needs.
>
> For these reasons, we should try to find a way to express the operations:
> * activate()
> * suspend()
> * reactivate()
>
> in RESTful terms.
> But "putting user status" is not an option, IMO.
I will experiment with introducing methods that take a userId or
userName String. So at least we can cut half of the methods for status
changes.
Is it ok to remove the reactivate and use activate with a null token
instead or is this too different in meaning?
Best regards
Christian
--
Christian Schneider
http://www.liquid-reality.de
Open Source Architect
http://www.talend.com
