From 863b06ffa1493a32085dbafd1145ae0d74150689 Mon Sep 17 00:00:00 2001 From: Vladimir Kivernik Date: Sat, 26 Sep 2020 02:21:08 +0300 Subject: [PATCH] Add golangci-lint, fix linter issues --- .golangci.yml | 64 ++++++++++++ Makefile | 78 ++++----------- build/custom.mk | 10 +- build/deploy/main.go | 122 +++++++++++++++++++++++ build/manifest/.gitignore | 0 build/manifest/main.go | 5 +- build/setup.mk | 3 + go.mod | 1 + go.sum | 23 +++++ server/api.go | 15 ++- server/command.go | 77 +++++++------- server/command_test.go | 54 +++++----- server/configuration.go | 11 +- server/gitlab/api.go | 62 +++++++----- server/gitlab/gitlab.go | 28 +++--- server/gitlab/mocks/mock_gitlab.go | 32 +++--- server/gitlab/user.go | 16 +-- server/gitlab/webhook.go | 2 +- server/mocks/mock_gitlab.go | 30 +++--- server/plugin.go | 82 +++++++-------- server/subscription/subscription.go | 27 +++-- server/subscription/subscription_test.go | 6 +- server/subscriptions.go | 16 ++- server/subscriptions_test.go | 18 ++-- server/utils.go | 6 +- server/webhook/constants.go | 17 ++++ server/webhook/issue.go | 18 ++-- server/webhook/issue_fixture_test.go | 98 ------------------ server/webhook/issue_test.go | 5 +- server/webhook/merge_request.go | 34 ++++--- server/webhook/merge_request_test.go | 7 +- server/webhook/note_test.go | 2 +- server/webhook/pipeline.go | 8 +- server/webhook/pipeline_test.go | 7 +- server/webhook/push_test.go | 5 +- server/webhook/tag_fixture_test.go | 2 +- server/webhook/tag_test.go | 9 +- server/webhook/webhook.go | 12 --- server/webhook/webhook_test.go | 7 +- server/webhook_test.go | 11 +- 40 files changed, 566 insertions(+), 464 deletions(-) create mode 100644 .golangci.yml create mode 100644 build/deploy/main.go create mode 100644 build/manifest/.gitignore create mode 100644 server/webhook/constants.go diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..934f61c3 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,64 @@ +run: + timeout: 5m + modules-download-mode: readonly + +linters-settings: + goconst: + min-len: 2 + min-occurrences: 2 + gofmt: + simplify: true + goimports: + local-prefixes: github.com/mattermost/mattermost-plugin-gitlab + golint: + min-confidence: 0 + govet: + check-shadowing: true + enable-all: true + misspell: + locale: US + ignore-words: + - noteable + +linters: + disable-all: true + enable: + - bodyclose + - deadcode + - errcheck + - goconst + - gocritic + - gofmt + - goimports + - golint + - gosec + - gosimple + - govet + - ineffassign + - interfacer + - misspell + - nakedret + - staticcheck + - structcheck + - stylecheck + - typecheck + - unconvert + - unused + - varcheck + - whitespace + +issues: + exclude-rules: + - path: server/manifest.go + linters: + - deadcode + - unused + - varcheck + - path: server/configuration.go + linters: + - unused + - path: _test\.go + linters: + - bodyclose + - goconst + - scopelint # https://github.com/kyoh86/scopelint/issues/4 \ No newline at end of file diff --git a/Makefile b/Makefile index 376e8e96..fc3b2d24 100644 --- a/Makefile +++ b/Makefile @@ -30,54 +30,25 @@ all: check-style test dist apply: ./build/bin/manifest apply -## Runs govet and gofmt against all packages. +## Runs golangci-lint and eslint. .PHONY: check-style -check-style: webapp/.npminstall gofmt govet errcheck +check-style: webapp/.npminstall golangci-lint @echo Checking for style guide compliance ifneq ($(HAS_WEBAPP),) cd webapp && npm run lint endif -## Runs gofmt against all packages. -.PHONY: gofmt -gofmt: -ifneq ($(HAS_SERVER),) - @echo Running gofmt - @for package in $$(go list ./...); do \ - echo "Checking "$$package; \ - files=$$(go list -f '{{range .GoFiles}}{{$$.Dir}}/{{.}} {{end}}' $$package); \ - if [ "$$files" ]; then \ - gofmt_output=$$(gofmt -d -s $$files 2>&1); \ - if [ "$$gofmt_output" ]; then \ - echo "$$gofmt_output"; \ - echo "Gofmt failure"; \ - exit 1; \ - fi; \ - fi; \ - done - @echo Gofmt success -endif - -## Runs govet against all packages. -.PHONY: govet -govet: -ifneq ($(HAS_SERVER),) - @echo Running govet - @# Workaround because you can't install binaries without adding them to go.mod - env GO111MODULE=off $(GO) get golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow - $(GO) vet ./... - $(GO) vet -vettool=$(GOPATH)/bin/shadow ./... - @echo Govet success -endif - -## Runs golint against all packages. -.PHONY: golint -golint: - @echo Running lint - env GO111MODULE=off $(GO) get golang.org/x/lint/golint - $(GOPATH)/bin/golint -set_exit_status ./... - @echo lint success +## Run golangci-lint on codebase. +.PHONY: golangci-lint +golangci-lint: + @if ! [ -x "$$(command -v golangci-lint)" ]; then \ + echo "golangci-lint is not installed. Please see https://github.com/golangci/golangci-lint#install for installation instructions."; \ + exit 1; \ + fi; \ + + @echo Running golangci-lint + golangci-lint run ./... ## Builds the server, if it exists, including support for multiple architectures. .PHONY: server @@ -104,6 +75,7 @@ ifneq ($(HAS_WEBAPP),) endif ## Builds the webapp in debug mode, if it exists. +.PHONY: webapp-debug webapp-debug: webapp/.npminstall ifneq ($(HAS_WEBAPP),) cd webapp && \ @@ -139,23 +111,11 @@ endif dist: apply server webapp bundle ## Installs the plugin to a (development) server. +## It uses the API if appropriate environment variables are defined, +## and otherwise falls back to trying to copy the plugin to a sibling mattermost-server directory. .PHONY: deploy deploy: dist -## It uses the API if appropriate environment variables are defined, -## or copying the files directly to a sibling mattermost-server directory. -ifneq ($(and $(MM_SERVICESETTINGS_SITEURL),$(MM_ADMIN_USERNAME),$(MM_ADMIN_PASSWORD),$(CURL)),) - @echo "Installing plugin via API" - $(eval TOKEN := $(shell curl -i --post301 --location $(MM_SERVICESETTINGS_SITEURL) -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/users/login -d '{"login_id": "$(MM_ADMIN_USERNAME)", "password": "$(MM_ADMIN_PASSWORD)"}' | grep Token | cut -f2 -d' ' 2> /dev/null)) - @curl -s --post301 --location $(MM_SERVICESETTINGS_SITEURL) -H "Authorization: Bearer $(TOKEN)" -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/plugins -F "plugin=@dist/$(BUNDLE_NAME)" -F "force=true" > /dev/null && \ - curl -s --post301 --location $(MM_SERVICESETTINGS_SITEURL) -H "Authorization: Bearer $(TOKEN)" -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/plugins/$(PLUGIN_ID)/enable > /dev/null && \ - echo "OK." || echo "Sorry, something went wrong." -else ifneq ($(wildcard ../mattermost-server/.*),) - @echo "Installing plugin via filesystem. Server restart and manual plugin enabling required" - mkdir -p ../mattermost-server/plugins - tar -C ../mattermost-server/plugins -zxvf dist/$(BUNDLE_NAME) -else - @echo "No supported deployment method available. Install plugin manually." -endif + ./build/bin/deploy $(PLUGIN_ID) dist/$(BUNDLE_NAME) .PHONY: debug-deploy debug-deploy: debug-dist deploy @@ -197,15 +157,17 @@ endif clean: rm -fr dist/ ifneq ($(HAS_SERVER),) + rm -fr server/coverage.txt rm -fr server/dist endif ifneq ($(HAS_WEBAPP),) rm -fr webapp/.npminstall + rm -fr webapp/junit.xml rm -fr webapp/dist rm -fr webapp/node_modules endif rm -fr build/bin/ -# Help documentatin à la https://marmelab.com/blog/2016/02/29/auto-documented-makefile.html +# Help documentation à la https://marmelab.com/blog/2016/02/29/auto-documented-makefile.html help: - @cat Makefile | grep -v '\.PHONY' | grep -v '\help:' | grep -B1 -E '^[a-zA-Z0-9_.-]+:.*' | sed -e "s/:.*//" | sed -e "s/^## //" | grep -v '\-\-' | sed '1!G;h;$$!d' | awk 'NR%2{printf "\033[36m%-30s\033[0m",$$0;next;}1' | sort + @cat Makefile | grep -v '\.PHONY' | grep -v '\help:' | grep -B1 -E '^[a-zA-Z0-9_.-]+:.*' | sed -e "s/:.*//" | sed -e "s/^## //" | grep -v '\-\-' | sed '1!G;h;$$!d' | awk 'NR%2{printf "\033[36m%-30s\033[0m",$$0;next;}1' | sort \ No newline at end of file diff --git a/build/custom.mk b/build/custom.mk index b19691dd..efad4fbd 100644 --- a/build/custom.mk +++ b/build/custom.mk @@ -1,9 +1 @@ -## Runs errcheck against all packages. -.PHONY: errcheck -errcheck: -ifneq ($(HAS_SERVER),) - @echo Running errcheck - @# Workaroung because you can't install binaries without adding them to go.mod - env GO111MODULE=off $(GO) get github.com/kisielk/errcheck - errcheck ./server/... -endif +# Include custome targets and environment variables here diff --git a/build/deploy/main.go b/build/deploy/main.go new file mode 100644 index 00000000..186bb492 --- /dev/null +++ b/build/deploy/main.go @@ -0,0 +1,122 @@ +// main handles deployment of the plugin to a development server using either the Client4 API +// or by copying the plugin bundle into a sibling mattermost-server/plugin directory. +package main + +import ( + "fmt" + "log" + "os" + "path/filepath" + + "github.com/mattermost/mattermost-server/v5/model" + "github.com/mholt/archiver/v3" + "github.com/pkg/errors" +) + +func main() { + err := deploy() + if err != nil { + fmt.Printf("Failed to deploy: %s\n", err.Error()) + fmt.Println() + fmt.Println("Usage:") + fmt.Println(" deploy ") + os.Exit(1) + } +} + +// deploy handles deployment of the plugin to a development server. +func deploy() error { + if len(os.Args) < 3 { + return errors.New("invalid number of arguments") + } + + pluginID := os.Args[1] + bundlePath := os.Args[2] + + siteURL := os.Getenv("MM_SERVICESETTINGS_SITEURL") + adminToken := os.Getenv("MM_ADMIN_TOKEN") + adminUsername := os.Getenv("MM_ADMIN_USERNAME") + adminPassword := os.Getenv("MM_ADMIN_PASSWORD") + copyTargetDirectory, _ := filepath.Abs("../mattermost-server") + + if siteURL != "" { + client := model.NewAPIv4Client(siteURL) + + if adminToken != "" { + log.Printf("Authenticating using token against %s.", siteURL) + client.SetToken(adminToken) + + return uploadPlugin(client, pluginID, bundlePath) + } + + if adminUsername != "" && adminPassword != "" { + client := model.NewAPIv4Client(siteURL) + log.Printf("Authenticating as %s against %s.", adminUsername, siteURL) + _, resp := client.Login(adminUsername, adminPassword) + if resp.Error != nil { + return errors.Wrapf(resp.Error, "failed to login as %s", adminUsername) + } + + return uploadPlugin(client, pluginID, bundlePath) + } + } + + _, err := os.Stat(copyTargetDirectory) + if os.IsNotExist(err) { + return errors.New("no supported deployment method available, please install plugin manually") + } else if err != nil { + return errors.Wrapf(err, "failed to stat %s", copyTargetDirectory) + } + + log.Printf("Installing plugin to mattermost-server found in %s.", copyTargetDirectory) + log.Print("Server restart required to load updated plugin.") + return copyPlugin(pluginID, copyTargetDirectory, bundlePath) +} + +// uploadPlugin attempts to upload and enable a plugin via the Client4 API. +// It will fail if plugin uploads are disabled. +func uploadPlugin(client *model.Client4, pluginID, bundlePath string) error { + pluginBundle, err := os.Open(bundlePath) + if err != nil { + return errors.Wrapf(err, "failed to open %s", bundlePath) + } + defer pluginBundle.Close() + + log.Print("Uploading plugin via API.") + _, resp := client.UploadPluginForced(pluginBundle) + if resp.Error != nil { + return errors.Wrap(resp.Error, "failed to upload plugin bundle") + } + + log.Print("Enabling plugin.") + _, resp = client.EnablePlugin(pluginID) + if resp.Error != nil { + return errors.Wrap(resp.Error, "Failed to enable plugin") + } + + return nil +} + +// copyPlugin attempts to install a plugin by copying it to a sibling ../mattermost-server/plugin +// directory. A server restart is required before the plugin will start. +func copyPlugin(pluginID, targetPath, bundlePath string) error { + targetPath = filepath.Join(targetPath, "plugins") + + err := os.MkdirAll(targetPath, 0777) + if err != nil { + return errors.Wrapf(err, "failed to create %s", targetPath) + } + + existingPluginPath := filepath.Join(targetPath, pluginID) + err = os.RemoveAll(existingPluginPath) + if err != nil { + return errors.Wrapf(err, "failed to remove existing existing plugin directory %s", existingPluginPath) + } + + err = archiver.Unarchive(bundlePath, targetPath) + if err != nil { + return errors.Wrapf(err, "failed to unarchive %s into %s", bundlePath, targetPath) + } + + return nil +} diff --git a/build/manifest/.gitignore b/build/manifest/.gitignore new file mode 100644 index 00000000..e69de29b diff --git a/build/manifest/main.go b/build/manifest/main.go index 89a397c1..a924723e 100644 --- a/build/manifest/main.go +++ b/build/manifest/main.go @@ -106,14 +106,15 @@ func applyManifest(manifest *model.Manifest) error { if err := ioutil.WriteFile( "server/manifest.go", []byte(fmt.Sprintf(pluginIDGoFileTemplate, manifest.Id, manifest.Version)), - 0644, + 0600, ); err != nil { return errors.Wrap(err, "failed to write server/manifest.go") } } if manifest.HasWebapp() { - if err := ioutil.WriteFile( + // #nosec G306 + if err := ioutil.WriteFile( // #nosec G306 "webapp/src/manifest.js", []byte(fmt.Sprintf(pluginIDJSFileTemplate, manifest.Id, manifest.Version)), 0644, diff --git a/build/setup.mk b/build/setup.mk index d692b445..bc1fdc35 100644 --- a/build/setup.mk +++ b/build/setup.mk @@ -7,6 +7,9 @@ endif # Ensure that the build tools are compiled. Go's caching makes this quick. $(shell cd build/manifest && $(GO) build -o ../bin/manifest) +# Ensure that the deployment tools are compiled. Go's caching makes this quick. +$(shell cd build/deploy && $(GO) build -o ../bin/deploy) + # Extract the plugin id from the manifest. PLUGIN_ID ?= $(shell build/bin/manifest id) ifeq ($(PLUGIN_ID),) diff --git a/go.mod b/go.mod index 37c96dd8..35ddb9d8 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.13 require ( github.com/golang/mock v1.4.3 github.com/mattermost/mattermost-server/v5 v5.25.1 + github.com/mholt/archiver/v3 v3.3.0 github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.6.1 github.com/xanzy/go-gitlab v0.33.0 diff --git a/go.sum b/go.sum index 886b16e5..c5ec1d90 100644 --- a/go.sum +++ b/go.sum @@ -30,6 +30,8 @@ github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuy github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= +github.com/andybalholm/brotli v0.0.0-20190621154722-5f990b63d2d6 h1:bZ28Hqta7TFAK3Q08CMvv8y3/8ATaEqv2nGoc6yff6c= +github.com/andybalholm/brotli v0.0.0-20190621154722-5f990b63d2d6/go.mod h1:+lx6/Aqd1kLJ1GQfkvOnaZ1WGmLpMpbprPuIOOZX30U= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c= github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= @@ -93,6 +95,9 @@ github.com/die-net/lrucache v0.0.0-20181227122439-19a39ef22a11/go.mod h1:ew0MSjC github.com/disintegration/imaging v1.6.0/go.mod h1:xuIt+sRxDFrHS0drzXUlCJthkJ8k7lkkUojDSR247MQ= github.com/disintegration/imaging v1.6.2/go.mod h1:44/5580QXChDfwIclfc/PCwrr44amcmDAg8hxG0Ewe4= github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyGc8n1E= +github.com/dsnet/compress v0.0.1 h1:PlZu0n3Tuv04TzpfPbrnI0HW/YwodEXDS+oPKahKF0Q= +github.com/dsnet/compress v0.0.1/go.mod h1:Aw8dCMJ7RioblQeTqt88akK31OvO8Dhf5JflhBbQEHo= +github.com/dsnet/golib v0.0.0-20171103203638-1ea166775780/go.mod h1:Lj+Z9rebOhdfkVLjJ8T6VcRQv3SXugXy999NBtR9aFY= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/dyatlov/go-opengraph v0.0.0-20180429202543-816b6608b3c8 h1:6muCmMJat6z7qptVrIf/+OWPxsjAfvhw5/6t+FwEkgg= github.com/dyatlov/go-opengraph v0.0.0-20180429202543-816b6608b3c8/go.mod h1:nYia/MIs9OyvXXYboPmNOj0gVWo97Wx0sde+ZuKkoM4= @@ -141,6 +146,8 @@ github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7a github.com/gogo/protobuf v1.2.0/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGwJL78qG/PmXZO1EjYhfJinVAhrmmHX6Z8B9k= +github.com/golang/gddo v0.0.0-20190419222130-af0f2af80721 h1:KRMr9A3qfbVM7iV/WcLY/rL5LICqwMHLhwRXKu99fXw= +github.com/golang/gddo v0.0.0-20190419222130-af0f2af80721/go.mod h1:xEhNfoBDX1hzLm2Nf80qUvZ2sVwoMZ8d6IE2SrsQfh4= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= @@ -164,6 +171,7 @@ github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvq github.com/golang/protobuf v1.4.2 h1:+Z5KGCizgyZCbGh1KZqA0fcLLkwbsjIzS4aV2v7wJX0= github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= +github.com/golang/snappy v0.0.1 h1:Qgr9rKW7uDUkrbSmQeiDsGa8SjGyCOGtuasMWwvp2P4= github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/gomodule/redigo v2.0.0+incompatible/go.mod h1:B4C85qUVwatsJoIUNIfCRsp7qO0iAmpGFZ4EELWSbC4= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= @@ -259,6 +267,12 @@ github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfV github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w= github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/klauspost/compress v1.4.1/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A= +github.com/klauspost/compress v1.9.2 h1:LfVyl+ZlLlLDeQ/d2AqfGIIH4qEDu0Ed2S5GyhCWIWY= +github.com/klauspost/compress v1.9.2/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A= +github.com/klauspost/cpuid v1.2.0/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek= +github.com/klauspost/pgzip v1.2.1 h1:oIPZROsWuPHpOdMVWLuJZXwgjhrW8r1yEX8UqMyeNHM= +github.com/klauspost/pgzip v1.2.1/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs= github.com/kljensen/snowball v0.6.0/go.mod h1:27N7E8fVU5H68RlUmnWwZCfxgt4POBJfENGMvNRhldw= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= @@ -311,6 +325,8 @@ github.com/mattn/go-sqlite3 v1.9.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOq github.com/mattn/go-sqlite3 v1.11.0 h1:LDdKkqtYlom37fkvqs8rMPFKAMe8+SgjbwZ6ex1/A/Q= github.com/mattn/go-sqlite3 v1.11.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= +github.com/mholt/archiver/v3 v3.3.0 h1:vWjhY8SQp5yzM9P6OJ/eZEkmi3UAbRrxCq48MxjAzig= +github.com/mholt/archiver/v3 v3.3.0/go.mod h1:YnQtqsp+94Rwd0D/rk5cnLrxusUBUXg+08Ebtr1Mqao= github.com/microcosm-cc/bluemonday v1.0.1/go.mod h1:hsXNsILzKxV+sX77C5b8FSuKF00vh2OMYv+xgHpAMF4= github.com/miekg/dns v1.1.26/go.mod h1:bPDLeHnStXmXAq1m/Ch/hvfNHr14JKNPMBo3VZKjuso= github.com/miekg/dns v1.1.29/go.mod h1:KNUDUusw/aVsxyTYZM1oqvCicbwhgbNgztCETuNZ7xM= @@ -337,6 +353,8 @@ github.com/neelance/astrewrite v0.0.0-20160511093645-99348263ae86/go.mod h1:kHJE github.com/neelance/sourcemap v0.0.0-20151028013722-8c68805598ab/go.mod h1:Qr6/a/Q4r9LP1IltGz7tA7iOK1WonHEYhu1HRBA7ZiM= github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646/go.mod h1:jpp1/29i3P1S/RLdc7JQKbRpFeM1dOBd8T9ki5s+AY8= github.com/ngdinhtoan/glide-cleanup v0.2.0/go.mod h1:UQzsmiDOb8YV3nOsCxK/c9zPpCZVNoHScRE3EO9pVMM= +github.com/nwaples/rardecode v1.0.0 h1:r7vGuS5akxOnR4JQSkko62RJ1ReCMXxQRPtxsiFMBOs= +github.com/nwaples/rardecode v1.0.0/go.mod h1:5DzqNKiOdpKKBH87u8VlvAnPZMXcGRhxWkRpHbbfGS0= github.com/oklog/run v1.0.0 h1:Ru7dDtJNOyC66gQ5dQmaCa0qIsAUFY3sFpK1Xk8igrw= github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA= github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA= @@ -360,6 +378,7 @@ github.com/pelletier/go-toml v1.7.0 h1:7utD74fnzVc/cpcyy8sjrlFr5vYpypUixARcHIMIG github.com/pelletier/go-toml v1.7.0/go.mod h1:vwGMzjaWMwyfHwgIBhI2YUM4fB6nL6lVAvS1LBMMhTE= github.com/peterbourgon/diskv v0.0.0-20171120014656-2973218375c3/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= github.com/philhofer/fwd v1.0.0/go.mod h1:gk3iGcWd9+svBvR0sR+KPcfE+RNWozjowpeBVG3ZVNU= +github.com/pierrec/lz4 v2.0.5+incompatible h1:2xWsjqPFWcplujydGg4WmhC/6fZqK42wMM8aXeqhl0I= github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= @@ -488,6 +507,8 @@ github.com/uber/jaeger-client-go v2.23.0+incompatible/go.mod h1:WVhlPFC8FDjOFMMW github.com/uber/jaeger-lib v2.2.0+incompatible/go.mod h1:ComeNDZlWwrWnDv8aPp0Ba6+uUTzImX/AauajbLI56U= github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= +github.com/ulikunitz/xz v0.5.6 h1:jGHAfXawEGZQ3blwU5wnWKQJvAraT7Ftq9EXjnXYgt8= +github.com/ulikunitz/xz v0.5.6/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4ABRW8= github.com/viant/assertly v0.4.8/go.mod h1:aGifi++jvCrUaklKEKT0BU95igDNaqkvz+49uaYMPRU= github.com/viant/toolbox v0.24.0/go.mod h1:OxMCG57V0PXuIP2HNQrtJf2CjqdmbrOx5EkMILuUhzM= github.com/wiggin77/cfg v1.0.2/go.mod h1:b3gotba2e5bXTqTW48DwIFoLc+4lWKP7WPi/CdvZ4aE= @@ -497,6 +518,8 @@ github.com/wiggin77/srslog v1.0.1/go.mod h1:fehkyYDq1QfuYn60TDPu9YdY2bB85VUW2mvN github.com/willf/bitset v1.1.10/go.mod h1:RjeCKbqT1RxIR/KWY6phxZiaY1IyutSBfGjNPySAYV4= github.com/xanzy/go-gitlab v0.33.0 h1:MUJZknbLhVXSFzBA5eqGGhQ2yHSu8tPbGBPeB3sN4B0= github.com/xanzy/go-gitlab v0.33.0/go.mod h1:sPLojNBn68fMUWSxIJtdVVIP8uSBYqesTfDUseX11Ug= +github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 h1:nIPpBwaJSVYIxUFsDv3M8ofmx9yWTog9BfvIu0q41lo= +github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8/go.mod h1:HUYIGzjTL3rfEspMxjDjgmT5uz5wzYJKVo23qUhYTos= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= github.com/xtgo/uuid v0.0.0-20140804021211-a0b114877d4c/go.mod h1:UrdRz5enIKZ63MEE3IF9l2/ebyx59GyGgPi+tICQdmM= diff --git a/server/api.go b/server/api.go index b8276284..a670b216 100644 --- a/server/api.go +++ b/server/api.go @@ -12,16 +12,15 @@ import ( "strings" "time" - "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin" - "golang.org/x/oauth2" + + "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" ) const ( - API_ERROR_ID_NOT_CONNECTED = "not_connected" - GITLAB_USERNAME = "GitLab Plugin" + APIErrorIDNotConnected = "not_connected" ) type APIErrorResponse struct { @@ -161,7 +160,7 @@ func (p *Plugin) completeConnectUserToGitlab(w http.ResponseWriter, r *http.Requ userInfo, err := p.GitlabClient.GetCurrentUser(userID, *tok) if err != nil { - p.API.LogError("can't retreive user info from gitLab API", "err", err.Error()) + p.API.LogError("can't retrieve user info from gitLab API", "err", err.Error()) http.Error(w, "Unable to connect user to GitLab", http.StatusInternalServerError) return } @@ -194,14 +193,14 @@ func (p *Plugin) completeConnectUserToGitlab(w http.ResponseWriter, r *http.Requ "* The fifth will refresh the numbers.\n\n"+ "Click on them!\n\n"+ "##### Slash Commands\n"+ - strings.Replace(commandHelp, "|", "`", -1), userInfo.GitlabUsername) + strings.ReplaceAll(commandHelp, "|", "`"), userInfo.GitlabUsername) if err := p.CreateBotDMPost(userID, message, "custom_git_welcome"); err != nil { p.API.LogError("can't send help message with bot dm", "err", err.Error()) } p.API.PublishWebSocketEvent( - WS_EVENT_CONNECT, + WsEventConnect, map[string]interface{}{ "connected": true, "gitlab_username": userInfo.GitlabUsername, @@ -290,7 +289,7 @@ func (p *Plugin) getGitlabUser(w http.ResponseWriter, r *http.Request) { userInfo, apiErr := p.getGitlabUserInfoByMattermostID(req.UserID) if apiErr != nil { - if apiErr.ID == API_ERROR_ID_NOT_CONNECTED { + if apiErr.ID == APIErrorIDNotConnected { p.writeAPIError(w, &APIErrorResponse{ID: "", Message: "User is not connected to a GitLab account.", StatusCode: http.StatusNotFound}) } else { p.writeAPIError(w, apiErr) diff --git a/server/command.go b/server/command.go index c29ea660..174052fb 100644 --- a/server/command.go +++ b/server/command.go @@ -54,6 +54,7 @@ const commandHelp = `* |/gitlab connect| - Connect your Mattermost account to yo const ( webhookHowToURL = "https://github.com/mattermost/mattermost-plugin-gitlab#step-3-create-a-gitlab-webhook" inboundWebhookURL = "plugins/com.github.manland.mattermost-plugin-gitlab/webhook" + specifyRepositoryMessage = "Please specify a repository." unknownActionMessage = "Unknown action, please use `/gitlab help` to see all actions available." newWebhookEmptySiteURLmessage = "Unable to create webhook. The Mattermot Site URL is not set. " + "Set it in the Admin Console or rerun /gitlab webhook add group/project URL including the desired URL." @@ -69,6 +70,12 @@ const ( invalidSubscribeSubCommand = "Invalid subscribe command. Available commands are add, delete, and list" ) +const ( + commandAdd = "add" + commandDelete = "delete" + commandList = "list" +) + func getCommand() *model.Command { return &model.Command{ Trigger: "gitlab", @@ -95,7 +102,7 @@ func (p *Plugin) getCommandResponse(args *model.CommandArgs, text string) *model return &model.CommandResponse{} } -//ExecuteCommand is the entrypoint for /gitlab commands. It returns a message to display to the user or an error. +// ExecuteCommand is the entrypoint for /gitlab commands. It returns a message to display to the user or an error. func (p *Plugin) ExecuteCommand(_ *plugin.Context, args *model.CommandArgs) (*model.CommandResponse, *model.AppError) { var ( split = strings.Fields(args.Command) @@ -124,14 +131,14 @@ func (p *Plugin) ExecuteCommand(_ *plugin.Context, args *model.CommandArgs) (*mo } if action == "help" || action == "" { - text := "###### Mattermost GitLab Plugin - Slash Command Help\n" + strings.Replace(commandHelp, "|", "`", -1) + text := "###### Mattermost GitLab Plugin - Slash Command Help\n" + strings.ReplaceAll(commandHelp, "|", "`") return p.getCommandResponse(args, text), nil } info, apiErr := p.getGitlabUserInfoByMattermostID(args.UserId) if apiErr != nil { text := "Unknown error." - if apiErr.ID == API_ERROR_ID_NOT_CONNECTED { + if apiErr.ID == APIErrorIDNotConnected { text = "You must connect your account to GitLab first. Either click on the GitLab logo in the bottom left of the screen or enter `/gitlab connect`." } return p.getCommandResponse(args, text), nil @@ -145,12 +152,12 @@ func (p *Plugin) ExecuteCommand(_ *plugin.Context, args *model.CommandArgs) (*mo response := p.getCommandResponse(args, message) return response, nil case "unsubscribe": - //subcommand subscriptions delete is preferred but unsubscribe remains to prevent breaking existing workflows + // subcommand subscriptions delete is preferred but unsubscribe remains to prevent breaking existing workflows var message string var err error if len(parameters) == 0 { - message = "Please specify a repository." + message = specifyRepositoryMessage } else { message, err = p.subscriptionDelete(info, config, parameters[0], args.ChannelId) if err != nil { @@ -185,29 +192,29 @@ func (p *Plugin) ExecuteCommand(_ *plugin.Context, args *model.CommandArgs) (*mo setting := parameters[0] strValue := parameters[1] value := false - if strValue == SETTING_ON { + if strValue == SettingOn { value = true - } else if strValue != SETTING_OFF { + } else if strValue != SettingOff { return p.getCommandResponse(args, "Invalid value. Accepted values are: \"on\" or \"off\"."), nil } - if setting == SETTING_NOTIFICATIONS { + switch setting { + case SettingNotifications: if value { if err := p.storeGitlabToUserIDMapping(info.GitlabUsername, info.UserID); err != nil { p.API.LogError("can't store GitLab user id mapping", "err", err.Error()) return p.getCommandResponse(args, "Unknown error please retry or ask to an administrator to look at logs"), nil } } else { - if err := p.API.KVDelete(info.GitlabUsername + GITLAB_USERNAME_KEY); err != nil { + if err := p.API.KVDelete(info.GitlabUsername + GitlabUsernameKey); err != nil { p.API.LogError("can't delete GitLab username in kvstore", "err", err.Error()) return p.getCommandResponse(args, "Unknown error please retry or ask to an administrator to look at logs"), nil } } - info.Settings.Notifications = value - } else if setting == SETTING_REMINDERS { + case SettingReminders: info.Settings.DailyReminder = value - } else { + default: return p.getCommandResponse(args, "Unknown setting."), nil } @@ -229,14 +236,14 @@ func (p *Plugin) ExecuteCommand(_ *plugin.Context, args *model.CommandArgs) (*mo } // webhookCommand processes the /gitlab webhook commands -func (p *Plugin) webhookCommand(parameters []string, info *gitlab.GitlabUserInfo) string { +func (p *Plugin) webhookCommand(parameters []string, info *gitlab.UserInfo) string { if len(parameters) < 1 { return unknownActionMessage } subCommand := parameters[0] switch subCommand { - case "list": + case commandList: if len(parameters) != 2 { return unknownActionMessage } @@ -257,7 +264,6 @@ func (p *Plugin) webhookCommand(parameters []string, info *gitlab.GitlabUserInfo } return err.Error() } - } else { owner := fullPath[0] webhookInfo, err = p.GitlabClient.GetGroupHooks(info, owner) @@ -277,7 +283,7 @@ func (p *Plugin) webhookCommand(parameters []string, info *gitlab.GitlabUserInfo } return formatedWebhooks - case "add": + case commandAdd: namespace := parameters[1] fullPath := strings.Split(namespace, "/") @@ -291,7 +297,7 @@ func (p *Plugin) webhookCommand(parameters []string, info *gitlab.GitlabUserInfo urlPath = parameters[3] } - //default to all triggers unless specified + // default to all triggers unless specified hookOptions := parseTriggers("*") if len(parameters) > 2 { triggersCsv := parameters[2] @@ -305,7 +311,7 @@ func (p *Plugin) webhookCommand(parameters []string, info *gitlab.GitlabUserInfo hookOptions.Token = p.getConfiguration().WebhookSecret } - //if project scope + // if project scope if len(fullPath) == 2 { owner := fullPath[0] repo := fullPath[1] @@ -327,7 +333,7 @@ func (p *Plugin) webhookCommand(parameters []string, info *gitlab.GitlabUserInfo } return fmt.Sprintf("Webhook Created:\n%s", newWebhook.String()) } - return fmt.Sprintf("Invalid command") + return fmt.Sprintf("Invalid command: %s", subCommand) default: return fmt.Sprintf("Unknown webhook command: %s", subCommand) @@ -398,7 +404,7 @@ func parseTriggers(triggersCsv string) *gitlab.AddWebhookOptions { } } -func (p *Plugin) subscriptionDelete(info *gitlab.GitlabUserInfo, config *configuration, fullPath, channelID string) (string, error) { +func (p *Plugin) subscriptionDelete(info *gitlab.UserInfo, config *configuration, fullPath, channelID string) (string, error) { normalizedPath := normalizePath(fullPath, config.GitlabURL) deleted, err := p.Unsubscribe(channelID, normalizedPath) if err != nil { @@ -413,7 +419,7 @@ func (p *Plugin) subscriptionDelete(info *gitlab.GitlabUserInfo, config *configu return fmt.Sprintf("Successfully deleted subscription for %s.", normalizedPath), nil } -//subscriptionsListCommand list GitLab subscriptions in a channel +// subscriptionsListCommand list GitLab subscriptions in a channel func (p *Plugin) subscriptionsListCommand(channelID string) string { var txt string subs, err := p.GetSubscriptionsByChannel(channelID) @@ -432,8 +438,8 @@ func (p *Plugin) subscriptionsListCommand(channelID string) string { return txt } -//subscriptionsAddCommand subscripes to A GitLab Project -func (p *Plugin) subscriptionsAddCommand(info *gitlab.GitlabUserInfo, config *configuration, fullPath, channelID, features string) string { +// subscriptionsAddCommand subscripes to A GitLab Project +func (p *Plugin) subscriptionsAddCommand(info *gitlab.UserInfo, config *configuration, fullPath, channelID, features string) string { var err error namespace, project, err := p.GitlabClient. ResolveNamespaceAndProject(info, fullPath, config.EnablePrivateRepo) @@ -480,7 +486,7 @@ func (p *Plugin) subscriptionsAddCommand(info *gitlab.GitlabUserInfo, config *co } var hookStatusMessage string if !hasHook { - //no web hook found + // no web hook found hookStatusMessage = fmt.Sprintf( "\nA Webhook is needed, run ```/gitlab webhook add %s``` to create one now.", fullPath, @@ -489,9 +495,9 @@ func (p *Plugin) subscriptionsAddCommand(info *gitlab.GitlabUserInfo, config *co return fmt.Sprintf("Successfully subscribed to %s.%s", fullPath, hookStatusMessage) } -// subscribeCommand proccess the /gitlab subscribe command. +// subscribeCommand process the /gitlab subscribe command. // It returns a message and handles all errors my including helpful information in the message -func (p *Plugin) subscribeCommand(parameters []string, channelID string, config *configuration, info *gitlab.GitlabUserInfo) string { +func (p *Plugin) subscribeCommand(parameters []string, channelID string, config *configuration, info *gitlab.UserInfo) string { if len(parameters) == 0 { return invalidSubscribeSubCommand } @@ -499,9 +505,9 @@ func (p *Plugin) subscribeCommand(parameters []string, channelID string, config subcommand := parameters[0] switch subcommand { - case "list": + case commandList: return p.subscriptionsListCommand(channelID) - case "add": + case commandAdd: features := "merges,issues,tag" if len(parameters) > 2 { features = strings.Join(parameters[2:], " ") @@ -510,9 +516,9 @@ func (p *Plugin) subscribeCommand(parameters []string, channelID string, config fullPath := normalizePath(parameters[1], config.GitlabURL) return p.subscriptionsAddCommand(info, config, fullPath, channelID, features) - case "delete": + case commandDelete: if len(parameters) < 2 { - return "Please specify a repository." + return specifyRepositoryMessage } message, err := p.subscriptionDelete(info, config, parameters[1], channelID) @@ -523,7 +529,6 @@ func (p *Plugin) subscribeCommand(parameters []string, channelID string, config default: return invalidSubscribeSubCommand } - } func getAutocompleteData() *model.AutocompleteData { @@ -540,15 +545,15 @@ func getAutocompleteData() *model.AutocompleteData { subscriptions := model.NewAutocompleteData("subscriptions", "[command]", "Available commands: Add, List, Delete") - subscriptionsList := model.NewAutocompleteData("list", "", "List current channel subscriptions") + subscriptionsList := model.NewAutocompleteData(commandList, "", "List current channel subscriptions") subscriptions.AddCommand(subscriptionsList) - subscriptionsAdd := model.NewAutocompleteData("add", "owner[/repo] [features]", "Subscribe the current channel to receive notifications from a project") + subscriptionsAdd := model.NewAutocompleteData(commandAdd, "owner[/repo] [features]", "Subscribe the current channel to receive notifications from a project") subscriptionsAdd.AddTextArgument("Project path: includes user or group name with optional slash project name", "owner[/repo]", "") subscriptionsAdd.AddTextArgument("Features: comma-delimited list of features to subscribe to", "[issues,][merges,][pushes,][issue_comments,][merge_request_comments,][pipeline,][tag,][pull_reviews,][label:]", "") subscriptions.AddCommand(subscriptionsAdd) - subscriptionsDelete := model.NewAutocompleteData("delete", "owner[/repo]", "Unsubscribe the current channel from a repository") + subscriptionsDelete := model.NewAutocompleteData(commandDelete, "owner[/repo]", "Unsubscribe the current channel from a repository") subscriptionsDelete.AddTextArgument("Project path: includes user or group name with optional slash project name", "owner[/repo]", "") subscriptions.AddCommand(subscriptionsDelete) @@ -578,11 +583,11 @@ func getAutocompleteData() *model.AutocompleteData { gitlabCommand.AddCommand(settings) webhook := model.NewAutocompleteData("webhook", "[command]", "Available Commands: list, add") - webhookList := model.NewAutocompleteData("list", "owner/[repo]", "List existing project or group webhooks") + webhookList := model.NewAutocompleteData(commandList, "owner/[repo]", "List existing project or group webhooks") webhookList.AddTextArgument("Project path: includes user or group name with optional slash project name", "owner[/repo]", "") webhook.AddCommand(webhookList) - webhookAdd := model.NewAutocompleteData("add", "owner/[repo] [options] [url] [token]", "Add a project or group webhook") + webhookAdd := model.NewAutocompleteData(commandAdd, "owner/[repo] [options] [url] [token]", "Add a project or group webhook") webhookAdd.AddTextArgument("Group or Project path: includes user or group name with optional slash project name", "owner[/repo]", "") webhookAdd.AddTextArgument("[Optional] options: comma-delimited list of actions to trigger a webhook, defaults to all with SSL verification", "[* or *noSSL] or [PushEvents,][TagPushEvents,][Comments,][ConfidentialComments,][IssuesEvents,][ConfidentialIssuesEvents,][MergeRequestsEvents,][JobEvents,][PipelineEvents,][WikiPageEvents,][SSLverification]", "") webhookAdd.AddTextArgument("[Optional] url: URL to be triggered triggered. Defaults to this plugins URL", "[url]", "") diff --git a/server/command_test.go b/server/command_test.go index f865de16..46413e8d 100644 --- a/server/command_test.go +++ b/server/command_test.go @@ -5,18 +5,19 @@ import ( "testing" "github.com/golang/mock/gomock" - "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" - mocks "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab/mocks" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin/plugintest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" gitLabAPI "github.com/xanzy/go-gitlab" + + "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" + mocks "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab/mocks" ) type subscribeCommandTest struct { testName string - paramaters []string + parameters []string want string webhookInfo []*gitlab.WebhookInfo mattermostURL string @@ -29,12 +30,12 @@ const subscribeSuccessMessage = "Successfully subscribed to group/project.\nA We var subscribeCommandTests = []subscribeCommandTest{ { testName: "No Subscriptions", - paramaters: []string{"list"}, + parameters: []string{"list"}, want: "Currently there are no subscriptions in this channel", }, { testName: "Hook Found", - paramaters: []string{"add", "group/project"}, + parameters: []string{"add", "group/project"}, mockGitlab: true, want: "Successfully subscribed to group/project.", webhookInfo: []*gitlab.WebhookInfo{{URL: "example.com/somewebhookURL"}}, @@ -42,7 +43,7 @@ var subscribeCommandTests = []subscribeCommandTest{ }, { testName: "No webhooks", - paramaters: []string{"add", "group/project"}, + parameters: []string{"add", "group/project"}, mattermostURL: "example.com", webhookInfo: []*gitlab.WebhookInfo{{}}, mockGitlab: true, @@ -50,7 +51,7 @@ var subscribeCommandTests = []subscribeCommandTest{ }, { testName: "Multiple un-matching hooks", - paramaters: []string{"add", "group/project"}, + parameters: []string{"add", "group/project"}, mattermostURL: "example.com", mockGitlab: true, webhookInfo: []*gitlab.WebhookInfo{{URL: "www.anotherhook.io/wrong"}, {URL: "www.213210948239324.edu/notgood"}}, @@ -58,49 +59,46 @@ var subscribeCommandTests = []subscribeCommandTest{ }, { testName: "Error getting webhooks", - paramaters: []string{"add", "group"}, + parameters: []string{"add", "group"}, mattermostURL: "example.com", mockGitlab: true, webhookInfo: []*gitlab.WebhookInfo{{}}, want: "Unable to determine status of Webhook. See [setup instructions](https://github.com/mattermost/mattermost-plugin-gitlab#step-3-create-a-gitlab-webhook) to validate.", - projectHookErr: errors.New("Unable to get project hooks"), //true, + projectHookErr: errors.New("unable to get project hooks"), }, } func TestSubscribeCommand(t *testing.T) { for _, test := range subscribeCommandTests { t.Run(test.testName, func(t *testing.T) { - mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() channelID := "12345" - userInfo := &gitlab.GitlabUserInfo{} + userInfo := &gitlab.UserInfo{} p := getTestPlugin(t, mockCtrl, test.webhookInfo, test.mattermostURL, test.projectHookErr, test.mockGitlab) - subscribeMessage := p.subscribeCommand(test.paramaters, channelID, &configuration{}, userInfo) + subscribeMessage := p.subscribeCommand(test.parameters, channelID, &configuration{}, userInfo) assert.Equal(t, test.want, subscribeMessage, "Subscribe command message should be the same.") }) - } } type webhookCommandTest struct { testName string - paramaters []string + parameters []string scope string webhookInfo []*gitlab.WebhookInfo want string siteURL string webhook *gitlab.WebhookInfo - secretToken string } var listWebhookCommandTests = []webhookCommandTest{ { testName: "List Project hooks", - paramaters: []string{"list", "group/project"}, + parameters: []string{"list", "group/project"}, scope: "project", webhookInfo: []*gitlab.WebhookInfo{ { @@ -128,7 +126,7 @@ Triggers: }, { testName: "List multiple project hooks", - paramaters: []string{"list", "group/project"}, + parameters: []string{"list", "group/project"}, scope: "project", webhookInfo: []*gitlab.WebhookInfo{ { @@ -150,7 +148,7 @@ Triggers: }, { testName: "List Group hooks", - paramaters: []string{"list", "group"}, + parameters: []string{"list", "group"}, scope: "group", webhookInfo: []*gitlab.WebhookInfo{ { @@ -178,12 +176,12 @@ Triggers: }, { testName: "Unrecognized sub command", - paramaters: []string{"invalid", "group"}, + parameters: []string{"invalid", "group"}, want: "Unknown webhook command: invalid", }, { testName: "List missing namespace", - paramaters: []string{"list"}, + parameters: []string{"list"}, want: "Unknown action, please use `/gitlab help` to see all actions available.", }, } @@ -205,7 +203,7 @@ func TestListWebhookCommand(t *testing.T) { p.GitlabClient = mockedClient } - got := p.webhookCommand(test.paramaters, &gitlab.GitlabUserInfo{}) + got := p.webhookCommand(test.parameters, &gitlab.UserInfo{}) assert.Equal(t, test.want, got) }) } @@ -271,21 +269,21 @@ Triggers: var addWebhookCommandTests = []webhookCommandTest{ { testName: "Create project hook with defaults", - paramaters: []string{"add", "group/project"}, + parameters: []string{"add", "group/project"}, want: "Webhook Created:\n\n\n`https://example.com`" + allTriggersFormated, siteURL: "https://example.com", webhook: exampleWebhookWithAlltriggers, }, { testName: "Create project hook with all trigers", - paramaters: []string{"add", "group/project", "*"}, + parameters: []string{"add", "group/project", "*"}, want: "Webhook Created:\n\n\n`https://example.com`" + allTriggersFormated, siteURL: "https://example.com", webhook: exampleWebhookWithAlltriggers, }, { testName: "Create project hook with explicit trigers", - paramaters: []string{"add", "group/project", "PushEvents,MergeRequestEvents"}, + parameters: []string{"add", "group/project", "PushEvents,MergeRequestEvents"}, want: "Webhook Created:\n\n\n`https://example.com`" + ` Triggers: * Push Events @@ -300,7 +298,7 @@ Triggers: }, { testName: "Create project hook with explicit URL", - paramaters: []string{"add", "group/project", "*", "https://anothersite.com"}, + parameters: []string{"add", "group/project", "*", "https://anothersite.com"}, want: "Webhook Created:\n\n\n`https://anothersite.com`" + allTriggersFormated, siteURL: "https://example.com", webhook: &gitlab.WebhookInfo{ @@ -320,14 +318,14 @@ Triggers: }, { testName: "Create project hook with explicit token", - paramaters: []string{"add", "group/project", "*", "https://example.com", "1234abcd"}, + parameters: []string{"add", "group/project", "*", "https://example.com", "1234abcd"}, want: "Webhook Created:\n\n\n`https://example.com`" + allTriggersFormated, siteURL: "https://example.com", webhook: exampleWebhookWithAlltriggers, }, { testName: "Create Group hook with defaults", - paramaters: []string{"add", "group"}, + parameters: []string{"add", "group"}, want: "Webhook Created:\n\n\n`https://example.com`" + allTriggersFormated, siteURL: "https://example.com", scope: "group", @@ -359,7 +357,7 @@ func TestAddWebhookCommand(t *testing.T) { api.On("GetConfig", mock.Anything).Return(conf) p.SetAPI(api) - got := p.webhookCommand(test.paramaters, &gitlab.GitlabUserInfo{}) + got := p.webhookCommand(test.parameters, &gitlab.UserInfo{}) assert.Equal(t, test.want, got) }) diff --git a/server/configuration.go b/server/configuration.go index bc37bc3d..0a48ae22 100644 --- a/server/configuration.go +++ b/server/configuration.go @@ -5,9 +5,10 @@ import ( "net/url" "reflect" - "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" "github.com/mattermost/mattermost-server/v5/model" "github.com/pkg/errors" + + "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" ) // configuration captures the plugin's external configuration as exposed in the Mattermost server @@ -42,18 +43,18 @@ func (c *configuration) Clone() *configuration { // IsValid checks if all needed fields are set. func (c *configuration) IsValid() error { if _, err := url.ParseRequestURI(c.GitlabURL); err != nil { - return errors.New("Must have a valid GitLab URL") + return errors.New("must have a valid GitLab URL") } if c.GitlabOAuthClientID == "" { - return fmt.Errorf("Must have a GitLab oauth client id") + return fmt.Errorf("must have a GitLab oauth client id") } if c.GitlabOAuthClientSecret == "" { - return fmt.Errorf("Must have a GitLab oauth client secret") + return fmt.Errorf("must have a GitLab oauth client secret") } if c.EncryptionKey == "" { - return fmt.Errorf("Must have an encryption key") + return fmt.Errorf("must have an encryption key") } return nil diff --git a/server/gitlab/api.go b/server/gitlab/api.go index 7302ac40..a5a2ee1f 100644 --- a/server/gitlab/api.go +++ b/server/gitlab/api.go @@ -11,8 +11,13 @@ import ( "golang.org/x/sync/errgroup" ) +const ( + stateOpened = "opened" + scopeAll = "all" +) + // NewGroupHook creates a webhook associated with a GitLab group -func (g *gitlab) NewGroupHook(user *GitlabUserInfo, groupName string, webhookOptions *AddWebhookOptions) (*WebhookInfo, error) { +func (g *gitlab) NewGroupHook(user *UserInfo, groupName string, webhookOptions *AddWebhookOptions) (*WebhookInfo, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err @@ -50,7 +55,7 @@ func (g *gitlab) NewGroupHook(user *GitlabUserInfo, groupName string, webhookOpt } // NewProjectHook creates a webhook associated with a GitLab project -func (g *gitlab) NewProjectHook(user *GitlabUserInfo, projectID interface{}, webhookOptions *AddWebhookOptions) (*WebhookInfo, error) { +func (g *gitlab) NewProjectHook(user *UserInfo, projectID interface{}, webhookOptions *AddWebhookOptions) (*WebhookInfo, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err @@ -83,7 +88,7 @@ func (g *gitlab) NewProjectHook(user *GitlabUserInfo, projectID interface{}, web } // GetGroupHooks gathers all the group level hooks for a GitLab group. -func (g *gitlab) GetGroupHooks(user *GitlabUserInfo, owner string) ([]*WebhookInfo, error) { +func (g *gitlab) GetGroupHooks(user *UserInfo, owner string) ([]*WebhookInfo, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err @@ -102,7 +107,7 @@ func (g *gitlab) GetGroupHooks(user *GitlabUserInfo, owner string) ([]*WebhookIn return webhooks, nil } -//String produces a multiline bulleted string for displaying webhook information. +// String produces a multiline bulleted string for displaying webhook information. func (w *WebhookInfo) String() string { var formatedTriggers string if w.EnableSSLVerification { @@ -187,7 +192,7 @@ func getGroupHookInfo(hook *internGitlab.GroupHook) *WebhookInfo { } // GetProjectHooks gathers all the project level hooks from a single GitLab project. -func (g *gitlab) GetProjectHooks(user *GitlabUserInfo, owner string, repo string) ([]*WebhookInfo, error) { +func (g *gitlab) GetProjectHooks(user *UserInfo, owner string, repo string) ([]*WebhookInfo, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err @@ -205,7 +210,7 @@ func (g *gitlab) GetProjectHooks(user *GitlabUserInfo, owner string, repo string return webhooks, err } -func (g *gitlab) GetProject(user *GitlabUserInfo, owner, repo string) (*internGitlab.Project, error) { +func (g *gitlab) GetProject(user *UserInfo, owner, repo string) (*internGitlab.Project, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err @@ -215,27 +220,27 @@ func (g *gitlab) GetProject(user *GitlabUserInfo, owner, repo string) (*internGi return result, err } -func (g *gitlab) GetReviews(user *GitlabUserInfo) ([]*internGitlab.MergeRequest, error) { +func (g *gitlab) GetReviews(user *UserInfo) ([]*internGitlab.MergeRequest, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err } - opened := "opened" - scope := "all" + opened := stateOpened + scope := scopeAll var result []*internGitlab.MergeRequest var errRequest error if g.gitlabGroup == "" { result, _, errRequest = client.MergeRequests.ListMergeRequests(&internGitlab.ListMergeRequestsOptions{ - AssigneeID: &user.GitlabUserId, + AssigneeID: &user.GitlabUserID, State: &opened, Scope: &scope, }) } else { result, _, errRequest = client.MergeRequests.ListGroupMergeRequests(g.gitlabGroup, &internGitlab.ListGroupMergeRequestsOptions{ - AssigneeID: &user.GitlabUserId, + AssigneeID: &user.GitlabUserID, State: &opened, Scope: &scope, }) @@ -244,27 +249,27 @@ func (g *gitlab) GetReviews(user *GitlabUserInfo) ([]*internGitlab.MergeRequest, return result, errRequest } -func (g *gitlab) GetYourPrs(user *GitlabUserInfo) ([]*internGitlab.MergeRequest, error) { +func (g *gitlab) GetYourPrs(user *UserInfo) ([]*internGitlab.MergeRequest, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err } - opened := "opened" - scope := "all" + opened := stateOpened + scope := scopeAll var result []*internGitlab.MergeRequest var errRequest error if g.gitlabGroup == "" { result, _, errRequest = client.MergeRequests.ListMergeRequests(&internGitlab.ListMergeRequestsOptions{ - AuthorID: &user.GitlabUserId, + AuthorID: &user.GitlabUserID, State: &opened, Scope: &scope, }) } else { result, _, errRequest = client.MergeRequests.ListGroupMergeRequests(g.gitlabGroup, &internGitlab.ListGroupMergeRequestsOptions{ - AuthorID: &user.GitlabUserId, + AuthorID: &user.GitlabUserID, State: &opened, Scope: &scope, }) @@ -273,27 +278,27 @@ func (g *gitlab) GetYourPrs(user *GitlabUserInfo) ([]*internGitlab.MergeRequest, return result, errRequest } -func (g *gitlab) GetYourAssignments(user *GitlabUserInfo) ([]*internGitlab.Issue, error) { +func (g *gitlab) GetYourAssignments(user *UserInfo) ([]*internGitlab.Issue, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err } - opened := "opened" - scope := "all" + opened := stateOpened + scope := scopeAll var result []*internGitlab.Issue var errRequest error if g.gitlabGroup == "" { result, _, errRequest = client.Issues.ListIssues(&internGitlab.ListIssuesOptions{ - AssigneeID: &user.GitlabUserId, + AssigneeID: &user.GitlabUserID, State: &opened, Scope: &scope, }) } else { result, _, errRequest = client.Issues.ListGroupIssues(g.gitlabGroup, &internGitlab.ListGroupIssuesOptions{ - AssigneeID: &user.GitlabUserId, + AssigneeID: &user.GitlabUserID, State: &opened, Scope: &scope, }) @@ -302,7 +307,7 @@ func (g *gitlab) GetYourAssignments(user *GitlabUserInfo) ([]*internGitlab.Issue return result, errRequest } -func (g *gitlab) GetUnreads(user *GitlabUserInfo) ([]*internGitlab.Todo, error) { +func (g *gitlab) GetUnreads(user *UserInfo) ([]*internGitlab.Todo, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err @@ -324,11 +329,10 @@ func (g *gitlab) GetUnreads(user *GitlabUserInfo) ([]*internGitlab.Todo, error) } func (g *gitlab) ResolveNamespaceAndProject( - userInfo *GitlabUserInfo, + userInfo *UserInfo, fullPath string, allowPrivate bool, ) (owner string, repo string, err error) { - // Initialize client client, err := g.gitlabConnect(*userInfo.Token) if err != nil { @@ -385,15 +389,19 @@ func (g *gitlab) ResolveNamespaceAndProject( // Decide what to return if user != nil { return user.Username, "", nil - } else if group != nil { - if !allowPrivate && &group.Visibility != nil && group.Visibility != internGitlab.PublicVisibility { + } + + if group != nil { + if !allowPrivate && group.Visibility != internGitlab.PublicVisibility { return "", "", fmt.Errorf( "you can't add a private group on this Mattermost instance: %w", ErrPrivateResource, ) } return group.FullPath, "", nil - } else if project != nil { + } + + if project != nil { if !allowPrivate && project.Visibility != internGitlab.PublicVisibility { return "", "", fmt.Errorf( "you can't add a private project on this Mattermost instance: %w", diff --git a/server/gitlab/gitlab.go b/server/gitlab/gitlab.go index 7e9d423d..9b992912 100644 --- a/server/gitlab/gitlab.go +++ b/server/gitlab/gitlab.go @@ -22,24 +22,24 @@ var ( // Gitlab is a client to call GitLab api see New() to build one type Gitlab interface { - GetCurrentUser(userID string, token oauth2.Token) (*GitlabUserInfo, error) - GetUserDetails(user *GitlabUserInfo) (*internGitlab.User, error) - GetProject(user *GitlabUserInfo, owner, repo string) (*internGitlab.Project, error) - GetReviews(user *GitlabUserInfo) ([]*internGitlab.MergeRequest, error) - GetYourPrs(user *GitlabUserInfo) ([]*internGitlab.MergeRequest, error) - GetYourAssignments(user *GitlabUserInfo) ([]*internGitlab.Issue, error) - GetUnreads(user *GitlabUserInfo) ([]*internGitlab.Todo, error) - GetProjectHooks(user *GitlabUserInfo, owner string, repo string) ([]*WebhookInfo, error) - GetGroupHooks(user *GitlabUserInfo, owner string) ([]*WebhookInfo, error) - NewProjectHook(user *GitlabUserInfo, projectID interface{}, projectHookOptions *AddWebhookOptions) (*WebhookInfo, error) - NewGroupHook(user *GitlabUserInfo, groupName string, groupHookOptions *AddWebhookOptions) (*WebhookInfo, error) + GetCurrentUser(userID string, token oauth2.Token) (*UserInfo, error) + GetUserDetails(user *UserInfo) (*internGitlab.User, error) + GetProject(user *UserInfo, owner, repo string) (*internGitlab.Project, error) + GetReviews(user *UserInfo) ([]*internGitlab.MergeRequest, error) + GetYourPrs(user *UserInfo) ([]*internGitlab.MergeRequest, error) + GetYourAssignments(user *UserInfo) ([]*internGitlab.Issue, error) + GetUnreads(user *UserInfo) ([]*internGitlab.Todo, error) + GetProjectHooks(user *UserInfo, owner string, repo string) ([]*WebhookInfo, error) + GetGroupHooks(user *UserInfo, owner string) ([]*WebhookInfo, error) + NewProjectHook(user *UserInfo, projectID interface{}, projectHookOptions *AddWebhookOptions) (*WebhookInfo, error) + NewGroupHook(user *UserInfo, groupName string, groupHookOptions *AddWebhookOptions) (*WebhookInfo, error) // ResolveNamespaceAndProject accepts full path to User, Group or namespaced Project and returns corresponding // namespace and project name. // // ErrNotFound will be returned if no resource can be found. // If allowPrivate is set to false, and resolved group/project is private, ErrPrivateResource will be returned. ResolveNamespaceAndProject( - userInfo *GitlabUserInfo, + userInfo *UserInfo, fullPath string, allowPrivate bool, ) (namespace string, project string, err error) @@ -55,9 +55,9 @@ type gitlab struct { type Scope int const ( - //Group is a type for group hooks + // Group is a type for group hooks Group Scope = iota - //Project is a type for project hooks + // Project is a type for project hooks Project ) diff --git a/server/gitlab/mocks/mock_gitlab.go b/server/gitlab/mocks/mock_gitlab.go index 8e640612..85efafd2 100644 --- a/server/gitlab/mocks/mock_gitlab.go +++ b/server/gitlab/mocks/mock_gitlab.go @@ -5,11 +5,13 @@ package mock_gitlab import ( + reflect "reflect" + gomock "github.com/golang/mock/gomock" - gitlab "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" gitlab0 "github.com/xanzy/go-gitlab" oauth2 "golang.org/x/oauth2" - reflect "reflect" + + gitlab "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" ) // MockGitlab is a mock of Gitlab interface @@ -36,10 +38,10 @@ func (m *MockGitlab) EXPECT() *MockGitlabMockRecorder { } // GetCurrentUser mocks base method -func (m *MockGitlab) GetCurrentUser(userID string, token oauth2.Token) (*gitlab.GitlabUserInfo, error) { +func (m *MockGitlab) GetCurrentUser(userID string, token oauth2.Token) (*gitlab.UserInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetCurrentUser", userID, token) - ret0, _ := ret[0].(*gitlab.GitlabUserInfo) + ret0, _ := ret[0].(*gitlab.UserInfo) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -51,7 +53,7 @@ func (mr *MockGitlabMockRecorder) GetCurrentUser(userID, token interface{}) *gom } // GetUserDetails mocks base method -func (m *MockGitlab) GetUserDetails(user *gitlab.GitlabUserInfo) (*gitlab0.User, error) { +func (m *MockGitlab) GetUserDetails(user *gitlab.UserInfo) (*gitlab0.User, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetUserDetails", user) ret0, _ := ret[0].(*gitlab0.User) @@ -66,7 +68,7 @@ func (mr *MockGitlabMockRecorder) GetUserDetails(user interface{}) *gomock.Call } // GetProject mocks base method -func (m *MockGitlab) GetProject(user *gitlab.GitlabUserInfo, owner, repo string) (*gitlab0.Project, error) { +func (m *MockGitlab) GetProject(user *gitlab.UserInfo, owner, repo string) (*gitlab0.Project, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetProject", user, owner, repo) ret0, _ := ret[0].(*gitlab0.Project) @@ -81,7 +83,7 @@ func (mr *MockGitlabMockRecorder) GetProject(user, owner, repo interface{}) *gom } // GetReviews mocks base method -func (m *MockGitlab) GetReviews(user *gitlab.GitlabUserInfo) ([]*gitlab0.MergeRequest, error) { +func (m *MockGitlab) GetReviews(user *gitlab.UserInfo) ([]*gitlab0.MergeRequest, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetReviews", user) ret0, _ := ret[0].([]*gitlab0.MergeRequest) @@ -96,7 +98,7 @@ func (mr *MockGitlabMockRecorder) GetReviews(user interface{}) *gomock.Call { } // GetYourPrs mocks base method -func (m *MockGitlab) GetYourPrs(user *gitlab.GitlabUserInfo) ([]*gitlab0.MergeRequest, error) { +func (m *MockGitlab) GetYourPrs(user *gitlab.UserInfo) ([]*gitlab0.MergeRequest, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetYourPrs", user) ret0, _ := ret[0].([]*gitlab0.MergeRequest) @@ -111,7 +113,7 @@ func (mr *MockGitlabMockRecorder) GetYourPrs(user interface{}) *gomock.Call { } // GetYourAssignments mocks base method -func (m *MockGitlab) GetYourAssignments(user *gitlab.GitlabUserInfo) ([]*gitlab0.Issue, error) { +func (m *MockGitlab) GetYourAssignments(user *gitlab.UserInfo) ([]*gitlab0.Issue, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetYourAssignments", user) ret0, _ := ret[0].([]*gitlab0.Issue) @@ -126,7 +128,7 @@ func (mr *MockGitlabMockRecorder) GetYourAssignments(user interface{}) *gomock.C } // GetUnreads mocks base method -func (m *MockGitlab) GetUnreads(user *gitlab.GitlabUserInfo) ([]*gitlab0.Todo, error) { +func (m *MockGitlab) GetUnreads(user *gitlab.UserInfo) ([]*gitlab0.Todo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetUnreads", user) ret0, _ := ret[0].([]*gitlab0.Todo) @@ -141,7 +143,7 @@ func (mr *MockGitlabMockRecorder) GetUnreads(user interface{}) *gomock.Call { } // GetProjectHooks mocks base method -func (m *MockGitlab) GetProjectHooks(user *gitlab.GitlabUserInfo, owner, repo string) ([]*gitlab.WebhookInfo, error) { +func (m *MockGitlab) GetProjectHooks(user *gitlab.UserInfo, owner, repo string) ([]*gitlab.WebhookInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetProjectHooks", user, owner, repo) ret0, _ := ret[0].([]*gitlab.WebhookInfo) @@ -156,7 +158,7 @@ func (mr *MockGitlabMockRecorder) GetProjectHooks(user, owner, repo interface{}) } // GetGroupHooks mocks base method -func (m *MockGitlab) GetGroupHooks(user *gitlab.GitlabUserInfo, owner string) ([]*gitlab.WebhookInfo, error) { +func (m *MockGitlab) GetGroupHooks(user *gitlab.UserInfo, owner string) ([]*gitlab.WebhookInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetGroupHooks", user, owner) ret0, _ := ret[0].([]*gitlab.WebhookInfo) @@ -171,7 +173,7 @@ func (mr *MockGitlabMockRecorder) GetGroupHooks(user, owner interface{}) *gomock } // NewProjectHook mocks base method -func (m *MockGitlab) NewProjectHook(user *gitlab.GitlabUserInfo, projectID interface{}, projectHookOptions *gitlab.AddWebhookOptions) (*gitlab.WebhookInfo, error) { +func (m *MockGitlab) NewProjectHook(user *gitlab.UserInfo, projectID interface{}, projectHookOptions *gitlab.AddWebhookOptions) (*gitlab.WebhookInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewProjectHook", user, projectID, projectHookOptions) ret0, _ := ret[0].(*gitlab.WebhookInfo) @@ -186,7 +188,7 @@ func (mr *MockGitlabMockRecorder) NewProjectHook(user, projectID, projectHookOpt } // NewGroupHook mocks base method -func (m *MockGitlab) NewGroupHook(user *gitlab.GitlabUserInfo, groupName string, groupHookOptions *gitlab.AddWebhookOptions) (*gitlab.WebhookInfo, error) { +func (m *MockGitlab) NewGroupHook(user *gitlab.UserInfo, groupName string, groupHookOptions *gitlab.AddWebhookOptions) (*gitlab.WebhookInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewGroupHook", user, groupName, groupHookOptions) ret0, _ := ret[0].(*gitlab.WebhookInfo) @@ -201,7 +203,7 @@ func (mr *MockGitlabMockRecorder) NewGroupHook(user, groupName, groupHookOptions } // ResolveNamespaceAndProject mocks base method -func (m *MockGitlab) ResolveNamespaceAndProject(userInfo *gitlab.GitlabUserInfo, fullPath string, allowPrivate bool) (string, string, error) { +func (m *MockGitlab) ResolveNamespaceAndProject(userInfo *gitlab.UserInfo, fullPath string, allowPrivate bool) (string, string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ResolveNamespaceAndProject", userInfo, fullPath, allowPrivate) ret0, _ := ret[0].(string) diff --git a/server/gitlab/user.go b/server/gitlab/user.go index 04cc2786..fbf3567b 100644 --- a/server/gitlab/user.go +++ b/server/gitlab/user.go @@ -6,13 +6,13 @@ import ( "golang.org/x/oauth2" ) -const SETTING_BUTTONS_TEAM = "team" +const SettingButtonsTeam = "team" -type GitlabUserInfo struct { +type UserInfo struct { UserID string Token *oauth2.Token GitlabUsername string - GitlabUserId int + GitlabUserID int LastToDoPostAt int64 Settings *UserSettings AllowedPrivateRepos bool @@ -24,7 +24,7 @@ type UserSettings struct { Notifications bool `json:"notifications"` } -func (g *gitlab) GetCurrentUser(userID string, token oauth2.Token) (*GitlabUserInfo, error) { +func (g *gitlab) GetCurrentUser(userID string, token oauth2.Token) (*UserInfo, error) { client, err := g.gitlabConnect(token) if err != nil { return nil, err @@ -35,21 +35,21 @@ func (g *gitlab) GetCurrentUser(userID string, token oauth2.Token) (*GitlabUserI return nil, err } - return &GitlabUserInfo{ + return &UserInfo{ UserID: userID, - GitlabUserId: gitUser.ID, + GitlabUserID: gitUser.ID, Token: &token, GitlabUsername: gitUser.Username, LastToDoPostAt: model.GetMillis(), Settings: &UserSettings{ - SidebarButtons: SETTING_BUTTONS_TEAM, + SidebarButtons: SettingButtonsTeam, DailyReminder: true, Notifications: true, }, }, nil } -func (g *gitlab) GetUserDetails(user *GitlabUserInfo) (*internGitlab.User, error) { +func (g *gitlab) GetUserDetails(user *UserInfo) (*internGitlab.User, error) { client, err := g.gitlabConnect(*user.Token) if err != nil { return nil, err diff --git a/server/gitlab/webhook.go b/server/gitlab/webhook.go index d0b284af..2f25fbaa 100644 --- a/server/gitlab/webhook.go +++ b/server/gitlab/webhook.go @@ -21,7 +21,7 @@ type WebhookInfo struct { Scope Scope } -//AddWebhookOptions is a paramater object with options for creating a project or group hook. +// AddWebhookOptions is a paramater object with options for creating a project or group hook. type AddWebhookOptions struct { URL string ConfidentialNoteEvents bool diff --git a/server/mocks/mock_gitlab.go b/server/mocks/mock_gitlab.go index fbbf7a5f..e2469e3b 100644 --- a/server/mocks/mock_gitlab.go +++ b/server/mocks/mock_gitlab.go @@ -5,11 +5,13 @@ package mock_gitlab import ( + reflect "reflect" + gomock "github.com/golang/mock/gomock" - gitlab "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" gitlab0 "github.com/xanzy/go-gitlab" oauth2 "golang.org/x/oauth2" - reflect "reflect" + + gitlab "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" ) // MockGitlab is a mock of Gitlab interface @@ -36,10 +38,10 @@ func (m *MockGitlab) EXPECT() *MockGitlabMockRecorder { } // GetCurrentUser mocks base method -func (m *MockGitlab) GetCurrentUser(userID string, token oauth2.Token) (*gitlab.GitlabUserInfo, error) { +func (m *MockGitlab) GetCurrentUser(userID string, token oauth2.Token) (*gitlab.UserInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetCurrentUser", userID, token) - ret0, _ := ret[0].(*gitlab.GitlabUserInfo) + ret0, _ := ret[0].(*gitlab.UserInfo) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -51,7 +53,7 @@ func (mr *MockGitlabMockRecorder) GetCurrentUser(userID, token interface{}) *gom } // GetUserDetails mocks base method -func (m *MockGitlab) GetUserDetails(user *gitlab.GitlabUserInfo) (*gitlab0.User, error) { +func (m *MockGitlab) GetUserDetails(user *gitlab.UserInfo) (*gitlab0.User, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetUserDetails", user) ret0, _ := ret[0].(*gitlab0.User) @@ -66,7 +68,7 @@ func (mr *MockGitlabMockRecorder) GetUserDetails(user interface{}) *gomock.Call } // GetProject mocks base method -func (m *MockGitlab) GetProject(user *gitlab.GitlabUserInfo, owner, repo string) (*gitlab0.Project, error) { +func (m *MockGitlab) GetProject(user *gitlab.UserInfo, owner, repo string) (*gitlab0.Project, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetProject", user, owner, repo) ret0, _ := ret[0].(*gitlab0.Project) @@ -81,7 +83,7 @@ func (mr *MockGitlabMockRecorder) GetProject(user, owner, repo interface{}) *gom } // GetReviews mocks base method -func (m *MockGitlab) GetReviews(user *gitlab.GitlabUserInfo) ([]*gitlab0.MergeRequest, error) { +func (m *MockGitlab) GetReviews(user *gitlab.UserInfo) ([]*gitlab0.MergeRequest, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetReviews", user) ret0, _ := ret[0].([]*gitlab0.MergeRequest) @@ -96,7 +98,7 @@ func (mr *MockGitlabMockRecorder) GetReviews(user interface{}) *gomock.Call { } // GetYourPrs mocks base method -func (m *MockGitlab) GetYourPrs(user *gitlab.GitlabUserInfo) ([]*gitlab0.MergeRequest, error) { +func (m *MockGitlab) GetYourPrs(user *gitlab.UserInfo) ([]*gitlab0.MergeRequest, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetYourPrs", user) ret0, _ := ret[0].([]*gitlab0.MergeRequest) @@ -111,7 +113,7 @@ func (mr *MockGitlabMockRecorder) GetYourPrs(user interface{}) *gomock.Call { } // GetYourAssignments mocks base method -func (m *MockGitlab) GetYourAssignments(user *gitlab.GitlabUserInfo) ([]*gitlab0.Issue, error) { +func (m *MockGitlab) GetYourAssignments(user *gitlab.UserInfo) ([]*gitlab0.Issue, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetYourAssignments", user) ret0, _ := ret[0].([]*gitlab0.Issue) @@ -126,7 +128,7 @@ func (mr *MockGitlabMockRecorder) GetYourAssignments(user interface{}) *gomock.C } // GetUnreads mocks base method -func (m *MockGitlab) GetUnreads(user *gitlab.GitlabUserInfo) ([]*gitlab0.Todo, error) { +func (m *MockGitlab) GetUnreads(user *gitlab.UserInfo) ([]*gitlab0.Todo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetUnreads", user) ret0, _ := ret[0].([]*gitlab0.Todo) @@ -141,7 +143,7 @@ func (mr *MockGitlabMockRecorder) GetUnreads(user interface{}) *gomock.Call { } // GetProjectHooks mocks base method -func (m *MockGitlab) GetProjectHooks(user *gitlab.GitlabUserInfo, owner, repo string) ([]*gitlab.WebhookInfo, error) { +func (m *MockGitlab) GetProjectHooks(user *gitlab.UserInfo, owner, repo string) ([]*gitlab.WebhookInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetProjectHooks", user, owner, repo) ret0, _ := ret[0].([]*gitlab.WebhookInfo) @@ -156,7 +158,7 @@ func (mr *MockGitlabMockRecorder) GetProjectHooks(user, owner, repo interface{}) } // GetGroupHooks mocks base method -func (m *MockGitlab) GetGroupHooks(user *gitlab.GitlabUserInfo, owner string) ([]*gitlab.WebhookInfo, error) { +func (m *MockGitlab) GetGroupHooks(user *gitlab.UserInfo, owner string) ([]*gitlab.WebhookInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetGroupHooks", user, owner) ret0, _ := ret[0].([]*gitlab.WebhookInfo) @@ -171,7 +173,7 @@ func (mr *MockGitlabMockRecorder) GetGroupHooks(user, owner interface{}) *gomock } // NewProjectHook mocks base method -func (m *MockGitlab) NewProjectHook(user *gitlab.GitlabUserInfo, projectID interface{}, projectHookOptions *gitlab0.AddProjectHookOptions) (*gitlab0.ProjectHook, error) { +func (m *MockGitlab) NewProjectHook(user *gitlab.UserInfo, projectID interface{}, projectHookOptions *gitlab0.AddProjectHookOptions) (*gitlab0.ProjectHook, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewProjectHook", user, projectID, projectHookOptions) ret0, _ := ret[0].(*gitlab0.ProjectHook) @@ -186,7 +188,7 @@ func (mr *MockGitlabMockRecorder) NewProjectHook(user, projectID, projectHookOpt } // ResolveNamespaceAndProject mocks base method -func (m *MockGitlab) ResolveNamespaceAndProject(userInfo *gitlab.GitlabUserInfo, fullPath string, allowPrivate bool) (string, string, error) { +func (m *MockGitlab) ResolveNamespaceAndProject(userInfo *gitlab.UserInfo, fullPath string, allowPrivate bool) (string, string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ResolveNamespaceAndProject", userInfo, fullPath, allowPrivate) ret0, _ := ret[0].(string) diff --git a/server/plugin.go b/server/plugin.go index d5bbcb80..75b95914 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -11,29 +11,29 @@ import ( "strings" "sync" - "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" - "github.com/mattermost/mattermost-plugin-gitlab/server/webhook" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin" "github.com/pkg/errors" - "golang.org/x/oauth2" + + "github.com/mattermost/mattermost-plugin-gitlab/server/gitlab" + "github.com/mattermost/mattermost-plugin-gitlab/server/webhook" ) const ( - GITLAB_TOKEN_KEY = "_gitlabtoken" - GITLAB_USERNAME_KEY = "_gitlabusername" - GITLAB_IDUSERNAME_KEY = "_gitlabidusername" - WS_EVENT_CONNECT = "gitlab_connect" - WS_EVENT_DISCONNECT = "gitlab_disconnect" - WS_EVENT_REFRESH = "gitlab_refresh" - SETTING_NOTIFICATIONS = "notifications" - SETTING_REMINDERS = "reminders" - SETTING_ON = "on" - SETTING_OFF = "off" + GitlabTokenKey = "_gitlabtoken" + GitlabUsernameKey = "_gitlabusername" + GitlabIDUsernameKey = "_gitlabidusername" + WsEventConnect = "gitlab_connect" + WsEventDisconnect = "gitlab_disconnect" + WsEventRefresh = "gitlab_refresh" + SettingNotifications = "notifications" + SettingReminders = "reminders" + SettingOn = "on" + SettingOff = "off" ) -var emptySiteURLErr = errors.New("SiteURL is not set. Please set it and restart the plugin.") +var errEmptySiteURL = errors.New("siteURL is not set. Please set it and restart the plugin") type Plugin struct { plugin.MattermostPlugin @@ -73,7 +73,7 @@ func (p *Plugin) OnActivate() error { bundlePath, err := p.API.GetBundlePath() if err != nil { - return errors.Wrap(err, "can't retreive bundle path") + return errors.Wrap(err, "can't retrieve bundle path") } profileImage, err := ioutil.ReadFile(filepath.Join(bundlePath, "assets", "profile.png")) if err != nil { @@ -85,7 +85,7 @@ func (p *Plugin) OnActivate() error { siteURL := *p.API.GetConfig().ServiceSettings.SiteURL if siteURL == "" { - return emptySiteURLErr + return errEmptySiteURL } return nil @@ -112,7 +112,7 @@ func (p *Plugin) getOAuthConfig() *oauth2.Config { } } -func (p *Plugin) storeGitlabUserInfo(info *gitlab.GitlabUserInfo) error { +func (p *Plugin) storeGitlabUserInfo(info *gitlab.UserInfo) error { config := p.getConfiguration() encryptedToken, err := encrypt([]byte(config.EncryptionKey), info.Token.AccessToken) @@ -127,20 +127,20 @@ func (p *Plugin) storeGitlabUserInfo(info *gitlab.GitlabUserInfo) error { return err } - if err := p.API.KVSet(info.UserID+GITLAB_TOKEN_KEY, jsonInfo); err != nil { + if err := p.API.KVSet(info.UserID+GitlabTokenKey, jsonInfo); err != nil { return err } return nil } -func (p *Plugin) getGitlabUserInfoByMattermostID(userID string) (*gitlab.GitlabUserInfo, *APIErrorResponse) { +func (p *Plugin) getGitlabUserInfoByMattermostID(userID string) (*gitlab.UserInfo, *APIErrorResponse) { config := p.getConfiguration() - var userInfo gitlab.GitlabUserInfo + var userInfo gitlab.UserInfo - if infoBytes, err := p.API.KVGet(userID + GITLAB_TOKEN_KEY); err != nil || infoBytes == nil { - return nil, &APIErrorResponse{ID: API_ERROR_ID_NOT_CONNECTED, Message: "Must connect user account to GitLab first.", StatusCode: http.StatusBadRequest} + if infoBytes, err := p.API.KVGet(userID + GitlabTokenKey); err != nil || infoBytes == nil { + return nil, &APIErrorResponse{ID: APIErrorIDNotConnected, Message: "Must connect user account to GitLab first.", StatusCode: http.StatusBadRequest} } else if err := json.Unmarshal(infoBytes, &userInfo); err != nil { return nil, &APIErrorResponse{ID: "", Message: "Unable to parse token.", StatusCode: http.StatusInternalServerError} } @@ -157,17 +157,17 @@ func (p *Plugin) getGitlabUserInfoByMattermostID(userID string) (*gitlab.GitlabU } func (p *Plugin) storeGitlabToUserIDMapping(gitlabUsername, userID string) error { - if err := p.API.KVSet(gitlabUsername+GITLAB_USERNAME_KEY, []byte(userID)); err != nil { - return fmt.Errorf("Encountered error saving GitLab username mapping") + if err := p.API.KVSet(gitlabUsername+GitlabUsernameKey, []byte(userID)); err != nil { + return fmt.Errorf("encountered error saving GitLab username mapping") } - if err := p.API.KVSet(userID+GITLAB_IDUSERNAME_KEY, []byte(gitlabUsername)); err != nil { - return fmt.Errorf("Encountered error saving GitLab id mapping") + if err := p.API.KVSet(userID+GitlabIDUsernameKey, []byte(gitlabUsername)); err != nil { + return fmt.Errorf("encountered error saving GitLab id mapping") } return nil } func (p *Plugin) getGitlabToUserIDMapping(gitlabUsername string) string { - userID, err := p.API.KVGet(gitlabUsername + GITLAB_USERNAME_KEY) + userID, err := p.API.KVGet(gitlabUsername + GitlabUsernameKey) if err != nil { p.API.LogError("can't get userId from store with username", "err", err.DetailedError, "username", gitlabUsername) } @@ -175,7 +175,7 @@ func (p *Plugin) getGitlabToUserIDMapping(gitlabUsername string) string { } func (p *Plugin) getGitlabIDToUsernameMapping(gitlabUserID string) string { - gitlabUsername, err := p.API.KVGet(gitlabUserID + GITLAB_IDUSERNAME_KEY) + gitlabUsername, err := p.API.KVGet(gitlabUserID + GitlabIDUsernameKey) if err != nil { p.API.LogError("can't get user id by login", "err", err.DetailedError) } @@ -192,14 +192,14 @@ func (p *Plugin) disconnectGitlabAccount(userID string) { return } - if err := p.API.KVDelete(userID + GITLAB_TOKEN_KEY); err != nil { + if err := p.API.KVDelete(userID + GitlabTokenKey); err != nil { p.API.LogError("can't delete token in store", "err", err.DetailedError, "userId", userID) } - if err := p.API.KVDelete(userInfo.GitlabUsername + GITLAB_USERNAME_KEY); err != nil { + if err := p.API.KVDelete(userInfo.GitlabUsername + GitlabUsernameKey); err != nil { p.API.LogError("can't delete username in store", "err", err.DetailedError, "username", userInfo.GitlabUsername) } - if err := p.API.KVDelete(fmt.Sprintf("%d%s", userInfo.GitlabUserId, GITLAB_IDUSERNAME_KEY)); err != nil { - p.API.LogError("can't delete user id in sotre", "err", err.DetailedError, "id", userInfo.GitlabUserId) + if err := p.API.KVDelete(fmt.Sprintf("%d%s", userInfo.GitlabUserID, GitlabIDUsernameKey)); err != nil { + p.API.LogError("can't delete user id in sotre", "err", err.DetailedError, "id", userInfo.GitlabUserID) } if user, err := p.API.GetUser(userID); err == nil && user.Props != nil && len(user.Props["git_user"]) > 0 { @@ -210,7 +210,7 @@ func (p *Plugin) disconnectGitlabAccount(userID string) { } p.API.PublishWebSocketEvent( - WS_EVENT_DISCONNECT, + WsEventDisconnect, nil, &model.WebsocketBroadcast{UserId: userID}, ) @@ -238,7 +238,7 @@ func (p *Plugin) CreateBotDMPost(userID, message, postType string) *model.AppErr return nil } -func (p *Plugin) PostToDo(info *gitlab.GitlabUserInfo) { +func (p *Plugin) PostToDo(info *gitlab.UserInfo) { text, err := p.GetToDo(info) if err != nil { p.API.LogError("can't post todo", "err", err.Error()) @@ -250,7 +250,7 @@ func (p *Plugin) PostToDo(info *gitlab.GitlabUserInfo) { } } -func (p *Plugin) GetToDo(user *gitlab.GitlabUserInfo) (string, error) { +func (p *Plugin) GetToDo(user *gitlab.UserInfo) (string, error) { unreads, err := p.GitlabClient.GetUnreads(user) if err != nil { return "", err @@ -340,7 +340,7 @@ func (p *Plugin) isNamespaceAllowed(namespace string) error { func (p *Plugin) sendRefreshEvent(userID string) { p.API.PublishWebSocketEvent( - WS_EVENT_REFRESH, + WsEventRefresh, nil, &model.WebsocketBroadcast{UserId: userID}, ) @@ -348,13 +348,13 @@ func (p *Plugin) sendRefreshEvent(userID string) { // HasProjectHook checks if the subscribed GitLab Project or its parrent Group has a webhook // with a URL that matches the Mattermost Site URL. -func (p *Plugin) HasProjectHook(user *gitlab.GitlabUserInfo, namespace string, project string) (bool, error) { +func (p *Plugin) HasProjectHook(user *gitlab.UserInfo, namespace string, project string) (bool, error) { hooks, err := p.GitlabClient.GetProjectHooks(user, namespace, project) if err != nil { - return false, errors.New("Unable to connect to GitLab") + return false, errors.New("unable to connect to GitLab") } - //ignore error because many project won't be part of groups + // ignore error because many project won't be part of groups hasGroupHook, _ := p.HasGroupHook(user, namespace) if hasGroupHook { @@ -374,10 +374,10 @@ func (p *Plugin) HasProjectHook(user *gitlab.GitlabUserInfo, namespace string, p // HasGroupHook checks if the subscribed GitLab Group has a webhook // with a URL that matches the Mattermost Site URL. -func (p *Plugin) HasGroupHook(user *gitlab.GitlabUserInfo, namespace string) (bool, error) { +func (p *Plugin) HasGroupHook(user *gitlab.UserInfo, namespace string) (bool, error) { hooks, err := p.GitlabClient.GetGroupHooks(user, namespace) if err != nil { - return false, errors.New("Unable to connect to GitLab") + return false, errors.New("unable to connect to GitLab") } siteURL := *p.API.GetConfig().ServiceSettings.SiteURL diff --git a/server/subscription/subscription.go b/server/subscription/subscription.go index fee98986..1f6eea62 100644 --- a/server/subscription/subscription.go +++ b/server/subscription/subscription.go @@ -25,29 +25,28 @@ type Subscription struct { Repository string } -func New(ChannelID, CreatorID, Features, Repository string) (*Subscription, error) { - if strings.Contains(Features, "label:") && len(strings.Split(Features, "\"")) < 3 { - return nil, errors.New("The label is formatted incorrectly") +func New(channelID, creatorID, features, repository string) (*Subscription, error) { + if strings.Contains(features, "label:") && len(strings.Split(features, "\"")) < 3 { + return nil, errors.New("the label is formatted incorrectly") } - if strings.Contains(Features, "label:") && len(strings.Split(Features, "\"")) > 3 { - return nil, errors.New("Can't add multiple labels on the same subscription") + if strings.Contains(features, "label:") && len(strings.Split(features, "\"")) > 3 { + return nil, errors.New("can't add multiple labels on the same subscription") } - features := strings.Split(Features, ",") badFeatures := make([]string, 0) - for _, f := range features { - if _, ok := allFeatures[f]; !strings.HasPrefix(f, "label:") && !ok { - badFeatures = append(badFeatures, f) + for _, feature := range strings.Split(features, ",") { + if _, ok := allFeatures[feature]; !strings.HasPrefix(feature, "label:") && !ok { + badFeatures = append(badFeatures, feature) } } if len(badFeatures) > 0 { - return nil, fmt.Errorf("Unknown features %s", strings.Join(badFeatures, ",")) + return nil, fmt.Errorf("unknown features %s", strings.Join(badFeatures, ",")) } return &Subscription{ - ChannelID: ChannelID, - CreatorID: CreatorID, - Features: Features, - Repository: Repository, + ChannelID: channelID, + CreatorID: creatorID, + Features: features, + Repository: repository, }, nil } diff --git a/server/subscription/subscription_test.go b/server/subscription/subscription_test.go index 670334eb..7600f832 100644 --- a/server/subscription/subscription_test.go +++ b/server/subscription/subscription_test.go @@ -54,7 +54,7 @@ func TestNewSubscriptionAll(t *testing.T) { func TestNewSubscriptionUnknown(t *testing.T) { s, err := New("", "", "unknown,merges,missing", "") assert.Nil(t, s) - assert.Equal(t, err.Error(), "Unknown features unknown,missing") + assert.Equal(t, err.Error(), "unknown features unknown,missing") } func TestNewSubscriptionLabel(t *testing.T) { @@ -68,11 +68,11 @@ func TestNewSubscriptionLabel(t *testing.T) { func TestNewSubscriptionBadFormated(t *testing.T) { s, err := New("", "", `label:"`, "") assert.Nil(t, s) - assert.Equal(t, err.Error(), "The label is formatted incorrectly") + assert.Equal(t, err.Error(), "the label is formatted incorrectly") } func TestNewSubscriptionMultipleLabel(t *testing.T) { s, err := New("", "", `label:"1",label:"2"`, "") assert.Nil(t, s) - assert.Equal(t, err.Error(), "Can't add multiple labels on the same subscription") + assert.Equal(t, err.Error(), "can't add multiple labels on the same subscription") } diff --git a/server/subscriptions.go b/server/subscriptions.go index 2d8978e4..241cee05 100644 --- a/server/subscriptions.go +++ b/server/subscriptions.go @@ -10,14 +10,14 @@ import ( ) const ( - SUBSCRIPTIONS_KEY = "subscriptions" + SubscriptionsKey = "subscriptions" ) type Subscriptions struct { Repositories map[string][]*subscription.Subscription } -func (p *Plugin) Subscribe(info *gitlab.GitlabUserInfo, namespace, project, channelID, features string) error { +func (p *Plugin) Subscribe(info *gitlab.UserInfo, namespace, project, channelID, features string) error { if err := p.isNamespaceAllowed(namespace); err != nil { return err } @@ -84,7 +84,7 @@ func (p *Plugin) AddSubscription(fullPath string, sub *subscription.Subscription func (p *Plugin) GetSubscriptions() (*Subscriptions, error) { var subscriptions *Subscriptions - value, err := p.API.KVGet(SUBSCRIPTIONS_KEY) + value, err := p.API.KVGet(SubscriptionsKey) if err != nil { p.API.LogError("can't get subscriptions from kvstore", "err", err.DetailedError) return nil, err @@ -92,10 +92,8 @@ func (p *Plugin) GetSubscriptions() (*Subscriptions, error) { if value == nil { subscriptions = &Subscriptions{Repositories: map[string][]*subscription.Subscription{}} - } else { - if err := json.NewDecoder(bytes.NewReader(value)).Decode(&subscriptions); err != nil { - return nil, err - } + } else if err := json.NewDecoder(bytes.NewReader(value)).Decode(&subscriptions); err != nil { + return nil, err } return subscriptions, nil @@ -106,7 +104,7 @@ func (p *Plugin) StoreSubscriptions(s *Subscriptions) error { if err != nil { return err } - if err := p.API.KVSet(SUBSCRIPTIONS_KEY, b); err != nil { + if err := p.API.KVSet(SubscriptionsKey, b); err != nil { p.API.LogError("can't set subscriptions in kvstore", "err", err.DetailedError) } return nil @@ -117,7 +115,6 @@ func (p *Plugin) GetSubscribedChannelsForProject( project string, isPublicVisibility bool, ) []*subscription.Subscription { - var subsForRepo []*subscription.Subscription subs, err := p.GetSubscriptions() @@ -169,7 +166,6 @@ func (p *Plugin) Unsubscribe(channelID string, fullPath string) (bool, error) { // We don't know whether fullPath is a namespace or project, so we have to check both cases for _, path := range []string{fullPath, fullPath + "/"} { - pathSubs := subs.Repositories[path] if pathSubs == nil { continue diff --git a/server/subscriptions_test.go b/server/subscriptions_test.go index 42a854c6..9a62907e 100644 --- a/server/subscriptions_test.go +++ b/server/subscriptions_test.go @@ -18,15 +18,15 @@ type dataUnsubscribeTestStruct struct { var dataUnsubscribeTest = []dataUnsubscribeTestStruct{ { - name: "should delete existing subscripion", + name: "should delete existing subscription", channelID: "1", repoName: "owner/project", initMock: func() *plugintest.API { m := &plugintest.API{} kvget := `{"Repositories":{"owner/project":[{"ChannelID":"1","CreatorID":"1","Features":"all","Repository":"owner/project"}]}}` kvset := `{"Repositories":{}}` - m.On("KVGet", SUBSCRIPTIONS_KEY).Return([]byte(kvget), nil).Once() - m.On("KVSet", SUBSCRIPTIONS_KEY, []byte(kvset)).Return(nil).Once() + m.On("KVGet", SubscriptionsKey).Return([]byte(kvget), nil).Once() + m.On("KVSet", SubscriptionsKey, []byte(kvset)).Return(nil).Once() return m }, shouldDelete: true, @@ -39,8 +39,8 @@ var dataUnsubscribeTest = []dataUnsubscribeTestStruct{ m := &plugintest.API{} kvget := `{"Repositories":{"owner/project":[{"ChannelID":"1","CreatorID":"1","Features":"all","Repository":"owner/project"},{"ChannelID":"2","CreatorID":"1","Features":"all","Repository":"owner/project"}]}}` kvset := `{"Repositories":{"owner/project":[{"ChannelID":"2","CreatorID":"1","Features":"all","Repository":"owner/project"}]}}` - m.On("KVGet", SUBSCRIPTIONS_KEY).Return([]byte(kvget), nil).Once() - m.On("KVSet", SUBSCRIPTIONS_KEY, []byte(kvset)).Return(nil).Once() + m.On("KVGet", SubscriptionsKey).Return([]byte(kvget), nil).Once() + m.On("KVSet", SubscriptionsKey, []byte(kvset)).Return(nil).Once() return m }, shouldDelete: true, @@ -52,7 +52,7 @@ var dataUnsubscribeTest = []dataUnsubscribeTestStruct{ initMock: func() *plugintest.API { m := &plugintest.API{} kvget := `{"Repositories":{"owner/project":[{"ChannelID":"1","CreatorID":"1","Features":"all","Repository":"owner/project"}]}}` - m.On("KVGet", SUBSCRIPTIONS_KEY).Return([]byte(kvget), nil).Once() + m.On("KVGet", SubscriptionsKey).Return([]byte(kvget), nil).Once() return m }, shouldDelete: false, @@ -67,15 +67,15 @@ var dataUnsubscribeTest = []dataUnsubscribeTestStruct{ shouldDelete: false, shouldError: true, }, { - name: "should delete organisation", + name: "should delete organization", channelID: "1", repoName: "owner", initMock: func() *plugintest.API { m := &plugintest.API{} kvget := `{"Repositories":{"owner/":[{"ChannelID":"1","CreatorID":"1","Features":"all","Repository":"owner/"}]}}` kvset := `{"Repositories":{}}` - m.On("KVGet", SUBSCRIPTIONS_KEY).Return([]byte(kvget), nil).Once() - m.On("KVSet", SUBSCRIPTIONS_KEY, []byte(kvset)).Return(nil).Once() + m.On("KVGet", SubscriptionsKey).Return([]byte(kvget), nil).Once() + m.On("KVSet", SubscriptionsKey, []byte(kvset)).Return(nil).Once() return m }, shouldDelete: true, diff --git a/server/utils.go b/server/utils.go index 0a20d6a1..89c83c70 100644 --- a/server/utils.go +++ b/server/utils.go @@ -44,7 +44,7 @@ func encrypt(key []byte, text string) (string, error) { } cfb := cipher.NewCFBEncrypter(block, iv) - cfb.XORKeyStream(ciphertext[aes.BlockSize:], []byte(msg)) + cfb.XORKeyStream(ciphertext[aes.BlockSize:], msg) finalMsg := base64.URLEncoding.EncodeToString(ciphertext) return finalMsg, nil } @@ -61,7 +61,7 @@ func decrypt(key []byte, text string) (string, error) { } if (len(decodedMsg) % aes.BlockSize) != 0 { - return "", errors.New("blocksize must be multipe of decoded message length") + return "", errors.New("blocksize must be multiple of decoded message length") } iv := decodedMsg[:aes.BlockSize] @@ -83,7 +83,7 @@ func normalizePath(full, baseURL string) string { if baseURL == "" { baseURL = "https://gitlab.com/" } else if !strings.HasSuffix(baseURL, "/") { - baseURL = baseURL + "/" + baseURL += "/" } return strings.TrimSuffix(strings.TrimSpace(strings.Replace(full, baseURL, "", 1)), "/") diff --git a/server/webhook/constants.go b/server/webhook/constants.go new file mode 100644 index 00000000..808b3708 --- /dev/null +++ b/server/webhook/constants.go @@ -0,0 +1,17 @@ +package webhook + +const ( + actionOpen = "open" + actionClose = "close" + actionMerge = "merge" + actionReopen = "reopen" + actionUpdate = "update" + + stateOpened = "opened" + stateClosed = "closed" + stateMerged = "merged" + + statusSuccess = "success" + statusRunning = "running" + statusFailed = "failed" +) diff --git a/server/webhook/issue.go b/server/webhook/issue.go index 3c0240cc..9b425d06 100644 --- a/server/webhook/issue.go +++ b/server/webhook/issue.go @@ -24,11 +24,14 @@ func (w *webhook) handleDMIssue(event *gitlab.IssueEvent) ([]*HandleWebhook, err message := "" - if event.ObjectAttributes.Action == "open" && len(event.Assignees) > 0 { - message = fmt.Sprintf("[%s](%s) assigned you to issue [%s#%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.Project.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) - } else if event.ObjectAttributes.Action == "close" { + switch event.ObjectAttributes.Action { + case actionOpen: + if len(event.Assignees) > 0 { + message = fmt.Sprintf("[%s](%s) assigned you to issue [%s#%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.Project.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) + } + case actionClose: message = fmt.Sprintf("[%s](%s) closed your issue [%s#%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.Project.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) - } else if event.ObjectAttributes.Action == "reopen" { + case actionReopen: message = fmt.Sprintf("[%s](%s) reopened your issue [%s#%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.Project.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) } @@ -67,11 +70,12 @@ func (w *webhook) handleChannelIssue(event *gitlab.IssueEvent) ([]*HandleWebhook message := "" - if issue.Action == "open" { + switch issue.Action { + case actionOpen: message = fmt.Sprintf("#### %s\n##### [%s#%v](%s)\n###### new issue by [%s](%s) on [%s](%s)\n\n%s", issue.Title, repo.PathWithNamespace, issue.IID, issue.URL, senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), issue.CreatedAt, issue.URL, issue.Description) - } else if issue.Action == "close" { + case actionClose: message = fmt.Sprintf("[%s] Issue [%s](%s) closed by [%s](%s)", repo.PathWithNamespace, issue.Title, issue.URL, senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername)) - } else if issue.Action == "update" && len(event.Changes.Labels.Current) > 0 && !sameLabels(event.Changes.Labels.Current, event.Changes.Labels.Previous) { + case actionUpdate: message = fmt.Sprintf("#### %s\n##### [%s#%v](%s)\n###### issue labeled `%s` by [%s](%s) on [%s](%s)\n\n%s", issue.Title, repo.PathWithNamespace, issue.IID, issue.URL, labelToString(event.Changes.Labels.Current), event.User.Username, event.User.WebsiteURL, issue.UpdatedAt, issue.URL, issue.Description) } diff --git a/server/webhook/issue_fixture_test.go b/server/webhook/issue_fixture_test.go index ac3f2fda..e81367cf 100644 --- a/server/webhook/issue_fixture_test.go +++ b/server/webhook/issue_fixture_test.go @@ -316,104 +316,6 @@ const CloseIssue = `{ "avatar_url":"https://www.gravatar.com/avatar/c6b552a4cd47f7cf1701ea5b650cd2e3?s=80\\u0026d=identicon" }] }` -const LabelIssue = `{ - "object_kind":"issue", - "event_type":"issue", - "user":{ - "name":"manland", - "username":"manland", - "avatar_url":"https://www.gravatar.com/avatar/c6b552a4cd47f7cf1701ea5b650cd2e3?s=80\\u0026d=identicon" - }, - "project":{ - "id":24, - "name":"webhook", - "description":"", - "web_url":"http://localhost:3000/manland/webhook", - "avatar_url":null, - "git_ssh_url":"ssh://rmaneschi@localhost:2222/manland/webhook.git", - "git_http_url":"http://localhost:3000/manland/webhook.git", - "namespace":"manland", - "visibility_level":20, - "path_with_namespace":"manland/webhook", - "default_branch":"master", - "ci_config_path":null, - "homepage":"http://localhost:3000/manland/webhook", - "url":"ssh://rmaneschi@localhost:2222/manland/webhook.git", - "ssh_url":"ssh://rmaneschi@localhost:2222/manland/webhook.git", - "http_url":"http://localhost:3000/manland/webhook.git" - }, - "object_attributes":{ - "author_id":50, - "closed_at":null, - "confidential":false, - "created_at":"2019-04-06 21:20:45 UTC", - "description":"test label issue", - "due_date":null, - "id":183, - "iid":3, - "last_edited_at":null, - "last_edited_by_id":null, - "milestone_id":null, - "moved_to_id":null, - "project_id":24, - "relative_position":1073743323, - "state":"opened", - "time_estimate":0, - "title":"Test label issue", - "updated_at":"2019-04-06 21:21:08 UTC", - "updated_by_id":50, - "url":"http://localhost:3000/manland/webhook/issues/3", - "total_time_spent":0, - "human_total_time_spent":null, - "human_time_estimate":null, - "assignee_ids":[], - "assignee_id":null, - "action":"update" - }, - "labels":[{ - "id":36, - "title":"violet", - "color":"#A295D6", - "project_id":24, - "created_at":"2019-04-06 21:21:06 UTC", - "updated_at":"2019-04-06 21:21:06 UTC", - "template":false, - "description":null, - "type":"ProjectLabel", - "group_id":null - }], - "changes":{ - "updated_at":{ - "previous":"2019-04-06 21:20:45 UTC", - "current":"2019-04-06 21:21:08 UTC" - }, - "updated_by_id":{ - "previous":null, - "current":50 - }, - "labels":{ - "previous":[], - "current":[{ - "id":36, - "title":"violet", - "color":"#A295D6", - "project_id":24, - "created_at":"2019-04-06 21:21:06 UTC", - "updated_at":"2019-04-06 21:21:06 UTC", - "template":false, - "description":null, - "type":"ProjectLabel", - "group_id":null - }] - } - }, - "repository":{ - "name":"webhook", - "url":"ssh://rmaneschi@localhost:2222/manland/webhook.git", - "description":"", - "homepage":"http://localhost:3000/manland/webhook" - } - }` const ReopenIssue = `{ "object_kind":"issue", diff --git a/server/webhook/issue_test.go b/server/webhook/issue_test.go index 174877a4..4eeca2b4 100644 --- a/server/webhook/issue_test.go +++ b/server/webhook/issue_test.go @@ -5,9 +5,10 @@ import ( "strings" "testing" - "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" "github.com/stretchr/testify/assert" "github.com/xanzy/go-gitlab" + + "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" ) type testDataIssueStr struct { @@ -37,7 +38,7 @@ var testDataIssue = []testDataIssueStr{ }}, }, { testTitle: "root open issue with manland assignee and display in channel1 (subgroup)", - fixture: strings.Replace(NewIssue, "manland/webhook", "manland/subgroup/webhook", -1), + fixture: strings.ReplaceAll(NewIssue, "manland/webhook", "manland/subgroup/webhook"), gitlabRetreiver: newFakeWebhook([]*subscription.Subscription{ {ChannelID: "channel1", CreatorID: "1", Features: "issues", Repository: "manland/subgroup/webhook"}, }), diff --git a/server/webhook/merge_request.go b/server/webhook/merge_request.go index b4e3cede..f245c0e6 100644 --- a/server/webhook/merge_request.go +++ b/server/webhook/merge_request.go @@ -24,17 +24,23 @@ func (w *webhook) handleDMMergeRequest(event *gitlab.MergeEvent) ([]*HandleWebho message := "" - if event.ObjectAttributes.State == "opened" && event.ObjectAttributes.Action == "open" { - message = fmt.Sprintf("[%s](%s) requested your review on [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) - } else if event.ObjectAttributes.State == "closed" { + switch event.ObjectAttributes.State { + case stateOpened: + switch event.ObjectAttributes.Action { + case actionOpen: + message = fmt.Sprintf("[%s](%s) requested your review on [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) + case actionReopen: + message = fmt.Sprintf("[%s](%s) reopen your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) + case actionUpdate: + // TODO not enough check (opened/update) to say assigned to you... + message = fmt.Sprintf("[%s](%s) assigned you to merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) + } + case stateClosed: message = fmt.Sprintf("[%s](%s) closed your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) - } else if event.ObjectAttributes.State == "opened" && event.ObjectAttributes.Action == "reopen" { - message = fmt.Sprintf("[%s](%s) reopen your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) - } else if event.ObjectAttributes.State == "opened" && event.ObjectAttributes.Action == "update" { - // TODO not enough check (opened/update) to say assigned to you... - message = fmt.Sprintf("[%s](%s) assigned you to merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) - } else if event.ObjectAttributes.State == "merged" && event.ObjectAttributes.Action == "merge" { - message = fmt.Sprintf("[%s](%s) merged your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) + case stateMerged: + if event.ObjectAttributes.Action == actionMerge { + message = fmt.Sprintf("[%s](%s) merged your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL) + } } if len(message) > 0 { @@ -68,13 +74,13 @@ func (w *webhook) handleChannelMergeRequest(event *gitlab.MergeEvent) ([]*Handle message := "" switch pr.Action { - case "open": + case actionOpen: message = fmt.Sprintf("#### %s\n##### [%s!%v](%s) new merge-request by [%s](%s) on [%s](%s)\n\n%s", pr.Title, repo.PathWithNamespace, pr.IID, pr.URL, senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), pr.CreatedAt, pr.URL, pr.Description) - case "merge": + case actionMerge: message = fmt.Sprintf("[%s] Merge request [!%v %s](%s) was merged by [%s](%s)", repo.PathWithNamespace, pr.IID, pr.Title, pr.URL, senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername)) - case "close": + case actionClose: message = fmt.Sprintf("[%s] Merge request [!%v %s](%s) was closed by [%s](%s)", repo.PathWithNamespace, pr.IID, pr.Title, pr.URL, senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername)) - case "reopen": + case actionReopen: message = fmt.Sprintf("[%s] Merge request [!%v %s](%s) was reopened by [%s](%s)", repo.PathWithNamespace, pr.IID, pr.Title, pr.URL, senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername)) } diff --git a/server/webhook/merge_request_test.go b/server/webhook/merge_request_test.go index a181f737..70dac52f 100644 --- a/server/webhook/merge_request_test.go +++ b/server/webhook/merge_request_test.go @@ -5,9 +5,10 @@ import ( "strings" "testing" - "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" "github.com/stretchr/testify/assert" "github.com/xanzy/go-gitlab" + + "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" ) type testDataMergeRequestStr struct { @@ -37,7 +38,7 @@ var testDataMergeRequest = []testDataMergeRequestStr{ }}, }, { testTitle: "root open merge request for manland and display in channel1 (subgroup)", - fixture: strings.Replace(OpenMergeRequest, "manland/webhook", "manland/subgroup/webhook", -1), + fixture: strings.ReplaceAll(OpenMergeRequest, "manland/webhook", "manland/subgroup/webhook"), gitlabRetreiver: newFakeWebhook([]*subscription.Subscription{ {ChannelID: "channel1", CreatorID: "1", Features: "merges", Repository: "manland/subgroup/webhook"}, }), @@ -121,7 +122,7 @@ var testDataMergeRequest = []testDataMergeRequestStr{ }), res: []*HandleWebhook{{ Message: "[root](http://my.gitlab.com/root) closed your merge request [manland/webhook!1](http://localhost:3000/manland/webhook/merge_requests/1)", - ToUsers: []string{}, //no assignee + ToUsers: []string{}, // no assignee ToChannels: []string{}, From: "root", }, { diff --git a/server/webhook/note_test.go b/server/webhook/note_test.go index 3ba130cb..75ff60c9 100644 --- a/server/webhook/note_test.go +++ b/server/webhook/note_test.go @@ -41,7 +41,7 @@ var testDataNote = []testDataNoteStr{ }, { testTitle: "manland comment issue of root (subgroup)", kind: "issue", - fixture: strings.Replace(IssueComment, "manland/webhook", "manland/subgroup/webhook", -1), + fixture: strings.ReplaceAll(IssueComment, "manland/webhook", "manland/subgroup/webhook"), gitlabRetreiver: newFakeWebhook([]*subscription.Subscription{ {ChannelID: "channel1", CreatorID: "1", Features: "issue_comments", Repository: "manland/subgroup/webhook"}, }), diff --git a/server/webhook/pipeline.go b/server/webhook/pipeline.go index 2a393bb2..95fec88c 100644 --- a/server/webhook/pipeline.go +++ b/server/webhook/pipeline.go @@ -24,7 +24,7 @@ func (w *webhook) handleDMPipeline(event *gitlab.PipelineEvent) ([]*HandleWebhoo handlers := []*HandleWebhook{} - if event.ObjectAttributes.Status == "failed" { + if event.ObjectAttributes.Status == statusFailed { message := fmt.Sprintf("[%s](%s) Your pipeline has failed for %s[View Commit](%s)", repo.PathWithNamespace, repo.WebURL, event.Commit.Message, event.Commit.URL) handlers = append(handlers, &HandleWebhook{ Message: message, @@ -54,11 +54,11 @@ func (w *webhook) handleChannelPipeline(event *gitlab.PipelineEvent) ([]*HandleW message := "" switch event.ObjectAttributes.Status { - case "running": + case statusRunning: message = fmt.Sprintf("[%s](%s) New pipeline by [%s](%s) for %s[%s](%s)", repo.PathWithNamespace, repo.WebURL, senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.Commit.Message, "View Commit", event.Commit.URL) - case "success": + case statusSuccess: message = fmt.Sprintf("[%s](%s) Pipeline by [%s](%s) success for %s[%s](%s)", repo.PathWithNamespace, repo.WebURL, senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.Commit.Message, "View Commit", event.Commit.URL) - case "failed": + case statusFailed: message = fmt.Sprintf("[%s](%s) Pipeline by [%s](%s) fail for %s[%s](%s)", repo.PathWithNamespace, repo.WebURL, senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.Commit.Message, "View Commit", event.Commit.URL) default: return res, nil diff --git a/server/webhook/pipeline_test.go b/server/webhook/pipeline_test.go index 745d5b25..93da0898 100644 --- a/server/webhook/pipeline_test.go +++ b/server/webhook/pipeline_test.go @@ -5,9 +5,10 @@ import ( "strings" "testing" - "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" "github.com/stretchr/testify/assert" "github.com/xanzy/go-gitlab" + + "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" ) type testDataPipelineStr struct { @@ -24,7 +25,7 @@ var testDataPipeline = []testDataPipelineStr{ gitlabRetreiver: newFakeWebhook([]*subscription.Subscription{ {ChannelID: "channel1", CreatorID: "1", Features: "pipeline", Repository: "manland/webhook"}, }), - res: []*HandleWebhook{}, //we don't care about pending pipeline + res: []*HandleWebhook{}, // we don't care about pending pipeline }, { testTitle: "root start a pipeline in running", fixture: PipelineRun, @@ -39,7 +40,7 @@ var testDataPipeline = []testDataPipelineStr{ }}, }, { testTitle: "root start a pipeline in running (subgroup)", - fixture: strings.Replace(PipelineRun, "manland/webhook", "manland/subgroup/webhook", -1), + fixture: strings.ReplaceAll(PipelineRun, "manland/webhook", "manland/subgroup/webhook"), gitlabRetreiver: newFakeWebhook([]*subscription.Subscription{ {ChannelID: "channel1", CreatorID: "1", Features: "pipeline", Repository: "manland/subgroup/webhook"}, }), diff --git a/server/webhook/push_test.go b/server/webhook/push_test.go index 803b764b..aa216fa6 100644 --- a/server/webhook/push_test.go +++ b/server/webhook/push_test.go @@ -5,9 +5,10 @@ import ( "strings" "testing" - "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" "github.com/stretchr/testify/assert" "github.com/xanzy/go-gitlab" + + "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" ) type testDataPushStr struct { @@ -33,7 +34,7 @@ var testDataPush = []testDataPushStr{ }}, }, { testTitle: "manland push 1 commit (subgroup)", - fixture: strings.Replace(PushEvent, "manland/webhook", "manland/subgroup/webhook", -1), + fixture: strings.ReplaceAll(PushEvent, "manland/webhook", "manland/subgroup/webhook"), gitlabRetreiver: newFakeWebhook([]*subscription.Subscription{ {ChannelID: "channel1", CreatorID: "1", Features: "pushes", Repository: "manland/subgroup/webhook"}, }), diff --git a/server/webhook/tag_fixture_test.go b/server/webhook/tag_fixture_test.go index 66f7a2db..73cdd02c 100644 --- a/server/webhook/tag_fixture_test.go +++ b/server/webhook/tag_fixture_test.go @@ -7,7 +7,7 @@ const SimpleTag = `{ "after":"48bb9a42241a84ec7c03850f0096ac671bb06641", "ref":"refs/tags/tag1", "checkout_sha":"c30217b62542c586fdbadc7b5ee762bfdca10663", - "message":"Really beatiful tag", + "message":"Really beautiful tag", "user_id":50, "user_name":"manland", "user_username":"manland", diff --git a/server/webhook/tag_test.go b/server/webhook/tag_test.go index ed43ecdb..ef3e396c 100644 --- a/server/webhook/tag_test.go +++ b/server/webhook/tag_test.go @@ -5,9 +5,10 @@ import ( "strings" "testing" - "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" "github.com/stretchr/testify/assert" "github.com/xanzy/go-gitlab" + + "github.com/mattermost/mattermost-plugin-gitlab/server/subscription" ) type testDataTagStr struct { @@ -25,7 +26,7 @@ var testDataTag = []testDataTagStr{ {ChannelID: "channel1", CreatorID: "1", Features: "tag", Repository: "manland/webhook"}, }), res: []*HandleWebhook{{ - Message: "[manland/webhook](http://localhost:3000/manland/webhook) New tag [tag1](http://localhost:3000/manland/webhook/commit/c30217b62542c586fdbadc7b5ee762bfdca10663) by [manland](http://my.gitlab.com/manland): Really beatiful tag", + Message: "[manland/webhook](http://localhost:3000/manland/webhook) New tag [tag1](http://localhost:3000/manland/webhook/commit/c30217b62542c586fdbadc7b5ee762bfdca10663) by [manland](http://my.gitlab.com/manland): Really beautiful tag", ToUsers: []string{}, // No DM because user know he has created a tag ToChannels: []string{"channel1"}, From: "manland", @@ -33,12 +34,12 @@ var testDataTag = []testDataTagStr{ }, { testTitle: "manland create a tag (subgroup)", - fixture: strings.Replace(SimpleTag, "manland/webhook", "manland/subgroup/webhook", -1), + fixture: strings.ReplaceAll(SimpleTag, "manland/webhook", "manland/subgroup/webhook"), gitlabRetreiver: newFakeWebhook([]*subscription.Subscription{ {ChannelID: "channel1", CreatorID: "1", Features: "tag", Repository: "manland/subgroup/webhook"}, }), res: []*HandleWebhook{{ - Message: "[manland/subgroup/webhook](http://localhost:3000/manland/subgroup/webhook) New tag [tag1](http://localhost:3000/manland/subgroup/webhook/commit/c30217b62542c586fdbadc7b5ee762bfdca10663) by [manland](http://my.gitlab.com/manland): Really beatiful tag", + Message: "[manland/subgroup/webhook](http://localhost:3000/manland/subgroup/webhook) New tag [tag1](http://localhost:3000/manland/subgroup/webhook/commit/c30217b62542c586fdbadc7b5ee762bfdca10663) by [manland](http://my.gitlab.com/manland): Really beautiful tag", ToUsers: []string{}, // No DM because user know he has created a tag ToChannels: []string{"channel1"}, From: "manland", diff --git a/server/webhook/webhook.go b/server/webhook/webhook.go index a44926a6..8df0de6c 100644 --- a/server/webhook/webhook.go +++ b/server/webhook/webhook.go @@ -105,18 +105,6 @@ func (w *webhook) handleMention(m mentionDetails) *HandleWebhook { return nil } -func sameLabels(a []gitlab.Label, b []gitlab.Label) bool { - if len(a) != len(b) { - return false - } - for index, l := range a { - if l.ID != b[index].ID { - return false - } - } - return true -} - func containsLabel(a []gitlab.Label, labelName string) bool { for _, l := range a { if l.Name == labelName { diff --git a/server/webhook/webhook_test.go b/server/webhook/webhook_test.go index 8d0760ea..6146a131 100644 --- a/server/webhook/webhook_test.go +++ b/server/webhook/webhook_test.go @@ -24,11 +24,12 @@ func (*fakeWebhook) GetUserURL(username string) string { } func (*fakeWebhook) GetUsernameByID(id int) string { - if id == 1 { + switch id { + case 1: return "root" - } else if id == 50 { + case 50: return "manland" - } else { + default: return "" } } diff --git a/server/webhook_test.go b/server/webhook_test.go index 3173ea3e..34057028 100644 --- a/server/webhook_test.go +++ b/server/webhook_test.go @@ -6,11 +6,12 @@ import ( "net/http/httptest" "testing" - "github.com/mattermost/mattermost-plugin-gitlab/server/webhook" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin/plugintest" "github.com/stretchr/testify/assert" gitlabLib "github.com/xanzy/go-gitlab" + + "github.com/mattermost/mattermost-plugin-gitlab/server/webhook" ) type fakeWebhookHandler struct{} @@ -75,7 +76,7 @@ func TestHandleWebhookWithKnowAuthorButUnknowToUser(t *testing.T) { mock := &plugintest.API{} mock.On("KVGet", "test_gitlabusername").Return([]byte("1"), nil).Once() mock.On("KVGet", "unknown_gitlabusername").Return(nil, nil).Once() - mock.On("PublishWebSocketEvent", WS_EVENT_REFRESH, map[string]interface{}(nil), &model.WebsocketBroadcast{UserId: "1"}).Return(nil).Once() + mock.On("PublishWebSocketEvent", WsEventRefresh, map[string]interface{}(nil), &model.WebsocketBroadcast{UserId: "1"}).Return(nil).Once() mock.On("LogInfo", "new msg", "message", "hello", "from", "test").Return(nil) mock.On("LogInfo", "userFrom", "from", "1").Return(nil) p.SetAPI(mock) @@ -92,7 +93,7 @@ func TestHandleWebhookWithKnowAuthorButUnknowToUser(t *testing.T) { mock.AssertCalled(t, "KVGet", "test_gitlabusername") mock.AssertCalled(t, "KVGet", "unknown_gitlabusername") mock.AssertNumberOfCalls(t, "KVGet", 2) - mock.AssertCalled(t, "PublishWebSocketEvent", WS_EVENT_REFRESH, map[string]interface{}(nil), &model.WebsocketBroadcast{UserId: "1"}) + mock.AssertCalled(t, "PublishWebSocketEvent", WsEventRefresh, map[string]interface{}(nil), &model.WebsocketBroadcast{UserId: "1"}) mock.AssertNumberOfCalls(t, "PublishWebSocketEvent", 1) } @@ -101,7 +102,7 @@ func TestHandleWebhookToChannel(t *testing.T) { mock := &plugintest.API{} mock.On("KVGet", "test_gitlabusername").Return([]byte("1"), nil).Once() - mock.On("PublishWebSocketEvent", WS_EVENT_REFRESH, map[string]interface{}(nil), &model.WebsocketBroadcast{UserId: "1"}).Return(nil).Once() + mock.On("PublishWebSocketEvent", WsEventRefresh, map[string]interface{}(nil), &model.WebsocketBroadcast{UserId: "1"}).Return(nil).Once() mock.On("LogInfo", "new msg", "message", "hello", "from", "test").Return(nil) mock.On("LogInfo", "userFrom", "from", "1").Return(nil) mock.On("CreatePost", &model.Post{Id: "", CreateAt: 0, UpdateAt: 0, EditAt: 0, DeleteAt: 0, IsPinned: false, UserId: "", ChannelId: "town-square", RootId: "", ParentId: "", OriginalId: "", Message: "hello", MessageSource: "", Type: "", Hashtags: "", Filenames: model.StringArray(nil), FileIds: model.StringArray(nil), PendingPostId: "", HasReactions: false, Metadata: (*model.PostMetadata)(nil)}).Return(nil, nil) @@ -118,7 +119,7 @@ func TestHandleWebhookToChannel(t *testing.T) { assert.Equal(t, http.StatusOK, resp.StatusCode) mock.AssertCalled(t, "KVGet", "test_gitlabusername") mock.AssertNumberOfCalls(t, "KVGet", 1) - mock.AssertCalled(t, "PublishWebSocketEvent", WS_EVENT_REFRESH, map[string]interface{}(nil), &model.WebsocketBroadcast{UserId: "1"}) + mock.AssertCalled(t, "PublishWebSocketEvent", WsEventRefresh, map[string]interface{}(nil), &model.WebsocketBroadcast{UserId: "1"}) mock.AssertNumberOfCalls(t, "PublishWebSocketEvent", 1) mock.AssertNumberOfCalls(t, "CreatePost", 1) }