MM-33359 corrected unread replies update (#17068)

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
2021-03-09 16:10:47 +02:00
--- a/store/opentracinglayer/opentracinglayer.go
+++ b/store/opentracinglayer/opentracinglayer.go
@@ -7738,24 +7738,6 @@ func (s *OpenTracingLayerThreadStore) CollectThreadsWithNewerReplies(userId stri
 	return result, err
 }

-func (s *OpenTracingLayerThreadStore) CreateMembershipIfNeeded(userId string, postID string, following bool, incrementMentions bool, updateFollowing bool) error {
-	origCtx := s.Root.Store.Context()
-	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ThreadStore.CreateMembershipIfNeeded")
-	s.Root.Store.SetContext(newCtx)
-	defer func() {
-		s.Root.Store.SetContext(origCtx)
-	}()
-
-	defer span.Finish()
-	err := s.ThreadStore.CreateMembershipIfNeeded(userId, postID, following, incrementMentions, updateFollowing)
-	if err != nil {
-		span.LogFields(spanlog.Error(err))
-		ext.Error.Set(span, true)
-	}
-
-	return err
-}
-
 func (s *OpenTracingLayerThreadStore) Delete(postId string) error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ThreadStore.Delete")
@@ -7918,6 +7900,24 @@ func (s *OpenTracingLayerThreadStore) GetThreadsForUser(userId string, teamId st
 	return result, err
 }

+func (s *OpenTracingLayerThreadStore) MaintainMembership(userId string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool) error {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ThreadStore.MaintainMembership")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	err := s.ThreadStore.MaintainMembership(userId, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
+	if err != nil {
+		span.LogFields(spanlog.Error(err))
+		ext.Error.Set(span, true)
+	}
+
+	return err
+}
+
 func (s *OpenTracingLayerThreadStore) MarkAllAsRead(userId string, teamID string) error {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ThreadStore.MarkAllAsRead")
--- a/store/retrylayer/retrylayer.go
+++ b/store/retrylayer/retrylayer.go
@@ -8398,26 +8398,6 @@ func (s *RetryLayerThreadStore) CollectThreadsWithNewerReplies(userId string, ch

 }

-func (s *RetryLayerThreadStore) CreateMembershipIfNeeded(userId string, postID string, following bool, incrementMentions bool, updateFollowing bool) error {
-
-	tries := 0
-	for {
-		err := s.ThreadStore.CreateMembershipIfNeeded(userId, postID, following, incrementMentions, updateFollowing)
-		if err == nil {
-			return nil
-		}
-		if !isRepeatableError(err) {
-			return err
-		}
-		tries++
-		if tries >= 3 {
-			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
-			return err
-		}
-	}
-
-}
-
 func (s *RetryLayerThreadStore) Delete(postId string) error {

 	tries := 0
@@ -8598,6 +8578,26 @@ func (s *RetryLayerThreadStore) GetThreadsForUser(userId string, teamId string,

 }

+func (s *RetryLayerThreadStore) MaintainMembership(userId string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool) error {
+
+	tries := 0
+	for {
+		err := s.ThreadStore.MaintainMembership(userId, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
+		if err == nil {
+			return nil
+		}
+		if !isRepeatableError(err) {
+			return err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return err
+		}
+	}
+
+}
+
 func (s *RetryLayerThreadStore) MarkAllAsRead(userId string, teamID string) error {

 	tries := 0
--- a/store/sqlstore/thread_store.go
+++ b/store/sqlstore/thread_store.go
@@ -501,14 +501,22 @@ func (s *SqlThreadStore) DeleteMembershipForUser(userId string, postId string) e
 	return nil
 }

-func (s *SqlThreadStore) CreateMembershipIfNeeded(userId, postId string, following, incrementMentions, updateFollowing bool) error {
+func (s *SqlThreadStore) MaintainMembership(userId, postId string, following, incrementMentions, updateFollowing, updateViewedTimestamp bool) error {
 	membership, err := s.GetMembershipForUser(userId, postId)
 	now := utils.MillisFromTime(time.Now())
+	// if memebership exists, update it if:
+	// a. user started/stopped following a thread
+	// b. mention count changed
+	// c. user viewed a thread
 	if err == nil {
-		if (updateFollowing && !membership.Following || membership.Following != following) || incrementMentions {
-			if updateFollowing {
+		followingNeedsUpdate := (updateFollowing && !membership.Following || membership.Following != following)
+		if followingNeedsUpdate || incrementMentions || updateViewedTimestamp {
+			if followingNeedsUpdate {
 				membership.Following = following
 			}
+			if updateViewedTimestamp {
+				membership.LastViewed = now
+			}
 			membership.LastUpdated = now
 			if incrementMentions {
 				membership.UnreadMentions += 1
--- a/store/store.go
+++ b/store/store.go
@@ -265,7 +265,7 @@ type ThreadStore interface {
 	GetMembershipsForUser(userId, teamID string) ([]*model.ThreadMembership, error)
 	GetMembershipForUser(userId, postID string) (*model.ThreadMembership, error)
 	DeleteMembershipForUser(userId, postID string) error
-	CreateMembershipIfNeeded(userId, postID string, following, incrementMentions, updateFollowing bool) error
+	MaintainMembership(userId, postID string, following, incrementMentions, updateFollowing, updateViewedTimestamp bool) error
 	CollectThreadsWithNewerReplies(userId string, channelIds []string, timestamp int64) ([]string, error)
 	UpdateUnreadsByChannel(userId string, changedThreads []string, timestamp int64, updateViewedTimestamp bool) error
 }
--- a/store/storetest/mocks/ThreadStore.go
+++ b/store/storetest/mocks/ThreadStore.go
@@ -37,20 +37,6 @@ func (_m *ThreadStore) CollectThreadsWithNewerReplies(userId string, channelIds
 	return r0, r1
 }

-// CreateMembershipIfNeeded provides a mock function with given fields: userId, postID, following, incrementMentions, updateFollowing
-func (_m *ThreadStore) CreateMembershipIfNeeded(userId string, postID string, following bool, incrementMentions bool, updateFollowing bool) error {
-	ret := _m.Called(userId, postID, following, incrementMentions, updateFollowing)
-
-	var r0 error
-	if rf, ok := ret.Get(0).(func(string, string, bool, bool, bool) error); ok {
-		r0 = rf(userId, postID, following, incrementMentions, updateFollowing)
-	} else {
-		r0 = ret.Error(0)
-	}
-
-	return r0
-}
-
 // Delete provides a mock function with given fields: postId
 func (_m *ThreadStore) Delete(postId string) error {
 	ret := _m.Called(postId)
@@ -240,6 +226,20 @@ func (_m *ThreadStore) GetThreadsForUser(userId string, teamId string, opts mode
 	return r0, r1
 }

+// MaintainMembership provides a mock function with given fields: userId, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp
+func (_m *ThreadStore) MaintainMembership(userId string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool) error {
+	ret := _m.Called(userId, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, string, bool, bool, bool, bool) error); ok {
+		r0 = rf(userId, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // MarkAllAsRead provides a mock function with given fields: userId, teamID
 func (_m *ThreadStore) MarkAllAsRead(userId string, teamID string) error {
 	ret := _m.Called(userId, teamID)
--- a/store/storetest/mocks/dbSelecter.go
+++ b/store/storetest/mocks/dbSelecter.go
@@ -0,0 +1,38 @@
+// Code generated by mockery v1.0.0. DO NOT EDIT.
+
+// Regenerate this file using `make store-mocks`.
+
+package mocks
+
+import mock "github.com/stretchr/testify/mock"
+
+// dbSelecter is an autogenerated mock type for the dbSelecter type
+type dbSelecter struct {
+	mock.Mock
+}
+
+// Select provides a mock function with given fields: i, query, args
+func (_m *dbSelecter) Select(i interface{}, query string, args ...interface{}) ([]interface{}, error) {
+	var _ca []interface{}
+	_ca = append(_ca, i, query)
+	_ca = append(_ca, args...)
+	ret := _m.Called(_ca...)
+
+	var r0 []interface{}
+	if rf, ok := ret.Get(0).(func(interface{}, string, ...interface{}) []interface{}); ok {
+		r0 = rf(i, query, args...)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]interface{})
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(interface{}, string, ...interface{}) error); ok {
+		r1 = rf(i, query, args...)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
--- a/store/storetest/thread_store.go
+++ b/store/storetest/thread_store.go
@@ -235,7 +235,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
 	t.Run("Thread last updated is changed when channel is updated after UpdateLastViewedAtPost", func(t *testing.T) {
 		newPosts := makeSomePosts()

-		require.NoError(t, ss.Thread().CreateMembershipIfNeeded(newPosts[0].UserId, newPosts[0].Id, true, false, true))
+		require.NoError(t, ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false))
 		m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
 		require.NoError(t, err1)
 		m.LastUpdated -= 1000
@@ -255,7 +255,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
 	t.Run("Thread last updated is changed when channel is updated after IncrementMentionCount", func(t *testing.T) {
 		newPosts := makeSomePosts()

-		require.NoError(t, ss.Thread().CreateMembershipIfNeeded(newPosts[0].UserId, newPosts[0].Id, true, false, true))
+		require.NoError(t, ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false))
 		m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
 		require.NoError(t, err1)
 		m.LastUpdated -= 1000
@@ -275,7 +275,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
 	t.Run("Thread last updated is changed when channel is updated after UpdateLastViewedAt", func(t *testing.T) {
 		newPosts := makeSomePosts()

-		require.NoError(t, ss.Thread().CreateMembershipIfNeeded(newPosts[0].UserId, newPosts[0].Id, true, false, true))
+		require.NoError(t, ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false))
 		m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
 		require.NoError(t, err1)
 		m.LastUpdated -= 1000
@@ -292,10 +292,24 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
 		}, time.Second, 10*time.Millisecond)
 	})

+	t.Run("Thread membership 'viewed' timestamp is updated properly", func(t *testing.T) {
+		newPosts := makeSomePosts()
+
+		require.NoError(t, ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false))
+		m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
+		require.NoError(t, err1)
+		require.Equal(t, int64(0), m.LastViewed)
+
+		require.NoError(t, ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, true))
+		m2, err2 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
+		require.NoError(t, err2)
+		require.Greater(t, m2.LastViewed, int64(0))
+	})
+
 	t.Run("Thread last updated is changed when channel is updated after UpdateLastViewedAtPost for mark unread", func(t *testing.T) {
 		newPosts := makeSomePosts()

-		require.NoError(t, ss.Thread().CreateMembershipIfNeeded(newPosts[0].UserId, newPosts[0].Id, true, false, true))
+		require.NoError(t, ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false))
 		m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
 		require.NoError(t, err1)
 		m.LastUpdated += 1000
--- a/store/timerlayer/timerlayer.go
+++ b/store/timerlayer/timerlayer.go
@@ -6984,22 +6984,6 @@ func (s *TimerLayerThreadStore) CollectThreadsWithNewerReplies(userId string, ch
 	return result, err
 }

-func (s *TimerLayerThreadStore) CreateMembershipIfNeeded(userId string, postID string, following bool, incrementMentions bool, updateFollowing bool) error {
-	start := timemodule.Now()
-
-	err := s.ThreadStore.CreateMembershipIfNeeded(userId, postID, following, incrementMentions, updateFollowing)
-
-	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
-	if s.Root.Metrics != nil {
-		success := "false"
-		if err == nil {
-			success = "true"
-		}
-		s.Root.Metrics.ObserveStoreMethodDuration("ThreadStore.CreateMembershipIfNeeded", success, elapsed)
-	}
-	return err
-}
-
 func (s *TimerLayerThreadStore) Delete(postId string) error {
 	start := timemodule.Now()

@@ -7144,6 +7128,22 @@ func (s *TimerLayerThreadStore) GetThreadsForUser(userId string, teamId string,
 	return result, err
 }

+func (s *TimerLayerThreadStore) MaintainMembership(userId string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool) error {
+	start := timemodule.Now()
+
+	err := s.ThreadStore.MaintainMembership(userId, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
+
+	elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("ThreadStore.MaintainMembership", success, elapsed)
+	}
+	return err
+}
+
 func (s *TimerLayerThreadStore) MarkAllAsRead(userId string, teamID string) error {
 	start := timemodule.Now()