mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-01 15:00:08 +08:00
[MM-69688] Fix /share-channel status always showing "--" for Last Sync (#37345)
* [MM-69688] Fix /share-channel status Last Sync always showing -- GetRemotesStatus never selected the sync cursor columns, so SharedChannelRemoteStatus.NextSyncAt was always the zero value and formatTimestamp rendered "--". Select GREATEST of the LastPostUpdateAt and LastMembersSyncAt cursors as LastSyncAt and render that field. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69688] Cover LastSyncAt in GetRemotesStatus store test Advance the post and members sync cursors and assert GetRemotesStatus reports LastSyncAt as the greater of the two, and reports zero when a remote has never synced. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69688] Add command and formatTimestamp coverage for Last Sync Add doStatus command-layer tests asserting the /share-channel status output renders a timestamp for a synced remote and "--" for an unsynced one, a formatTimestamp unit test for the zero/today/older branches, and harden the store test so both GREATEST operands are non-zero. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69688] Assert the non-today timestamp format in formatTimestamp test Co-authored-by: mattermost-code <matty-code@mattermost.com> * Trigger CI re-run after transient Postgres shard 3 Docker Hub failure Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69688] Include LastPostCreateAt in Last Sync computation --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com>
This commit is contained in:
@@ -301,7 +301,7 @@ func (sp *ShareProvider) doStatus(a *app.App, args *model.CommandArgs, _ map[str
|
||||
accepted := formatBool(args.T, status.IsInviteAccepted)
|
||||
online := formatBool(args.T, isOnline(status.LastPingAt))
|
||||
|
||||
lastSync := formatTimestamp(status.NextSyncAt)
|
||||
lastSync := formatTimestamp(status.LastSyncAt)
|
||||
|
||||
fmt.Fprintf(&sb, "| %s | %s | %s | %s | %s | %s |\n",
|
||||
status.DisplayName, status.SiteURL, readonly, accepted, online, lastSync)
|
||||
|
||||
@@ -127,6 +127,87 @@ func TestShareProviderDoCommand(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestShareProviderDoStatus(t *testing.T) {
|
||||
seedInvitedRemote := func(t *testing.T, th *TestHelper, channelID string) *model.SharedChannelRemote {
|
||||
t.Helper()
|
||||
rc, err := th.App.AddRemoteCluster(&model.RemoteCluster{
|
||||
RemoteId: model.NewId(),
|
||||
Name: "remote-" + model.NewId(),
|
||||
DisplayName: "Remote Status Display",
|
||||
SiteURL: "https://remote-status.example.com",
|
||||
Token: model.NewId(),
|
||||
Topics: "topic",
|
||||
CreateAt: model.GetMillis(),
|
||||
LastPingAt: model.GetMillis(),
|
||||
CreatorId: model.NewId(),
|
||||
})
|
||||
require.Nil(t, err)
|
||||
|
||||
scr, saveErr := th.App.SaveSharedChannelRemote(&model.SharedChannelRemote{
|
||||
Id: model.NewId(),
|
||||
ChannelId: channelID,
|
||||
RemoteId: rc.RemoteId,
|
||||
CreatorId: th.BasicUser.Id,
|
||||
IsInviteAccepted: true,
|
||||
IsInviteConfirmed: true,
|
||||
})
|
||||
require.NoError(t, saveErr)
|
||||
return scr
|
||||
}
|
||||
|
||||
statusArgs := func(th *TestHelper, channelID string) *model.CommandArgs {
|
||||
return &model.CommandArgs{
|
||||
T: func(s string, args ...any) string { return s },
|
||||
ChannelId: channelID,
|
||||
UserId: th.BasicUser.Id,
|
||||
TeamId: th.BasicTeam.Id,
|
||||
Command: "/share-channel status",
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("renders the last sync timestamp for a remote that has synced", func(t *testing.T) {
|
||||
th := setupForSharedChannels(t).initBasic(t)
|
||||
th.addPermissionToRole(t, model.PermissionManageSharedChannels.Id, th.BasicUser.Roles)
|
||||
|
||||
mockSyncService := app.NewMockSharedChannelService(th.Server.GetSharedChannelSyncService())
|
||||
th.Server.SetSharedChannelSyncService(mockSyncService)
|
||||
|
||||
channel := th.CreateChannel(t, th.BasicTeam, WithShared(true))
|
||||
scr := seedInvitedRemote(t, th, channel.Id)
|
||||
|
||||
syncAt := model.GetMillis() - 5000
|
||||
require.NoError(t, th.App.Srv().Store().SharedChannel().UpdateRemoteCursor(scr.Id, model.GetPostsSinceForSyncCursor{
|
||||
LastPostUpdateAt: syncAt,
|
||||
LastPostUpdateID: model.NewId(),
|
||||
}))
|
||||
|
||||
commandProvider := ShareProvider{}
|
||||
response := commandProvider.DoCommand(th.App, th.Context, statusArgs(th, channel.Id), "")
|
||||
|
||||
require.Contains(t, response.Text, "Remote Status Display")
|
||||
assert.Contains(t, response.Text, "Today ", "expected a rendered timestamp in the Last Sync column, got:\n%s", response.Text)
|
||||
assert.NotContains(t, response.Text, "| -- |", "Last Sync column should not be the placeholder for a synced remote:\n%s", response.Text)
|
||||
})
|
||||
|
||||
t.Run("renders a placeholder when the remote has never synced", func(t *testing.T) {
|
||||
th := setupForSharedChannels(t).initBasic(t)
|
||||
th.addPermissionToRole(t, model.PermissionManageSharedChannels.Id, th.BasicUser.Roles)
|
||||
|
||||
mockSyncService := app.NewMockSharedChannelService(th.Server.GetSharedChannelSyncService())
|
||||
th.Server.SetSharedChannelSyncService(mockSyncService)
|
||||
|
||||
channel := th.CreateChannel(t, th.BasicTeam, WithShared(true))
|
||||
seedInvitedRemote(t, th, channel.Id)
|
||||
|
||||
commandProvider := ShareProvider{}
|
||||
response := commandProvider.DoCommand(th.App, th.Context, statusArgs(th, channel.Id), "")
|
||||
|
||||
require.Contains(t, response.Text, "Remote Status Display")
|
||||
assert.Contains(t, response.Text, "| -- |", "expected the Last Sync placeholder for an unsynced remote, got:\n%s", response.Text)
|
||||
assert.NotContains(t, response.Text, "Today ", "unsynced remote should not render a timestamp:\n%s", response.Text)
|
||||
})
|
||||
}
|
||||
|
||||
func TestShareProviderGetAutoCompleteListItemsPermission(t *testing.T) {
|
||||
connectionIDArg := func() *model.AutocompleteArg {
|
||||
return &model.AutocompleteArg{Name: "connectionID"}
|
||||
|
||||
@@ -4,9 +4,13 @@
|
||||
package slashcommands
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/mattermost/mattermost/server/public/model"
|
||||
)
|
||||
|
||||
func TestParseNamedArgs(t *testing.T) {
|
||||
@@ -38,3 +42,22 @@ func TestParseNamedArgs(t *testing.T) {
|
||||
assert.Equal(t, tt.m, m, tt.name)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatTimestamp(t *testing.T) {
|
||||
t.Run("zero renders as placeholder", func(t *testing.T) {
|
||||
assert.Equal(t, "--", formatTimestamp(0))
|
||||
})
|
||||
|
||||
t.Run("today renders with a Today prefix", func(t *testing.T) {
|
||||
got := formatTimestamp(model.GetMillis())
|
||||
assert.True(t, strings.HasPrefix(got, "Today "), "expected a Today prefix, got %q", got)
|
||||
})
|
||||
|
||||
t.Run("an earlier day renders the full date without the placeholder", func(t *testing.T) {
|
||||
twoDaysAgo := model.GetMillis() - int64(48*time.Hour/time.Millisecond)
|
||||
got := formatTimestamp(twoDaysAgo)
|
||||
assert.NotEqual(t, "--", got)
|
||||
assert.False(t, strings.HasPrefix(got, "Today "), "expected a full date, got %q", got)
|
||||
assert.Equal(t, model.GetTimeForMillis(twoDaysAgo).Format("Jan 2 15:04:05 MST 2006"), got)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -679,7 +679,7 @@ func (s SqlSharedChannelStore) GetRemotesStatus(channelId string) ([]*model.Shar
|
||||
status := []*model.SharedChannelRemoteStatus{}
|
||||
|
||||
query := s.getQueryBuilder().
|
||||
Select("scr.ChannelId, scr.RemoteId, rc.DisplayName, rc.SiteURL, rc.LastPingAt, sc.ReadOnly, scr.IsInviteAccepted").
|
||||
Select("scr.ChannelId, scr.RemoteId, rc.DisplayName, rc.SiteURL, rc.LastPingAt, sc.ReadOnly, scr.IsInviteAccepted, GREATEST(COALESCE(scr.LastPostCreateAt, 0), COALESCE(scr.LastPostUpdateAt, 0), COALESCE(scr.LastMembersSyncAt, 0)) AS LastSyncAt").
|
||||
From("SharedChannelRemotes scr, RemoteClusters rc, SharedChannels sc").
|
||||
Where("scr.RemoteId = rc.RemoteId").
|
||||
Where("scr.DeleteAt = 0").
|
||||
|
||||
@@ -815,6 +815,37 @@ func testGetRemotesStatus(t *testing.T, rctx request.CTX, ss store.Store) {
|
||||
_, err = ss.SharedChannel().SaveRemote(scr2)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Advance the sync cursors so the reported LastSyncAt reflects real activity.
|
||||
// scr1's members sync is newer than its post sync, so LastSyncAt must track the
|
||||
// members cursor. scr2's post-create cursor is the newest of all three cursors,
|
||||
// so LastSyncAt must track it. The post-create cursor advances independently of
|
||||
// the post-update cursor (see GetPostsSinceForSync), so it can be the most recent
|
||||
// sync activity and must be one of the GREATEST operands.
|
||||
now := model.GetMillis()
|
||||
scr1PostSyncAt := now - 5000
|
||||
scr1MembersSyncAt := now - 1000
|
||||
scr2PostCreateAt := now - 500
|
||||
scr2PostUpdateAt := now - 3000
|
||||
scr2MembersSyncAt := now - 7000
|
||||
|
||||
err = ss.SharedChannel().UpdateRemoteCursor(scr1.Id, model.GetPostsSinceForSyncCursor{
|
||||
LastPostUpdateAt: scr1PostSyncAt,
|
||||
LastPostUpdateID: model.NewId(),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
err = ss.SharedChannel().UpdateRemoteMembershipCursor(scr1.Id, scr1MembersSyncAt)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = ss.SharedChannel().UpdateRemoteCursor(scr2.Id, model.GetPostsSinceForSyncCursor{
|
||||
LastPostCreateAt: scr2PostCreateAt,
|
||||
LastPostCreateID: model.NewId(),
|
||||
LastPostUpdateAt: scr2PostUpdateAt,
|
||||
LastPostUpdateID: model.NewId(),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
err = ss.SharedChannel().UpdateRemoteMembershipCursor(scr2.Id, scr2MembersSyncAt)
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Run("Get remotes status for channel", func(t *testing.T) {
|
||||
statuses, err := ss.SharedChannel().GetRemotesStatus(channel.Id)
|
||||
require.NoError(t, err)
|
||||
@@ -833,6 +864,8 @@ func testGetRemotesStatus(t *testing.T, rctx request.CTX, ss store.Store) {
|
||||
assert.Equal(t, rc1.DisplayName, s1.DisplayName)
|
||||
assert.Equal(t, rc1.SiteURL, s1.SiteURL)
|
||||
assert.True(t, s1.IsInviteAccepted)
|
||||
// LastSyncAt is the greater of the post and members cursors.
|
||||
assert.Equal(t, scr1MembersSyncAt, s1.LastSyncAt)
|
||||
|
||||
s2 := statusMap[rc2.RemoteId]
|
||||
require.NotNil(t, s2)
|
||||
@@ -841,6 +874,34 @@ func testGetRemotesStatus(t *testing.T, rctx request.CTX, ss store.Store) {
|
||||
assert.Equal(t, rc2.DisplayName, s2.DisplayName)
|
||||
assert.Equal(t, rc2.SiteURL, s2.SiteURL)
|
||||
assert.False(t, s2.IsInviteAccepted)
|
||||
// The post-create cursor is the newest cursor, so LastSyncAt tracks it. If the
|
||||
// post-create cursor were excluded from GREATEST, this would report the older
|
||||
// post-update cursor instead.
|
||||
assert.Equal(t, scr2PostCreateAt, s2.LastSyncAt)
|
||||
})
|
||||
|
||||
t.Run("Reports zero last sync when the remote has never synced", func(t *testing.T) {
|
||||
unsyncedChannel, err := createTestChannel(ss, rctx, "test_remotes_status_unsynced")
|
||||
require.NoError(t, err)
|
||||
|
||||
_, scErr := shareChannel(ss, unsyncedChannel, true, "")
|
||||
require.NoError(t, scErr)
|
||||
|
||||
scrUnsynced := &model.SharedChannelRemote{
|
||||
Id: model.NewId(),
|
||||
ChannelId: unsyncedChannel.Id,
|
||||
RemoteId: rc1.RemoteId,
|
||||
CreatorId: rc1.CreatorId,
|
||||
IsInviteAccepted: true,
|
||||
IsInviteConfirmed: true,
|
||||
}
|
||||
_, err = ss.SharedChannel().SaveRemote(scrUnsynced)
|
||||
require.NoError(t, err)
|
||||
|
||||
statuses, err := ss.SharedChannel().GetRemotesStatus(unsyncedChannel.Id)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, statuses, 1)
|
||||
assert.Zero(t, statuses[0].LastSyncAt)
|
||||
})
|
||||
|
||||
t.Run("Get remotes status for channel with no remotes", func(t *testing.T) {
|
||||
|
||||
@@ -167,7 +167,7 @@ type SharedChannelRemoteStatus struct {
|
||||
DisplayName string `json:"display_name"`
|
||||
SiteURL string `json:"site_url"`
|
||||
LastPingAt int64 `json:"last_ping_at"`
|
||||
NextSyncAt int64 `json:"next_sync_at"`
|
||||
LastSyncAt int64 `json:"last_sync_at"`
|
||||
ReadOnly bool `json:"readonly"`
|
||||
IsInviteAccepted bool `json:"is_invite_accepted"`
|
||||
Token string `json:"token"`
|
||||
|
||||
Reference in New Issue
Block a user