[PATCH] 35 Fix for ticket 214 - Proper error message while adding duplicate user
Please review the patch which fixes the ticket 214. It requires Patch 34 which is also attached to be applied first. --Abhishek
On 12/7/2012 9:59 PM, Abhishek Koneru wrote:
Please review the patch which fixes the ticket 214. It requires Patch 34 which is also attached to be applied first.
Patch #34 is ACKed and pushed after rebase. I have some comments about patch #35: 1. The patch is adding a new message: CMS_USRGRP_USER_ADD_FAILED_2=An user already exits with the user id \"{0}\". In this project it looks like the convention is that the number at the end of the message name indicates the number of parameters, which in this case should be 1. 2. In UsrGrpAdminServlet.java I'm not sure the failure there is caused by the user already exists. It could be that the user ID is already specified in the request but there's another error. 3. In UGSubsystem.java instead of checking whether the user already exists using getUser(), you could try to check the LDAP return code of the add() operation. This way we don't need to do any extra checking. -- Endi S. Dewata
On 01/04/13 07:17, Endi Sukma Dewata wrote:
On 12/7/2012 9:59 PM, Abhishek Koneru wrote:
Please review the patch which fixes the ticket 214. It requires Patch 34 which is also attached to be applied first.
Patch #34 is ACKed and pushed after rebase.
I have some comments about patch #35:
1. The patch is adding a new message:
CMS_USRGRP_USER_ADD_FAILED_2=An user already exits with the user id \"{0}\".
In this project it looks like the convention is that the number at the end of the message name indicates the number of parameters, which in this case should be 1.
Also, fix the typos "An user" --> "A user" and "exits" --> "exists".
2. In UsrGrpAdminServlet.java I'm not sure the failure there is caused by the user already exists. It could be that the user ID is already specified in the request but there's another error.
3. In UGSubsystem.java instead of checking whether the user already exists using getUser(), you could try to check the LDAP return code of the add() operation. This way we don't need to do any extra checking.
participants (3)
-
Abhishek Koneru -
Endi Sukma Dewata -
Matthew Harmsen