fix: do not skip properties on creating templates #5060
Conversation
| assert.Equal(t, template.Description, updated.Description) | ||
| assert.Equal(t, template.DisplayName, updated.DisplayName) | ||
| // Icon is removed, as the API considers it as "delete" request | ||
| assert.Equal(t, "", updated.Icon) |
There was a problem hiding this comment.
This is really unfortunate. If somebody runs coder templates edit <template-name>, they will automatically remove the icon. This is an outcome of not having a clear difference between delete icon and do not modify. Do you think that it's worth opening another issue?
EDIT:
Side issue - it isn't possible to clear the template description with UI. User can submit an empty description field, but the content is not deleted.
There was a problem hiding this comment.
Considering the behavior here:
Lines 481 to 496 in 4b3d211
I would definitely consider it a bug. It's inconsistent how fields are affected. A missing field (null) should produce no change, an empty string should remove it and a non empty string change it. Feel free to open an issue about this. 👍🏻
| assert.Equal(t, template.Description, updated.Description) | ||
| assert.Equal(t, template.DisplayName, updated.DisplayName) | ||
| // Icon is removed, as the API considers it as "delete" request | ||
| assert.Equal(t, "", updated.Icon) |
There was a problem hiding this comment.
Considering the behavior here:
Lines 481 to 496 in 4b3d211
I would definitely consider it a bug. It's inconsistent how fields are affected. A missing field (null) should produce no change, an empty string should remove it and a non empty string change it. Feel free to open an issue about this. 👍🏻
While working on #3321 I found that
POST /templatesrejects properties, even though the API contract expects it. This PR fixes the behavior.