Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions client/playbook.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,20 @@ type ChecklistItem struct {
LastSkipped int64 `json:"delete_at"`
DueDate int64 `json:"due_date"`
TaskActions []TaskAction `json:"task_actions"`
Requirements []TaskRequirement `json:"requirements"`
ConditionID string `json:"condition_id"`
ConditionAction string `json:"condition_action"`
ConditionReason string `json:"condition_reason"`
UpdateAt int64 `json:"update_at"`
}

// TaskRequirement is a labeled field that must be completed when checking off a task.
type TaskRequirement struct {
ID string `json:"id"`
Label string `json:"label"`
Value string `json:"value"`
}

// TaskAction represents a task action in an item
type TaskAction struct {
Trigger TriggerAction `json:"trigger"`
Expand Down
1 change: 1 addition & 0 deletions client/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

type GlobalSettings struct {
EnableExperimentalFeatures bool `json:"enable_experimental_features"`
EnableTaskRequirements bool `json:"enable_task_requirements"`
}

// SettingsService handles communication with the settings related methods.
Expand Down
9 changes: 9 additions & 0 deletions plugin.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@
"display_name": "Expose Playbooks MCP tools externally (experimental)",
"help_text": "Requires Enable Experimental Features to be enabled. When enabled, allows the Agents plugin to expose Playbooks MCP tools through its external MCP endpoint. The Agents plugin admin settings must also allow and enable the server/tools.",
"default": false
},
{
"key": "BetaFeatures",
"type": "custom",
"display_name": "Beta Features",
"help_text": "Opt-in beta features. Expand the section to enable individual features.",
"default": {
"task_requirements": false
}
}
]
}
Expand Down
5 changes: 3 additions & 2 deletions server/api/graphql_playbook.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,9 @@ type UpdateChecklistItem struct {
Description string `json:"description"`
LastSkipped float64 `json:"delete_at"`
DueDate float64 `json:"due_date"`
TaskActions *[]app.TaskAction `json:"task_actions"`
ConditionID string `json:"condition_id"`
TaskActions *[]app.TaskAction `json:"task_actions"`
ConditionID string `json:"condition_id"`
Requirements *[]app.TaskRequirement `json:"requirements"`
}

func (ci *UpdateChecklistItem) GetAssigneeID() string {
Expand Down
10 changes: 8 additions & 2 deletions server/api/playbook_runs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1464,7 +1464,8 @@ func (h *PlaybookRunHandler) itemSetState(c *Context, w http.ResponseWriter, r *
userID := r.Header.Get("Mattermost-User-ID")

var params struct {
NewState string `json:"new_state"`
NewState string `json:"new_state"`
RequirementValues map[string]string `json:"requirement_values"`
}
if err := json.NewDecoder(r.Body).Decode(&params); err != nil {
h.HandleErrorWithCode(w, c.logger, http.StatusBadRequest, "failed to unmarshal", err)
Expand All @@ -1476,7 +1477,12 @@ func (h *PlaybookRunHandler) itemSetState(c *Context, w http.ResponseWriter, r *
return
}

if err := h.playbookRunService.ModifyCheckedState(id, userID, params.NewState, checklistNum, itemNum); err != nil {
var opts []app.ModifyCheckedStateOptions
if params.RequirementValues != nil {
opts = append(opts, app.ModifyCheckedStateOptions{RequirementValues: params.RequirementValues})
}

if err := h.playbookRunService.ModifyCheckedState(id, userID, params.NewState, checklistNum, itemNum, opts...); err != nil {
h.HandleError(w, c.logger, err)
return
}
Expand Down
14 changes: 14 additions & 0 deletions server/api/schema.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ input ChecklistItemUpdates {
dueDate: Float!
taskActions: [TaskActionUpdates!]
conditionID: String!
requirements: [TaskRequirementUpdates!]
}

input TaskRequirementUpdates {
id: String!
label: String!
value: String!
}

input TaskActionUpdates {
Expand Down Expand Up @@ -214,6 +221,13 @@ type ChecklistItem {
conditionID: String!
conditionAction: String!
conditionReason: String!
requirements: [TaskRequirement!]!
}

type TaskRequirement {
id: String!
label: String!
value: String!
}

type TaskAction {
Expand Down
1 change: 1 addition & 0 deletions server/api/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func NewSettingsHandler(router *mux.Router, api *pluginapi.Client, configService
func (h *SettingsHandler) getSettings(w http.ResponseWriter, r *http.Request) {
settings := client.GlobalSettings{
EnableExperimentalFeatures: h.config.IsExperimentalFeaturesEnabled(),
EnableTaskRequirements: h.config.IsTaskRequirementsEnabled(),
}

ReturnJSON(w, &settings, http.StatusOK)
Expand Down
2 changes: 1 addition & 1 deletion server/app/permissions_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ func (s *stubRunService) IsOwner(string, string) bool {
func (s *stubRunService) ChangeOwner(string, string, string) error {
panic("stubRunService: ChangeOwner not implemented")
}
func (s *stubRunService) ModifyCheckedState(string, string, string, int, int) error {
func (s *stubRunService) ModifyCheckedState(string, string, string, int, int, ...ModifyCheckedStateOptions) error {
panic("stubRunService: ModifyCheckedState not implemented")
}
func (s *stubRunService) ToggleCheckedState(string, string, int, int) error {
Expand Down
10 changes: 10 additions & 0 deletions server/app/playbook.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,9 @@ type ChecklistItem struct {
// TaskActions is an array of all the task actions associated with this task.
TaskActions []TaskAction `json:"task_actions" export:"-"`

// Requirements are fields that must be filled when checking off this task in a run.
Requirements []TaskRequirement `json:"requirements" export:"requirements"`

// UpdateAt is when this checklist item was last modified
UpdateAt int64 `json:"update_at" export:"-"`

Expand All @@ -355,6 +358,13 @@ type ChecklistItem struct {
ConditionReason string `json:"condition_reason" export:"-"`
}

// TaskRequirement is a labeled field that must be completed when checking off a task.
type TaskRequirement struct {
ID string `json:"id" export:"id"`
Label string `json:"label" export:"label"`
Value string `json:"value" export:"-"`
}

func (ci *ChecklistItem) GetAssigneeID() string {
return ci.AssigneeID
}
Expand Down
14 changes: 12 additions & 2 deletions server/app/playbook_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,9 @@ func GetChecklistItemUpdates(previous, current []ChecklistItem) ItemChanges {
if !reflect.DeepEqual(prev.TaskActions, item.TaskActions) {
fields["task_actions"] = item.TaskActions
}
if !reflect.DeepEqual(prev.Requirements, item.Requirements) {
fields["requirements"] = item.Requirements
}
if prev.UpdateAt != item.UpdateAt {
fields["update_at"] = item.UpdateAt
}
Expand Down Expand Up @@ -1254,6 +1257,12 @@ const (
TriggerTypeStatusUpdatePosted = "status_update_posted"
)

// ModifyCheckedStateOptions holds optional parameters for ModifyCheckedState.
type ModifyCheckedStateOptions struct {
// RequirementValues maps requirement IDs to the values filled in when checking off a task.
RequirementValues map[string]string
}

// PlaybookRunService is the playbook run service interface.
type PlaybookRunService interface {
// GetPlaybookRuns returns filtered playbook runs and the total count before paging.
Expand Down Expand Up @@ -1321,8 +1330,9 @@ type PlaybookRunService interface {
ChangeOwner(playbookRunID string, userID string, ownerID string) error

// ModifyCheckedState modifies the state of the specified checklist item
// Idempotent, will not perform any actions if the checklist item is already in the specified state
ModifyCheckedState(playbookRunID, userID, newState string, checklistNumber int, itemNumber int) error
// Idempotent, will not perform any actions if the checklist item is already in the specified state.
// Optional opts may include requirement values to apply when checking off a task.
ModifyCheckedState(playbookRunID, userID, newState string, checklistNumber int, itemNumber int, opts ...ModifyCheckedStateOptions) error

// ToggleCheckedState checks or unchecks the specified checklist item
ToggleCheckedState(playbookRunID, userID string, checklistNumber, itemNumber int) error
Expand Down
82 changes: 61 additions & 21 deletions server/app/playbook_run_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2265,8 +2265,10 @@ func (s *PlaybookRunServiceImpl) ChangeOwner(playbookRunID, userID, ownerID stri
}

// ModifyCheckedState checks or unchecks the specified checklist item. Idempotent, will not perform
// any action if the checklist item is already in the given checked state
func (s *PlaybookRunServiceImpl) ModifyCheckedState(playbookRunID, userID, newState string, checklistNumber, itemNumber int) error {
// any action if the checklist item is already in the given checked state.
// When checking off a task that has requirements, opts may supply RequirementValues; all
// requirements must have non-empty values or the call fails.
func (s *PlaybookRunServiceImpl) ModifyCheckedState(playbookRunID, userID, newState string, checklistNumber, itemNumber int, opts ...ModifyCheckedStateOptions) error {
auditRec := plugin.MakeAuditRecord("modifyChecklistItemState", model.AuditStatusFail)
defer s.api.LogAuditRec(auditRec)

Expand Down Expand Up @@ -2302,11 +2304,46 @@ func (s *PlaybookRunServiceImpl) ModifyCheckedState(playbookRunID, userID, newSt
originalRun = playbookRunToModify.Clone()
}

if newState == itemToCheck.State {
var requirementValues map[string]string
if len(opts) > 0 {
requirementValues = opts[0].RequirementValues
}

wasClosed := itemToCheck.State == ChecklistItemStateClosed

requirementsChanged := false
if len(itemToCheck.Requirements) > 0 && requirementValues != nil {
reqs := make([]TaskRequirement, len(itemToCheck.Requirements))
copy(reqs, itemToCheck.Requirements)
itemToCheck.Requirements = reqs
for i := range itemToCheck.Requirements {
if val, ok := requirementValues[itemToCheck.Requirements[i].ID]; ok {
if itemToCheck.Requirements[i].Value != val {
itemToCheck.Requirements[i].Value = val
requirementsChanged = true
}
}
}
}

// Only require all fields when checking off while beta features are enabled.
// Saving values alone may leave some fields empty.
if newState == ChecklistItemStateClosed && !wasClosed && len(itemToCheck.Requirements) > 0 && s.configService.IsTaskRequirementsEnabled() {
for _, req := range itemToCheck.Requirements {
if strings.TrimSpace(req.Value) == "" {
return errors.Wrap(ErrMalformedPlaybookRun, "all task requirements must be filled before checking off")
}
}
}

if newState == itemToCheck.State && !requirementsChanged {
auditRec.Success()
return nil
}

stateChanged := newState != itemToCheck.State
timestamp := model.GetMillis()

details := Details{
Action: "check",
Task: stripmd.Strip(itemToCheck.Title),
Expand All @@ -2326,9 +2363,10 @@ func (s *PlaybookRunServiceImpl) ModifyCheckedState(playbookRunID, userID, newSt
modifyMessage = fmt.Sprintf("restored checklist item **%v**", stripmd.Strip(itemToCheck.Title))
}

itemToCheck.State = newState
timestamp := model.GetMillis()
itemToCheck.StateModified = timestamp
if stateChanged {
itemToCheck.State = newState
itemToCheck.StateModified = timestamp
}
updateChecklistAndItemTimestamp(&playbookRunToModify.Checklists[checklistNumber], &itemToCheck, timestamp)
playbookRunToModify.Checklists[checklistNumber].Items[itemNumber] = itemToCheck

Expand All @@ -2337,23 +2375,25 @@ func (s *PlaybookRunServiceImpl) ModifyCheckedState(playbookRunID, userID, newSt
return errors.Wrapf(err, "failed to update playbook run, is now in inconsistent state")
}

detailsJSON, err := json.Marshal(details)
if err != nil {
return errors.Wrap(err, "failed to encode timeline event details")
}
if stateChanged {
detailsJSON, err := json.Marshal(details)
if err != nil {
return errors.Wrap(err, "failed to encode timeline event details")
}

event := &TimelineEvent{
PlaybookRunID: playbookRunID,
CreateAt: itemToCheck.StateModified,
EventAt: itemToCheck.StateModified,
EventType: TaskStateModified,
Summary: modifyMessage,
SubjectUserID: userID,
Details: string(detailsJSON),
}
event := &TimelineEvent{
PlaybookRunID: playbookRunID,
CreateAt: itemToCheck.StateModified,
EventAt: itemToCheck.StateModified,
EventType: TaskStateModified,
Summary: modifyMessage,
SubjectUserID: userID,
Details: string(detailsJSON),
}

if _, err = s.store.CreateTimelineEvent(event); err != nil {
return errors.Wrap(err, "failed to create timeline event")
if _, err = s.store.CreateTimelineEvent(event); err != nil {
return errors.Wrap(err, "failed to create timeline event")
}
}
s.sendPlaybookRunObjectUpdatedWS(playbookRunID, originalRun, nil)

Expand Down
53 changes: 53 additions & 0 deletions server/app/task_requirements_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright (c) 2020-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

package app

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestGetChecklistItemUpdates_Requirements(t *testing.T) {
previous := []ChecklistItem{
{
ID: "item-1",
Title: "Task",
Requirements: []TaskRequirement{
{ID: "req-1", Label: "Ticket URL", Value: ""},
},
},
}
current := []ChecklistItem{
{
ID: "item-1",
Title: "Task",
Requirements: []TaskRequirement{
{ID: "req-1", Label: "Ticket URL", Value: "https://example.com"},
},
},
}

updates := GetChecklistItemUpdates(previous, current)
require.Len(t, updates.Updates, 1)
assert.Equal(t, "item-1", updates.Updates[0].ID)

reqs, ok := updates.Updates[0].Fields["requirements"]
require.True(t, ok, "requirements field should be included when values change")
assert.Equal(t, current[0].Requirements, reqs)
}

func TestGetChecklistItemUpdates_RequirementsUnchanged(t *testing.T) {
item := ChecklistItem{
ID: "item-1",
Title: "Task",
Requirements: []TaskRequirement{
{ID: "req-1", Label: "Ticket URL", Value: "abc"},
},
}

updates := GetChecklistItemUpdates([]ChecklistItem{item}, []ChecklistItem{item})
assert.Empty(t, updates.Updates)
}
41 changes: 41 additions & 0 deletions server/config/beta_features_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) 2020-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

package config

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestIsTaskRequirementsEnabled(t *testing.T) {
t.Run("disabled by default", func(t *testing.T) {
svc := &ServiceImpl{configuration: &Configuration{}}
assert.False(t, svc.IsTaskRequirementsEnabled())
})

t.Run("enabled when task_requirements is true", func(t *testing.T) {
svc := &ServiceImpl{
configuration: &Configuration{
BetaFeatures: BetaFeaturesConfig{TaskRequirements: true},
},
}
assert.True(t, svc.IsTaskRequirementsEnabled())
})
}

func TestSerializeIncludesBetaFeatures(t *testing.T) {
cfg := &Configuration{
BetaFeatures: BetaFeaturesConfig{TaskRequirements: true},
}
serialized := cfg.serialize()

raw, ok := serialized["BetaFeatures"]
require.True(t, ok, "serialize() must include BetaFeatures key matching plugin.json")

beta, ok := raw.(BetaFeaturesConfig)
require.True(t, ok)
assert.True(t, beta.TaskRequirements)
}
Loading
Loading