Merge branch 'master' into refactor_issues-subscription

models/issue_comment.go

@@ -535,6 +535,10 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
 		return nil, err
 	}
 
+	if err = updateCommentInfos(e, opts, comment); err != nil {
+		return nil, err
+	}
+
 	if err = sendCreateCommentAction(e, opts, comment); err != nil {
 		return nil, err
 	}
@@ -546,6 +550,56 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
 	return comment, nil
 }
 
+func updateCommentInfos(e *xorm.Session, opts *CreateCommentOptions, comment *Comment) (err error) {
+	// Check comment type.
+	switch opts.Type {
+	case CommentTypeCode:
+		if comment.ReviewID != 0 {
+			if comment.Review == nil {
+				if err := comment.loadReview(e); err != nil {
+					return err
+				}
+			}
+			if comment.Review.Type <= ReviewTypePending {
+				return nil
+			}
+		}
+		fallthrough
+	case CommentTypeComment:
+		if _, err = e.Exec("UPDATE `issue` SET num_comments=num_comments+1 WHERE id=?", opts.Issue.ID); err != nil {
+			return err
+		}
+
+		// Check attachments
+		attachments := make([]*Attachment, 0, len(opts.Attachments))
+		for _, uuid := range opts.Attachments {
+			attach, err := getAttachmentByUUID(e, uuid)
+			if err != nil {
+				if IsErrAttachmentNotExist(err) {
+					continue
+				}
+				return fmt.Errorf("getAttachmentByUUID [%s]: %v", uuid, err)
+			}
+			attachments = append(attachments, attach)
+		}
+
+		for i := range attachments {
+			attachments[i].IssueID = opts.Issue.ID
+			attachments[i].CommentID = comment.ID
+			// No assign value could be 0, so ignore AllCols().
+			if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil {
+				return fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err)
+			}
+		}
+	case CommentTypeReopen, CommentTypeClose:
+		if err = opts.Issue.updateClosedNum(e); err != nil {
+			return err
+		}
+	}
+	// update the issue's updated_unix column
+	return updateIssueCols(e, opts.Issue, "updated_unix")
+}
+
 func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, comment *Comment) (err error) {
 	// Compose comment action, could be plain comment, close or reopen issue/pull request.
 	// This object will be used to notify watchers in the end of function.
@@ -575,56 +629,16 @@ func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, commen
 		fallthrough
 	case CommentTypeComment:
 		act.OpType = ActionCommentIssue
-
-		if _, err = e.Exec("UPDATE `issue` SET num_comments=num_comments+1 WHERE id=?", opts.Issue.ID); err != nil {
-			return err
-		}
-
-		// Check attachments
-		attachments := make([]*Attachment, 0, len(opts.Attachments))
-		for _, uuid := range opts.Attachments {
-			attach, err := getAttachmentByUUID(e, uuid)
-			if err != nil {
-				if IsErrAttachmentNotExist(err) {
-					continue
-				}
-				return fmt.Errorf("getAttachmentByUUID [%s]: %v", uuid, err)
-			}
-			attachments = append(attachments, attach)
-		}
-
-		for i := range attachments {
-			attachments[i].IssueID = opts.Issue.ID
-			attachments[i].CommentID = comment.ID
-			// No assign value could be 0, so ignore AllCols().
-			if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil {
-				return fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err)
-			}
-		}
-
 	case CommentTypeReopen:
 		act.OpType = ActionReopenIssue
 		if opts.Issue.IsPull {
 			act.OpType = ActionReopenPullRequest
 		}
-
-		if err = opts.Issue.updateClosedNum(e); err != nil {
-			return err
-		}
-
 	case CommentTypeClose:
 		act.OpType = ActionCloseIssue
 		if opts.Issue.IsPull {
 			act.OpType = ActionClosePullRequest
 		}
-
-		if err = opts.Issue.updateClosedNum(e); err != nil {
-			return err
-		}
-	}
-	// update the issue's updated_unix column
-	if err = updateIssueCols(e, opts.Issue, "updated_unix"); err != nil {
-		return err
 	}
 	// Notify watchers for whatever action comes in, ignore if no action type.
 	if act.OpType > 0 {

models/migrations/migrations.go

@@ -264,6 +264,8 @@ var migrations = []Migration{
 	NewMigration("Add WhitelistDeployKeys to protected branch", addWhitelistDeployKeysToBranches),
 	// v104 -> v105
 	NewMigration("remove unnecessary columns from label", removeLabelUneededCols),
+	// v105 -> v106
+	NewMigration("add includes_all_repositories to teams", addTeamIncludesAllRepositories),
 }
 
 // Migrate database to current version

models/migrations/v105.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 migrations
+
+import (
+	"xorm.io/xorm"
+)
+
+func addTeamIncludesAllRepositories(x *xorm.Engine) error {
+
+	type Team struct {
+		ID                      int64 `xorm:"pk autoincr"`
+		IncludesAllRepositories bool  `xorm:"NOT NULL DEFAULT false"`
+	}
+
+	if err := x.Sync2(new(Team)); err != nil {
+		return err
+	}
+
+	_, err := x.Exec("UPDATE `team` SET `includes_all_repositories` = ? WHERE `name`=?",
+		true, "Owners")
+	return err
+}

models/org.go

@@ -48,6 +48,9 @@ func (org *User) GetOwnerTeam() (*Team, error) {
 }
 
 func (org *User) getTeams(e Engine) error {
+	if org.Teams != nil {
+		return nil
+	}
 	return e.
 		Where("org_id=?", org.ID).
 		OrderBy("CASE WHEN name LIKE '" + ownerTeamName + "' THEN '' ELSE name END").
@@ -149,11 +152,12 @@ func CreateOrganization(org, owner *User) (err error) {
 
 	// Create default owner team.
 	t := &Team{
-		OrgID:      org.ID,
-		LowerName:  strings.ToLower(ownerTeamName),
-		Name:       ownerTeamName,
-		Authorize:  AccessModeOwner,
-		NumMembers: 1,
+		OrgID:                   org.ID,
+		LowerName:               strings.ToLower(ownerTeamName),
+		Name:                    ownerTeamName,
+		Authorize:               AccessModeOwner,
+		NumMembers:              1,
+		IncludesAllRepositories: true,
 	}
 	if _, err = sess.Insert(t); err != nil {
 		return fmt.Errorf("insert owner team: %v", err)

models/org_team.go

@@ -22,17 +22,18 @@ const ownerTeamName = "Owners"
 
 // Team represents a organization team.
 type Team struct {
-	ID          int64 `xorm:"pk autoincr"`
-	OrgID       int64 `xorm:"INDEX"`
-	LowerName   string
-	Name        string
-	Description string
-	Authorize   AccessMode
-	Repos       []*Repository `xorm:"-"`
-	Members     []*User       `xorm:"-"`
-	NumRepos    int
-	NumMembers  int
-	Units       []*TeamUnit `xorm:"-"`
+	ID                      int64 `xorm:"pk autoincr"`
+	OrgID                   int64 `xorm:"INDEX"`
+	LowerName               string
+	Name                    string
+	Description             string
+	Authorize               AccessMode
+	Repos                   []*Repository `xorm:"-"`
+	Members                 []*User       `xorm:"-"`
+	NumRepos                int
+	NumMembers              int
+	Units                   []*TeamUnit `xorm:"-"`
+	IncludesAllRepositories bool        `xorm:"NOT NULL DEFAULT false"`
 }
 
 // SearchTeamOptions holds the search options
@@ -149,6 +150,9 @@ func (t *Team) IsMember(userID int64) bool {
 }
 
 func (t *Team) getRepositories(e Engine) error {
+	if t.Repos != nil {
+		return nil
+	}
 	return e.Join("INNER", "team_repo", "repository.id = team_repo.repo_id").
 		Where("team_repo.team_id=?", t.ID).
 		OrderBy("repository.name").
@@ -220,6 +224,25 @@ func (t *Team) addRepository(e Engine, repo *Repository) (err error) {
 	return nil
 }
 
+// addAllRepositories adds all repositories to the team.
+// If the team already has some repositories they will be left unchanged.
+func (t *Team) addAllRepositories(e Engine) error {
+	var orgRepos []Repository
+	if err := e.Where("owner_id = ?", t.OrgID).Find(&orgRepos); err != nil {
+		return fmt.Errorf("get org repos: %v", err)
+	}
+
+	for _, repo := range orgRepos {
+		if !t.hasRepository(e, repo.ID) {
+			if err := t.addRepository(e, &repo); err != nil {
+				return fmt.Errorf("addRepository: %v", err)
+			}
+		}
+	}
+
+	return nil
+}
+
 // AddRepository adds new repository to team of organization.
 func (t *Team) AddRepository(repo *Repository) (err error) {
 	if repo.OwnerID != t.OrgID {
@@ -241,6 +264,8 @@ func (t *Team) AddRepository(repo *Repository) (err error) {
 	return sess.Commit()
 }
 
+// removeRepository removes a repository from a team and recalculates access
+// Note: Repository shall not be removed from team if it includes all repositories (unless the repository is deleted)
 func (t *Team) removeRepository(e Engine, repo *Repository, recalculate bool) (err error) {
 	if err = removeTeamRepo(e, t.ID, repo.ID); err != nil {
 		return err
@@ -284,11 +309,16 @@ func (t *Team) removeRepository(e Engine, repo *Repository, recalculate bool) (e
 }
 
 // RemoveRepository removes repository from team of organization.
+// If the team shall include all repositories the request is ignored.
 func (t *Team) RemoveRepository(repoID int64) error {
 	if !t.HasRepository(repoID) {
 		return nil
 	}
 
+	if t.IncludesAllRepositories {
+		return nil
+	}
+
 	repo, err := GetRepositoryByID(repoID)
 	if err != nil {
 		return err
@@ -394,6 +424,14 @@ func NewTeam(t *Team) (err error) {
 		}
 	}
 
+	// Add all repositories to the team if it has access to all of them.
+	if t.IncludesAllRepositories {
+		err = t.addAllRepositories(sess)
+		if err != nil {
+			return fmt.Errorf("addAllRepositories: %v", err)
+		}
+	}
+
 	// Update organization number of teams.
 	if _, err = sess.Exec("UPDATE `user` SET num_teams=num_teams+1 WHERE id = ?", t.OrgID); err != nil {
 		errRollback := sess.Rollback()
@@ -446,7 +484,7 @@ func GetTeamByID(teamID int64) (*Team, error) {
 }
 
 // UpdateTeam updates information of team.
-func UpdateTeam(t *Team, authChanged bool) (err error) {
+func UpdateTeam(t *Team, authChanged bool, includeAllChanged bool) (err error) {
 	if len(t.Name) == 0 {
 		return errors.New("empty team name")
 	}
@@ -511,6 +549,14 @@ func UpdateTeam(t *Team, authChanged bool) (err error) {
 		}
 	}
 
+	// Add all repositories to the team if it has access to all of them.
+	if includeAllChanged && t.IncludesAllRepositories {
+		err = t.addAllRepositories(sess)
+		if err != nil {
+			return fmt.Errorf("addAllRepositories: %v", err)
+		}
+	}
+
 	return sess.Commit()
 }
 

models/org_team_test.go

@@ -5,9 +5,12 @@
 package models
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/modules/structs"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -206,7 +209,7 @@ func TestUpdateTeam(t *testing.T) {
 	team.Name = "newName"
 	team.Description = strings.Repeat("A long description!", 100)
 	team.Authorize = AccessModeAdmin
-	assert.NoError(t, UpdateTeam(team, true))
+	assert.NoError(t, UpdateTeam(team, true, false))
 
 	team = AssertExistsAndLoadBean(t, &Team{Name: "newName"}).(*Team)
 	assert.True(t, strings.HasPrefix(team.Description, "A long description!"))
@@ -225,7 +228,7 @@ func TestUpdateTeam2(t *testing.T) {
 	team.LowerName = "owners"
 	team.Name = "Owners"
 	team.Description = strings.Repeat("A long description!", 100)
-	err := UpdateTeam(team, true)
+	err := UpdateTeam(team, true, false)
 	assert.True(t, IsErrTeamAlreadyExist(err))
 
 	CheckConsistencyFor(t, &Team{ID: team.ID})
@@ -374,3 +377,133 @@ func TestUsersInTeamsCount(t *testing.T) {
 	test([]int64{1, 2, 3, 4, 5}, []int64{2, 5}, 2)    // userid 2,4
 	test([]int64{1, 2, 3, 4, 5}, []int64{2, 3, 5}, 3) // userid 2,4,5
 }
+
+func TestIncludesAllRepositoriesTeams(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	testTeamRepositories := func(teamID int64, repoIds []int64) {
+		team := AssertExistsAndLoadBean(t, &Team{ID: teamID}).(*Team)
+		assert.NoError(t, team.GetRepositories(), "%s: GetRepositories", team.Name)
+		assert.Len(t, team.Repos, team.NumRepos, "%s: len repo", team.Name)
+		assert.Equal(t, len(repoIds), len(team.Repos), "%s: repo count", team.Name)
+		for i, rid := range repoIds {
+			if rid > 0 {
+				assert.True(t, team.HasRepository(rid), "%s: HasRepository(%d) %d", rid, i)
+			}
+		}
+	}
+
+	// Get an admin user.
+	user, err := GetUserByID(1)
+	assert.NoError(t, err, "GetUserByID")
+
+	// Create org.
+	org := &User{
+		Name:       "All repo",
+		IsActive:   true,
+		Type:       UserTypeOrganization,
+		Visibility: structs.VisibleTypePublic,
+	}
+	assert.NoError(t, CreateOrganization(org, user), "CreateOrganization")
+
+	// Check Owner team.
+	ownerTeam, err := org.GetOwnerTeam()
+	assert.NoError(t, err, "GetOwnerTeam")
+	assert.True(t, ownerTeam.IncludesAllRepositories, "Owner team includes all repositories")
+
+	// Create repos.
+	repoIds := make([]int64, 0)
+	for i := 0; i < 3; i++ {
+		r, err := CreateRepository(user, org, CreateRepoOptions{Name: fmt.Sprintf("repo-%d", i)})
+		assert.NoError(t, err, "CreateRepository %d", i)
+		if r != nil {
+			repoIds = append(repoIds, r.ID)
+		}
+	}
+	// Get fresh copy of Owner team after creating repos.
+	ownerTeam, err = org.GetOwnerTeam()
+	assert.NoError(t, err, "GetOwnerTeam")
+
+	// Create teams and check repositories.
+	teams := []*Team{
+		ownerTeam,
+		{
+			OrgID:                   org.ID,
+			Name:                    "team one",
+			Authorize:               AccessModeRead,
+			IncludesAllRepositories: true,
+		},
+		{
+			OrgID:                   org.ID,
+			Name:                    "team 2",
+			Authorize:               AccessModeRead,
+			IncludesAllRepositories: false,
+		},
+		{
+			OrgID:                   org.ID,
+			Name:                    "team three",
+			Authorize:               AccessModeWrite,
+			IncludesAllRepositories: true,
+		},
+		{
+			OrgID:                   org.ID,
+			Name:                    "team 4",
+			Authorize:               AccessModeWrite,
+			IncludesAllRepositories: false,
+		},
+	}
+	teamRepos := [][]int64{
+		repoIds,
+		repoIds,
+		{},
+		repoIds,
+		{},
+	}
+	for i, team := range teams {
+		if i > 0 { // first team is Owner.
+			assert.NoError(t, NewTeam(team), "%s: NewTeam", team.Name)
+		}
+		testTeamRepositories(team.ID, teamRepos[i])
+	}
+
+	// Update teams and check repositories.
+	teams[3].IncludesAllRepositories = false
+	teams[4].IncludesAllRepositories = true
+	teamRepos[4] = repoIds
+	for i, team := range teams {
+		assert.NoError(t, UpdateTeam(team, false, true), "%s: UpdateTeam", team.Name)
+		testTeamRepositories(team.ID, teamRepos[i])
+	}
+
+	// Create repo and check teams repositories.
+	org.Teams = nil // Reset teams to allow their reloading.
+	r, err := CreateRepository(user, org, CreateRepoOptions{Name: "repo-last"})
+	assert.NoError(t, err, "CreateRepository last")
+	if r != nil {
+		repoIds = append(repoIds, r.ID)
+	}
+	teamRepos[0] = repoIds
+	teamRepos[1] = repoIds
+	teamRepos[4] = repoIds
+	for i, team := range teams {
+		testTeamRepositories(team.ID, teamRepos[i])
+	}
+
+	// Remove repo and check teams repositories.
+	assert.NoError(t, DeleteRepository(user, org.ID, repoIds[0]), "DeleteRepository")
+	teamRepos[0] = repoIds[1:]
+	teamRepos[1] = repoIds[1:]
+	teamRepos[3] = repoIds[1:3]
+	teamRepos[4] = repoIds[1:]
+	for i, team := range teams {
+		testTeamRepositories(team.ID, teamRepos[i])
+	}
+
+	// Wipe created items.
+	for i, rid := range repoIds {
+		if i > 0 { // first repo already deleted.
+			assert.NoError(t, DeleteRepository(user, org.ID, rid), "DeleteRepository %d", i)
+		}
+	}
+	assert.NoError(t, DeleteOrganization(org), "DeleteOrganization")
+}

models/repo.go

@@ -1447,14 +1447,17 @@ func createRepository(e *xorm.Session, doer, u *User, repo *Repository) (err err
 	}
 	u.NumRepos++
 
-	// Give access to all members in owner team.
+	// Give access to all members in teams with access to all repositories.
 	if u.IsOrganization() {
-		t, err := u.getOwnerTeam(e)
-		if err != nil {
-			return fmt.Errorf("getOwnerTeam: %v", err)
+		if err := u.GetTeams(); err != nil {
+			return fmt.Errorf("GetTeams: %v", err)
 		}
-		if err = t.addRepository(e, repo); err != nil {
-			return fmt.Errorf("addRepository: %v", err)
+		for _, t := range u.Teams {
+			if t.IncludesAllRepositories {
+				if err := t.addRepository(e, repo); err != nil {
+					return fmt.Errorf("addRepository: %v", err)
+				}
+			}
 		}
 	} else if err = repo.recalculateAccesses(e); err != nil {
 		// Organization automatically called this in addRepository method.
@@ -1641,11 +1644,15 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
 	}
 
 	if newOwner.IsOrganization() {
-		t, err := newOwner.getOwnerTeam(sess)
-		if err != nil {
-			return fmt.Errorf("getOwnerTeam: %v", err)
-		} else if err = t.addRepository(sess, repo); err != nil {
-			return fmt.Errorf("add to owner team: %v", err)
+		if err := newOwner.GetTeams(); err != nil {
+			return fmt.Errorf("GetTeams: %v", err)
+		}
+		for _, t := range newOwner.Teams {
+			if t.IncludesAllRepositories {
+				if err := t.addRepository(sess, repo); err != nil {
+					return fmt.Errorf("addRepository: %v", err)
+				}
+			}
 		}
 	} else if err = repo.recalculateAccesses(sess); err != nil {
 		// Organization called this in addRepository method.

models/review.go

@@ -129,13 +129,17 @@ func (r *Review) publish(e *xorm.Engine) error {
 				go func(en *xorm.Engine, review *Review, comm *Comment) {
 					sess := en.NewSession()
 					defer sess.Close()
-					if err := sendCreateCommentAction(sess, &CreateCommentOptions{
+					opts := &CreateCommentOptions{
 						Doer:    comm.Poster,
 						Issue:   review.Issue,
 						Repo:    review.Issue.Repo,
 						Type:    comm.Type,
 						Content: comm.Content,
-					}, comm); err != nil {
+					}
+					if err := updateCommentInfos(sess, opts, comm); err != nil {
+						log.Warn("updateCommentInfos: %v", err)
+					}
+					if err := sendCreateCommentAction(sess, opts, comm); err != nil {
 						log.Warn("sendCreateCommentAction: %v", err)
 					}
 				}(e, r, comment)