<html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 09/01/2014 01:08 PM, Petr Viktorin wrote: </div> <blockquote cite="mid:54045399.3030404@redhat.com" type="cite">On 08/08/2014 03:54 PM, thierry bordaz wrote: <blockquote type="cite">Hi, The attached patch is related to 'User Life Cycle' (<a class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/3813">https://fedorahosted.org/freeipa/ticket/3813</a>) It creates a stageuser plugin with a first function stageuser-add. Stage user entries are provisioned under 'cn=staged users,cn=accounts,cn=provisioning,SUFFIX'. Thanks thierry </blockquote> Avoid `from ipalib.plugins.baseldap import *` in new code; instead import the module itself and use e.g. `baseldap.LDAPObject`. The stageuser help (docstring) is copied from the user plugin, and discusses things like account lockout and disabling users. It should rather explain what stageuser itself does. (And I don't very much like the Note about the interface being badly designed...) Also decide if the docs should call it "staged user" or "stage user" or "stageuser". </blockquote> Hello Petr, <blockquote>Thanks for your review. I will rewrite the docstring to be only staged user related and I will adopt 'stage user' and 'stageuser'. About the interface, that is correct that I was not able to find a good solution. I need to add a 'stageuser' I use 'stageuser-add' and '--first' and '--last' are required. Now when a user got deleted ('user-del') and is later move to 'stage user' it also use the command line 'stageuser-add'. At this time the delete user is know/found by it 'user login'/uid. So '--first' and '--last' are not required to find it (can be used to check givenname/sn). Now I do not expect that 'stageuser-add' will be frequently used to move a Delete user to Stage user, so it is not a so painful constraint. </blockquote> <blockquote cite="mid:54045399.3030404@redhat.com" type="cite"> A lot of the code is copied and pasted over from the users plugin. Don't do that. Either import things (e.g. validate_nsaccountlock) from the users plugin, or move the reused code into a shared module. For the `user` object, since so much is the same, it might be best to create a common base class for user and stageuser; and similarly for the Command plugins. </blockquote> <blockquote>I agree that user and stageuser have a lot of code in common. So it would be beneficial to have a common base class. Is it ok to put this in a file under freeipa/ipalib ? About the name of this class, what about 'accounts' or 'user_accounts' ? What do you mean by 'similarly for Command plugins' ?. If I create, for example, a freeipa/ipalib/accounts.py containing all the common code for 'user' and 'stageuser'. Then import it into freeipa/ipalib/plugins/user and freeipa/ipalib/plugins/stageuser I believe it will refactore the 'Command plugins'. </blockquote> <blockquote cite="mid:54045399.3030404@redhat.com" type="cite"> The default permissions need different names, and you don't need another copy of the 'non_object' ones. Also, run the makeaci script. </blockquote> Ok. I will update the names. 'non_object' is not clear to me, what does this exactly mean when a managed_permission has 'non_object' True. In update_managed_permissions.py it is said that if True, it target a defaults value. But on the other side, the managed permissions also define 'ipapermtarget' (like ipauser or UPG) many thanks Petr thierry <blockquote> </blockquote> </body> </html>