Remove IsOwner, NumTeams from OrgUser (#3269)

7 files changed, 63 insertions, 77 deletions

diff --git a/models/action_test.go b/models/action_test.go
index 0169179050..e0a3e21237 100644
--- a/models/action_test.go
+++ b/models/action_test.go
@@ -425,7 +425,7 @@ func TestGetFeeds2(t *testing.T) {
 	// test with an organization user
 	assert.NoError(t, PrepareTestDatabase())
 	org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User)
-	userID := AssertExistsAndLoadBean(t, &OrgUser{OrgID: org.ID, IsOwner: true}).(*OrgUser).UID
+	const userID = 2 // user2 is an owner of the organization
 
 	actions, err := GetFeeds(GetFeedsOptions{
 		RequestedUser:    org,
diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml
index 709a1997b9..5bb23571fc 100644
--- a/models/fixtures/org_user.yml
+++ b/models/fixtures/org_user.yml
@@ -3,53 +3,39 @@
   uid: 2
   org_id: 3
   is_public: true
-  is_owner: true
-  num_teams: 1
 
 -
   id: 2
   uid: 4
   org_id: 3
   is_public: false
-  is_owner: false
-  num_teams: 0
 
 -
   id: 3
   uid: 5
   org_id: 6
   is_public: true
-  is_owner: true
-  num_teams: 1
 
 -
   id: 4
   uid: 5
   org_id: 7
   is_public: false
-  is_owner: true
-  num_teams: 1
 
 -
   id: 5
   uid: 15
   org_id: 17
   is_public: true
-  is_owner: true
-  num_teams: 1
 
 -
   id: 6
   uid: 18
   org_id: 17
   is_public: false
-  is_owner: true
-  num_teams: 1
 
 -
   id: 7
   uid: 20
   org_id: 19
   is_public: true
-  is_owner: true
-  num_teams: 1
\ No newline at end of file
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index 37f3717ff4..23235f7819 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -164,6 +164,8 @@ var migrations = []Migration{
 	NewMigration("add pull request options", addPullRequestOptions),
 	// v55 -> v56
 	NewMigration("add writable deploy keys", addModeToDeploKeys),
+	// v56 -> v57
+	NewMigration("remove is_owner, num_teams columns from org_user", removeIsOwnerColumnFromOrgUser),
 }
 
 // Migrate database to current version
diff --git a/models/migrations/v56.go b/models/migrations/v56.go
new file mode 100644
index 0000000000..bbaee28a92
--- /dev/null
+++ b/models/migrations/v56.go
@@ -0,0 +1,33 @@
+// Copyright 2017 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 migrations
+
+import (
+	"fmt"
+
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
+
+	"github.com/go-xorm/xorm"
+)
+
+func removeIsOwnerColumnFromOrgUser(x *xorm.Engine) (err error) {
+	switch {
+	case setting.UseSQLite3:
+		log.Warn("Unable to drop columns in SQLite")
+	case setting.UseMySQL, setting.UseTiDB, setting.UsePostgreSQL:
+		if _, err := x.Exec("ALTER TABLE org_user DROP COLUMN is_owner, DROP COLUMN num_teams"); err != nil {
+			return fmt.Errorf("DROP COLUMN org_user.is_owner, org_user.num_teams: %v", err)
+		}
+	case setting.UseMSSQL:
+		if _, err := x.Exec("ALTER TABLE org_user DROP COLUMN is_owner, num_teams"); err != nil {
+			return fmt.Errorf("DROP COLUMN org_user.is_owner, org_user.num_teams: %v", err)
+		}
+	default:
+		log.Fatal(4, "Unrecognized DB")
+	}
+
+	return nil
+}
diff --git a/models/org.go b/models/org.go
index a28a8e28e1..095265a12e 100644
--- a/models/org.go
+++ b/models/org.go
@@ -10,6 +10,8 @@ import (
 	"os"
 	"strings"
 
+	"code.gitea.io/gitea/modules/log"
+
 	"github.com/Unknwon/com"
 	"github.com/go-xorm/builder"
 	"github.com/go-xorm/xorm"
@@ -139,10 +141,8 @@ func CreateOrganization(org, owner *User) (err error) {
 
 	// Add initial creator to organization and owner team.
 	if _, err = sess.Insert(&OrgUser{
-		UID:      owner.ID,
-		OrgID:    org.ID,
-		IsOwner:  true,
-		NumTeams: 1,
+		UID:   owner.ID,
+		OrgID: org.ID,
 	}); err != nil {
 		return fmt.Errorf("insert org-user relation: %v", err)
 	}
@@ -280,18 +280,25 @@ type OrgUser struct {
 	UID      int64 `xorm:"INDEX UNIQUE(s)"`
 	OrgID    int64 `xorm:"INDEX UNIQUE(s)"`
 	IsPublic bool  `xorm:"INDEX"`
-	IsOwner  bool
-	NumTeams int
+}
+
+func isOrganizationOwner(e Engine, orgID, uid int64) (bool, error) {
+	ownerTeam := &Team{
+		OrgID: orgID,
+		Name:  ownerTeamName,
+	}
+	if has, err := e.Get(ownerTeam); err != nil {
+		return false, err
+	} else if !has {
+		log.Error(4, "Organization does not have owner team: %d", orgID)
+		return false, nil
+	}
+	return isTeamMember(e, orgID, ownerTeam.ID, uid)
 }
 
 // IsOrganizationOwner returns true if given user is in the owner team.
 func IsOrganizationOwner(orgID, uid int64) (bool, error) {
-	return x.
-		Where("is_owner=?", true).
-		And("uid=?", uid).
-		And("org_id=?", orgID).
-		Table("org_user").
-		Exist()
+	return isOrganizationOwner(x, orgID, uid)
 }
 
 // IsOrganizationMember returns true if given user is member of organization.
@@ -336,9 +343,10 @@ func GetOrgsByUserID(userID int64, showAll bool) ([]*User, error) {
 func getOwnedOrgsByUserID(sess *xorm.Session, userID int64) ([]*User, error) {
 	orgs := make([]*User, 0, 10)
 	return orgs, sess.
-		Where("`org_user`.uid=?", userID).
-		And("`org_user`.is_owner=?", true).
-		Join("INNER", "`org_user`", "`org_user`.org_id=`user`.id").
+		Join("INNER", "`team_user`", "`team_user`.org_id=`user`.id").
+		Join("INNER", "`team`", "`team`.id=`team_user`.team_id").
+		Where("`team_user`.uid=?", userID).
+		And("`team`.authorize=?", AccessModeOwner).
 		Asc("`user`.name").
 		Find(&orgs)
 }
diff --git a/models/org_team.go b/models/org_team.go
index c667cfb8e2..941b7ed2a1 100644
--- a/models/org_team.go
+++ b/models/org_team.go
@@ -518,22 +518,6 @@ func AddTeamMember(team *Team, userID int64) error {
 		}
 	}
 
-	// We make sure it exists before.
-	ou := new(OrgUser)
-	if _, err := sess.
-		Where("uid = ?", userID).
-		And("org_id = ?", team.OrgID).
-		Get(ou); err != nil {
-		return err
-	}
-	ou.NumTeams++
-	if team.IsOwnerTeam() {
-		ou.IsOwner = true
-	}
-	if _, err := sess.ID(ou.ID).Cols("num_teams, is_owner").Update(ou); err != nil {
-		return err
-	}
-
 	return sess.Commit()
 }
 
@@ -574,25 +558,6 @@ func removeTeamMember(e Engine, team *Team, userID int64) error {
 		}
 	}
 
-	// This must exist.
-	ou := new(OrgUser)
-	_, err = e.
-		Where("uid = ?", userID).
-		And("org_id = ?", team.OrgID).
-		Get(ou)
-	if err != nil {
-		return err
-	}
-	ou.NumTeams--
-	if team.IsOwnerTeam() {
-		ou.IsOwner = false
-	}
-	if _, err = e.
-		ID(ou.ID).
-		Cols("num_teams").
-		Update(ou); err != nil {
-		return err
-	}
 	return nil
 }
 
diff --git a/models/org_test.go b/models/org_test.go
index aef313d05e..42ab4a2a45 100644
--- a/models/org_test.go
+++ b/models/org_test.go
@@ -368,16 +368,12 @@ func TestGetOrgUsersByUserID(t *testing.T) {
 			ID:       orgUsers[0].ID,
 			OrgID:    6,
 			UID:      5,
-			IsOwner:  true,
-			IsPublic: true,
-			NumTeams: 1}, *orgUsers[0])
+			IsPublic: true}, *orgUsers[0])
 		assert.Equal(t, OrgUser{
 			ID:       orgUsers[1].ID,
 			OrgID:    7,
 			UID:      5,
-			IsOwner:  true,
-			IsPublic: false,
-			NumTeams: 1}, *orgUsers[1])
+			IsPublic: false}, *orgUsers[1])
 	}
 
 	publicOrgUsers, err := GetOrgUsersByUserID(5, false)
@@ -400,16 +396,12 @@ func TestGetOrgUsersByOrgID(t *testing.T) {
 			ID:       orgUsers[0].ID,
 			OrgID:    3,
 			UID:      2,
-			IsOwner:  true,
-			IsPublic: true,
-			NumTeams: 1}, *orgUsers[0])
+			IsPublic: true}, *orgUsers[0])
 		assert.Equal(t, OrgUser{
 			ID:       orgUsers[1].ID,
 			OrgID:    3,
 			UID:      4,
-			IsOwner:  false,
-			IsPublic: false,
-			NumTeams: 0}, *orgUsers[1])
+			IsPublic: false}, *orgUsers[1])
 	}
 
 	orgUsers, err = GetOrgUsersByOrgID(NonexistentID)