[Freeipa-devel] Replace stageuser-add --from-delete with user-undel --to-staged

[Martin Basti]

On 08/12/2015 02:08 PM, Tomas Capek wrote:
> On 12/08/15 13:15, David Kupka wrote:
>> On 12/08/15 12:45, thierry bordaz wrote:
>>> On 08/12/2015 12:35 PM, Martin Basti wrote:
>>>>
>>>>
>>>> On 08/11/2015 10:40 AM, thierry bordaz wrote:
>>>>> On 08/11/2015 09:32 AM, Martin Basti wrote:
>>>>>> On 11/08/15 09:17, Jan Cholasta wrote:
>>>>>>> On 5.8.2015 12:34, thierry bordaz wrote:
>>>>>>>> On 08/05/2015 12:13 PM, Jan Cholasta wrote:
>>>>>>>>> Dne 5.8.2015 v 11:55 thierry bordaz napsal(a):
>>>>>>>>>> On 08/05/2015 11:27 AM, Martin Basti wrote:
>>>>>>>>>>>
>>>>>>>>>>> ----- Original Message -----
>>>>>>>>>>> From: "thierry bordaz" <tbordaz at redhat.com>
>>>>>>>>>>> To: "Jan Cholasta" <jcholast at redhat.com>
>>>>>>>>>>> Cc: freeipa-devel at redhat.com
>>>>>>>>>>> Sent: Monday, August 3, 2015 5:34:02 PM
>>>>>>>>>>> Subject: Re: [Freeipa-devel] Replace stageuser-add
>>>>>>>>>>> --from-delete with
>>>>>>>>>>> user-undel --to-staged
>>>>>>>>>>>
>>>>>>>>>>> On 07/28/2015 12:34 PM, Jan Cholasta wrote:
>>>>>>>>>>>> Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a):
>>>>>>>>>>>>>
>>>>>>>>>>>>> Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a):
>>>>>>>>>>>>>> Dne 27.7.2015 v 17:59 Martin Basti napsal(a):
>>>>>>>>>>>>>>> On 23/07/15 14:43, Martin Basti wrote:
>>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I tried to fix #5145 and I partially succeeded.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> However, I cannot fix this part of ticket, where user is
>>>>>>>>>>>>>>>> prompted to
>>>>>>>>>>>>>>>> write name and surname.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> $ ipa stageuser-add tuser --from-delete
>>>>>>>>>>>>>>>> First name: this will be ignored
>>>>>>>>>>>>>>>> Last name: this will be also ignored
>>>>>>>>>>>>>>>> ------------------------
>>>>>>>>>>>>>>>> Added stage user "tuser"
>>>>>>>>>>>>>>>> ------------------------
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> As the first name and last name are mandatory 
>>>>>>>>>>>>>>>> attributes of
>>>>>>>>>>>>>>>> stageuser-add command, but they are not needed by when the
>>>>>>>>>>>>>>>> --from-delete option is used.
>>>>>>>>>>>>>>>> I would like to ask how to fix this issue, IMO this will
>>>>>>>>>>>>>>>> be huge
>>>>>>>>>>>>>>>> hack
>>>>>>>>>>>>>>>> in internal API. Or should we just document this bug as 
>>>>>>>>>>>>>>>> known
>>>>>>>>>>>>>>>> issue
>>>>>>>>>>>>>>>> (thierry wrote that this is not use case that should be 
>>>>>>>>>>>>>>>> used
>>>>>>>>>>>>>>>> often)?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> The best solution would be separate command, but this idea
>>>>>>>>>>>>>>>> was
>>>>>>>>>>>>>>>> rejected in thread "[Freeipa-devel] User life cycle: 
>>>>>>>>>>>>>>>> question
>>>>>>>>>>>>>>>> regarding the design"
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>>> Martin^2
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> as was mentioned before, we have issue with current
>>>>>>>>>>>>>>> internal API
>>>>>>>>>>>>>>> and the
>>>>>>>>>>>>>>> stageuser-add --from-delete command.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> We discussed this today, and we did not find a nice way how
>>>>>>>>>>>>>>> to fix
>>>>>>>>>>>>>>> it,
>>>>>>>>>>>>>>> so we propose this (which is IMO the best solution):
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> * stageuser-add --from-delete should be deprecated
>>>>>>>>>>>>>> +1
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> * create new option for user-undel: used-undel 
>>>>>>>>>>>>>>> --to-staged (or
>>>>>>>>>>>>>>> create
>>>>>>>>>>>>>>> new command) that will handle moving deleted users to 
>>>>>>>>>>>>>>> staged
>>>>>>>>>>>>>>> area as
>>>>>>>>>>>>>>> --from-delete did.
>>>>>>>>>>>>>> Make it new command please.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Instead of stageuser-add and option --from-delete, which 
>>>>>>>>>>>>>>> work
>>>>>>>>>>>>>>> totally
>>>>>>>>>>>>>>> different, the command user-undel does similar operation 
>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>> stage-user
>>>>>>>>>>>>>>> --from-delete, it just uses different container.
>>>>>>>>>>>>>> NACK on stuffing everything into a single command just
>>>>>>>>>>>>>> because it
>>>>>>>>>>>>>> does
>>>>>>>>>>>>>> something similar.
>>>>>>>>>>>>> How about making it a 'stageuser-undel'? The 'user-undel' 
>>>>>>>>>>>>> moves
>>>>>>>>>>>>> preserved user to active, so the 'stageuser-undel' would move
>>>>>>>>>>>>> preserved
>>>>>>>>>>>>> to staged. The action is similar, but has slightly different
>>>>>>>>>>>>> specifics
>>>>>>>>>>>>> (which attributes are preserved etc.), and for me the
>>>>>>>>>>>>> 'stageuser-undel'
>>>>>>>>>>>>> feels more natural than 'user-undel --to-staged' since it's
>>>>>>>>>>>>> basically
>>>>>>>>>>>>> the same as there is 'stageuser-add' for creating a staged
>>>>>>>>>>>>> user, not
>>>>>>>>>>>>> 'user-add --to-staged'. It would be in the same style as 
>>>>>>>>>>>>> all the
>>>>>>>>>>>>> other
>>>>>>>>>>>>> commands concerning operations with users in staged 
>>>>>>>>>>>>> container.
>>>>>>>>>>>> Well, user-undel is the opposite of user-del, and 
>>>>>>>>>>>> stageuser-undel
>>>>>>>>>>>> should be the opposite of stageuser-del. The stageuser-undel
>>>>>>>>>>>> you are
>>>>>>>>>>>> suggesting is not.
>>>>>>>>>>>>
>>>>>>>>>>>> Also I'm not sure if we want to (always) remove the deleted
>>>>>>>>>>>> user once
>>>>>>>>>>>> a staged user is created from it, but -undel behaves like 
>>>>>>>>>>>> that.
>>>>>>>>>>>>
>>>>>>>>>>>> I don't think the command should be limited to deleted users
>>>>>>>>>>>> only.
>>>>>>>>>>>> Active and deleted users share the same namespace, so it is an
>>>>>>>>>>>> arbitrary limitation.
>>>>>>>>>>> preserved users has been valid active user. In that sense
>>>>>>>>>>> active/preserved are managed by a same set of CLI
>>>>>>>>>>> (user-find,user-del,user-show) because a preserved user is a
>>>>>>>>>>> 'user'. So
>>>>>>>>>>> I would vote for continuing with a 'user-*' commands and use
>>>>>>>>>>> 'user-undel
>>>>>>>>>>> --to-stage'.
>>>>>>>>>>>
>>>>>>>>>>> But then if we will make any incompatible change between
>>>>>>>>>>> "user-undel"
>>>>>>>>>>> and "user-undel --to-stage" we may hit this issue again. I
>>>>>>>>>>> agree with
>>>>>>>>>>> Honza, this should be a separate command.
>>>>>>>>>> What do you mean 'incompatible change' ?
>>>>>>>>>>
>>>>>>>>>> --to-stage option would only select a different container 
>>>>>>>>>> that the
>>>>>>>>>> 'Active' one ?
>>>>>>>>>
>>>>>>>>> That's not sufficient. The command should do the reverse of
>>>>>>>>> stageuser-activate, which is ADD and DELETE, possibly with some
>>>>>>>>> modifications of the entry between them, not MODRDN like
>>>>>>>>> user-undel does.
>>>>>>>>>
>>>>>>>> That is a good point. Do we need to modify anything from a deleted
>>>>>>>> entry
>>>>>>>> to a add it in the stage container.
>>>>>>>> Already delete entry have been purged of several values (password,
>>>>>>>> krb
>>>>>>>> keys, membership,..) do you think of other attributes to remove ?
>>>>>>>>
>>>>>>>> IIRC the use case is a support engineer who activated too early an
>>>>>>>> entry. So you are right he wants to unactivate it. A question 
>>>>>>>> is does
>>>>>>>> the unactivation requires more modifications than the one did by
>>>>>>>> 'user-del --preserve'. Note that we can not retrieve the attribute
>>>>>>>> values when the entry was activated from stage.
>>>>>>>
>>>>>>> I don't know if any modifications are needed ATM (doesn't mean
>>>>>>> there can't be any in the future), but the point is that if you are
>>>>>>> creating object A from object B using operation X, you should be
>>>>>>> creating object B from object A using the reverse of operation X,
>>>>>>> otherwise there *will* be inconsistencies, and we don't want that.
>>>>>>>
>>>>>> +1
>>>>>> I agree with this
>>>>>>
>>>>> So my understanding is that you think a new verb should be created to
>>>>> allow: 'Active' -> 'Stage'
>>>>> I do not recall why this was not discussed or if it as already been
>>>>> rejected.
>>>>>
>>>>> I like the idea and I think it could be useful to Support Engineer.
>>>>> Now I am not sure we want to make 'easy' the action to 'unactivate' a
>>>>> user (currently it requires two operations).
>>>>> In your opinion, does it replace 'stageuser-add --from-delete' or
>>>>> 'user-undel --to-stage' ? or is it an additional subcommand.
>>>>> Also, activate/unactivate is not a NULL operation. Some values has
>>>>> been changed (uid,gid,uniqueuiid...) and some values will be lost
>>>>> (membership).
>>>>>
>>>>> About the verb, this is not because the previous action is 'activate'
>>>>> that we should use 'unactivate'. For example, Thomas raised the point
>>>>> that after 'user-del', 'user-restore' would have been more user
>>>>> friendly than 'user-undel'
>>>>>
>>>>> Thanks
>>>>> thierry
>>>>>
>>>> We had discussion off-list discussion, and result is following 
>>>> proposal:
>>>>
>>>> * remove 'stageuser-add --from-delete'
>>>> * add new command: 'user-stage'
>>>>
>>>> the user-stage command will move both deleted or active users to
>>>> staged area.
>>>> $ user-stage <deleted_user>
>>>> replaces the stage-user --from-delete, keeps the same behavior
>>>>
>>>> $ user-stage <active_user>
>>>> this is stretch goal, nice to have, but I don't know how easy is to
>>>> implement this
>>>>
>>>> For better visualization, here is link to our board screen
>>>> https://pvoborni.fedorapeople.org/images/user-lifecycle.jpg
>>>>
>>>> Thierry, do you agree with this?
>>>> Martin^2
>>>
>>>
>>> Hello,
>>>
>>> I really like the idea (as well as the drawing) of having the same cli
>>> for both active/deleted user.
>>> About the exact verb 'user-stage', I am always bad at this exercise and
>>> it would be great to have Thomas ack on that.
>>>
>>> Just a question about the other verbs user-disable/user-enable. I know
>>> they are doing something different but do you think there is a risk of
>>> confusion for admin when he should do user-stage or user-disable ?
>>>
>>> thanks
>>> thierry
>>>
>>>
>>>
>>
>> Adding Tomas to the loop.
> Hello everyone,
>
>   I probably don't have all the information and perhaps cannot see all 
> possible side effects but on the handover session for this feature, I 
> was concerned that some commands did not match in GUI and in CLI 
> (restore, undel).
>
> Also, from the UX perspective, I thought it would be more friendly to 
> have symmetrical commands and not confuse users with two delete modes 
> as in "do you want to mock-delete the user or delete delete it?"
>
> Therefore, I proposed to have two simple pairs of commands, also 
> reflected in the UI:
>
> "add" and "delete"  - for just adding and completely removing users
>
> "retire" and "restore" - for just putting a user to or taking it out 
> of the user "archive"
>
Sounds good, but we will not change all commands.

> as for the "--from-delete" option , I think the proposed "user-stage" 
> overlaps a little with the existing "stageuser-*" commands. As the 
> command would move a user to the initial state of the lifecycle, be it 
> an active or retired user, I'd try to emphasize the fact by proposing 
> a similar but perhaps more cogent "user-restage".
>
> Do you think it would work?
How about case, when user was created via 'user-add',  then 
'user-restage' may confuse users, because that user hasn't been in stage 
area.

Martin^2
>
> Tomas
>
>