Adds default team to the remote cluster entity (#27863)

* Adds default team to the remote cluster entity

A new DefaultTeamId field is added to the RemoteCluster entity and its
endpoints, and used when receiving channel invites to choose in which
team to create a new channel.

This will be later extended with the ability for the system admin to
manually accept invites, choosing which team to create the channel on
each. This use case will be triggered when the DefaultTeamId field is
empty, which now simply chooses the first team it finds in the
database as a fallback.

* Fix migrations list

* Fixes channelinvite test case

* Fix i18n

* Fix migration list
This commit is contained in:
Miguel de la Cruz
2024-08-08 16:36:27 +02:00
committed by GitHub
parent 3d8d0f5e3e
commit 59873a2a50
14 changed files with 99 additions and 39 deletions

View File

@@ -3499,6 +3499,9 @@ components:
site_url:
description: URL of the remote cluster
type: string
default_team_id:
description: The team where channels from invites are created
type: string
create_at:
description: Time in milliseconds that the remote cluster was created
type: integer

View File

@@ -171,6 +171,9 @@
properties:
display_name:
type: string
default_team_id:
type: string
description: The team where channels from invites are created
responses:
"200":
description: Remote Cluster patch successful

View File

@@ -378,11 +378,12 @@ func createRemoteCluster(c *Context, w http.ResponseWriter, r *http.Request) {
}
rc := &model.RemoteCluster{
Name: rcWithTeamAndPassword.Name,
DisplayName: rcWithTeamAndPassword.DisplayName,
SiteURL: model.SiteURLPending + model.NewId(),
Token: model.NewId(),
CreatorId: c.AppContext.Session().UserId,
Name: rcWithTeamAndPassword.Name,
DisplayName: rcWithTeamAndPassword.DisplayName,
SiteURL: model.SiteURLPending + model.NewId(),
DefaultTeamId: rcWithTeamAndPassword.DefaultTeamId,
Token: model.NewId(),
CreatorId: c.AppContext.Session().UserId,
}
audit.AddEventParameterAuditable(auditRec, "remotecluster", rc)

View File

@@ -158,9 +158,10 @@ func TestGetRemoteClusters(t *testing.T) {
func TestCreateRemoteCluster(t *testing.T) {
rcWithTeamAndPassword := &model.RemoteClusterWithPassword{
RemoteCluster: &model.RemoteCluster{
Name: "remotecluster",
SiteURL: "http://example.com",
Token: model.NewId(),
Name: "remotecluster",
SiteURL: "http://example.com",
DefaultTeamId: model.NewId(),
Token: model.NewId(),
},
Password: "mysupersecret",
}
@@ -231,6 +232,7 @@ func TestCreateRemoteCluster(t *testing.T) {
CheckCreatedStatus(t, resp)
require.NoError(t, err)
require.Equal(t, rcWithTeamAndPassword.Name, rcWithInvite.RemoteCluster.Name)
require.Equal(t, rcWithTeamAndPassword.DefaultTeamId, rcWithInvite.RemoteCluster.DefaultTeamId)
require.NotZero(t, rcWithInvite.Invite)
require.Zero(t, rcWithInvite.RemoteCluster.Token)
require.Zero(t, rcWithInvite.RemoteCluster.RemoteToken)
@@ -431,6 +433,7 @@ func TestGetRemoteCluster(t *testing.T) {
defer th.TearDown()
newRC.CreatorId = th.SystemAdminUser.Id
newRC.DefaultTeamId = th.BasicTeam.Id
rc, appErr := th.App.AddRemoteCluster(newRC)
require.Nil(t, appErr)
@@ -455,6 +458,7 @@ func TestGetRemoteCluster(t *testing.T) {
CheckOKStatus(t, resp)
require.NoError(t, err)
require.Equal(t, rc.RemoteId, fetchedRC.RemoteId)
require.Equal(t, th.BasicTeam.Id, fetchedRC.DefaultTeamId)
require.Empty(t, fetchedRC.Token)
})
}
@@ -509,12 +513,17 @@ func TestPatchRemoteCluster(t *testing.T) {
})
t.Run("should correctly patch the remote cluster", func(t *testing.T) {
rcp := &model.RemoteClusterPatch{DisplayName: model.NewPointer("patched!")}
newTeamId := model.NewId()
rcp := &model.RemoteClusterPatch{
DisplayName: model.NewPointer("patched!"),
DefaultTeamId: model.NewPointer(newTeamId),
}
patchedRC, resp, err := th.SystemAdminClient.PatchRemoteCluster(context.Background(), rc.RemoteId, rcp)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.Equal(t, "patched!", patchedRC.DisplayName)
require.Equal(t, newTeamId, patchedRC.DefaultTeamId)
})
}

View File

@@ -246,6 +246,8 @@ channels/db/migrations/mysql/000123_remove_upload_file_permission.down.sql
channels/db/migrations/mysql/000123_remove_upload_file_permission.up.sql
channels/db/migrations/mysql/000124_remove_manage_team_permission.down.sql
channels/db/migrations/mysql/000124_remove_manage_team_permission.up.sql
channels/db/migrations/mysql/000125_remoteclusters_add_default_team_id.down.sql
channels/db/migrations/mysql/000125_remoteclusters_add_default_team_id.up.sql
channels/db/migrations/postgres/000001_create_teams.down.sql
channels/db/migrations/postgres/000001_create_teams.up.sql
channels/db/migrations/postgres/000002_create_team_members.down.sql
@@ -492,3 +494,5 @@ channels/db/migrations/postgres/000123_remove_upload_file_permission.down.sql
channels/db/migrations/postgres/000123_remove_upload_file_permission.up.sql
channels/db/migrations/postgres/000124_remove_manage_team_permission.down.sql
channels/db/migrations/postgres/000124_remove_manage_team_permission.up.sql
channels/db/migrations/postgres/000125_remoteclusters_add_default_team_id.down.sql
channels/db/migrations/postgres/000125_remoteclusters_add_default_team_id.up.sql

View File

@@ -0,0 +1 @@
-- Skipping it because the forward migrations are destructive

View File

@@ -0,0 +1,14 @@
SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS
WHERE table_name = 'RemoteClusters'
AND table_schema = DATABASE()
AND column_name = 'DefaultTeamId'
) > 0,
'SELECT 1',
'ALTER TABLE RemoteClusters ADD DefaultTeamId VARCHAR(26) DEFAULT "";'
));
PREPARE alterIfNotExists FROM @preparedStatement;
EXECUTE alterIfNotExists;
DEALLOCATE PREPARE alterIfNotExists;

View File

@@ -0,0 +1 @@
-- Skipping it because the forward migrations are destructive

View File

@@ -0,0 +1 @@
ALTER TABLE remoteclusters ADD COLUMN IF NOT EXISTS defaultteamid character varying(26) DEFAULT '';

View File

@@ -34,6 +34,7 @@ func remoteClusterFields(prefix string) []string {
prefix + "Name",
prefix + "DisplayName",
prefix + "SiteURL",
prefix + "DefaultTeamId",
prefix + "CreateAt",
prefix + "LastPingAt",
prefix + "Token",
@@ -65,10 +66,10 @@ func (s sqlRemoteClusterStore) Save(remoteCluster *model.RemoteCluster) (*model.
}
query := `INSERT INTO RemoteClusters
(RemoteId, RemoteTeamId, Name, DisplayName, SiteURL, CreateAt,
(RemoteId, RemoteTeamId, Name, DisplayName, SiteURL, DefaultTeamId, CreateAt,
LastPingAt, Token, RemoteToken, Topics, CreatorId, PluginID, Options)
VALUES
(:RemoteId, :RemoteTeamId, :Name, :DisplayName, :SiteURL, :CreateAt,
(:RemoteId, :RemoteTeamId, :Name, :DisplayName, :SiteURL, :DefaultTeamId, :CreateAt,
:LastPingAt, :Token, :RemoteToken, :Topics, :CreatorId, :PluginID, :Options)`
if _, err := s.GetMasterX().NamedExec(query, remoteCluster); err != nil {
@@ -93,6 +94,7 @@ func (s sqlRemoteClusterStore) Update(remoteCluster *model.RemoteCluster) (*mode
CreatorId = :CreatorId,
DisplayName = :DisplayName,
SiteURL = :SiteURL,
DefaultTeamId = :DefaultTeamId,
Topics = :Topics,
PluginID = :PluginID,
Options = :Options

View File

@@ -255,8 +255,8 @@ func (scs *Service) handleChannelCreation(invite channelInviteMsg, rc *model.Rem
return scs.createDirectChannel(invite, rc)
}
teamId := invite.TeamId
// if the invite doesn't have a teamId associated and until the
teamId := rc.DefaultTeamId
// if the remote doesn't have a teamId associated and until the
// acceptance of an invite includes selecting a team, we use the
// first team of the list
if teamId == "" {

View File

@@ -62,7 +62,7 @@ func TestOnReceiveChannelInvite(t *testing.T) {
}
mockStore := &mocks.Store{}
remoteCluster := &model.RemoteCluster{Name: "test"}
remoteCluster := &model.RemoteCluster{Name: "test", DefaultTeamId: model.NewId()}
invitation := channelInviteMsg{
ChannelId: model.NewId(),
TeamId: model.NewId(),
@@ -144,9 +144,15 @@ func TestOnReceiveChannelInvite(t *testing.T) {
channel := &model.Channel{
Id: invitation.ChannelId,
}
mockTeamStore := mocks.TeamStore{}
team := &model.Team{
Id: model.NewId(),
}
mockChannelStore.On("Get", invitation.ChannelId, true).Return(nil, &store.ErrNotFound{})
mockTeamStore.On("GetAllPage", 0, 1, mock.Anything).Return([]*model.Team{team}, nil)
mockStore.On("Channel").Return(&mockChannelStore)
mockStore.On("Team").Return(&mockTeamStore)
mockServer = scs.server.(*MockServerIface)
mockServer.On("GetStore").Return(mockStore)

View File

@@ -50,33 +50,35 @@ func (bm *Bitmask) UnsetBit(flag Bitmask) {
}
type RemoteCluster struct {
RemoteId string `json:"remote_id"`
RemoteTeamId string `json:"remote_team_id"` // Deprecated: this field is no longer used. It's only kept for backwards compatibility.
Name string `json:"name"`
DisplayName string `json:"display_name"`
SiteURL string `json:"site_url"`
CreateAt int64 `json:"create_at"`
LastPingAt int64 `json:"last_ping_at"`
Token string `json:"token"`
RemoteToken string `json:"remote_token"`
Topics string `json:"topics"`
CreatorId string `json:"creator_id"`
PluginID string `json:"plugin_id"` // non-empty when sync message are to be delivered via plugin API
Options Bitmask `json:"options"` // bit-flag set of options
RemoteId string `json:"remote_id"`
RemoteTeamId string `json:"remote_team_id"` // Deprecated: this field is no longer used. It's only kept for backwards compatibility.
Name string `json:"name"`
DisplayName string `json:"display_name"`
SiteURL string `json:"site_url"`
DefaultTeamId string `json:"default_team_id"`
CreateAt int64 `json:"create_at"`
LastPingAt int64 `json:"last_ping_at"`
Token string `json:"token"`
RemoteToken string `json:"remote_token"`
Topics string `json:"topics"`
CreatorId string `json:"creator_id"`
PluginID string `json:"plugin_id"` // non-empty when sync message are to be delivered via plugin API
Options Bitmask `json:"options"` // bit-flag set of options
}
func (rc *RemoteCluster) Auditable() map[string]interface{} {
return map[string]interface{}{
"remote_id": rc.RemoteId,
"remote_team_id": rc.RemoteTeamId,
"name": rc.Name,
"display_name": rc.DisplayName,
"site_url": rc.SiteURL,
"create_at": rc.CreateAt,
"last_ping_at": rc.LastPingAt,
"creator_id": rc.CreatorId,
"plugin_id": rc.PluginID,
"options": rc.Options,
"remote_id": rc.RemoteId,
"remote_team_id": rc.RemoteTeamId,
"name": rc.Name,
"display_name": rc.DisplayName,
"site_url": rc.SiteURL,
"default_team_id": rc.DefaultTeamId,
"create_at": rc.CreateAt,
"last_ping_at": rc.LastPingAt,
"creator_id": rc.CreatorId,
"plugin_id": rc.PluginID,
"options": rc.Options,
}
}
@@ -123,6 +125,11 @@ func (rc *RemoteCluster) IsValid() *AppError {
if !IsValidId(rc.CreatorId) {
return NewAppError("RemoteCluster.IsValid", "model.cluster.is_valid.id.app_error", nil, "creator_id="+rc.CreatorId, http.StatusBadRequest)
}
if rc.DefaultTeamId != "" && !IsValidId(rc.DefaultTeamId) {
return NewAppError("RemoteCluster.IsValid", "model.cluster.is_valid.id.app_error", nil, "default_team_id="+rc.DefaultTeamId, http.StatusBadRequest)
}
return nil
}
@@ -132,12 +139,14 @@ func (rc *RemoteCluster) Sanitize() {
}
type RemoteClusterPatch struct {
DisplayName *string `json:"display_name"`
DisplayName *string `json:"display_name"`
DefaultTeamId *string `json:"default_team_id"`
}
func (rcp *RemoteClusterPatch) Auditable() map[string]interface{} {
return map[string]interface{}{
"display_name": rcp.DisplayName,
"display_name": rcp.DisplayName,
"default_team_id": rcp.DefaultTeamId,
}
}
@@ -145,6 +154,10 @@ func (rc *RemoteCluster) Patch(patch *RemoteClusterPatch) {
if patch.DisplayName != nil {
rc.DisplayName = *patch.DisplayName
}
if patch.DefaultTeamId != nil {
rc.DefaultTeamId = *patch.DefaultTeamId
}
}
type RemoteClusterWithPassword struct {

View File

@@ -27,6 +27,8 @@ func TestRemoteClusterIsValid(t *testing.T) {
{name: "Missing create_at", rc: &RemoteCluster{RemoteId: id, Name: NewId(), SiteURL: "example.com"}, valid: false},
{name: "Missing last_ping_at", rc: &RemoteCluster{RemoteId: id, Name: NewId(), SiteURL: "example.com", CreatorId: creator, CreateAt: now}, valid: true},
{name: "Missing creator", rc: &RemoteCluster{RemoteId: id, Name: NewId(), SiteURL: "example.com", CreateAt: now, LastPingAt: now}, valid: false},
{name: "Bad default_team_id", rc: &RemoteCluster{RemoteId: id, Name: NewId(), SiteURL: "example.com", CreateAt: now, LastPingAt: now, CreatorId: creator, DefaultTeamId: "bad-id"}, valid: false},
{name: "Valid default_team_id", rc: &RemoteCluster{RemoteId: id, Name: NewId(), SiteURL: "example.com", CreateAt: now, LastPingAt: now, CreatorId: creator, DefaultTeamId: NewId()}, valid: true},
{name: "RemoteCluster valid", rc: &RemoteCluster{RemoteId: id, Name: NewId(), SiteURL: "example.com", CreateAt: now, LastPingAt: now, CreatorId: creator}, valid: true},
{name: "Include protocol", rc: &RemoteCluster{RemoteId: id, Name: NewId(), SiteURL: "http://example.com", CreateAt: now, LastPingAt: now, CreatorId: creator}, valid: true},
{name: "Include protocol & port", rc: &RemoteCluster{RemoteId: id, Name: NewId(), SiteURL: "http://example.com:8065", CreateAt: now, LastPingAt: now, CreatorId: creator}, valid: true},