diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 7464802..76640cd 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -79,6 +79,18 @@ Derek add label: help wanted Derek remove label: bug ``` +To address multiple labels through a single action use a comma separated list. The maximum number of labels that can be managed in one comment defaults to 5; this can be set to preference through `multilabel_limit` in your `stack.yml` + +To add multiple labels: +``` +Derek add label: proposal, help wanted, skill/intermediate +``` + +To remove multiple labels: +``` +Derek remove label: proposal, help wanted +``` + #### Set a milestone for an issue or PR You can organize your issues in groups through existing milestones @@ -97,6 +109,9 @@ You can assign work to people too ``` Derek assign: alexellis ``` + +Use the `me` moniker to refer to yourself + ``` Derek unassign: me ``` @@ -107,6 +122,11 @@ You can assign people for a PR review as well ``` Derek set reviewer: alexellis +``` + +Use the `me` moniker to refer to yourself + +``` Derek clear reviewer: me ``` diff --git a/handler/commentHandler.go b/handler/commentHandler.go index da09c2d..6308618 100644 --- a/handler/commentHandler.go +++ b/handler/commentHandler.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "os" + "strconv" "strings" log "github.com/Sirupsen/logrus" @@ -35,8 +36,9 @@ const ( assignReviewerConstant string = "AssignReviewer" unassignReviewerConstant string = "UnassignReviewer" - commandTriggerDefault string = "Derek " - commandTriggerSlash string = "/" + noDCO string = "no-dco" + labelLimitDefault int = 5 + labelLimitEnvVar string = "multilabel_limit" ) func makeClient(installation int, config config.Config) (*github.Client, context.Context) { @@ -135,40 +137,89 @@ func findLabel(currentLabels []types.IssueLabel, cmdLabel string) bool { return false } +func classifyLabels(currentLabels []types.IssueLabel, labelAction string, labelValue string) ([]string, []string) { + + var actionableLabels, unactionableLabels []string + + requestedLabels := strings.Split(labelValue, ",") + + for _, requestedLabel := range requestedLabels { + + requestedLabel = strings.TrimSpace(requestedLabel) + + found := findLabel(currentLabels, requestedLabel) + + if validAction(found, labelAction, addLabelConstant, removeLabelConstant) { + actionableLabels = append(actionableLabels, requestedLabel) + } else { + unactionableLabels = append(unactionableLabels, requestedLabel) + } + + } + return actionableLabels, unactionableLabels +} + func manageLabel(req types.IssueCommentOuter, cmdType string, labelValue string, config config.Config) (string, error) { var buffer bytes.Buffer labelAction := strings.Replace(strings.ToLower(cmdType), "label", "", 1) + buffer.WriteString(fmt.Sprintf("%s wants to %s label(s) of '%s' on issue #%d.\n", req.Comment.User.Login, labelAction, labelValue, req.Issue.Number)) - buffer.WriteString(fmt.Sprintf("%s wants to %s label of '%s' on issue #%d \n", req.Comment.User.Login, labelAction, labelValue, req.Issue.Number)) + actionableLabels, unactionableLabels := classifyLabels(req.Issue.Labels, cmdType, labelValue) - found := findLabel(req.Issue.Labels, labelValue) + if len(unactionableLabels) > 0 { + buffer.WriteString(fmt.Sprintf("Request to %s label(s) of '%s' on issue #%d was unnecessary.\n", labelAction, strings.Join(unactionableLabels, ", "), req.Issue.Number)) - if !validAction(found, cmdType, addLabelConstant, removeLabelConstant) { - buffer.WriteString(fmt.Sprintf("Request to %s label of '%s' on issue #%d was unnecessary.", labelAction, labelValue, req.Issue.Number)) - return buffer.String(), nil + if len(actionableLabels) == 0 { + buffer.WriteString(fmt.Sprintf("No further valid labels found - no action taken on issue #%d.\n", req.Issue.Number)) + return buffer.String(), nil + } } client, ctx := makeClient(req.Installation.ID, config) var err error + maxActionableLabels := getMultiLabelLimit() + + if len(actionableLabels) > maxActionableLabels { + buffer.WriteString(fmt.Sprintf("Label(s) '%s' on issue #%d were ignored as they fall outside of the configured limit of %d.\n", strings.Join(actionableLabels[maxActionableLabels:], ", "), req.Issue.Number, maxActionableLabels)) + actionableLabels = actionableLabels[:maxActionableLabels] + } + if cmdType == addLabelConstant { - _, _, err = client.Issues.AddLabelsToIssue(ctx, req.Repository.Owner.Login, req.Repository.Name, req.Issue.Number, []string{labelValue}) - } else { - if isDcoLabel(labelValue) { - buffer.WriteString(fmt.Sprintf("%s the request is not allowed.Label `%s` can be removed by owner or by signing out the commit.", req.Repository.Owner.Login, labelValue)) - return buffer.String(), nil - } else { - _, err = client.Issues.RemoveLabelForIssue(ctx, req.Repository.Owner.Login, req.Repository.Name, req.Issue.Number, labelValue) + + _, _, err = client.Issues.AddLabelsToIssue(ctx, req.Repository.Owner.Login, req.Repository.Name, req.Issue.Number, actionableLabels) + + if err != nil { + return buffer.String(), err } - } - if err != nil { - return buffer.String(), err + } else { + + actionedLabels := actionableLabels[:0] + + for _, actionableLabel := range actionableLabels { + + if isDcoLabel(actionableLabel) { + + buffer.WriteString(fmt.Sprintf("The request to remove `%s` by %s was not allowed - label can be removed by owner or by signing off the commit.\n", actionableLabel, req.Repository.Owner.Login)) + + } else { + + _, err = client.Issues.RemoveLabelForIssue(ctx, req.Repository.Owner.Login, req.Repository.Name, req.Issue.Number, actionableLabel) + + if err != nil { + return buffer.String(), err + } + + actionedLabels = append(actionedLabels, actionableLabel) + } + } + actionableLabels = actionedLabels } - buffer.WriteString(fmt.Sprintf("Request to %s label of '%s' on issue #%d was successfully completed.", labelAction, labelValue, req.Issue.Number)) + buffer.WriteString(fmt.Sprintf("Request to %s label(s) of '%s' on issue #%d was successfully completed.\n", labelAction, strings.Join(actionableLabels, ", "), req.Issue.Number)) return buffer.String(), nil } @@ -361,6 +412,8 @@ func parse(body string, commandTriggers []string) *types.CommentAction { commands := map[string]string{ commandTrigger + "add label: ": addLabelConstant, commandTrigger + "remove label: ": removeLabelConstant, + commandTrigger + "add labels: ": addLabelConstant, + commandTrigger + "remove labels: ": removeLabelConstant, commandTrigger + "assign: ": assignConstant, commandTrigger + "unassign: ": unassignConstant, commandTrigger + "close": closeConstant, @@ -378,10 +431,8 @@ func parse(body string, commandTriggers []string) *types.CommentAction { for trigger, commandType := range commands { if isValidCommand(body, trigger) { - val := body[len(trigger):] - val = strings.Trim(val, " \t.,\n\r") commentAction.Type = commandType - commentAction.Value = val + commentAction.Value = getCommandValue(body, len(trigger)) break } } @@ -390,6 +441,14 @@ func parse(body string, commandTriggers []string) *types.CommentAction { return &commentAction } +func getCommandValue(commentBody string, triggerLength int) string { + + val := commentBody[triggerLength:] + val = strings.Split(val, "\n")[0] + val = strings.Trim(val, " \t.,\n\r") + return val +} + func isValidCommand(body string, trigger string) bool { return (len(body) > len(trigger) && body[0:len(trigger)] == trigger) || (body == trigger && !strings.HasSuffix(trigger, ": ")) @@ -428,9 +487,22 @@ func removeMilestone(client *github.Client, ctx context.Context, URL string) err } func isDcoLabel(labelValue string) bool { - return strings.ToLower(labelValue) == "no-dco" + return strings.ToLower(labelValue) == noDCO } func getCommandTriggers() []string { return []string{"Derek ", "/"} } + +func getMultiLabelLimit() int { + + val, ok := os.LookupEnv(labelLimitEnvVar) + if ok { + configuredLimit, err := strconv.Atoi(val) + if err != nil { + return labelLimitDefault + } + return configuredLimit + } + return labelLimitDefault +} diff --git a/handler/commentHandler_test.go b/handler/commentHandler_test.go index 0c60bb9..f49e346 100644 --- a/handler/commentHandler_test.go +++ b/handler/commentHandler_test.go @@ -4,6 +4,8 @@ package handler import ( + "os" + "strings" "testing" "github.com/alexellis/derek/types" @@ -551,3 +553,255 @@ func Test_Parsing_Reviewers(t *testing.T) { }) } } +func Test_classifyLabels(t *testing.T) { + + var classifyOptions = []struct { + title string + currentLabels []types.IssueLabel + cmdType string + labelValue string + expectedActionable []string + expectedUnactionable []string + }{ + {title: "Add when all Labels exist", + currentLabels: []types.IssueLabel{ + types.IssueLabel{ + Name: "rod", + }, + types.IssueLabel{ + Name: "jane", + }, + types.IssueLabel{ + Name: "freddie", + }, + }, + cmdType: addLabelConstant, + labelValue: "rod, jane, freddie", + expectedActionable: []string{}, + expectedUnactionable: []string{"rod", "jane", "freddie"}, + }, + {title: "Remove when all Labels exist", + currentLabels: []types.IssueLabel{ + types.IssueLabel{ + Name: "rod", + }, + types.IssueLabel{ + Name: "jane", + }, + types.IssueLabel{ + Name: "freddie", + }, + }, + cmdType: removeLabelConstant, + labelValue: "rod, jane, freddie", + expectedActionable: []string{"rod", "jane", "freddie"}, + expectedUnactionable: []string{}, + }, + {title: "Add when no Labels exist", + currentLabels: []types.IssueLabel{}, + cmdType: addLabelConstant, + labelValue: "rod, jane, freddie", + expectedActionable: []string{"rod", "jane", "freddie"}, + expectedUnactionable: []string{}, + }, + {title: "Remove when no Labels exist", + currentLabels: []types.IssueLabel{}, + cmdType: removeLabelConstant, + labelValue: "rod, jane, freddie", + expectedActionable: []string{}, + expectedUnactionable: []string{"rod", "jane", "freddie"}, + }, + {title: "Add when subset of Labels exist", + currentLabels: []types.IssueLabel{ + types.IssueLabel{ + Name: "rod", + }, + types.IssueLabel{ + Name: "jane", + }, + }, + cmdType: addLabelConstant, + labelValue: "rod, jane, freddie", + expectedActionable: []string{"freddie"}, + expectedUnactionable: []string{"rod", "jane"}, + }, + {title: "Remove when subset of Labels exist", + currentLabels: []types.IssueLabel{ + types.IssueLabel{ + Name: "rod", + }, + types.IssueLabel{ + Name: "jane", + }, + }, + cmdType: removeLabelConstant, + labelValue: "rod, jane, freddie", + expectedActionable: []string{"rod", "jane"}, + expectedUnactionable: []string{"freddie"}, + }, + {title: "Add new value to set", + currentLabels: []types.IssueLabel{ + types.IssueLabel{ + Name: "rod", + }, + types.IssueLabel{ + Name: "jane", + }, + }, + cmdType: addLabelConstant, + labelValue: "freddie, burt", + expectedActionable: []string{"freddie", "burt"}, + expectedUnactionable: []string{}, + }, + {title: "remove existing values from set", + currentLabels: []types.IssueLabel{ + types.IssueLabel{ + Name: "rod", + }, + types.IssueLabel{ + Name: "jane", + }, + types.IssueLabel{ + Name: "freddie", + }, + types.IssueLabel{ + Name: "burt", + }, + }, + cmdType: removeLabelConstant, + labelValue: "rod, jane", + expectedActionable: []string{"freddie", "burt"}, + expectedUnactionable: []string{}, + }, + } + + for _, test := range classifyOptions { + t.Run(test.title, func(t *testing.T) { + + actionableLabels, unactionableLabels := classifyLabels(test.currentLabels, test.cmdType, test.labelValue) + + if len(actionableLabels) != len(test.expectedActionable) || len(unactionableLabels) != len(test.expectedUnactionable) { + t.Errorf("Label Classification (%s) - wanted: Actionable(%s) Unactionable(%s), got Actionable(%s) Unactionable(%s)\n", test.title, strings.Join(test.expectedActionable, ", "), strings.Join(test.expectedUnactionable, ", "), strings.Join(actionableLabels, ", "), strings.Join(unactionableLabels, ", ")) + } + }) + } +} + +func Test_getMultiLabelLimit(t *testing.T) { + + var labelLimits = []struct { + title string + envVar string + envVal string + expectedVal int + }{ + { + title: "No ENV var", + envVar: "random", + envVal: "10", + expectedVal: labelLimitDefault, + }, + { + title: "ENV var exists - all valid", + envVar: labelLimitEnvVar, + envVal: "8", + expectedVal: 8, + }, + { + title: "ENV var exists but cannot be cast as int", + envVar: labelLimitEnvVar, + envVal: "fred", + expectedVal: labelLimitDefault, + }, + } + + for _, test := range labelLimits { + t.Run(test.title, func(t *testing.T) { + + os.Setenv(test.envVar, test.envVal) + + maxActionableLabels := getMultiLabelLimit() + + os.Unsetenv(test.envVar) + + if maxActionableLabels != test.expectedVal { + t.Errorf("multi-label limit wrong value found - wanted: %d, found %d", test.expectedVal, maxActionableLabels) + } + }) + } +} + +func Test_getCommandValue(t *testing.T) { + + var commandValues = []struct { + title string + commentBody string + trigger string + expectedVal string + }{ + { + title: "Single Label", + commentBody: "Derek add label: burt", + trigger: "Derek add label: ", + expectedVal: "burt", + }, + { + title: "Single Label trailing spaces", + commentBody: "Derek add label: burt ", + trigger: "Derek add label: ", + expectedVal: "burt", + }, + { + title: "Single Label trailing dots", + commentBody: "Derek add label: burt........", + trigger: "Derek add label: ", + expectedVal: "burt", + }, + { + title: "Single Label trailing commas", + commentBody: "Derek add label: burt,,,,,,,,,,,", + trigger: "Derek add label: ", + expectedVal: "burt", + }, + { + title: "Single Label trailing mixure", + commentBody: "Derek add label: burt,,. , ,, ,.,", + trigger: "Derek add label: ", + expectedVal: "burt", + }, + { + title: "Multiple Labels", + commentBody: "Derek add label: burt, and, ernie", + trigger: "Derek add label: ", + expectedVal: "burt, and, ernie", + }, + { + title: "Multi-line Labels", + commentBody: `Derek add label: burt + , and + , ernie`, + trigger: "Derek add label: ", + expectedVal: "burt", + }, + { + title: "Multi-line Labels with a trailing comma", + commentBody: `Derek add label: burt, + and, + ernie`, + trigger: "Derek add label: ", + expectedVal: "burt", + }, + } + + for _, test := range commandValues { + t.Run(test.title, func(t *testing.T) { + + val := getCommandValue(test.commentBody, len(test.trigger)) + + if val != test.expectedVal { + t.Errorf("command value error - wanted: %s, found %s", test.expectedVal, val) + } + }) + } + +} diff --git a/stack.example.yml b/stack.example.yml index 919e0c6..f07c7ab 100644 --- a/stack.example.yml +++ b/stack.example.yml @@ -9,6 +9,7 @@ functions: lang: Dockerfile environment: application_id: + multilabel_limit: 5 debug: true customers_url: https://raw.githubusercontent.com/alexellis/derek/master/.CUSTOMERS validate_hmac: false