Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 2 additions & 57 deletions .github/workflows/docs-impact-review.yml

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -1074,106 +1074,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) {
return allClustersReceived
}, 10*time.Second, 100*time.Millisecond, "All clusters should receive removal sync")
})
t.Run("Test 7: Feature flag disabled", func(t *testing.T) {
// This test verifies that the shared channel membership sync functionality respects the feature flag.
// It tests two scenarios:
// 1. When the feature flag is disabled, no sync messages should be sent even when NotifyMembershipChanged is called
// 2. When the feature flag is enabled, sync messages should be sent as expected
// This ensures that the feature can be safely disabled in production without triggering unintended syncs
EnsureCleanState(t, th, ss)
var syncMessageCount atomic.Int32

// Disable feature flag from the beginning to prevent any automatic sync
th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableSharedChannelsMemberSync = false })
t.Cleanup(func() {
th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableSharedChannelsMemberSync = true })
})

// Create test HTTP server that counts sync messages
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/api/v4/remotecluster/msg" {
syncMessageCount.Add(1)
}
writeOKResponse(w)
}))
defer testServer.Close()

// Create and share channel
channel := th.CreateChannel(t, th.BasicTeam)
sc := &model.SharedChannel{
ChannelId: channel.Id,
TeamId: th.BasicTeam.Id,
Home: true,
ShareName: "feature-flag-test",
CreatorId: th.BasicUser.Id,
RemoteId: "",
}
_, shareErr := th.App.ShareChannel(th.Context, sc)
require.NoError(t, shareErr)

// Create self-referential remote cluster
selfCluster := &model.RemoteCluster{
RemoteId: model.NewId(),
Name: "self-cluster-flag-test",
SiteURL: testServer.URL,
CreateAt: model.GetMillis(),
LastPingAt: model.GetMillis(),
Token: model.NewId(),
CreatorId: th.BasicUser.Id,
RemoteToken: model.NewId(),
}
selfCluster, err = ss.RemoteCluster().Save(selfCluster)
require.NoError(t, err)

// Share channel with self
scr := &model.SharedChannelRemote{
Id: model.NewId(),
ChannelId: channel.Id,
CreatorId: th.BasicUser.Id,
RemoteId: selfCluster.RemoteId,
IsInviteAccepted: true,
IsInviteConfirmed: true,
LastMembersSyncAt: 0,
}
_, err = ss.SharedChannel().SaveRemote(scr)
require.NoError(t, err)

// Test 1: Sync with feature flag disabled
th.App.UpdateConfig(func(cfg *model.Config) {
cfg.FeatureFlags.EnableSharedChannelsMemberSync = false
})

// Add users to the channel after disabling the feature flag
for range 3 {
user := th.CreateUser(t)
_, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user.Id, th.BasicUser.Id)
require.Nil(t, appErr)
_, appErr = th.App.AddUserToChannel(th.Context, user, channel, false)
require.Nil(t, appErr)
}

syncMessageCount.Store(0)
service.NotifyMembershipChanged(channel.Id, "")

// Verify no sync messages were sent
require.Never(t, func() bool {
return syncMessageCount.Load() > 0
}, 2*time.Second, 100*time.Millisecond, "No sync should occur with feature flag disabled")

// Test 2: Sync with feature flag enabled
th.App.UpdateConfig(func(cfg *model.Config) {
cfg.FeatureFlags.EnableSharedChannelsMemberSync = true
})

syncMessageCount.Store(0)
service.NotifyMembershipChanged(channel.Id, "")

// Verify sync messages were sent
require.Eventually(t, func() bool {
return syncMessageCount.Load() > 0
}, 5*time.Second, 100*time.Millisecond, "Sync should occur with feature flag enabled")
})
t.Run("Test 8: Sync Task After Connection Becomes Available", func(t *testing.T) {
t.Run("Test 7: Sync Task After Connection Becomes Available", func(t *testing.T) {
// This test verifies that membership sync works correctly when a remote cluster
// reconnects after being offline. We simulate a cluster that has been unavailable
// (old LastPingAt timestamp) and then comes back online.
Expand Down
1 change: 0 additions & 1 deletion server/channels/app/shared_channel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func setupSharedChannels(tb testing.TB) *TestHelper {
return SetupConfig(tb, func(cfg *model.Config) {
*cfg.ConnectedWorkspacesSettings.EnableRemoteClusterService = true
*cfg.ConnectedWorkspacesSettings.EnableSharedChannels = true
cfg.FeatureFlags.EnableSharedChannelsMemberSync = true
cfg.ClusterSettings.ClusterName = new("test-remote")
})
}
Expand Down
2 changes: 1 addition & 1 deletion server/config/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func GenerateClientConfig(c *model.Config, telemetryID string, license *model.Li

if license.HasSharedChannels() {
props["ExperimentalSharedChannels"] = strconv.FormatBool(*c.ConnectedWorkspacesSettings.EnableSharedChannels)
props["ExperimentalRemoteClusterService"] = strconv.FormatBool(c.FeatureFlags.EnableRemoteClusterService && *c.ConnectedWorkspacesSettings.EnableRemoteClusterService)
props["ExperimentalRemoteClusterService"] = strconv.FormatBool(*c.ConnectedWorkspacesSettings.EnableRemoteClusterService)
}

if model.MinimumProfessionalLicense(license) {
Expand Down
3 changes: 0 additions & 3 deletions server/platform/services/sharedchannel/channelinvite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,10 @@ var _ remotecluster.RemoteClusterServiceIFace = (*stubRemoteClusterService)(nil)

// setupMockServerWithConfig sets up the standard mocks that all tests need
func setupMockServerWithConfig(mockServer *MockServerIface) {
// Mock Config for feature flag check - disable membership sync to avoid complex mocking
mockConfig := model.Config{}
mockConfig.SetDefaults()
mockConfig.FeatureFlags.EnableSharedChannelsMemberSync = false
mockServer.On("Config").Return(&mockConfig)

// Mock GetRemoteClusterService for feature flag check
mockServer.On("GetRemoteClusterService").Return(nil)
}

Expand Down
5 changes: 1 addition & 4 deletions server/platform/services/sharedchannel/membership.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,8 @@ import (
"github.com/mattermost/mattermost/server/public/shared/mlog"
)

// isChannelMemberSyncEnabled checks if the feature flag is enabled and remote cluster service is available
func (scs *Service) isChannelMemberSyncEnabled() bool {
featureFlagEnabled := scs.server.Config().FeatureFlags.EnableSharedChannelsMemberSync
remoteClusterService := scs.server.GetRemoteClusterService()
return featureFlagEnabled && remoteClusterService != nil
return scs.server.GetRemoteClusterService() != nil
}

// NotifyMembershipChanged is called when users are added or removed from a shared channel.
Expand Down
5 changes: 0 additions & 5 deletions server/platform/services/sharedchannel/membership_recv.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ const (
// Out-of-order messages resolve naturally: if an old "add" arrives after a newer "remove",
// the sender's next sync cycle will send a corrective "remove" because the history shows the user left.
func (scs *Service) onReceiveMembershipChanges(syncMsg *model.SyncMsg, rc *model.RemoteCluster) error {
// Check if feature flag is enabled
if !scs.server.Config().FeatureFlags.EnableSharedChannelsMemberSync {
return nil
}

if len(syncMsg.MembershipChanges) == 0 {
return fmt.Errorf("onReceiveMembershipChanges: no membership changes")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,8 @@ func setupMembershipTest(t *testing.T) (*Service, *MockServerIface, *MockAppIfac
mockStore.On("User").Return(mockUserStore)
mockServer.On("GetStore").Return(mockStore)

// Enable membership sync feature flag
mockConfig := model.Config{}
mockConfig.SetDefaults()
mockConfig.FeatureFlags.EnableSharedChannelsMemberSync = true
mockServer.On("Config").Return(&mockConfig)

return scs, mockServer, mockApp, mockStore, mockSharedChannelStore, mockChannelStore, mockUserStore
Expand Down
34 changes: 9 additions & 25 deletions server/platform/services/sharedchannel/membership_send_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/mattermost/mattermost/server/v8/channels/store/storetest/mocks"
)

func setupSendTest(t *testing.T, enableMemberSync bool) (*Service, *mocks.Store, *mocks.ChannelMemberHistoryStore, *mocks.SharedChannelStore, *mocks.UserStore, *mocks.RemoteClusterStore) {
func setupSendTest(t *testing.T) (*Service, *mocks.Store, *mocks.ChannelMemberHistoryStore, *mocks.SharedChannelStore, *mocks.UserStore, *mocks.RemoteClusterStore) {
t.Helper()

mockServer := &MockServerIface{}
Expand Down Expand Up @@ -44,29 +44,13 @@ func setupSendTest(t *testing.T, enableMemberSync bool) (*Service, *mocks.Store,

mockConfig := model.Config{}
mockConfig.SetDefaults()
mockConfig.FeatureFlags.EnableSharedChannelsMemberSync = enableMemberSync
mockServer.On("Config").Return(&mockConfig)

return scs, mockStore, mockCMHStore, mockSharedChannelStore, mockUserStore, mockRCStore
}

func TestFetchMembershipsForSync_FeatureFlagDisabled(t *testing.T) {
scs, _, _, _, _, _ := setupSendTest(t, false)

sd := &syncData{
task: syncTask{channelID: model.NewId()},
rc: &model.RemoteCluster{RemoteId: model.NewId()},
scr: &model.SharedChannelRemote{LastMembersSyncAt: 0},
users: make(map[string]*model.User),
}

err := scs.fetchMembershipsForSync(sd)
require.NoError(t, err)
assert.Empty(t, sd.membershipChanges, "should not fetch when feature flag disabled")
}

func TestFetchMembershipsForSync_NoChanges(t *testing.T) {
scs, _, mockCMHStore, _, _, _ := setupSendTest(t, true)
scs, _, mockCMHStore, _, _, _ := setupSendTest(t)

channelID := model.NewId()
mockCMHStore.On("GetMembershipChanges", channelID, int64(0), mock.AnythingOfType("int")).
Expand All @@ -86,7 +70,7 @@ func TestFetchMembershipsForSync_NoChanges(t *testing.T) {
}

func TestFetchMembershipsForSync_DeduplicatesJoinLeaveRejoin(t *testing.T) {
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t, true)
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t)

channelID := model.NewId()
remoteID := model.NewId()
Expand Down Expand Up @@ -128,7 +112,7 @@ func TestFetchMembershipsForSync_DeduplicatesJoinLeaveRejoin(t *testing.T) {
}

func TestFetchMembershipsForSync_DeduplicatesJoinThenLeave(t *testing.T) {
scs, _, mockCMHStore, _, mockUserStore, _ := setupSendTest(t, true)
scs, _, mockCMHStore, _, mockUserStore, _ := setupSendTest(t)

channelID := model.NewId()
remoteID := model.NewId()
Expand Down Expand Up @@ -160,7 +144,7 @@ func TestFetchMembershipsForSync_DeduplicatesJoinThenLeave(t *testing.T) {
}

func TestFetchMembershipsForSync_MultipleUsers(t *testing.T) {
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t, true)
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t)

channelID := model.NewId()
remoteID := model.NewId()
Expand Down Expand Up @@ -221,7 +205,7 @@ func TestFetchMembershipsForSync_MultipleUsers(t *testing.T) {
}

func TestFetchMembershipsForSync_SetsRepeatWhenLimitHit(t *testing.T) {
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t, true)
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t)

channelID := model.NewId()
remoteID := model.NewId()
Expand Down Expand Up @@ -260,7 +244,7 @@ func TestFetchMembershipsForSync_SetsRepeatWhenLimitHit(t *testing.T) {
}

func TestFetchMembershipsForSync_CursorFromSCR(t *testing.T) {
scs, _, mockCMHStore, _, _, _ := setupSendTest(t, true)
scs, _, mockCMHStore, _, _, _ := setupSendTest(t)

channelID := model.NewId()
remoteID := model.NewId()
Expand All @@ -282,7 +266,7 @@ func TestFetchMembershipsForSync_CursorFromSCR(t *testing.T) {
}

func TestFetchMembershipsForSync_RepeatedTimestampsAtBoundary(t *testing.T) {
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t, true)
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t)

channelID := model.NewId()
remoteID := model.NewId()
Expand Down Expand Up @@ -363,7 +347,7 @@ func TestFetchMembershipsForSync_RepeatedTimestampsAtBoundary(t *testing.T) {
}

func TestFetchMembershipsForSync_SkipsUsersFromTargetRemote(t *testing.T) {
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t, true)
scs, _, mockCMHStore, mockSCStore, mockUserStore, _ := setupSendTest(t)

channelID := model.NewId()
remoteID := model.NewId()
Expand Down
11 changes: 1 addition & 10 deletions server/platform/services/sharedchannel/sync_recv.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,6 @@ func (scs *Service) ProcessSyncMessage(rctx request.CTX, syncMsg *model.SyncMsg,
MembershipErrors: make([]string, 0),
}

// Check if feature flag is enabled for membership changes
membershipSyncEnabled := scs.server.Config().FeatureFlags.EnableSharedChannelsMemberSync
hasMembershipChanges := len(syncMsg.MembershipChanges) > 0

// If this message only contains membership changes and feature is disabled, skip it
if hasMembershipChanges && !membershipSyncEnabled && len(syncMsg.Users) == 0 && len(syncMsg.Posts) == 0 && len(syncMsg.Reactions) == 0 {
return syncResp, nil
}

scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Sync msg received",
mlog.String("remote", rc.Name),
mlog.String("channel_id", syncMsg.ChannelId),
Expand Down Expand Up @@ -282,7 +273,7 @@ func (scs *Service) ProcessSyncMessage(rctx request.CTX, syncMsg *model.SyncMsg,
}

// Process membership changes after users have been synced
if hasMembershipChanges && membershipSyncEnabled {
if len(syncMsg.MembershipChanges) > 0 {
if err := scs.onReceiveMembershipChanges(syncMsg, rc); err != nil {
scs.server.Log().LogM(mlog.MlvlSharedChannelServiceError, "Error processing membership changes",
mlog.String("remote", rc.Name),
Expand Down
1 change: 0 additions & 1 deletion server/platform/services/sharedchannel/sync_recv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ func TestProcessSyncMessage(t *testing.T) {
makeConfig := func() *model.Config {
cfg := &model.Config{}
cfg.SetDefaults()
cfg.FeatureFlags.EnableSharedChannelsMemberSync = false
return cfg
}

Expand Down
4 changes: 0 additions & 4 deletions server/platform/services/sharedchannel/sync_send_remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,6 @@ func (scs *Service) fetchPostsForSync(sd *syncData) error {

// fetchMembershipsForSync populates the sync data with membership changes from ChannelMemberHistory.
func (scs *Service) fetchMembershipsForSync(sd *syncData) error {
if !scs.server.Config().FeatureFlags.EnableSharedChannelsMemberSync {
return nil
}

start := time.Now()
defer func() {
if metrics := scs.server.GetMetrics(); metrics != nil {
Expand Down
12 changes: 0 additions & 12 deletions server/public/model/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,9 @@ type FeatureFlags struct {
// all other values as false.
TestBoolFeature bool

// Enable the remote cluster service for shared channels.
EnableRemoteClusterService bool

// Enable DMs and GMs for shared channels.
EnableSharedChannelsDMs bool

// Enable plugins in shared channels.
EnableSharedChannelsPlugins bool

// Enable synchronization of channel members in shared channels
EnableSharedChannelsMemberSync bool

// Enable syncing all users for remote clusters in shared channels
EnableSyncAllUsersForRemoteCluster bool

Expand Down Expand Up @@ -157,11 +148,8 @@ type FeatureFlags struct {
func (f *FeatureFlags) SetDefaults() {
f.TestFeature = "off"
f.TestBoolFeature = false
f.EnableRemoteClusterService = false
f.EnableSharedChannelsDMs = false
f.EnableSharedChannelsMemberSync = false
f.EnableSyncAllUsersForRemoteCluster = false
f.EnableSharedChannelsPlugins = true
f.AppsEnabled = false
f.NormalizeLdapDNs = false
f.WysiwygEditor = false
Expand Down
12 changes: 3 additions & 9 deletions webapp/channels/src/components/actions_menu/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ import type {Post} from '@mattermost/types/posts';
import {Permissions} from 'mattermost-redux/constants';
import {AppBindingLocations} from 'mattermost-redux/constants/apps';
import {appsEnabled} from 'mattermost-redux/selectors/entities/apps';
import {getChannel} from 'mattermost-redux/selectors/entities/channels';
import {isMarketplaceEnabled, getFeatureFlagValue} from 'mattermost-redux/selectors/entities/general';
import {isMarketplaceEnabled} from 'mattermost-redux/selectors/entities/general';
import {haveICurrentTeamPermission} from 'mattermost-redux/selectors/entities/roles';
import {getCurrentTeamId} from 'mattermost-redux/selectors/entities/teams';
import {getCurrentUser} from 'mattermost-redux/selectors/entities/users';
Expand Down Expand Up @@ -48,9 +47,6 @@ function mapStateToProps(state: GlobalState, ownProps: Props) {
const {post} = ownProps;

const systemMessage = isSystemMessage(post);
const channel = getChannel(state, post.channel_id);
const sharedChannelsPluginsEnabled = getFeatureFlagValue(state, 'EnableSharedChannelsPlugins') === 'true';

const apps = appsEnabled(state);
const showBindings = apps && !systemMessage && !isCombinedUserActivityPost(post.id);
let appBindings: AppBinding[] | null = emptyBindings;
Expand All @@ -60,14 +56,12 @@ function mapStateToProps(state: GlobalState, ownProps: Props) {
const currentUser = getCurrentUser(state);
const isSysAdmin = isSystemAdmin(currentUser.roles);

const pluginItemsVisible = !channel?.shared || sharedChannelsPluginsEnabled;

return {
appBindings,
appsEnabled: apps,
pluginMenuItemComponents: pluginItemsVisible ? state.plugins.components.PostDropdownMenuItem : [],
pluginMenuItemComponents: state.plugins.components.PostDropdownMenuItem,
isSysAdmin,
pluginMenuItems: pluginItemsVisible ? state.plugins.components.PostDropdownMenu : [],
pluginMenuItems: state.plugins.components.PostDropdownMenu,
teamId: getCurrentTeamId(state),
isMobileView: getIsMobileView(state),
canOpenMarketplace: (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ const AdvancedTextEditor = ({
}]));
}, [dispatch, currentUserId, getFormattingBarPreferenceName, isFormattingBarHidden]);

const pluginItems = usePluginItems(draft, textboxRef, handleDraftChange, channelId);
const pluginItems = usePluginItems(draft, textboxRef, handleDraftChange);
const focusTextbox = useTextboxFocus(textboxRef, channelId, isRHS, canPost, wysiwygRef);
const {
rewriteMenuProps,
Expand Down
Loading
Loading