Merge pull request #881 from hmhealey/plt194

PLT-194 Changes to ChannelMember notification settings

3 files changed, 103 insertions, 44 deletions

diff --git a/api/channel.go b/api/channel.go
index 896e22793..6494e3528 100644
--- a/api/channel.go
+++ b/api/channel.go
@@ -23,7 +23,7 @@ func InitChannel(r *mux.Router) {
 	sr.Handle("/create_direct", ApiUserRequired(createDirectChannel)).Methods("POST")
 	sr.Handle("/update", ApiUserRequired(updateChannel)).Methods("POST")
 	sr.Handle("/update_desc", ApiUserRequired(updateChannelDesc)).Methods("POST")
-	sr.Handle("/update_notify_level", ApiUserRequired(updateNotifyLevel)).Methods("POST")
+	sr.Handle("/update_notify_props", ApiUserRequired(updateNotifyProps)).Methods("POST")
 	sr.Handle("/{id:[A-Za-z0-9]+}/", ApiUserRequiredActivity(getChannel, false)).Methods("GET")
 	sr.Handle("/{id:[A-Za-z0-9]+}/extra_info", ApiUserRequired(getChannelExtraInfo)).Methods("GET")
 	sr.Handle("/{id:[A-Za-z0-9]+}/join", ApiUserRequired(joinChannel)).Methods("POST")
@@ -76,7 +76,7 @@ func CreateChannel(c *Context, channel *model.Channel, addMember bool) (*model.C
 
 		if addMember {
 			cm := &model.ChannelMember{ChannelId: sc.Id, UserId: c.Session.UserId,
-				Roles: model.CHANNEL_ROLE_ADMIN, NotifyLevel: model.CHANNEL_NOTIFY_ALL}
+				Roles: model.CHANNEL_ROLE_ADMIN, NotifyProps: model.GetDefaultChannelNotifyProps()}
 
 			if cmresult := <-Srv.Store.Channel().SaveMember(cm); cmresult.Err != nil {
 				return nil, cmresult.Err
@@ -134,8 +134,7 @@ func CreateDirectChannel(c *Context, otherUserId string) (*model.Channel, *model
 	if sc, err := CreateChannel(c, channel, true); err != nil {
 		return nil, err
 	} else {
-		cm := &model.ChannelMember{ChannelId: sc.Id, UserId: otherUserId,
-			Roles: "", NotifyLevel: model.CHANNEL_NOTIFY_ALL}
+		cm := &model.ChannelMember{ChannelId: sc.Id, UserId: otherUserId, Roles: "", NotifyProps: model.GetDefaultChannelNotifyProps()}
 
 		if cmresult := <-Srv.Store.Channel().SaveMember(cm); cmresult.Err != nil {
 			return nil, cmresult.Err
@@ -372,7 +371,8 @@ func JoinChannel(c *Context, channelId string, role string) {
 		}
 
 		if channel.Type == model.CHANNEL_OPEN {
-			cm := &model.ChannelMember{ChannelId: channel.Id, UserId: c.Session.UserId, NotifyLevel: model.CHANNEL_NOTIFY_ALL, Roles: role}
+			cm := &model.ChannelMember{ChannelId: channel.Id, UserId: c.Session.UserId,
+				Roles: role, NotifyProps: model.GetDefaultChannelNotifyProps()}
 
 			if cmresult := <-Srv.Store.Channel().SaveMember(cm); cmresult.Err != nil {
 				c.Err = cmresult.Err
@@ -405,7 +405,9 @@ func JoinDefaultChannels(user *model.User, channelRole string) *model.AppError {
 	if result := <-Srv.Store.Channel().GetByName(user.TeamId, "town-square"); result.Err != nil {
 		err = result.Err
 	} else {
-		cm := &model.ChannelMember{ChannelId: result.Data.(*model.Channel).Id, UserId: user.Id, NotifyLevel: model.CHANNEL_NOTIFY_ALL, Roles: channelRole}
+		cm := &model.ChannelMember{ChannelId: result.Data.(*model.Channel).Id, UserId: user.Id,
+			Roles: channelRole, NotifyProps: model.GetDefaultChannelNotifyProps()}
+
 		if cmResult := <-Srv.Store.Channel().SaveMember(cm); cmResult.Err != nil {
 			err = cmResult.Err
 		}
@@ -414,7 +416,9 @@ func JoinDefaultChannels(user *model.User, channelRole string) *model.AppError {
 	if result := <-Srv.Store.Channel().GetByName(user.TeamId, "off-topic"); result.Err != nil {
 		err = result.Err
 	} else {
-		cm := &model.ChannelMember{ChannelId: result.Data.(*model.Channel).Id, UserId: user.Id, NotifyLevel: model.CHANNEL_NOTIFY_ALL, Roles: channelRole}
+		cm := &model.ChannelMember{ChannelId: result.Data.(*model.Channel).Id, UserId: user.Id,
+			Roles: channelRole, NotifyProps: model.GetDefaultChannelNotifyProps()}
+
 		if cmResult := <-Srv.Store.Channel().SaveMember(cm); cmResult.Err != nil {
 			err = cmResult.Err
 		}
@@ -694,7 +698,7 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		} else {
 			oUser := oresult.Data.(*model.User)
 
-			cm := &model.ChannelMember{ChannelId: channel.Id, UserId: userId, NotifyLevel: model.CHANNEL_NOTIFY_ALL}
+			cm := &model.ChannelMember{ChannelId: channel.Id, UserId: userId, NotifyProps: model.GetDefaultChannelNotifyProps()}
 
 			if cmresult := <-Srv.Store.Channel().SaveMember(cm); cmresult.Err != nil {
 				l4g.Error("Failed to add member user_id=%v channel_id=%v err=%v", userId, id, cmresult.Err)
@@ -784,23 +788,18 @@ func removeChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 
 }
 
-func updateNotifyLevel(c *Context, w http.ResponseWriter, r *http.Request) {
+func updateNotifyProps(c *Context, w http.ResponseWriter, r *http.Request) {
 	data := model.MapFromJson(r.Body)
+
 	userId := data["user_id"]
 	if len(userId) != 26 {
-		c.SetInvalidParam("updateNotifyLevel", "user_id")
+		c.SetInvalidParam("updateMarkUnreadLevel", "user_id")
 		return
 	}
 
 	channelId := data["channel_id"]
 	if len(channelId) != 26 {
-		c.SetInvalidParam("updateNotifyLevel", "channel_id")
-		return
-	}
-
-	notifyLevel := data["notify_level"]
-	if len(notifyLevel) == 0 || !model.IsChannelNotifyLevelValid(notifyLevel) {
-		c.SetInvalidParam("updateNotifyLevel", "notify_level")
+		c.SetInvalidParam("updateMarkUnreadLevel", "channel_id")
 		return
 	}
 
@@ -814,10 +813,29 @@ func updateNotifyLevel(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if result := <-Srv.Store.Channel().UpdateNotifyLevel(channelId, userId, notifyLevel); result.Err != nil {
+	result := <-Srv.Store.Channel().GetMember(channelId, userId)
+	if result.Err != nil {
+		c.Err = result.Err
+		return
+	}
+
+	member := result.Data.(model.ChannelMember)
+
+	// update whichever notify properties have been provided, but don't change the others
+	if markUnread, exists := data["mark_unread"]; exists {
+		member.NotifyProps["mark_unread"] = markUnread
+	}
+
+	if desktop, exists := data["desktop"]; exists {
+		member.NotifyProps["desktop"] = desktop
+	}
+
+	if result := <-Srv.Store.Channel().UpdateMember(&member); result.Err != nil {
 		c.Err = result.Err
 		return
+	} else {
+		// return the updated notify properties including any unchanged ones
+		w.Write([]byte(model.MapToJson(member.NotifyProps)))
 	}
 
-	w.Write([]byte(model.MapToJson(data)))
 }
diff --git a/api/channel_benchmark_test.go b/api/channel_benchmark_test.go
index 77e679c14..7820f4a03 100644
--- a/api/channel_benchmark_test.go
+++ b/api/channel_benchmark_test.go
@@ -255,7 +255,7 @@ func BenchmarkRemoveChannelMember(b *testing.B) {
 	}
 }
 
-func BenchmarkUpdateNotifyLevel(b *testing.B) {
+func BenchmarkUpdateNotifyProps(b *testing.B) {
 	var (
 		NUM_CHANNELS_RANGE = utils.Range{NUM_CHANNELS, NUM_CHANNELS}
 	)
@@ -271,9 +271,10 @@ func BenchmarkUpdateNotifyLevel(b *testing.B) {
 
 	for i := range data {
 		newmap := map[string]string{
-			"channel_id":   channels[i].Id,
-			"user_id":      user.Id,
-			"notify_level": model.CHANNEL_NOTIFY_MENTION,
+			"channel_id":  channels[i].Id,
+			"user_id":     user.Id,
+			"desktop":     model.CHANNEL_NOTIFY_MENTION,
+			"mark_unread": model.CHANNEL_MARK_UNREAD_MENTION,
 		}
 		data[i] = newmap
 	}
@@ -282,7 +283,7 @@ func BenchmarkUpdateNotifyLevel(b *testing.B) {
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		for j := range channels {
-			Client.Must(Client.UpdateNotifyLevel(data[j]))
+			Client.Must(Client.UpdateNotifyProps(data[j]))
 		}
 	}
 }
diff --git a/api/channel_test.go b/api/channel_test.go
index 7845ac499..e6c7ed80e 100644
--- a/api/channel_test.go
+++ b/api/channel_test.go
@@ -803,7 +803,7 @@ func TestRemoveChannelMember(t *testing.T) {
 
 }
 
-func TestUpdateNotifyLevel(t *testing.T) {
+func TestUpdateNotifyProps(t *testing.T) {
 	Setup()
 
 	team := &model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: "test@nowhere.com", Type: model.TEAM_OPEN}
@@ -821,55 +821,94 @@ func TestUpdateNotifyLevel(t *testing.T) {
 	data := make(map[string]string)
 	data["channel_id"] = channel1.Id
 	data["user_id"] = user.Id
-	data["notify_level"] = model.CHANNEL_NOTIFY_MENTION
+	data["desktop"] = model.CHANNEL_NOTIFY_MENTION
 
 	timeBeforeUpdate := model.GetMillis()
 	time.Sleep(100 * time.Millisecond)
 
-	if _, err := Client.UpdateNotifyLevel(data); err != nil {
+	// test updating desktop
+	if result, err := Client.UpdateNotifyProps(data); err != nil {
 		t.Fatal(err)
+	} else if notifyProps := result.Data.(map[string]string); notifyProps["desktop"] != model.CHANNEL_NOTIFY_MENTION {
+		t.Fatal("NotifyProps[\"desktop\"] did not update properly")
+	} else if notifyProps["mark_unread"] != model.CHANNEL_MARK_UNREAD_ALL {
+		t.Fatalf("NotifyProps[\"mark_unread\"] changed to %v", notifyProps["mark_unread"])
 	}
 
 	rget := Client.Must(Client.GetChannels(""))
 	rdata := rget.Data.(*model.ChannelList)
-	if len(rdata.Members) == 0 || rdata.Members[channel1.Id].NotifyLevel != data["notify_level"] {
-		t.Fatal("NotifyLevel did not update properly")
+	if len(rdata.Members) == 0 || rdata.Members[channel1.Id].NotifyProps["desktop"] != data["desktop"] {
+		t.Fatal("NotifyProps[\"desktop\"] did not update properly")
+	} else if rdata.Members[channel1.Id].LastUpdateAt <= timeBeforeUpdate {
+		t.Fatal("LastUpdateAt did not update")
 	}
 
-	if rdata.Members[channel1.Id].LastUpdateAt <= timeBeforeUpdate {
-		t.Fatal("LastUpdateAt did not update")
+	// test an empty update
+	delete(data, "desktop")
+
+	if result, err := Client.UpdateNotifyProps(data); err != nil {
+		t.Fatal(err)
+	} else if notifyProps := result.Data.(map[string]string); notifyProps["mark_unread"] != model.CHANNEL_MARK_UNREAD_ALL {
+		t.Fatalf("NotifyProps[\"mark_unread\"] changed to %v", notifyProps["mark_unread"])
+	} else if notifyProps["desktop"] != model.CHANNEL_NOTIFY_MENTION {
+		t.Fatalf("NotifyProps[\"desktop\"] changed to %v", notifyProps["desktop"])
 	}
 
+	// test updating mark unread
+	data["mark_unread"] = model.CHANNEL_MARK_UNREAD_MENTION
+
+	if result, err := Client.UpdateNotifyProps(data); err != nil {
+		t.Fatal(err)
+	} else if notifyProps := result.Data.(map[string]string); notifyProps["mark_unread"] != model.CHANNEL_MARK_UNREAD_MENTION {
+		t.Fatal("NotifyProps[\"mark_unread\"] did not update properly")
+	} else if notifyProps["desktop"] != model.CHANNEL_NOTIFY_MENTION {
+		t.Fatalf("NotifyProps[\"desktop\"] changed to %v", notifyProps["desktop"])
+	}
+
+	// test updating both
+	data["desktop"] = model.CHANNEL_NOTIFY_NONE
+	data["mark_unread"] = model.CHANNEL_MARK_UNREAD_MENTION
+
+	if result, err := Client.UpdateNotifyProps(data); err != nil {
+		t.Fatal(err)
+	} else if notifyProps := result.Data.(map[string]string); notifyProps["desktop"] != model.CHANNEL_NOTIFY_NONE {
+		t.Fatal("NotifyProps[\"desktop\"] did not update properly")
+	} else if notifyProps["mark_unread"] != model.CHANNEL_MARK_UNREAD_MENTION {
+		t.Fatal("NotifyProps[\"mark_unread\"] did not update properly")
+	}
+
+	// test error cases
 	data["user_id"] = "junk"
-	if _, err := Client.UpdateNotifyLevel(data); err == nil {
+	if _, err := Client.UpdateNotifyProps(data); err == nil {
 		t.Fatal("Should have errored - bad user id")
 	}
 
 	data["user_id"] = "12345678901234567890123456"
-	if _, err := Client.UpdateNotifyLevel(data); err == nil {
+	if _, err := Client.UpdateNotifyProps(data); err == nil {
 		t.Fatal("Should have errored - bad user id")
 	}
 
 	data["user_id"] = user.Id
 	data["channel_id"] = "junk"
-	if _, err := Client.UpdateNotifyLevel(data); err == nil {
+	if _, err := Client.UpdateNotifyProps(data); err == nil {
 		t.Fatal("Should have errored - bad channel id")
 	}
 
 	data["channel_id"] = "12345678901234567890123456"
-	if _, err := Client.UpdateNotifyLevel(data); err == nil {
+	if _, err := Client.UpdateNotifyProps(data); err == nil {
 		t.Fatal("Should have errored - bad channel id")
 	}
 
-	data["channel_id"] = channel1.Id
-	data["notify_level"] = ""
-	if _, err := Client.UpdateNotifyLevel(data); err == nil {
-		t.Fatal("Should have errored - empty notify level")
+	data["desktop"] = "junk"
+	data["mark_unread"] = model.CHANNEL_MARK_UNREAD_ALL
+	if _, err := Client.UpdateNotifyProps(data); err == nil {
+		t.Fatal("Should have errored - bad desktop notify level")
 	}
 
-	data["notify_level"] = "junk"
-	if _, err := Client.UpdateNotifyLevel(data); err == nil {
-		t.Fatal("Should have errored - bad notify level")
+	data["desktop"] = model.CHANNEL_NOTIFY_ALL
+	data["mark_unread"] = "junk"
+	if _, err := Client.UpdateNotifyProps(data); err == nil {
+		t.Fatal("Should have errored - bad mark unread level")
 	}
 
 	user2 := &model.User{TeamId: team.Id, Email: model.NewId() + "corey@test.com", Nickname: "Corey Hulen", Password: "pwd"}
@@ -879,8 +918,9 @@ func TestUpdateNotifyLevel(t *testing.T) {
 
 	data["channel_id"] = channel1.Id
 	data["user_id"] = user2.Id
-	data["notify_level"] = model.CHANNEL_NOTIFY_MENTION
-	if _, err := Client.UpdateNotifyLevel(data); err == nil {
+	data["desktop"] = model.CHANNEL_NOTIFY_MENTION
+	data["mark_unread"] = model.CHANNEL_MARK_UNREAD_MENTION
+	if _, err := Client.UpdateNotifyProps(data); err == nil {
 		t.Fatal("Should have errored - user not in channel")
 	}
 }