Release 2.1.5 (#660)

* disable cgo

* bump version 2.1.5

* Bump version to 2.1.5

* [MM-51245] Handle relative paths in permalinks (#656)

* [MM-51269] Log errors from telemetry events (#653)

* keep telemetry init together and do the enable/disable thing (#659)

It places telemetry client and tracker initialization together at onActivate and uses the new enable/disable commands at onConfigurationChange.

Also, pass a logger to the telemetry tracker when ServiceSettings.EnableDeveloper is true (through NewTelemetryTracker), which will end up logging the telemetry activity (no matter if the tracking config or client is disabled). This is still 2/5 until backwards-compatibility details are discussed.

Added event account_connected when we have an oauth successful connection (through setup or plain connect).

* run ci on release branches

* remove check for release branch

* add nvmrc

* add cd workflow

---------

Co-authored-by: Pantelis Vratsalis <pvratsalis@gmail.com>
Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>
Co-authored-by: José Peso <trilopin@users.noreply.github.com>

.github/workflows/cd.yml

@@ -0,0 +1,18 @@
+name: cd
+on:
+  workflow_run:
+    workflows: ["ci"]
+    branches-ignore: ["*"]
+    types:
+      - completed
+  push:
+    tags:
+      - "v*"
+
+permissions:
+  contents: read
+
+jobs:
+  plugin-cd:
+    uses: mattermost/actions-workflows/.github/workflows/plugin-cd.yml@main
+    secrets: inherit

.github/workflows/ci.yml

@@ -0,0 +1,18 @@
+name: ci
+on:
+  schedule:
+    - cron: "0 0 * * *"
+  push:
+    branches:
+      - master
+    tags:
+      - "v*"
+  pull_request:
+
+permissions:
+  contents: read
+
+jobs:
+  plugin-ci:
+    uses: mattermost/actions-workflows/.github/workflows/plugin-ci.yml@main
+    secrets: inherit

.nvmrc

@@ -0,0 +1 @@
+14.21.1

go.mod

@@ -6,10 +6,11 @@ require (
 	github.com/Masterminds/sprig/v3 v3.2.2
 	github.com/google/go-github/v41 v41.0.0
 	github.com/gorilla/mux v1.8.0
-	github.com/mattermost/mattermost-plugin-api v0.0.27
-	github.com/mattermost/mattermost-server/v6 v6.5.0
-	github.com/microcosm-cc/bluemonday v1.0.18
+	github.com/mattermost/mattermost-plugin-api v0.1.3-0.20230323124751-86c7be7ffbac
+	// mmgoget: github.com/mattermost/mattermost-server/v6@v7.5.0 is replaced by -> github.com/mattermost/mattermost-server/v6@21aec2741b
+	github.com/mattermost/mattermost-server/v6 v6.0.0-20221109191448-21aec2741bfe
+	github.com/microcosm-cc/bluemonday v1.0.19
 	github.com/pkg/errors v0.9.1
-	github.com/stretchr/testify v1.7.0
+	github.com/stretchr/testify v1.8.0
 	golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8
 )

plugin.json

@@ -4,9 +4,9 @@
     "description": "GitHub plugin for Mattermost.",
     "homepage_url": "https://github.com/mattermost/mattermost-plugin-github",
     "support_url": "https://github.com/mattermost/mattermost-plugin-github/issues",
-    "release_notes_url": "https://github.com/mattermost/mattermost-plugin-github/releases/tag/v2.1.4",
+    "release_notes_url": "https://github.com/mattermost/mattermost-plugin-github/releases/tag/v2.1.5",
     "icon_path": "assets/icon.svg",
-    "version": "2.1.4",
+    "version": "2.1.5",
     "min_server_version": "6.5.0",
     "server": {
       "executables": {

server/plugin/api.go

@@ -404,6 +404,9 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter,
 		return
 	}
 
+	// track the successful connection
+	p.TrackUserEvent("account_connected", c.UserID, nil)
+
 	userInfo := &GitHubUserInfo{
 		UserID:         state.UserID,
 		Token:          tok,

server/plugin/configuration.go

@@ -212,16 +212,11 @@ func (p *Plugin) OnConfigurationChange() error {
 	if err != nil {
 		return errors.Wrap(err, "failed to register command")
 	}
-
-	enableDiagnostics := false
-	if config := p.API.GetConfig(); config != nil {
-		if configValue := config.LogSettings.EnableDiagnostics; configValue != nil {
-			enableDiagnostics = *configValue
-		}
+	// Some config changes require reloading tracking config
+	if p.tracker != nil {
+		p.tracker.ReloadConfig(telemetry.NewTrackerConfig(p.API.GetConfig()))
 	}
 
-	p.tracker = telemetry.NewTracker(p.telemetryClient, p.API.GetDiagnosticId(), p.API.GetServerVersion(), Manifest.Id, Manifest.Version, "github", enableDiagnostics)
-
 	return nil
 }
 

server/plugin/flows.go

@@ -12,7 +12,6 @@ import (
 	pluginapi "github.com/mattermost/mattermost-plugin-api"
 	"github.com/mattermost/mattermost-plugin-api/experimental/flow"
 
-	"github.com/mattermost/mattermost-plugin-api/experimental/telemetry"
 	"github.com/pkg/errors"
 
 	"github.com/mattermost/mattermost-server/v6/model"
@@ -23,6 +22,11 @@ type PingBroker interface {
 	SubscribePings() <-chan *github.PingEvent
 }
 
+type Tracker interface {
+	TrackEvent(event string, properties map[string]interface{})
+	TrackUserEvent(event, userID string, properties map[string]interface{})
+}
+
 type FlowManager struct {
 	client           *pluginapi.Client
 	pluginURL        string
@@ -32,7 +36,7 @@ type FlowManager struct {
 	getGitHubClient  func(ctx context.Context, userID string) (*github.Client, error)
 
 	pingBroker PingBroker
-	tracker    telemetry.Tracker
+	tracker    Tracker
 
 	setupFlow        *flow.Flow
 	oauthFlow        *flow.Flow
@@ -50,7 +54,7 @@ func (p *Plugin) NewFlowManager() *FlowManager {
 		getGitHubClient:  p.GetGitHubClient,
 
 		pingBroker: p.webhookBroker,
-		tracker:    p.tracker,
+		tracker:    p,
 	}
 
 	fm.setupFlow = fm.newFlow("setup").WithSteps(
@@ -223,14 +227,14 @@ func (fm *FlowManager) StartSetupWizard(userID string, delegatedFrom string) err
 }
 
 func (fm *FlowManager) trackStartSetupWizard(userID string, fromInvite bool) {
-	_ = fm.tracker.TrackUserEvent("setup_wizard_start", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("setup_wizard_start", userID, map[string]interface{}{
 		"from_invite": fromInvite,
 		"time":        model.GetMillis(),
 	})
 }
 
 func (fm *FlowManager) trackCompleteSetupWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("setup_wizard_complete", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("setup_wizard_complete", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
@@ -249,13 +253,13 @@ func (fm *FlowManager) StartOauthWizard(userID string) error {
 }
 
 func (fm *FlowManager) trackStartOauthWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("oauth_wizard_start", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("oauth_wizard_start", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
 
 func (fm *FlowManager) trackCompleteOauthWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("oauth_wizard_complete", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("oauth_wizard_complete", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
@@ -591,13 +595,13 @@ func (fm *FlowManager) StartWebhookWizard(userID string) error {
 }
 
 func (fm *FlowManager) trackStartWebhookWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("webhook_wizard_start", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("webhook_wizard_start", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
 
 func (fm *FlowManager) trackCompleteWebhookWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("webhook_wizard_complete", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("webhook_wizard_complete", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
@@ -755,13 +759,13 @@ func (fm *FlowManager) StartAnnouncementWizard(userID string) error {
 }
 
 func (fm *FlowManager) trackStartAnnouncementWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("announcement_wizard_start", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("announcement_wizard_start", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
 
 func (fm *FlowManager) trackCompletAnnouncementWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("announcement_wizard_complete", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("announcement_wizard_complete", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }

server/plugin/permalinks.go

@@ -3,6 +3,7 @@ package plugin
 import (
 	"context"
 	"encoding/hex"
+	"path"
 	"strings"
 	"time"
 
@@ -75,7 +76,7 @@ func (p *Plugin) getReplacements(msg string) []replacement {
 			case "commit":
 				r.permalinkInfo.commit = m[j]
 			case "path":
-				r.permalinkInfo.path = m[j]
+				r.permalinkInfo.path = strings.TrimPrefix(path.Join("/", m[j]), "/")
 			case "line":
 				r.permalinkInfo.line = m[j]
 			}

server/plugin/permalinks_test.go

@@ -48,7 +48,34 @@ func TestGetReplacements(t *testing.T) {
 					},
 				},
 			},
-		}, {
+		},
+		{
+			name:            "basic link with relative path",
+			input:           "start https://github.com/mattermost/mattermost-server/blob/cbb25838a61872b624ac512556d7bc932486a64c/../../../authentication.go#L15-L22 lorem ipsum",
+			numReplacements: 1,
+			replacements: []replacement{
+				{
+					index: 6,
+					word:  "https://github.com/mattermost/mattermost-server/blob/cbb25838a61872b624ac512556d7bc932486a64c/../../../authentication.go#L15-L22",
+					permalinkInfo: struct {
+						haswww string
+						commit string
+						user   string
+						repo   string
+						path   string
+						line   string
+					}{
+						haswww: "",
+						commit: "cbb25838a61872b624ac512556d7bc932486a64c",
+						line:   "L15-L22",
+						path:   "authentication.go",
+						user:   "mattermost",
+						repo:   "mattermost-server",
+					},
+				},
+			},
+		},
+		{
 			name:            "duplicate expansions",
 			input:           "start https://github.com/mattermost/mattermost-server/blob/cbb25838a61872b624ac512556d7bc932486a64c/app/authentication.go#L15-L22 lorem ipsum https://github.com/mattermost/mattermost-server/blob/cbb25838a61872b624ac512556d7bc932486a64c/app/authentication.go#L15-L22 lorem ipsum",
 			numReplacements: 2,

server/plugin/plugin.go

@@ -210,11 +210,7 @@ func (p *Plugin) OnActivate() error {
 	}
 
 	p.initializeAPI()
-
-	p.telemetryClient, err = telemetry.NewRudderClient()
-	if err != nil {
-		p.API.LogWarn("Telemetry client not started", "error", err.Error())
-	}
+	p.initializeTelemetry()
 
 	p.webhookBroker = NewWebhookBroker(p.sendGitHubPingEvent)
 	p.oauthBroker = NewOAuthBroker(p.sendOAuthCompleteEvent)
@@ -246,7 +242,9 @@ func (p *Plugin) OnActivate() error {
 func (p *Plugin) OnDeactivate() error {
 	p.webhookBroker.Close()
 	p.oauthBroker.Close()
-
+	if err := p.telemetryClient.Close(); err != nil {
+		p.API.LogWarn("Telemetry client failed to close", "error", err.Error())
+	}
 	return nil
 }
 

server/plugin/telemetry.go

@@ -4,13 +4,29 @@ import (
 	"strings"
 
 	pluginapi "github.com/mattermost/mattermost-plugin-api"
+	"github.com/mattermost/mattermost-plugin-api/experimental/bot/logger"
+	"github.com/mattermost/mattermost-plugin-api/experimental/telemetry"
 	"github.com/pkg/errors"
 )
 
 const (
 	keysPerPage = 1000
 )
 
+func (p *Plugin) TrackEvent(event string, properties map[string]interface{}) {
+	err := p.tracker.TrackEvent(event, properties)
+	if err != nil {
+		p.API.LogDebug("Error sending telemetry event", "event", event, "error", err.Error())
+	}
+}
+
+func (p *Plugin) TrackUserEvent(event, userID string, properties map[string]interface{}) {
+	err := p.tracker.TrackUserEvent(event, userID, properties)
+	if err != nil {
+		p.API.LogDebug("Error sending user telemetry event", "event", event, "error", err.Error())
+	}
+}
+
 func (p *Plugin) SendDailyTelemetry() {
 	config := p.getConfiguration()
 
@@ -19,7 +35,7 @@ func (p *Plugin) SendDailyTelemetry() {
 		p.API.LogWarn("Failed to get the number of connected users for telemetry", "error", err)
 	}
 
-	_ = p.tracker.TrackEvent("stats", map[string]interface{}{
+	p.TrackEvent("stats", map[string]interface{}{
 		"connected_user_count":          connectedUserCount,
 		"is_oauth_configured":           config.IsOAuthConfigured(),
 		"is_sass":                       config.IsSASS(),
@@ -53,3 +69,29 @@ func (p *Plugin) getConnectedUserCount() (int64, error) {
 
 	return count, nil
 }
+
+// Initialize telemetry setups the tracker/clients needed to send telemetry data.
+// The telemetry.NewTrackerConfig(...) param will take care of extract/parse the config to set rge right settings.
+// If you don't want the default behavior you still can pass a different telemetry.TrackerConfig data.
+func (p *Plugin) initializeTelemetry() {
+	var err error
+
+	// Telemetry client
+	p.telemetryClient, err = telemetry.NewRudderClient()
+	if err != nil {
+		p.API.LogWarn("Telemetry client not started", "error", err.Error())
+		return
+	}
+
+	// Get config values
+	p.tracker = telemetry.NewTracker(
+		p.telemetryClient,
+		p.API.GetDiagnosticId(),
+		p.API.GetServerVersion(),
+		Manifest.Id,
+		Manifest.Version,
+		"github",
+		telemetry.NewTrackerConfig(p.API.GetConfig()),
+		logger.New(p.API),
+	)
+}