diff --git a/models/organization/org.go b/models/organization/org.go index 82c3e0ea67..b6a7e8d1a0 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -510,10 +510,10 @@ func ChangeOrgUserStatus(ctx context.Context, orgID, uid int64, public bool) err // AddOrgUser adds new user to given organization. func AddOrgUser(ctx context.Context, orgID, uid int64) error { - isUser, err := user_model.IsUserByID(ctx, uid) + eligible, err := IsAnEligibleTeamMemberByID(ctx, uid) if err != nil { return err - } else if !isUser { + } else if !eligible { return user_model.ErrUserWrongType{UID: uid} } diff --git a/models/organization/org_user.go b/models/organization/org_user.go index 81671c5cf5..4c84fccbf3 100644 --- a/models/organization/org_user.go +++ b/models/organization/org_user.go @@ -112,6 +112,15 @@ func IsUserOrgOwner(ctx context.Context, users user_model.UserList, orgID int64) return results } +// Returns true if the given user ID is allowed to be a team member +func IsAnEligibleTeamMemberByID(ctx context.Context, uid int64) (bool, error) { + return db.GetEngine(ctx). + Where("id=?", uid). + In("type", user_model.UserTypeIndividual, user_model.UserTypeBot, user_model.UserTypeRemoteUser). + Table("user"). + Exist() +} + func loadOrganizationOwners(ctx context.Context, users user_model.UserList, orgID int64) (map[int64]*TeamUser, error) { if len(users) == 0 { return nil, nil diff --git a/models/organization/org_user_test.go b/models/organization/org_user_test.go index 4011e5df88..670a641214 100644 --- a/models/organization/org_user_test.go +++ b/models/organization/org_user_test.go @@ -162,3 +162,41 @@ func TestAddOrgUser(t *testing.T) { unittest.CheckConsistencyFor(t, &user_model.User{}, &organization.Team{}) } + +func TestIsAnEligibleTeamMemberByID(t *testing.T) { + defer unittest.OverrideFixtures("models/user/fixtures/")() + require.NoError(t, unittest.PrepareTestDatabase()) + + for _, testCase := range []struct { + name string + id int64 + eligible bool + }{ + { + name: "Regular user", + id: 1, + eligible: true, + }, + { + name: "Bot user", + id: 1042, + eligible: true, + }, + { + name: "Organization", + id: 3, + eligible: false, + }, + { + name: "F3 Remote user", + id: 1041, + eligible: true, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + eligible, err := organization.IsAnEligibleTeamMemberByID(t.Context(), testCase.id) + require.NoError(t, err) + assert.Equal(t, testCase.eligible, eligible) + }) + } +} diff --git a/models/user/user.go b/models/user/user.go index f962c19420..7e2101d7cc 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -464,15 +464,6 @@ func (u *User) IsUser() bool { return u.Type == UserTypeIndividual || u.Type == UserTypeBot } -// Returns true if the given user ID belongs to an actual user, not an organization -func IsUserByID(ctx context.Context, uid int64) (bool, error) { - return db.GetEngine(ctx). - Where("id=?", uid). - In("type", UserTypeIndividual, UserTypeBot). - Table("user"). - Exist() -} - // IsBot returns whether or not the user is of type bot func (u *User) IsBot() bool { return u.Type == UserTypeBot diff --git a/models/user/user_test.go b/models/user/user_test.go index 4db253df18..d1af3a750f 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -1113,20 +1113,3 @@ func TestGetUserByEmailSimple(t *testing.T) { assert.Nil(t, u) }) } - -func TestIsUserConsistency(t *testing.T) { - defer unittest.OverrideFixtures("models/user/fixtures/")() - require.NoError(t, unittest.PrepareTestDatabase()) - - test := func(userID int64) { - user, err := user_model.GetUserByID(t.Context(), userID) - require.NoError(t, err) - isUser, err := user_model.IsUserByID(t.Context(), userID) - require.NoError(t, err) - assert.Equal(t, user.IsUser(), isUser) - } - - test(1) - test(1041) - test(1042) -}