[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #783: GUACAMOLE-1728: Set User timezone attribute to empty when it's null so that we can save it.

[GitBox <gi...@apache.org>]

mike-jumper commented on code in PR #783:
URL: https://github.com/apache/guacamole-client/pull/783#discussion_r1058602103


##########
guacamole/src/main/java/org/apache/guacamole/rest/user/UserResource.java:
##########
@@ -150,6 +150,10 @@ public void updateObject(APIUser modifiedObject) throws GuacamoleException {
             throw e;
         }
 
+        /* Force updateObject to save the timezone when it is empty */
+        if (modifiedObject.getAttributes().get("timezone") == null)
+            modifiedObject.getAttributes().put("timezone", "");
+

Review Comment:
   Yeah, I agree this shouldn't be done this way:
   1. Within Guacamole object attributes, there is a distinction between `null` (unset) and `""` (set to empty string). They are both distinct, legitimate values. Setting to `null` typically is used to delete an attribute.
   2. The generic REST endpoint for attributes should definitely not be aware of the particular needs of any one attribute. It should just shuttle the data around.
   
   I think we need to establish exactly why this attribute doesn't get cleared out and correct _that_. It may be that the JS for the timezone field isn't properly submitting its value, or it may be that the database isn't properly handling those received attribute values.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org