From 7bd8d8e572956cf8a95ab0542782249bb0ce1f6d Mon Sep 17 00:00:00 2001 From: pikann22 Date: Wed, 8 Jul 2026 16:43:05 +0000 Subject: [PATCH 1/2] feat: enhance item update logic to handle optional assignee ID with explicit null support --- backend/checklists.go | 14 +++++- backend/items.go | 61 ++++++++++++++++++----- backend/plugin_test.go | 108 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 13 deletions(-) diff --git a/backend/checklists.go b/backend/checklists.go index e3281bb..f0e94aa 100644 --- a/backend/checklists.go +++ b/backend/checklists.go @@ -49,10 +49,20 @@ func (s *scanner) str(col string) string { if !ok || i >= len(s.row) || s.row[i] == nil { return "" } - if v, ok := s.row[i].(string); ok { + switch v := s.row[i].(type) { + case string: return v + case *string: + // updateItem re-inserts nullable columns as *string (nil = SQL NULL); + // a real driver dereferences this on the way in, but the row may + // still hold the pointer verbatim in tests, so unwrap it here too. + if v == nil { + return "" + } + return *v + default: + return fmt.Sprintf("%v", s.row[i]) } - return fmt.Sprintf("%v", s.row[i]) } func (s *scanner) strPtr(col string) *string { diff --git a/backend/items.go b/backend/items.go index 024c8fd..ceee1cb 100644 --- a/backend/items.go +++ b/backend/items.go @@ -1,9 +1,40 @@ package main import ( + "encoding/json" + plugin "github.com/Paca-AI/plugin-sdk-go" ) +// optionalString distinguishes three states for a JSON PATCH field: +// - key absent → Set=false (leave the stored value unchanged) +// - key = null → Set=true, Value=nil (explicitly clear the stored value) +// - key = "value" → Set=true, Value=non-nil (set the stored value) +// +// A plain *string cannot represent this: encoding/json leaves it nil both +// when the key is absent and when it is explicitly null, so a client asking +// to clear the field silently gets ignored instead. +type optionalString struct { + Set bool + Value *string +} + +// UnmarshalJSON implements json.Unmarshaler. It marks the field as Set and +// decodes the value, treating JSON null as a nil pointer. +func (o *optionalString) UnmarshalJSON(data []byte) error { + o.Set = true + if string(data) == "null" { + o.Value = nil + return nil + } + var s string + if err := json.Unmarshal(data, &s); err != nil { + return err + } + o.Value = &s + return nil +} + // ── Item route handlers ─────────────────────────────────────────────────────── // createItem handles POST /tasks/:taskId/checklists/:checklistId/items @@ -87,9 +118,9 @@ func (p *checklistPlugin) updateItem(req *plugin.Request, res *plugin.Response) } type body struct { - Title *string `json:"title"` - IsChecked *bool `json:"is_checked"` - AssigneeID *string `json:"assignee_id"` + Title *string `json:"title"` + IsChecked *bool `json:"is_checked"` + AssigneeID optionalString `json:"assignee_id"` } b, err := plugin.JSONBody[body](req) if err != nil { @@ -97,7 +128,7 @@ func (p *checklistPlugin) updateItem(req *plugin.Request, res *plugin.Response) return } - if b.Title == nil && b.IsChecked == nil && b.AssigneeID == nil { + if b.Title == nil && b.IsChecked == nil && !b.AssigneeID.Set { res.Error(400, "no fields to update") return } @@ -133,8 +164,8 @@ func (p *checklistPlugin) updateItem(req *plugin.Request, res *plugin.Response) if b.IsChecked != nil { updChecked = *b.IsChecked } - if b.AssigneeID != nil { - updAssignee = b.AssigneeID + if b.AssigneeID.Set { + updAssignee = b.AssigneeID.Value } // Simulate UPDATE as DELETE + re-INSERT. task_checklist_items has no child @@ -176,12 +207,20 @@ func (p *checklistPlugin) updateItem(req *plugin.Request, res *plugin.Response) changes = append(changes, map[string]any{"field": "is_checked", "old": oldChecked, "new": *b.IsChecked}) } oldAssignee := sc.strPtr("assignee_id") - if b.AssigneeID != nil && (oldAssignee == nil || *oldAssignee != *b.AssigneeID) { - var oldAssigneeActivity any - if oldAssignee != nil { - oldAssigneeActivity = *oldAssignee + if b.AssigneeID.Set { + newAssignee := b.AssigneeID.Value + assigneeChanged := (oldAssignee == nil) != (newAssignee == nil) || + (oldAssignee != nil && newAssignee != nil && *oldAssignee != *newAssignee) + if assigneeChanged { + var oldAssigneeActivity, newAssigneeActivity any + if oldAssignee != nil { + oldAssigneeActivity = *oldAssignee + } + if newAssignee != nil { + newAssigneeActivity = *newAssignee + } + changes = append(changes, map[string]any{"field": "assignee_id", "old": oldAssigneeActivity, "new": newAssigneeActivity}) } - changes = append(changes, map[string]any{"field": "assignee_id", "old": oldAssigneeActivity, "new": *b.AssigneeID}) } plugin.RecordActivity(taskID, projectID, req.Caller.UserID, "task.checklist_item.updated", map[string]any{"text": updTitle, "changes": changes, "_description": "updated checklist item: \"" + updTitle + "\""}) diff --git a/backend/plugin_test.go b/backend/plugin_test.go index a76b1bf..10a0e9d 100644 --- a/backend/plugin_test.go +++ b/backend/plugin_test.go @@ -306,6 +306,114 @@ func TestDeleteItem(t *testing.T) { } } +// TestUpdateItem_OmittedAssigneeUnchanged guards against regressing to +// unconditionally overwriting assignee_id on every PATCH, even when the +// request omits it. +func TestUpdateItem_OmittedAssigneeUnchanged(t *testing.T) { + tc := setupPlugin(t) + + createRes := tc.Call("POST", "/tasks/:taskId/checklists", + withPathParams(callerReq(), map[string]string{"taskId": testTaskID}). + WithJSONBody(map[string]string{"title": "CL"})) + var clEnv struct { + Data checklist `json:"data"` + } + _ = json.Unmarshal(createRes.Body, &clEnv) + clID := clEnv.Data.ID + + itemRes := tc.Call("POST", "/tasks/:taskId/checklists/:checklistId/items", + withPathParams(callerReq(), map[string]string{ + "taskId": testTaskID, + "checklistId": clID, + }).WithJSONBody(map[string]string{"title": "Step"})) + var itemEnv struct { + Data checklistItem `json:"data"` + } + _ = json.Unmarshal(itemRes.Body, &itemEnv) + itemID := itemEnv.Data.ID + + itemPath := map[string]string{"taskId": testTaskID, "checklistId": clID, "itemId": itemID} + + assignRes := tc.Call("PATCH", "/tasks/:taskId/checklists/:checklistId/items/:itemId", + withPathParams(callerReq(), itemPath).WithJSONBody(map[string]any{"assignee_id": "user-42"})) + if assignRes.StatusCode != 200 { + t.Fatalf("expected 200, got %d: %s", assignRes.StatusCode, assignRes.BodyString()) + } + var assignEnv struct { + Data checklistItem `json:"data"` + } + _ = json.Unmarshal(assignRes.Body, &assignEnv) + if assignEnv.Data.AssigneeID == nil || *assignEnv.Data.AssigneeID != "user-42" { + t.Fatalf("expected assignee_id=user-42, got %+v", assignEnv.Data.AssigneeID) + } + + // Patch only is_checked; assignee_id is absent from the body and must + // survive the update untouched. + patchRes := tc.Call("PATCH", "/tasks/:taskId/checklists/:checklistId/items/:itemId", + withPathParams(callerReq(), itemPath).WithJSONBody(map[string]any{"is_checked": true})) + if patchRes.StatusCode != 200 { + t.Fatalf("expected 200, got %d: %s", patchRes.StatusCode, patchRes.BodyString()) + } + var patchEnv struct { + Data checklistItem `json:"data"` + } + _ = json.Unmarshal(patchRes.Body, &patchEnv) + if !patchEnv.Data.IsChecked { + t.Fatal("expected is_checked=true") + } + if patchEnv.Data.AssigneeID == nil || *patchEnv.Data.AssigneeID != "user-42" { + t.Fatalf("expected assignee_id to remain user-42, got %+v", patchEnv.Data.AssigneeID) + } +} + +// TestUpdateItem_ExplicitNullClearsAssignee verifies the other half of the +// three-state contract: an explicit JSON null clears the assignee, matching +// the checklist MCP tool's documented "pass null to unassign" behavior. +func TestUpdateItem_ExplicitNullClearsAssignee(t *testing.T) { + tc := setupPlugin(t) + + createRes := tc.Call("POST", "/tasks/:taskId/checklists", + withPathParams(callerReq(), map[string]string{"taskId": testTaskID}). + WithJSONBody(map[string]string{"title": "CL"})) + var clEnv struct { + Data checklist `json:"data"` + } + _ = json.Unmarshal(createRes.Body, &clEnv) + clID := clEnv.Data.ID + + itemRes := tc.Call("POST", "/tasks/:taskId/checklists/:checklistId/items", + withPathParams(callerReq(), map[string]string{ + "taskId": testTaskID, + "checklistId": clID, + }).WithJSONBody(map[string]string{"title": "Step"})) + var itemEnv struct { + Data checklistItem `json:"data"` + } + _ = json.Unmarshal(itemRes.Body, &itemEnv) + itemID := itemEnv.Data.ID + + itemPath := map[string]string{"taskId": testTaskID, "checklistId": clID, "itemId": itemID} + + assignRes := tc.Call("PATCH", "/tasks/:taskId/checklists/:checklistId/items/:itemId", + withPathParams(callerReq(), itemPath).WithJSONBody(map[string]any{"assignee_id": "user-42"})) + if assignRes.StatusCode != 200 { + t.Fatalf("expected 200, got %d: %s", assignRes.StatusCode, assignRes.BodyString()) + } + + clearRes := tc.Call("PATCH", "/tasks/:taskId/checklists/:checklistId/items/:itemId", + withPathParams(callerReq(), itemPath).WithJSONBody(map[string]any{"assignee_id": nil})) + if clearRes.StatusCode != 200 { + t.Fatalf("expected 200, got %d: %s", clearRes.StatusCode, clearRes.BodyString()) + } + var clearEnv struct { + Data checklistItem `json:"data"` + } + _ = json.Unmarshal(clearRes.Body, &clearEnv) + if clearEnv.Data.AssigneeID != nil { + t.Fatalf("expected assignee_id to be cleared, got %q", *clearEnv.Data.AssigneeID) + } +} + // ── 404 guard tests ─────────────────────────────────────────────────────────── func TestListChecklists_UnknownTask(t *testing.T) { From 4577003803fe5723ab830be54c63c570d8b7fa01 Mon Sep 17 00:00:00 2001 From: pikann22 Date: Wed, 8 Jul 2026 16:43:31 +0000 Subject: [PATCH 2/2] chore: update version number to 0.2.7 in plugin.json --- plugin.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin.json b/plugin.json index 6d27147..805f213 100644 --- a/plugin.json +++ b/plugin.json @@ -2,7 +2,7 @@ "id": "com.paca.checklist", "displayName": "Checklist", "description": "Adds named checklists with checkable items to tasks.", - "version": "0.2.6", + "version": "0.2.7", "permissions": ["db.read", "db.write", "events.subscribe"], "backend": { "eventSubscriptions": ["task.deleted"],