Skip to content

Commit a640309

Browse files
committed
fix: handle missing TaskPage key in workflow activity mutation
When setting PAGE on a workflow activity where the TaskPage BSON key is absent (not just nil), dSet silently failed because it only updates existing keys. Added replaceActivity helper that appends the key to the activity document and replaces it in the BSON tree by match. Three cases now handled correctly: - TaskPage exists with value: update Page field in place - TaskPage exists with nil value: replace via dSet - TaskPage absent: append key, replace activity in tree
1 parent a6b0555 commit a640309

2 files changed

Lines changed: 68 additions & 13 deletions

File tree

mdl/backend/mpr/workflow_mutator.go

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,21 @@ func (m *mprWorkflowMutator) SetActivityProperty(activityRef string, atPos int,
162162
case "PAGE":
163163
taskPage := dGetDoc(actDoc, "TaskPage")
164164
if taskPage != nil {
165+
// TaskPage exists and has a value — update the Page field in place.
165166
dSet(taskPage, "Page", value)
167+
return nil
168+
}
169+
pageRef := bson.D{
170+
{Key: "$ID", Value: bsonutil.NewIDBsonBinary()},
171+
{Key: "$Type", Value: "Workflows$PageReference"},
172+
{Key: "Page", Value: value},
173+
}
174+
if dSet(actDoc, "TaskPage", pageRef) {
175+
// TaskPage key exists (nil value) — replaced in place via dSet.
166176
} else {
167-
pageRef := bson.D{
168-
{Key: "$ID", Value: bsonutil.NewIDBsonBinary()},
169-
{Key: "$Type", Value: "Workflows$PageReference"},
170-
{Key: "Page", Value: value},
171-
}
172-
dSet(actDoc, "TaskPage", pageRef)
177+
// TaskPage key absent — append to activity and replace in BSON tree.
178+
actDoc = append(actDoc, bson.E{Key: "TaskPage", Value: pageRef})
179+
m.replaceActivity(actDoc)
173180
}
174181
return nil
175182

@@ -555,6 +562,53 @@ func (m *mprWorkflowMutator) Save() error {
555562
// Internal helpers — activity search
556563
// ---------------------------------------------------------------------------
557564

565+
// replaceActivity replaces an activity document in the workflow's BSON tree
566+
// by matching on $ID. This is needed when appending new keys to an activity
567+
// document, because the slice header returned by findActivityByCaption cannot
568+
// propagate appends back to the parent bson.A.
569+
func (m *mprWorkflowMutator) replaceActivity(updated bson.D) {
570+
actID := extractBinaryIDFromDoc(dGet(updated, "$ID"))
571+
if actID == "" {
572+
return
573+
}
574+
flow := dGetDoc(m.rawData, "Flow")
575+
if flow == nil {
576+
return
577+
}
578+
replaceActivityRecursive(flow, actID, updated)
579+
}
580+
581+
func replaceActivityRecursive(flow bson.D, actID string, updated bson.D) bool {
582+
activitiesVal := dGet(flow, "Activities")
583+
arr := toBsonA(activitiesVal)
584+
if len(arr) == 0 {
585+
return false
586+
}
587+
// Skip the int32 type marker at index 0 if present.
588+
start := 0
589+
if _, ok := arr[0].(int32); ok {
590+
start = 1
591+
} else if _, ok := arr[0].(int); ok {
592+
start = 1
593+
}
594+
for i := start; i < len(arr); i++ {
595+
actDoc, ok := arr[i].(bson.D)
596+
if !ok {
597+
continue
598+
}
599+
if extractBinaryIDFromDoc(dGet(actDoc, "$ID")) == actID {
600+
arr[i] = updated
601+
return true
602+
}
603+
for _, nestedFlow := range getNestedFlows(actDoc) {
604+
if replaceActivityRecursive(nestedFlow, actID, updated) {
605+
return true
606+
}
607+
}
608+
}
609+
return false
610+
}
611+
558612
// findActivityByCaption searches the workflow for an activity matching caption.
559613
func (m *mprWorkflowMutator) findActivityByCaption(caption string, atPosition int) (bson.D, error) {
560614
flow := dGetDoc(m.rawData, "Flow")

mdl/backend/mpr/workflow_mutator_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,8 +1136,7 @@ func TestWorkflowMutator_InsertBoundaryEvent_NoDelay(t *testing.T) {
11361136
// ---------------------------------------------------------------------------
11371137

11381138
func TestWorkflowMutator_SetActivityProperty_Page_New(t *testing.T) {
1139-
// Note: When TaskPage key doesn't pre-exist in BSON, dSet silently fails.
1140-
// The key must be present (even as nil) for PAGE to work on a new activity.
1139+
// TaskPage key present with nil value — should be replaced with a new PageReference.
11411140
act := makeWfActivity("Workflows$UserTask", "Review", "task1")
11421141
act = append(act, bson.E{Key: "TaskPage", Value: nil})
11431142
m := newMutator(makeWorkflowDoc(act))
@@ -1157,21 +1156,23 @@ func TestWorkflowMutator_SetActivityProperty_Page_New(t *testing.T) {
11571156
}
11581157

11591158
func TestWorkflowMutator_SetActivityProperty_Page_MissingKey(t *testing.T) {
1160-
// BUG: dSet silently fails when TaskPage key is absent — pageRef is lost.
1159+
// Regression test: dSet silently failed when TaskPage key was absent.
1160+
// Fixed by appending the key to the activity and replacing it in the BSON tree.
11611161
act := makeWfActivity("Workflows$UserTask", "Review", "task1")
11621162
// No TaskPage field at all
11631163
m := newMutator(makeWorkflowDoc(act))
11641164

1165-
// No error returned, but the set is silently lost
11661165
if err := m.SetActivityProperty("Review", 0, "PAGE", "MyModule.TaskPage"); err != nil {
11671166
t.Fatalf("SetActivityProperty PAGE failed: %v", err)
11681167
}
11691168

11701169
actDoc, _ := m.findActivityByCaption("Review", 0)
11711170
taskPage := dGetDoc(actDoc, "TaskPage")
1172-
// This documents the bug: TaskPage is nil because dSet can't create new keys
1173-
if taskPage != nil {
1174-
t.Log("BUG FIXED: TaskPage is now set even when key was absent")
1171+
if taskPage == nil {
1172+
t.Fatal("TaskPage should be set even when key was absent")
1173+
}
1174+
if got := dGetString(taskPage, "Page"); got != "MyModule.TaskPage" {
1175+
t.Errorf("Page = %q, want MyModule.TaskPage", got)
11751176
}
11761177
}
11771178

0 commit comments

Comments
 (0)