Send unchanged write-only properties as adds on update #1395
Conversation
Write-only properties can't even be read internally within the CloudControl service so they must be included in PATCH requests to ensure the updated model validates. - Refactor where we unmarshal inputs to avoid duplicate work.
danielrbradley
added
the
impact/no-changelog-required
Does the PR have any schema changes?Looking good! No breaking changes found. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1395 +/- ##
==========================================
- Coverage 23.40% 23.39% -0.01%
==========================================
Files 25 25
Lines 4230 4236 +6
==========================================
+ Hits 990 991 +1
- Misses 3077 3082 +5
Partials 163 163 ☔ View full report in Codecov by Sentry. |
| newInputs, err := plugin.UnmarshalProperties(req.GetNews(), plugin.MarshalOptions{ | ||
| Label: fmt.Sprintf("%s.properties", label), | ||
| KeepUnknowns: true, | ||
| RejectAssets: true, |
There was a problem hiding this comment.
Not currently - Asset support is still a WIP PR which will change these settings everywhere.
| }, | ||
| }) | ||
|
|
||
| integration.ProgramTest(t, &test) |
There was a problem hiding this comment.
Does this need some more asserts? Or it used to panic or some such before the change?
There was a problem hiding this comment.
The update was a hard failure before this change.
|
A bit too much indirection and lack of familiarity for me to review this properly but I tried and added a few comments. |
Write-only properties can't even be read internally within the CloudControl service so they must be included in PATCH requests to ensure the updated model validates.
Fixes #906
Might also fix #1243 but there's no code to reproduce the issue.