From 602ba7b45384e204abbd7d9f8c9a6d33a17a00ed Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Fri, 26 Jul 2019 00:17:33 +0200 Subject: [PATCH 01/17] org/members: display 2FA state --- models/user.go | 6 ++++++ templates/org/member/members.tmpl | 10 +++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/models/user.go b/models/user.go index 1f684a594045a..addc70b96523a 100644 --- a/models/user.go +++ b/models/user.go @@ -552,6 +552,12 @@ func (u *User) IsUserOrgOwner(orgID int64) bool { return isOwner } +// IsTwoFaEnrolled return state of 2FA enrollement +func (u *User) IsTwoFaEnrolled() bool { + _, err := GetTwoFactorByUID(u.ID) + return err == nil //If we canno't generate a scratch token what so ever the reason 2FA is not enable +} + // IsUserPartOfOrg returns true if user with userID is part of the u organisation. func (u *User) IsUserPartOfOrg(userID int64) bool { return u.isUserPartOfOrg(x, userID) diff --git a/templates/org/member/members.tmpl b/templates/org/member/members.tmpl index 7f0a763610e3b..f62abaee93fcf 100644 --- a/templates/org/member/members.tmpl +++ b/templates/org/member/members.tmpl @@ -14,7 +14,7 @@
{{.Name}}
{{.FullName}}
-
+
{{$.i18n.Tr "org.members.membership_visibility"}}
@@ -37,6 +37,14 @@ {{if .IsUserOrgOwner $.Org.ID}} {{$.i18n.Tr "org.members.owner"}}{{else}}{{$.i18n.Tr "org.members.member"}}{{end}}
+
+
+ 2FA +
+
+ +
+
{{if eq $.SignedUser.ID .ID}} From 3dca80e0f796eb068d6166d87872c4055e32c196 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Fri, 26 Jul 2019 00:31:11 +0200 Subject: [PATCH 02/17] fix comment typo --- models/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/user.go b/models/user.go index addc70b96523a..58c84a3e31d70 100644 --- a/models/user.go +++ b/models/user.go @@ -555,7 +555,7 @@ func (u *User) IsUserOrgOwner(orgID int64) bool { // IsTwoFaEnrolled return state of 2FA enrollement func (u *User) IsTwoFaEnrolled() bool { _, err := GetTwoFactorByUID(u.ID) - return err == nil //If we canno't generate a scratch token what so ever the reason 2FA is not enable + return err == nil //If we can't generate a scratch token what so ever the reason -> 2FA is not enable } // IsUserPartOfOrg returns true if user with userID is part of the u organisation. From b2b2cbf2713bfddaa72328cc7f2f8a52c63bcdc1 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 27 Jul 2019 20:56:14 +0200 Subject: [PATCH 03/17] lay down UserList bases --- models/user.go | 2 +- models/userlist.go | 39 +++++++++++++++++++++++++++++++ routers/org/members.go | 3 +++ templates/org/member/members.tmpl | 8 +++---- 4 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 models/userlist.go diff --git a/models/user.go b/models/user.go index 58c84a3e31d70..aa4adc42105b5 100644 --- a/models/user.go +++ b/models/user.go @@ -142,7 +142,7 @@ type User struct { NumTeams int NumMembers int Teams []*Team `xorm:"-"` - Members []*User `xorm:"-"` + Members UserList `xorm:"-"` Visibility structs.VisibleType `xorm:"NOT NULL DEFAULT 0"` // Preferences diff --git a/models/userlist.go b/models/userlist.go new file mode 100644 index 0000000000000..be11b6e957e6d --- /dev/null +++ b/models/userlist.go @@ -0,0 +1,39 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +type UserList []*User + +//TODO paginate + +// IsPublicMember returns true if user public his/her membership in given organization. +func (ul UserList) IsPublicMember(orgID int64) []bool { + results := make([]bool, len(ul)) + //TODO use directly xorm + for i, u := range ul { + results[i] = u.IsPublicMember(orgID) + } + return results +} + +// IsUserOrgOwner returns true if user is in the owner team of given organization. +func (ul UserList) IsUserOrgOwner(orgID int64) []bool { + results := make([]bool, len(ul)) + //TODO use directly xorm + for i, u := range ul { + results[i] = u.IsUserOrgOwner(orgID) + } + return results +} + +// IsTwoFaEnrolled return state of 2FA enrollement +func (ul UserList) IsTwoFaEnrolled() []bool { + results := make([]bool, len(ul)) + //TODO use directly xorm + for i, u := range ul { + results[i] = u.IsTwoFaEnrolled() + } + return results +} diff --git a/routers/org/members.go b/routers/org/members.go index d65bc2a00844a..a345fae15b189 100644 --- a/routers/org/members.go +++ b/routers/org/members.go @@ -30,6 +30,9 @@ func Members(ctx *context.Context) { return } ctx.Data["Members"] = org.Members + ctx.Data["MembersIsPublicMember"] = org.Members.IsPublicMember(org.ID) + ctx.Data["MembersIsUserOrgOwner"] = org.Members.IsUserOrgOwner(org.ID) + ctx.Data["MembersIsTwoFaEnrolled"] = org.Members.IsTwoFaEnrolled() ctx.HTML(200, tplMembers) } diff --git a/templates/org/member/members.tmpl b/templates/org/member/members.tmpl index f62abaee93fcf..536281787ee97 100644 --- a/templates/org/member/members.tmpl +++ b/templates/org/member/members.tmpl @@ -5,7 +5,7 @@ {{template "base/alert" .}}
- {{range .Members}} + {{ range $id, $member := .Members}}
@@ -19,7 +19,7 @@ {{$.i18n.Tr "org.members.membership_visibility"}}
- {{ $isPublic := .IsPublicMember $.Org.ID}} + {{ $isPublic := index $.MembersIsPublicMember $id}} {{if $isPublic}} {{$.i18n.Tr "org.members.public"}} {{if or (eq $.SignedUser.ID .ID) $.IsOrganizationOwner}}({{$.i18n.Tr "org.members.public_helper"}}){{end}} @@ -34,7 +34,7 @@ {{$.i18n.Tr "org.members.member_role"}}
- {{if .IsUserOrgOwner $.Org.ID}} {{$.i18n.Tr "org.members.owner"}}{{else}}{{$.i18n.Tr "org.members.member"}}{{end}} + {{if index $.MembersIsUserOrgOwner $id}} {{$.i18n.Tr "org.members.owner"}}{{else}}{{$.i18n.Tr "org.members.member"}}{{end}}
@@ -42,7 +42,7 @@ 2FA
- +
From 225467f6b7a6ab917bfcff9ad0394b96ed648b0b Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 27 Jul 2019 22:40:19 +0200 Subject: [PATCH 04/17] add basic test for previous methods --- models/user_test.go | 78 +++++++++++++++++++++++++++++++++++++++++ models/userlist_test.go | 25 +++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 models/userlist_test.go diff --git a/models/user_test.go b/models/user_test.go index 10420a143f191..433e3f9417bce 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -5,6 +5,7 @@ package models import ( + "fmt" "math/rand" "strings" "testing" @@ -15,6 +16,83 @@ import ( "github.com/stretchr/testify/assert" ) +func TestUserIsPublicMember(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + tt := []struct { + uid int64 + orgid int64 + expected bool + }{ + {2, 3, true}, + {4, 3, false}, + {5, 6, true}, + {5, 7, false}, + } + for _, v := range tt { + t.Run(fmt.Sprintf("UserId%dIsPublicMemberOf%d", v.uid, v.orgid), func(t *testing.T) { + testUserIsPublicMember(t, v.uid, v.orgid, v.expected) + }) + } +} + +func testUserIsPublicMember(t *testing.T, uid int64, orgID int64, expected bool) { + user, err := GetUserByID(uid) + assert.NoError(t, err) + assert.Equal(t, expected, user.IsPublicMember(orgID)) +} + +func TestIsUserOrgOwner(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + tt := []struct { + uid int64 + orgid int64 + expected bool + }{ + {2, 3, true}, + {4, 3, false}, + {5, 6, true}, + {5, 7, false}, + } + for _, v := range tt { + t.Run(fmt.Sprintf("UserId%dIsOrgOwnerOf%d", v.uid, v.orgid), func(t *testing.T) { + testIsUserOrgOwner(t, v.uid, v.orgid, v.expected) + }) + } +} + +func testIsUserOrgOwner(t *testing.T, uid int64, orgID int64, expected bool) { + user, err := GetUserByID(uid) + assert.NoError(t, err) + assert.Equal(t, expected, user.IsUserOrgOwner(orgID)) +} + +func TestIsTwoFaEnrolled(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + tt := []struct { + uid int64 + expected bool + }{ + {2, false}, + {4, false}, + {5, false}, + {5, false}, + //TODO add a 2fa enabled user + } + for _, v := range tt { + t.Run(fmt.Sprintf("UserId%dIsTwoFaEnrolled", v.uid), func(t *testing.T) { + testIsTwoFaEnrolled(t, v.uid, v.expected) + }) + } +} + +func testIsTwoFaEnrolled(t *testing.T, uid int64, expected bool) { + user, err := GetUserByID(uid) + assert.NoError(t, err) + assert.Equal(t, expected, user.IsTwoFaEnrolled()) +} + func TestGetUserEmailsByNames(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) diff --git a/models/userlist_test.go b/models/userlist_test.go new file mode 100644 index 0000000000000..5400b641b0c35 --- /dev/null +++ b/models/userlist_test.go @@ -0,0 +1,25 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "github.com/stretchr/testify/assert" + "testing" +) + +func TestUserListIsPublicMember(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + //TODO +} + +func TestUserListIsUserOrgOwner(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + //TODO +} + +func TestUserListIsTwoFaEnrolled(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + //TODO +} From 564c313965ffd49262d636ef47e787c91d8bc065 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 27 Jul 2019 22:42:57 +0200 Subject: [PATCH 05/17] add comment for UserList type --- models/userlist.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/models/userlist.go b/models/userlist.go index be11b6e957e6d..cd084e57e97d6 100644 --- a/models/userlist.go +++ b/models/userlist.go @@ -4,6 +4,8 @@ package models +//UserList is a list of user. +// This type provide valuable methods to retrieve information for a group of users efficiently. type UserList []*User //TODO paginate From 68b15059bc6fe54060346e8a70a34d1e6bdbebdc Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 27 Jul 2019 23:07:59 +0200 Subject: [PATCH 06/17] add valid two-fa account --- models/fixtures/two_factor.yml | 9 +++++++++ models/user_test.go | 5 ++--- 2 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 models/fixtures/two_factor.yml diff --git a/models/fixtures/two_factor.yml b/models/fixtures/two_factor.yml new file mode 100644 index 0000000000000..59243d46fcd85 --- /dev/null +++ b/models/fixtures/two_factor.yml @@ -0,0 +1,9 @@ +- + id: 1 + uid: 2 + secret: KlDporn6Ile4vFcKI8z7Z6sqK1Scj2Qp0ovtUzCZO6jVbRW2lAoT7UDxDPtrab8d2B9zKOocBRdBJnS8orsrUNrsyETY+jJHb79M82uZRioKbRUz15sfOpmJmEzkFeSg6S4LicUBQos= + scratch_salt: Qb5bq2DyR2 + scratch_hash: 068eb9b8746e0bcfe332fac4457693df1bda55800eb0f6894d14ebb736ae6a24e0fc8fc5333c19f57f81599788f0b8e51ec1 + last_used_passcode: + created_unix: 1564253724 + updated_unix: 1564253724 diff --git a/models/user_test.go b/models/user_test.go index 433e3f9417bce..843ee63fd6b00 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -53,7 +53,7 @@ func TestIsUserOrgOwner(t *testing.T) { {2, 3, true}, {4, 3, false}, {5, 6, true}, - {5, 7, false}, + {5, 7, true}, } for _, v := range tt { t.Run(fmt.Sprintf("UserId%dIsOrgOwnerOf%d", v.uid, v.orgid), func(t *testing.T) { @@ -74,11 +74,10 @@ func TestIsTwoFaEnrolled(t *testing.T) { uid int64 expected bool }{ - {2, false}, + {2, true}, {4, false}, {5, false}, {5, false}, - //TODO add a 2fa enabled user } for _, v := range tt { t.Run(fmt.Sprintf("UserId%dIsTwoFaEnrolled", v.uid), func(t *testing.T) { From fd16091db29da020ef301bb0d9402611f6e2a661 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sat, 27 Jul 2019 23:24:10 +0200 Subject: [PATCH 07/17] test new UserList methods --- models/userlist_test.go | 64 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/models/userlist_test.go b/models/userlist_test.go index 5400b641b0c35..e8ed8c8c50dc0 100644 --- a/models/userlist_test.go +++ b/models/userlist_test.go @@ -5,21 +5,79 @@ package models import ( + "fmt" "github.com/stretchr/testify/assert" "testing" ) func TestUserListIsPublicMember(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - //TODO + tt := []struct { + orgid int64 + expected []bool + }{ + {3, []bool{true, false}}, + {6, []bool{true}}, + {7, []bool{false}}, + } + for _, v := range tt { + t.Run(fmt.Sprintf("IsPublicMemberOfOrdIg%d", v.orgid), func(t *testing.T) { + testUserListIsPublicMember(t, v.orgid, v.expected) + }) + } +} +func testUserListIsPublicMember(t *testing.T, orgID int64, expected []bool) { + org, err := GetUserByID(orgID) + assert.NoError(t, err) + assert.NoError(t, org.GetMembers()) + assert.Equal(t, expected, org.Members.IsPublicMember(orgID)) + } func TestUserListIsUserOrgOwner(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - //TODO + tt := []struct { + orgid int64 + expected []bool + }{ + {3, []bool{true, false}}, + {6, []bool{true}}, + {7, []bool{false}}, + } + for _, v := range tt { + t.Run(fmt.Sprintf("IsUserOrgOwnerOfOrdIg%d", v.orgid), func(t *testing.T) { + testUserListIsUserOrgOwner(t, v.orgid, v.expected) + }) + } +} +func testUserListIsUserOrgOwner(t *testing.T, orgID int64, expected []bool) { + org, err := GetUserByID(orgID) + assert.NoError(t, err) + assert.NoError(t, org.GetMembers()) + assert.Equal(t, expected, org.Members.IsPublicMember(orgID)) + } func TestUserListIsTwoFaEnrolled(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - //TODO + tt := []struct { + orgid int64 + expected []bool + }{ + {3, []bool{true, false}}, + {6, []bool{false}}, + {7, []bool{false}}, + } + for _, v := range tt { + t.Run(fmt.Sprintf("IsTwoFaEnrolledOfOrdIg%d", v.orgid), func(t *testing.T) { + testUserListIsTwoFaEnrolled(t, v.orgid, v.expected) + }) + } +} +func testUserListIsTwoFaEnrolled(t *testing.T, orgID int64, expected []bool) { + org, err := GetUserByID(orgID) + assert.NoError(t, err) + assert.NoError(t, org.GetMembers()) + assert.Equal(t, expected, org.Members.IsTwoFaEnrolled()) + } From 6839402c6b25ce639582d07062c8e316c742ef7c Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 28 Jul 2019 00:24:07 +0200 Subject: [PATCH 08/17] optimize MembersIsPublic by side loading info on GetMembers + fix integrations tests --- models/fixtures/org_user.yml | 6 +++++ models/fixtures/team.yml | 11 ++++++++- models/fixtures/team_user.yml | 8 ++++++- models/fixtures/two_factor.yml | 2 +- models/fixtures/user.yml | 37 ++++++++++++++++++++++++++++++- models/org.go | 3 +++ models/user.go | 13 ++++++----- models/user_test.go | 10 ++++----- models/userlist.go | 14 +++++------- models/userlist_test.go | 22 +++++++++--------- routers/org/members.go | 6 ++--- templates/org/member/members.tmpl | 2 +- 12 files changed, 97 insertions(+), 37 deletions(-) diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index 5bb23571fc266..385492dd68d11 100644 --- a/models/fixtures/org_user.yml +++ b/models/fixtures/org_user.yml @@ -39,3 +39,9 @@ uid: 20 org_id: 19 is_public: true + +- + id: 8 + uid: 24 + org_id: 25 + is_public: true diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index 2d0dd9cd56ca1..d99dc324692e9 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -77,4 +77,13 @@ name: review_team authorize: 1 # read num_repos: 1 - num_members: 1 \ No newline at end of file + num_members: 1 + +- + id: 10 + org_id: 25 + lower_name: owners + name: Owners + authorize: 4 # owner + num_repos: 0 + num_members: 1 diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index e20b5c9684bff..4fc609791d396 100644 --- a/models/fixtures/team_user.yml +++ b/models/fixtures/team_user.yml @@ -62,4 +62,10 @@ id: 11 org_id: 17 team_id: 9 - uid: 20 \ No newline at end of file + uid: 20 + +- + id: 12 + org_id: 25 + team_id: 10 + uid: 24 diff --git a/models/fixtures/two_factor.yml b/models/fixtures/two_factor.yml index 59243d46fcd85..d8cb85274b687 100644 --- a/models/fixtures/two_factor.yml +++ b/models/fixtures/two_factor.yml @@ -1,6 +1,6 @@ - id: 1 - uid: 2 + uid: 24 secret: KlDporn6Ile4vFcKI8z7Z6sqK1Scj2Qp0ovtUzCZO6jVbRW2lAoT7UDxDPtrab8d2B9zKOocBRdBJnS8orsrUNrsyETY+jJHb79M82uZRioKbRUz15sfOpmJmEzkFeSg6S4LicUBQos= scratch_salt: Qb5bq2DyR2 scratch_hash: 068eb9b8746e0bcfe332fac4457693df1bda55800eb0f6894d14ebb736ae6a24e0fc8fc5333c19f57f81599788f0b8e51ec1 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index ed60e7f5eab06..d89dc3c126731 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -365,4 +365,39 @@ is_active: true num_members: 0 num_teams: 0 - visibility: 2 \ No newline at end of file + visibility: 2 + +- + id: 24 + lower_name: user24 + name: user24 + full_name: "user24" + email: user24@example.com + keep_email_private: true + passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password + type: 0 # individual + salt: ZogKvWdyEx + is_admin: false + avatar: avatar24 + avatar_email: user24@example.com + num_repos: 0 + num_stars: 0 + num_followers: 0 + num_following: 0 + is_active: true + +- + id: 25 + lower_name: org25 + name: org25 + full_name: "org25" + email: org25@example.com + passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password + type: 1 # organization + salt: ZogKvWdyEx + is_admin: false + avatar: avatar25 + avatar_email: org25@example.com + num_repos: 0 + num_members: 1 + num_teams: 1 diff --git a/models/org.go b/models/org.go index d86109de57e10..2339bf7e67054 100644 --- a/models/org.go +++ b/models/org.go @@ -72,9 +72,12 @@ func (org *User) GetMembers() error { } var ids = make([]int64, len(ous)) + var idsIsPublic = make(map[int64]bool, len(ous)) for i, ou := range ous { ids[i] = ou.UID + idsIsPublic[ou.UID] = ou.IsPublic } + org.MembersIsPublic = idsIsPublic org.Members, err = GetUsersByIDs(ids) return err } diff --git a/models/user.go b/models/user.go index aa4adc42105b5..8a77db3e08c20 100644 --- a/models/user.go +++ b/models/user.go @@ -139,15 +139,18 @@ type User struct { NumRepos int // For organization - NumTeams int - NumMembers int - Teams []*Team `xorm:"-"` - Members UserList `xorm:"-"` - Visibility structs.VisibleType `xorm:"NOT NULL DEFAULT 0"` + NumTeams int + NumMembers int + Teams []*Team `xorm:"-"` + Members UserList `xorm:"-"` + MembersIsPublic map[int64]bool `xorm:"-"` + Visibility structs.VisibleType `xorm:"NOT NULL DEFAULT 0"` // Preferences DiffViewStyle string `xorm:"NOT NULL DEFAULT ''"` Theme string `xorm:"NOT NULL DEFAULT ''"` + + //IsTwoFaEnrolled bool `xorm:"-"` } // ColorFormat writes a colored string to identify this struct diff --git a/models/user_test.go b/models/user_test.go index 843ee63fd6b00..5f6ab8b3eb786 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/assert" ) - func TestUserIsPublicMember(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) @@ -74,10 +73,11 @@ func TestIsTwoFaEnrolled(t *testing.T) { uid int64 expected bool }{ - {2, true}, + {2, false}, {4, false}, {5, false}, {5, false}, + {24, true}, } for _, v := range tt { t.Run(fmt.Sprintf("UserId%dIsTwoFaEnrolled", v.uid), func(t *testing.T) { @@ -160,7 +160,7 @@ func TestSearchUsers(t *testing.T) { []int64{7, 17}) testOrgSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 3, PageSize: 2}, - []int64{19}) + []int64{19, 25}) testOrgSuccess(&SearchUserOptions{Page: 4, PageSize: 2}, []int64{}) @@ -172,13 +172,13 @@ func TestSearchUsers(t *testing.T) { } testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 1}, - []int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21}) + []int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24}) testUserSuccess(&SearchUserOptions{Page: 1, IsActive: util.OptionalBoolFalse}, []int64{9}) testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 1, IsActive: util.OptionalBoolTrue}, - []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21}) + []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24}) testUserSuccess(&SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", Page: 1, IsActive: util.OptionalBoolTrue}, []int64{1, 10, 11, 12, 13, 14, 15, 16, 18}) diff --git a/models/userlist.go b/models/userlist.go index cd084e57e97d6..51baf4d002eb4 100644 --- a/models/userlist.go +++ b/models/userlist.go @@ -8,16 +8,12 @@ package models // This type provide valuable methods to retrieve information for a group of users efficiently. type UserList []*User -//TODO paginate - -// IsPublicMember returns true if user public his/her membership in given organization. -func (ul UserList) IsPublicMember(orgID int64) []bool { - results := make([]bool, len(ul)) - //TODO use directly xorm - for i, u := range ul { - results[i] = u.IsPublicMember(orgID) +func (users UserList) getUserIDs() []int64 { + userIDs := make([]int64, len(users)) + for _, user := range users { + userIDs = append(userIDs, user.ID) //Considering that user id are unique in the list } - return results + return userIDs } // IsUserOrgOwner returns true if user is in the owner team of given organization. diff --git a/models/userlist_test.go b/models/userlist_test.go index e8ed8c8c50dc0..46e15f9527f6a 100644 --- a/models/userlist_test.go +++ b/models/userlist_test.go @@ -14,11 +14,12 @@ func TestUserListIsPublicMember(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) tt := []struct { orgid int64 - expected []bool + expected map[int64]bool }{ - {3, []bool{true, false}}, - {6, []bool{true}}, - {7, []bool{false}}, + {3, map[int64]bool{2: true, 4: false}}, + {6, map[int64]bool{5: true}}, + {7, map[int64]bool{5: false}}, + {25, map[int64]bool{24: true}}, } for _, v := range tt { t.Run(fmt.Sprintf("IsPublicMemberOfOrdIg%d", v.orgid), func(t *testing.T) { @@ -26,11 +27,11 @@ func TestUserListIsPublicMember(t *testing.T) { }) } } -func testUserListIsPublicMember(t *testing.T, orgID int64, expected []bool) { +func testUserListIsPublicMember(t *testing.T, orgID int64, expected map[int64]bool) { org, err := GetUserByID(orgID) assert.NoError(t, err) assert.NoError(t, org.GetMembers()) - assert.Equal(t, expected, org.Members.IsPublicMember(orgID)) + assert.Equal(t, expected, org.MembersIsPublic) } @@ -42,7 +43,8 @@ func TestUserListIsUserOrgOwner(t *testing.T) { }{ {3, []bool{true, false}}, {6, []bool{true}}, - {7, []bool{false}}, + {7, []bool{true}}, + {25, []bool{true}}, } for _, v := range tt { t.Run(fmt.Sprintf("IsUserOrgOwnerOfOrdIg%d", v.orgid), func(t *testing.T) { @@ -54,8 +56,7 @@ func testUserListIsUserOrgOwner(t *testing.T, orgID int64, expected []bool) { org, err := GetUserByID(orgID) assert.NoError(t, err) assert.NoError(t, org.GetMembers()) - assert.Equal(t, expected, org.Members.IsPublicMember(orgID)) - + assert.Equal(t, expected, org.Members.IsUserOrgOwner(orgID)) } func TestUserListIsTwoFaEnrolled(t *testing.T) { @@ -64,9 +65,10 @@ func TestUserListIsTwoFaEnrolled(t *testing.T) { orgid int64 expected []bool }{ - {3, []bool{true, false}}, + {3, []bool{false, false}}, {6, []bool{false}}, {7, []bool{false}}, + {25, []bool{true}}, } for _, v := range tt { t.Run(fmt.Sprintf("IsTwoFaEnrolledOfOrdIg%d", v.orgid), func(t *testing.T) { diff --git a/routers/org/members.go b/routers/org/members.go index a345fae15b189..0a6ff7814a867 100644 --- a/routers/org/members.go +++ b/routers/org/members.go @@ -19,7 +19,7 @@ const ( tplMembers base.TplName = "org/member/members" ) -// Members render orgnization users page +// Members render organization users page func Members(ctx *context.Context) { org := ctx.Org.Organization ctx.Data["Title"] = org.FullName @@ -30,14 +30,14 @@ func Members(ctx *context.Context) { return } ctx.Data["Members"] = org.Members - ctx.Data["MembersIsPublicMember"] = org.Members.IsPublicMember(org.ID) + ctx.Data["MembersIsPublicMember"] = org.MembersIsPublic ctx.Data["MembersIsUserOrgOwner"] = org.Members.IsUserOrgOwner(org.ID) ctx.Data["MembersIsTwoFaEnrolled"] = org.Members.IsTwoFaEnrolled() ctx.HTML(200, tplMembers) } -// MembersAction response for operation to a member of orgnization +// MembersAction response for operation to a member of organization func MembersAction(ctx *context.Context) { uid := com.StrTo(ctx.Query("uid")).MustInt64() if uid == 0 { diff --git a/templates/org/member/members.tmpl b/templates/org/member/members.tmpl index 536281787ee97..b0255a8397492 100644 --- a/templates/org/member/members.tmpl +++ b/templates/org/member/members.tmpl @@ -19,7 +19,7 @@ {{$.i18n.Tr "org.members.membership_visibility"}}
- {{ $isPublic := index $.MembersIsPublicMember $id}} + {{ $isPublic := index $.MembersIsPublicMember .ID}} {{if $isPublic}} {{$.i18n.Tr "org.members.public"}} {{if or (eq $.SignedUser.ID .ID) $.IsOrganizationOwner}}({{$.i18n.Tr "org.members.public_helper"}}){{end}} From 385e1d85e58373c248826eb1b3ab19649b70bda9 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 28 Jul 2019 00:44:10 +0200 Subject: [PATCH 09/17] respect fmt rules --- models/userlist.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/models/userlist.go b/models/userlist.go index 51baf4d002eb4..65ea0fdff2ec1 100644 --- a/models/userlist.go +++ b/models/userlist.go @@ -17,20 +17,20 @@ func (users UserList) getUserIDs() []int64 { } // IsUserOrgOwner returns true if user is in the owner team of given organization. -func (ul UserList) IsUserOrgOwner(orgID int64) []bool { - results := make([]bool, len(ul)) +func (users UserList) IsUserOrgOwner(orgID int64) []bool { + results := make([]bool, len(users)) //TODO use directly xorm - for i, u := range ul { + for i, u := range users { results[i] = u.IsUserOrgOwner(orgID) } return results } // IsTwoFaEnrolled return state of 2FA enrollement -func (ul UserList) IsTwoFaEnrolled() []bool { - results := make([]bool, len(ul)) +func (users UserList) IsTwoFaEnrolled() []bool { + results := make([]bool, len(users)) //TODO use directly xorm - for i, u := range ul { + for i, u := range users { results[i] = u.IsTwoFaEnrolled() } return results From f5046afa21fc0bb89a22e143cef0fd489c3db7eb Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 28 Jul 2019 00:50:57 +0200 Subject: [PATCH 10/17] use map for data --- models/user_test.go | 1 + models/userlist.go | 18 +++++++++--------- models/userlist_test.go | 26 +++++++++++++------------- routers/org/members.go | 2 +- templates/org/member/members.tmpl | 6 +++--- 5 files changed, 27 insertions(+), 26 deletions(-) diff --git a/models/user_test.go b/models/user_test.go index 5f6ab8b3eb786..a04fa88d191e3 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" ) + func TestUserIsPublicMember(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) diff --git a/models/userlist.go b/models/userlist.go index 65ea0fdff2ec1..cf4c5952f89af 100644 --- a/models/userlist.go +++ b/models/userlist.go @@ -17,21 +17,21 @@ func (users UserList) getUserIDs() []int64 { } // IsUserOrgOwner returns true if user is in the owner team of given organization. -func (users UserList) IsUserOrgOwner(orgID int64) []bool { - results := make([]bool, len(users)) +func (users UserList) IsUserOrgOwner(orgID int64) map[int64]bool { + results := make(map[int64]bool, len(users)) //TODO use directly xorm - for i, u := range users { - results[i] = u.IsUserOrgOwner(orgID) + for _, u := range users { + results[u.ID] = u.IsUserOrgOwner(orgID) } return results } -// IsTwoFaEnrolled return state of 2FA enrollement -func (users UserList) IsTwoFaEnrolled() []bool { - results := make([]bool, len(users)) +// GetTwoFaStatus return state of 2FA enrollement +func (users UserList) GetTwoFaStatus() map[int64]bool { + results := make(map[int64]bool, len(users)) //TODO use directly xorm - for i, u := range users { - results[i] = u.IsTwoFaEnrolled() + for _, u := range users { + results[u.ID] = u.IsTwoFaEnrolled() } return results } diff --git a/models/userlist_test.go b/models/userlist_test.go index 46e15f9527f6a..b41a4d0af5c7f 100644 --- a/models/userlist_test.go +++ b/models/userlist_test.go @@ -39,12 +39,12 @@ func TestUserListIsUserOrgOwner(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) tt := []struct { orgid int64 - expected []bool + expected map[int64]bool }{ - {3, []bool{true, false}}, - {6, []bool{true}}, - {7, []bool{true}}, - {25, []bool{true}}, + {3, map[int64]bool{2: true, 4: false}}, + {6, map[int64]bool{5: true}}, + {7, map[int64]bool{5: true}}, + {25, map[int64]bool{24: true}}, } for _, v := range tt { t.Run(fmt.Sprintf("IsUserOrgOwnerOfOrdIg%d", v.orgid), func(t *testing.T) { @@ -52,7 +52,7 @@ func TestUserListIsUserOrgOwner(t *testing.T) { }) } } -func testUserListIsUserOrgOwner(t *testing.T, orgID int64, expected []bool) { +func testUserListIsUserOrgOwner(t *testing.T, orgID int64, expected map[int64]bool) { org, err := GetUserByID(orgID) assert.NoError(t, err) assert.NoError(t, org.GetMembers()) @@ -63,12 +63,12 @@ func TestUserListIsTwoFaEnrolled(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) tt := []struct { orgid int64 - expected []bool + expected map[int64]bool }{ - {3, []bool{false, false}}, - {6, []bool{false}}, - {7, []bool{false}}, - {25, []bool{true}}, + {3, map[int64]bool{2: false, 4: false}}, + {6, map[int64]bool{5: false}}, + {7, map[int64]bool{5: false}}, + {25, map[int64]bool{24: true}}, } for _, v := range tt { t.Run(fmt.Sprintf("IsTwoFaEnrolledOfOrdIg%d", v.orgid), func(t *testing.T) { @@ -76,10 +76,10 @@ func TestUserListIsTwoFaEnrolled(t *testing.T) { }) } } -func testUserListIsTwoFaEnrolled(t *testing.T, orgID int64, expected []bool) { +func testUserListIsTwoFaEnrolled(t *testing.T, orgID int64, expected map[int64]bool) { org, err := GetUserByID(orgID) assert.NoError(t, err) assert.NoError(t, org.GetMembers()) - assert.Equal(t, expected, org.Members.IsTwoFaEnrolled()) + assert.Equal(t, expected, org.Members.GetTwoFaStatus()) } diff --git a/routers/org/members.go b/routers/org/members.go index 0a6ff7814a867..20f80cefcd1fb 100644 --- a/routers/org/members.go +++ b/routers/org/members.go @@ -32,7 +32,7 @@ func Members(ctx *context.Context) { ctx.Data["Members"] = org.Members ctx.Data["MembersIsPublicMember"] = org.MembersIsPublic ctx.Data["MembersIsUserOrgOwner"] = org.Members.IsUserOrgOwner(org.ID) - ctx.Data["MembersIsTwoFaEnrolled"] = org.Members.IsTwoFaEnrolled() + ctx.Data["MembersTwoFaStatus"] = org.Members.GetTwoFaStatus() ctx.HTML(200, tplMembers) } diff --git a/templates/org/member/members.tmpl b/templates/org/member/members.tmpl index b0255a8397492..9db506ee5bf9d 100644 --- a/templates/org/member/members.tmpl +++ b/templates/org/member/members.tmpl @@ -5,7 +5,7 @@ {{template "base/alert" .}}
- {{ range $id, $member := .Members}} + {{ range .Members}}
@@ -34,7 +34,7 @@ {{$.i18n.Tr "org.members.member_role"}}
- {{if index $.MembersIsUserOrgOwner $id}} {{$.i18n.Tr "org.members.owner"}}{{else}}{{$.i18n.Tr "org.members.member"}}{{end}} + {{if index $.MembersIsUserOrgOwner .ID}} {{$.i18n.Tr "org.members.owner"}}{{else}}{{$.i18n.Tr "org.members.member"}}{{end}}
@@ -42,7 +42,7 @@ 2FA
- +
From 2494db4759e00819af9a9aed7f5a8f24bdaa2583 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 28 Jul 2019 01:08:15 +0200 Subject: [PATCH 11/17] Optimize GetTwoFaStatus --- models/userlist.go | 32 +++++++++++++++++++++++++++++--- models/userlist_test.go | 2 +- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/models/userlist.go b/models/userlist.go index cf4c5952f89af..e03db111288d9 100644 --- a/models/userlist.go +++ b/models/userlist.go @@ -4,6 +4,10 @@ package models +import ( + "fmt" +) + //UserList is a list of user. // This type provide valuable methods to retrieve information for a group of users efficiently. type UserList []*User @@ -29,9 +33,31 @@ func (users UserList) IsUserOrgOwner(orgID int64) map[int64]bool { // GetTwoFaStatus return state of 2FA enrollement func (users UserList) GetTwoFaStatus() map[int64]bool { results := make(map[int64]bool, len(users)) - //TODO use directly xorm - for _, u := range users { - results[u.ID] = u.IsTwoFaEnrolled() + for _, user := range users { + results[user.ID] = false //Set default to false + } + tokenMaps, err := users.loadTwoFactorStatus(x) + if err == nil { + for _, token := range tokenMaps { + results[token.UID] = true + } } + return results } + +func (users UserList) loadTwoFactorStatus(e Engine) (map[int64]*TwoFactor, error) { + if len(users) == 0 { + return nil, nil + } + + userIDs := users.getUserIDs() + tokenMaps := make(map[int64]*TwoFactor, len(userIDs)) + err := e. + In("uid", userIDs). + Find(&tokenMaps) + if err != nil { + return nil, fmt.Errorf("find two factor: %v", err) + } + return tokenMaps, nil +} diff --git a/models/userlist_test.go b/models/userlist_test.go index b41a4d0af5c7f..d3d26d45dc898 100644 --- a/models/userlist_test.go +++ b/models/userlist_test.go @@ -76,10 +76,10 @@ func TestUserListIsTwoFaEnrolled(t *testing.T) { }) } } + func testUserListIsTwoFaEnrolled(t *testing.T, orgID int64, expected map[int64]bool) { org, err := GetUserByID(orgID) assert.NoError(t, err) assert.NoError(t, org.GetMembers()) assert.Equal(t, expected, org.Members.GetTwoFaStatus()) - } From 4f44e7254e5964e2d2267d24b50dd9f5754b5ac6 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 28 Jul 2019 01:27:17 +0200 Subject: [PATCH 12/17] rewrite by using existing sub func --- models/org.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/models/org.go b/models/org.go index 2339bf7e67054..a54293f12669a 100644 --- a/models/org.go +++ b/models/org.go @@ -301,15 +301,13 @@ type OrgUser struct { } func isOrganizationOwner(e Engine, orgID, uid int64) (bool, error) { - ownerTeam := &Team{ - OrgID: orgID, - Name: ownerTeamName, - } - if has, err := e.Get(ownerTeam); err != nil { + ownerTeam, err := getTeam(e, orgID, ownerTeamName) + if err != nil { + if err == ErrTeamNotExist { + log.Error("Organization does not have owner team: %d", orgID) + return false, nil + } return false, err - } else if !has { - log.Error("Organization does not have owner team: %d", orgID) - return false, nil } return isTeamMember(e, orgID, ownerTeam.ID, uid) } From b74810f6ee19fdf356fb0eba13d7af74c44badbb Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 28 Jul 2019 02:09:18 +0200 Subject: [PATCH 13/17] Optimize IsUserOrgOwner --- models/org.go | 2 +- models/org_team.go | 5 +++++ models/userlist.go | 37 ++++++++++++++++++++++++++++++++++--- models/userlist_test.go | 1 + 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/models/org.go b/models/org.go index a54293f12669a..7032f6698eeb0 100644 --- a/models/org.go +++ b/models/org.go @@ -301,7 +301,7 @@ type OrgUser struct { } func isOrganizationOwner(e Engine, orgID, uid int64) (bool, error) { - ownerTeam, err := getTeam(e, orgID, ownerTeamName) + ownerTeam, err := getOwnerTeam(e, orgID) if err != nil { if err == ErrTeamNotExist { log.Error("Organization does not have owner team: %d", orgID) diff --git a/models/org_team.go b/models/org_team.go index dcf07437403d3..1786376d02c81 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -362,6 +362,11 @@ func GetTeam(orgID int64, name string) (*Team, error) { return getTeam(x, orgID, name) } +// getOwnerTeam returns team by given team name and organization. +func getOwnerTeam(e Engine, orgID int64) (*Team, error) { + return getTeam(e, orgID, ownerTeamName) +} + func getTeamByID(e Engine, teamID int64) (*Team, error) { t := new(Team) has, err := e.ID(teamID).Get(t) diff --git a/models/userlist.go b/models/userlist.go index e03db111288d9..2e3a0679d707f 100644 --- a/models/userlist.go +++ b/models/userlist.go @@ -5,6 +5,7 @@ package models import ( + "code.gitea.io/gitea/modules/log" "fmt" ) @@ -23,13 +24,43 @@ func (users UserList) getUserIDs() []int64 { // IsUserOrgOwner returns true if user is in the owner team of given organization. func (users UserList) IsUserOrgOwner(orgID int64) map[int64]bool { results := make(map[int64]bool, len(users)) - //TODO use directly xorm - for _, u := range users { - results[u.ID] = u.IsUserOrgOwner(orgID) + for _, user := range users { + results[user.ID] = false //Set default to false + } + ownerMaps, err := users.loadOrganizationOwners(x, orgID) + if err == nil { + for _, owner := range ownerMaps { + results[owner.UID] = true + } } return results } +func (users UserList) loadOrganizationOwners(e Engine, orgID int64) (map[int64]*TeamUser, error) { + if len(users) == 0 { + return nil, nil + } + ownerTeam, err := getOwnerTeam(e, orgID) + if err != nil { + if err == ErrTeamNotExist { + log.Error("Organization does not have owner team: %d", orgID) + return nil, nil + } + return nil, err + } + + userIDs := users.getUserIDs() + ownerMaps := make(map[int64]*TeamUser) + err = e.In("uid", userIDs). + And("org_id=?", orgID). + And("team_id=?", ownerTeam.ID). + Find(&ownerMaps) + if err != nil { + return nil, fmt.Errorf("find team users: %v", err) + } + return ownerMaps, nil +} + // GetTwoFaStatus return state of 2FA enrollement func (users UserList) GetTwoFaStatus() map[int64]bool { results := make(map[int64]bool, len(users)) diff --git a/models/userlist_test.go b/models/userlist_test.go index d3d26d45dc898..223d19d71a6e9 100644 --- a/models/userlist_test.go +++ b/models/userlist_test.go @@ -52,6 +52,7 @@ func TestUserListIsUserOrgOwner(t *testing.T) { }) } } + func testUserListIsUserOrgOwner(t *testing.T, orgID int64, expected map[int64]bool) { org, err := GetUserByID(orgID) assert.NoError(t, err) From 371a4fb3f9d3bab4dfbd5635d53f553dfbba1f68 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 28 Jul 2019 03:04:34 +0200 Subject: [PATCH 14/17] remove un-used code --- models/user.go | 8 -------- models/user_test.go | 25 ------------------------- 2 files changed, 33 deletions(-) diff --git a/models/user.go b/models/user.go index 8a77db3e08c20..ab29683a7bf14 100644 --- a/models/user.go +++ b/models/user.go @@ -149,8 +149,6 @@ type User struct { // Preferences DiffViewStyle string `xorm:"NOT NULL DEFAULT ''"` Theme string `xorm:"NOT NULL DEFAULT ''"` - - //IsTwoFaEnrolled bool `xorm:"-"` } // ColorFormat writes a colored string to identify this struct @@ -555,12 +553,6 @@ func (u *User) IsUserOrgOwner(orgID int64) bool { return isOwner } -// IsTwoFaEnrolled return state of 2FA enrollement -func (u *User) IsTwoFaEnrolled() bool { - _, err := GetTwoFactorByUID(u.ID) - return err == nil //If we can't generate a scratch token what so ever the reason -> 2FA is not enable -} - // IsUserPartOfOrg returns true if user with userID is part of the u organisation. func (u *User) IsUserPartOfOrg(userID int64) bool { return u.isUserPartOfOrg(x, userID) diff --git a/models/user_test.go b/models/user_test.go index a04fa88d191e3..290253c4b1a0f 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -68,31 +68,6 @@ func testIsUserOrgOwner(t *testing.T, uid int64, orgID int64, expected bool) { assert.Equal(t, expected, user.IsUserOrgOwner(orgID)) } -func TestIsTwoFaEnrolled(t *testing.T) { - assert.NoError(t, PrepareTestDatabase()) - tt := []struct { - uid int64 - expected bool - }{ - {2, false}, - {4, false}, - {5, false}, - {5, false}, - {24, true}, - } - for _, v := range tt { - t.Run(fmt.Sprintf("UserId%dIsTwoFaEnrolled", v.uid), func(t *testing.T) { - testIsTwoFaEnrolled(t, v.uid, v.expected) - }) - } -} - -func testIsTwoFaEnrolled(t *testing.T, uid int64, expected bool) { - user, err := GetUserByID(uid) - assert.NoError(t, err) - assert.Equal(t, expected, user.IsTwoFaEnrolled()) -} - func TestGetUserEmailsByNames(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) From 3b6b0de53a5d03567db54ffb26bc5c421e70c296 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 29 Jul 2019 03:18:18 +0200 Subject: [PATCH 15/17] tests: cover empty org + fix import order --- models/userlist.go | 3 ++- models/userlist_test.go | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/models/userlist.go b/models/userlist.go index 2e3a0679d707f..43838a6804411 100644 --- a/models/userlist.go +++ b/models/userlist.go @@ -5,8 +5,9 @@ package models import ( - "code.gitea.io/gitea/modules/log" "fmt" + + "code.gitea.io/gitea/modules/log" ) //UserList is a list of user. diff --git a/models/userlist_test.go b/models/userlist_test.go index 223d19d71a6e9..11129ae6479fc 100644 --- a/models/userlist_test.go +++ b/models/userlist_test.go @@ -6,8 +6,9 @@ package models import ( "fmt" - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestUserListIsPublicMember(t *testing.T) { @@ -20,6 +21,7 @@ func TestUserListIsPublicMember(t *testing.T) { {6, map[int64]bool{5: true}}, {7, map[int64]bool{5: false}}, {25, map[int64]bool{24: true}}, + {22, map[int64]bool{}}, } for _, v := range tt { t.Run(fmt.Sprintf("IsPublicMemberOfOrdIg%d", v.orgid), func(t *testing.T) { @@ -45,6 +47,7 @@ func TestUserListIsUserOrgOwner(t *testing.T) { {6, map[int64]bool{5: true}}, {7, map[int64]bool{5: true}}, {25, map[int64]bool{24: true}}, + {22, map[int64]bool{}}, } for _, v := range tt { t.Run(fmt.Sprintf("IsUserOrgOwnerOfOrdIg%d", v.orgid), func(t *testing.T) { @@ -70,6 +73,7 @@ func TestUserListIsTwoFaEnrolled(t *testing.T) { {6, map[int64]bool{5: false}}, {7, map[int64]bool{5: false}}, {25, map[int64]bool{24: true}}, + {22, map[int64]bool{}}, } for _, v := range tt { t.Run(fmt.Sprintf("IsTwoFaEnrolledOfOrdIg%d", v.orgid), func(t *testing.T) { From dfcbb8972edc2921a0256411ed330d750a884e6b Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 29 Jul 2019 03:30:26 +0200 Subject: [PATCH 16/17] tests: add ErrTeamNotExist path --- models/fixtures/team.yml | 6 +++--- models/userlist_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index d99dc324692e9..b7265ec49e409 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -82,8 +82,8 @@ - id: 10 org_id: 25 - lower_name: owners - name: Owners - authorize: 4 # owner + lower_name: notowners + name: NotOwners + authorize: 1 # owner num_repos: 0 num_members: 1 diff --git a/models/userlist_test.go b/models/userlist_test.go index 11129ae6479fc..21d42aec20292 100644 --- a/models/userlist_test.go +++ b/models/userlist_test.go @@ -46,8 +46,8 @@ func TestUserListIsUserOrgOwner(t *testing.T) { {3, map[int64]bool{2: true, 4: false}}, {6, map[int64]bool{5: true}}, {7, map[int64]bool{5: true}}, - {25, map[int64]bool{24: true}}, - {22, map[int64]bool{}}, + {25, map[int64]bool{}}, // ErrTeamNotExist + {22, map[int64]bool{}}, // No member } for _, v := range tt { t.Run(fmt.Sprintf("IsUserOrgOwnerOfOrdIg%d", v.orgid), func(t *testing.T) { From 4371160423ad5f5f8f60e808e87a5b150f1181d5 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Mon, 29 Jul 2019 03:41:48 +0200 Subject: [PATCH 17/17] tests: fix wrong expected result --- models/userlist_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/userlist_test.go b/models/userlist_test.go index 21d42aec20292..ca08cc90ce89c 100644 --- a/models/userlist_test.go +++ b/models/userlist_test.go @@ -46,8 +46,8 @@ func TestUserListIsUserOrgOwner(t *testing.T) { {3, map[int64]bool{2: true, 4: false}}, {6, map[int64]bool{5: true}}, {7, map[int64]bool{5: true}}, - {25, map[int64]bool{}}, // ErrTeamNotExist - {22, map[int64]bool{}}, // No member + {25, map[int64]bool{24: false}}, // ErrTeamNotExist + {22, map[int64]bool{}}, // No member } for _, v := range tt { t.Run(fmt.Sprintf("IsUserOrgOwnerOfOrdIg%d", v.orgid), func(t *testing.T) {