refactor(custom_fields): per-key endpoints, drop from PUT /devices/:uid#6286
Conversation
|
Claude finished @gustavosbarreto's task in 2m 42s —— View job Code Review CompleteReviewed 21 files across code quality, security, testing, Go/TypeScript patterns, and architecture. 2 inline issues posted (2 high). Additional notes
To request another review round, comment |
gustavosbarreto
force-pushed
the
refactor/custom-fields-per-key-endpoints
branch
from
bf5bde1 to
bcd6952
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
gustavosbarreto
force-pushed
the
refactor/custom-fields-per-key-endpoints
branch
from
bcd6952 to
cd93243
Compare
Replace the single CustomFields field on DeviceUpdate with two dedicated requests, DeviceSetCustomField and DeviceDeleteCustomField, each parameterized by the field key. Add DeviceCustomFieldUpdate permission so role gates can distinguish custom_fields edits from device rename. Fix the validator order on DeviceUpdate.Name so omitempty short-circuits on empty input.
gustavosbarreto
force-pushed
the
refactor/custom-fields-per-key-endpoints
branch
from
cd93243 to
da156c4
Compare
|
/review |
Code Review
|
Adds PUT /devices/:uid/custom_fields/:key and DELETE /devices/:uid/custom_fields/:key for atomic per-key edits. Service enforces the per-device entry cap (defined by maxCustomFieldsPerDevice) when adding a new key. PG store uses jsonb_set / jsonb subtraction; Mongo store uses dot-notation $set / $unset. Both are atomic, removing the read-modify-write race that the previous bulk replace had. UpdateDevice now handles only rename, returning early when the name is empty or unchanged. The DeviceConflicts and DeviceUpdate calls only run when the name actually changes. Drops the CustomFields branch. Adds ErrDeviceCustomFieldLimitReached for the cap.
Adds /api/devices/{uid}/custom_fields/{key} with PUT and DELETE. PUT requires
{value} body; both responses are 200 only. Removes custom_fields and the
required:[name] from PUT /api/devices/{uid}, which is now strictly a rename.
Replaces useUpdateDeviceCustomFields with useSetDeviceCustomField and useDeleteDeviceCustomField. CustomFieldsSection now PUTs a single key on add and DELETEs a single key on remove, dropping the bulk map replace that forced sending name in the body. Gates the section on the new device:customField:update permission.
gustavosbarreto
force-pushed
the
refactor/custom-fields-per-key-endpoints
branch
from
da156c4 to
3c5e1ed
Compare
|
/review |
|
Claude finished @gustavosbarreto's task in 10m 1s —— View job Code Review CompleteReviewed 21 files across code quality, security, testing, Go/TypeScript patterns, and architecture — no issues found. The code looks good as-is. Notes from this round
To request another review round, comment |
|
Cloud mock regen tracked in shellhub-io/cloud#2360. |
1 participant
Summary
Move custom_fields off
PUT /devices/:uidand into dedicated per-key endpoints, mirroring the tag attach/detach pattern.PR #6248 added
custom_fieldsas a*map[string]stringfield onDeviceUpdate, sharing the body with rename. That forced three smells:nil= no change,{}= clear,{k:v}= replace) instead of explicit operations.Name(device_name,omitempty) made any payload that omittednamefail with 400. The UI worked around it by sendingname: "", which silently reverted external renames.After this change:
PUT /devices/:uid/custom_fields/:keywith{value}andDELETE /devices/:uid/custom_fields/:keyare atomic per-key. Storage stays as the same JSONB column, soGET /devicesand thecustom_fields containsfilter are unchanged.PUT /devices/:uidis rename-only. Returns early when name is empty or unchanged. The validator order is corrected.DeviceCustomFieldUpdatepermission gates the new endpoints (Operator and up).UI now calls
useSetDeviceCustomField/useDeleteDeviceCustomFieldinstead of replaying the full map.Decisions
PUT(notPOST) for set: idempotent set/upsert of the resource at the URL.devices.custom_fieldsJSONB column is untouched; only the write paths change.custom_fields containskeeps working over all values; per-key filtering and discovery are out of scope.