Merge branch 'master' into advanced-permissions-phase-1

3 files changed, 59 insertions, 14 deletions

diff --git a/app/notification.go b/app/notification.go
index 8cb63fbaf..bb0c8703f 100644
--- a/app/notification.go
+++ b/app/notification.go
@@ -55,10 +55,15 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 
 	if channel.Type == model.CHANNEL_DIRECT {
 		var otherUserId string
-		if userIds := strings.Split(channel.Name, "__"); userIds[0] == post.UserId {
-			otherUserId = userIds[1]
-		} else {
-			otherUserId = userIds[0]
+
+		userIds := strings.Split(channel.Name, "__")
+
+		if userIds[0] != userIds[1] {
+			if userIds[0] == post.UserId {
+				otherUserId = userIds[1]
+			} else {
+				otherUserId = userIds[0]
+			}
 		}
 
 		if _, ok := profileMap[otherUserId]; ok {
@@ -89,7 +94,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 			delete(mentionedUserIds, post.UserId)
 		}
 
-		if len(m.OtherPotentialMentions) > 0 {
+		if len(m.OtherPotentialMentions) > 0 && !post.IsSystemMessage() {
 			if result := <-a.Srv.Store.User().GetProfilesByUsernames(m.OtherPotentialMentions, team.Id); result.Err == nil {
 				outOfChannelMentions := result.Data.([]*model.User)
 				if channel.Type != model.CHANNEL_GROUP {
diff --git a/app/plugin.go b/app/plugin.go
index fe671d26a..6702e9227 100644
--- a/app/plugin.go
+++ b/app/plugin.go
@@ -91,18 +91,11 @@ func (a *App) ActivatePlugins() {
 		active := a.PluginEnv.IsPluginActive(id)
 
 		if pluginState.Enable && !active {
-			if err := a.PluginEnv.ActivatePlugin(id); err != nil {
-				l4g.Error(err.Error())
+			if err := a.activatePlugin(plugin.Manifest); err != nil {
+				l4g.Error("%v plugin enabled in config.json but failing to activate err=%v", plugin.Manifest.Id, err.DetailedError)
 				continue
 			}
 
-			if plugin.Manifest.HasClient() {
-				message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_ACTIVATED, "", "", "", nil)
-				message.Add("manifest", plugin.Manifest.ClientManifest())
-				a.Publish(message)
-			}
-
-			l4g.Info("Activated %v plugin", id)
 		} else if !pluginState.Enable && active {
 			if err := a.deactivatePlugin(plugin.Manifest); err != nil {
 				l4g.Error(err.Error())
@@ -111,6 +104,21 @@ func (a *App) ActivatePlugins() {
 	}
 }
 
+func (a *App) activatePlugin(manifest *model.Manifest) *model.AppError {
+	if err := a.PluginEnv.ActivatePlugin(manifest.Id); err != nil {
+		return model.NewAppError("activatePlugin", "app.plugin.activate.app_error", nil, err.Error(), http.StatusBadRequest)
+	}
+
+	if manifest.HasClient() {
+		message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_ACTIVATED, "", "", "", nil)
+		message.Add("manifest", manifest.ClientManifest())
+		a.Publish(message)
+	}
+
+	l4g.Info("Activated %v plugin", manifest.Id)
+	return nil
+}
+
 func (a *App) deactivatePlugin(manifest *model.Manifest) *model.AppError {
 	if err := a.PluginEnv.DeactivatePlugin(manifest.Id); err != nil {
 		return model.NewAppError("removePlugin", "app.plugin.deactivate.app_error", nil, err.Error(), http.StatusBadRequest)
@@ -301,11 +309,18 @@ func (a *App) EnablePlugin(id string) *model.AppError {
 		return model.NewAppError("EnablePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusBadRequest)
 	}
 
+	if err := a.activatePlugin(manifest); err != nil {
+		return err
+	}
+
 	a.UpdateConfig(func(cfg *model.Config) {
 		cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: true}
 	})
 
 	if err := a.SaveConfig(a.Config(), true); err != nil {
+		if err.Id == "ent.cluster.save_config.error" {
+			return model.NewAppError("EnablePlugin", "app.plugin.cluster.save_config.app_error", nil, "", http.StatusInternalServerError)
+		}
 		return model.NewAppError("EnablePlugin", "app.plugin.config.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}
 
diff --git a/app/plugin_test.go b/app/plugin_test.go
index 4794d2704..9ad5dc1fa 100644
--- a/app/plugin_test.go
+++ b/app/plugin_test.go
@@ -4,8 +4,10 @@
 package app
 
 import (
+	"errors"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/gorilla/mux"
@@ -195,3 +197,26 @@ func TestPluginCommands(t *testing.T) {
 	require.NotNil(t, err)
 	assert.Equal(t, http.StatusNotFound, err.StatusCode)
 }
+
+type pluginBadActivation struct {
+	testPlugin
+}
+
+func (p *pluginBadActivation) OnActivate(api plugin.API) error {
+	return errors.New("won't activate for some reason")
+}
+
+func TestPluginBadActivation(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	th.InstallPlugin(&model.Manifest{
+		Id: "foo",
+	}, &pluginBadActivation{})
+
+	t.Run("EnablePlugin bad activation", func(t *testing.T) {
+		err := th.App.EnablePlugin("foo")
+		assert.NotNil(t, err)
+		assert.True(t, strings.Contains(err.DetailedError, "won't activate for some reason"))
+	})
+}