search

unioslo / mreg-cli

Command Line Interface for Mreg
GNU General Public License v3.0
2 stars 7 forks source link

Fix `host set_comment` with empty comment #283

Closed pederhan closed 1 month ago

pederhan commented 1 month ago

This PR fixes passing in an empty string as the comment argument to host set_comment:

user@mreg.example.com> host set_comment myhost ""
ERROR: Patch failure! Tried to set comment to , but server returned None.

The error stemmed from the Host.comment field converting empty strings to None, which then subsequently caused revalidation to fail because our patch method expected to find an empty string in the comment field of the updated model (but got None).

This PR removes the nullability of the comment field, which matches the field on the server side.

pederhan commented 1 month ago

The problem[tm] is that users have made it very clear that they want to be able to "null" this field. I suppose they can set it to " ", but I'm not sure that... better? :)

Sending an empty string ("") works and is the only way to remove a comment through the API. Since the field on the model is not nullable, a PATCH request with the following JSON payload is the only way to unset a comment when sending JSON:

{"comment": ""}
terjekv commented 1 month ago

Ah. Good. Well, eh, good enough? :) Happy for merge.