Re: [Spacewalk-devel] [PATCH] Errors with unrequired field "Prefix"

2012-11-21 Thread Tomas Lestach
On Tuesday 20 of November 2012 13:53:14 Johannes Renner wrote:
> On 11/15/2012 03:57 PM, Johannes Renner wrote:
> > Hey again,
> > 
> > We found some errors with the prefix field of a user's properties. First,
> > when creating a new user with the UI, it is not possible to leave the
> > prefix empty, it complains about a non-valid value (even though an
> > 'empty' value is actually contained in the prefixes list). Further, when
> > editing a user's details it is not possible to empty the prefix. This is
> > especially annoying if you used e.g. spacecmd to create a user without a
> > prefix. It is then later on not even possible to edit and save such a
> > user without changing the prefix. I reproduced these issues reliably on
> > Oracle as well as on PG.
> > 
> > The attached patch fixes the problem by setting the apparently correct
> > empty value of " ", just in case it is empty (== ""). The select in the
> > form might even return it correctly as " ", but looks like it is maybe
> > trimmed away somewhere.. This works for me now, but I tested the fix on
> > PG only.
> > 
> > Thanks,
> > Johannes
> 
> Oh sorry, I just found out that my first patch had a subtle bug.. creating
> users with an empty prefix was still failing, even with the patch deployed.
> 
> Attached is now a fixed version that should really make it work.

Committed as: 12ee8d60a358cd3f435fdb88c61227e3d61f6b6e

Thank you!
Regards,
-- 
Tomas Lestach
RHN Satellite Engineering

> 
> Regards,
> Johannes

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] [PATCH] Errors with unrequired field "Prefix"

2012-11-20 Thread Johannes Renner
On 11/15/2012 03:57 PM, Johannes Renner wrote:
> Hey again,
> 
> We found some errors with the prefix field of a user's properties. First, 
> when creating
> a new user with the UI, it is not possible to leave the prefix empty, it 
> complains about
> a non-valid value (even though an 'empty' value is actually contained in the 
> prefixes
> list). Further, when editing a user's details it is not possible to empty the 
> prefix.
> This is especially annoying if you used e.g. spacecmd to create a user 
> without a prefix.
> It is then later on not even possible to edit and save such a user without 
> changing the
> prefix. I reproduced these issues reliably on Oracle as well as on PG.
> 
> The attached patch fixes the problem by setting the apparently correct empty 
> value of " ",
> just in case it is empty (== ""). The select in the form might even return it 
> correctly
> as " ", but looks like it is maybe trimmed away somewhere.. This works for me 
> now, but
> I tested the fix on PG only.
> 
> Thanks,
> Johannes

Oh sorry, I just found out that my first patch had a subtle bug.. creating 
users with an
empty prefix was still failing, even with the patch deployed.

Attached is now a fixed version that should really make it work.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
>From a34cdc50a0369e848f4aa6001a660fb7cad4ab84 Mon Sep 17 00:00:00 2001
From: Johannes Renner 
Date: Tue, 20 Nov 2012 13:35:04 +0100
Subject: [PATCH] Fix errors with unrequired field 'Prefix'

---
 .../redhat/rhn/frontend/action/user/UserEditActionHelper.java  |3 ++-
 .../src/com/redhat/rhn/manager/user/CreateUserCommand.java |8 ++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
index e7726ad..3056834 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
@@ -67,7 +67,8 @@ public abstract class UserEditActionHelper extends RhnAction {
 targetUser.setFirstNames((String)form.get("firstNames"));
 targetUser.setLastName((String)form.get("lastName"));
 targetUser.setTitle((String)form.get("title"));
-targetUser.setPrefix((String)form.get("prefix"));
+String prefix = (String)form.get("prefix");
+targetUser.setPrefix(prefix.isEmpty() ? " " : prefix);
 // Update PAM Authentication attribute
 updatePamAttribute(loggedInUser, targetUser, form);
 }
diff --git a/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java b/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java
index fa16d7c..c0097a0 100644
--- a/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java
+++ b/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java
@@ -224,12 +224,16 @@ public class CreateUserCommand {
  * errors list.
  */
 private void validatePrefix() {
-if (user.getPrefix() != null) {
+String prefix = user.getPrefix();
+if (prefix != null) {
 // Make sure whether prefix is valid, if it is set
 SortedSet validPrefixes = LocalizationService.getInstance().availablePrefixes();
+if (prefix.isEmpty()) {
+user.setPrefix(" ");
+}
 if (!validPrefixes.contains(user.getPrefix())) {
 errors.add(new ValidatorError("error.user_invalid_prefix",
-   user.getPrefix(), validPrefixes.toString()));
+   prefix, validPrefixes.toString()));
 }
 }
 }
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Errors with unrequired field "Prefix"

2012-11-15 Thread Johannes Renner
Hey again,

We found some errors with the prefix field of a user's properties. First, when 
creating
a new user with the UI, it is not possible to leave the prefix empty, it 
complains about
a non-valid value (even though an 'empty' value is actually contained in the 
prefixes
list). Further, when editing a user's details it is not possible to empty the 
prefix.
This is especially annoying if you used e.g. spacecmd to create a user without 
a prefix.
It is then later on not even possible to edit and save such a user without 
changing the
prefix. I reproduced these issues reliably on Oracle as well as on PG.

The attached patch fixes the problem by setting the apparently correct empty 
value of " ",
just in case it is empty (== ""). The select in the form might even return it 
correctly
as " ", but looks like it is maybe trimmed away somewhere.. This works for me 
now, but
I tested the fix on PG only.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
>From 7e095b96cc5d104b76cb0121ed8dc89e62ce50eb Mon Sep 17 00:00:00 2001
From: Johannes Renner 
Date: Thu, 15 Nov 2012 15:21:55 +0100
Subject: [PATCH] Fix errors with unrequired field 'Prefix'

---
 .../rhn/frontend/action/user/UserEditActionHelper.java   |3 ++-
 .../src/com/redhat/rhn/manager/user/CreateUserCommand.java   |   10 +++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
index e7726ad..3056834 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
@@ -67,7 +67,8 @@ public abstract class UserEditActionHelper extends RhnAction {
 targetUser.setFirstNames((String)form.get("firstNames"));
 targetUser.setLastName((String)form.get("lastName"));
 targetUser.setTitle((String)form.get("title"));
-targetUser.setPrefix((String)form.get("prefix"));
+String prefix = (String)form.get("prefix");
+targetUser.setPrefix(prefix.isEmpty() ? " " : prefix);
 // Update PAM Authentication attribute
 updatePamAttribute(loggedInUser, targetUser, form);
 }
diff --git a/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java b/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java
index fa16d7c..9930aad 100644
--- a/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java
+++ b/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java
@@ -224,12 +224,16 @@ public class CreateUserCommand {
  * errors list.
  */
 private void validatePrefix() {
-if (user.getPrefix() != null) {
+String prefix = user.getPrefix();
+if (prefix != null) {
 // Make sure whether prefix is valid, if it is set
 SortedSet validPrefixes = LocalizationService.getInstance().availablePrefixes();
-if (!validPrefixes.contains(user.getPrefix())) {
+if (prefix.isEmpty()) {
+user.setPrefix(" ");
+}
+if (!validPrefixes.contains(prefix)) {
 errors.add(new ValidatorError("error.user_invalid_prefix",
-   user.getPrefix(), validPrefixes.toString()));
+   prefix, validPrefixes.toString()));
 }
 }
 }
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel