fix ui bug when user can't create repo but org can

[a1012112796]

fix #15504

[techknowlogick]

Blocking per chat in case this may be a breaking change.

[a1012112796]

Blocking per chat in case this may be a breaking change.

@techknowlogick Would you please give more detail? Thanks.

[noerw]

@a1012112796 I raised that concern on discord:
It's not documented if the old behaviour is by design or a bug, but I seem to remember that the behaviour was like this since at least 1.12.x. (But I may be wrong, didn't verify with such an old release yet.)

Changing the behaviour would be a breaking change in permission handling, just take gitea.com for example: policy would need to be changed that users can't create organizations in order to keep the 5-repos-per-user limitation.

[noerw]

Ok I stand corrected:
Up to gitea 1.13.7 limited users were able to create repos in orgs (the error on the repo-create page was displayed, but the submit button not disabled, the repo creation goes through). Due to the displayed error message it seems like unintended behaviour to me that users can still create repos, but who knows 🤔
So maybe this isn't a breaking change after all.

edit: #12492 introduced the disabled button, but didn't touch on this topic either

[a1012112796]

@noerw I see, so you means the org's repos number should be calculate as owner's repo, then if owner can't create repo, his org also can't create repo, Isn't it?
But in fact a org maybe have more than one owner, how to handle it?
Add sadly gitea hasn't release this feature.

In fact, gitea only check the org's repo number limt when create org repo, see here:

gitea/modules/repository/create.go

Lines 18 to 24 in 77fa714

	// CreateRepository creates a repository for the user/organization.
	func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*models.Repository, error) {
	if !doer.IsAdmin && !u.CanCreateRepo() {
	return nil, models.ErrReachLimitOfRepo{
	Limit: u.MaxRepoCreation,
	}
	}

and this pull request just fix ui logic to make it same with post handle, hasn't change the repo create logic in post handle. I wonder why it breake this logic ...
That's just my thought, feel sorry if I missunderstood you, Thanks.

[lunny]

Proposal sollution about limit org's repo number.

Add a new identity named as main owner in a org, which is the creater of org, and can be changed but should be only one in a org, The orgs repo will be calculate as main owner's repo to limit repo number. Only when the main owner can create repo can this org create repo. and we can also limit the number of org one user can main owned to limit org create.

main logic:

diff --git a/models/user.go b/models/user.go
index c7b44bdb7..efedcc38b 100644
--- a/models/user.go
+++ b/models/user.go
@@ -161,6 +161,8 @@ type User struct {
 	MembersIsPublic           map[int64]bool      `xorm:"-"`
 	Visibility                structs.VisibleType `xorm:"NOT NULL DEFAULT 0"`
 	RepoAdminChangeTeamAccess bool                `xorm:"NOT NULL DEFAULT false"`
+	MainOwnerID               int64               `xorm:"INDEX"`
+	MainOwner                 *User               `xorm:"-"`
 
 	// Preferences
 	DiffViewStyle       string `xorm:"NOT NULL DEFAULT ''"`
@@ -264,19 +266,51 @@ func (u *User) MaxCreationLimit() int {
 	return u.MaxRepoCreation
 }
 
+// LoadMainOwner load main owner of a org
+func (u *User) LoadMainOwner() (err error) {
+	if !u.IsOrganization() || u.MainOwner != nil {
+		return nil
+	}
+
+	u.MainOwner, err = GetUserByID(u.MainOwnerID)
+	return err
+}
+
 // CanCreateRepo returns if user login can create a repository
 // NOTE: functions calling this assume a failure due to repository count limit; if new checks are added, those functions should be revised
 func (u *User) CanCreateRepo() bool {
 	if u.IsAdmin {
 		return true
 	}
+
+	total := 0
+	var err error
+	if u.IsOrganization() {
+		err = u.LoadMainOwner()
+		if err != nil {
+			log.Error("LoadMainOwner(): %v", err)
+			return false
+		}
+		if !u.MainOwner.CanCreateRepo() {
+			return false
+		}
+	} else {
+		total, err = u.TotalRepoNumInMainOwnedOrgs()
+		if err != nil {
+			log.Error("TotalRepoNumInMainOwnedOrgs(): %v", err)
+			return false
+		}
+	}
+
+	total += u.NumRepos
+
 	if u.MaxRepoCreation <= -1 {
 		if setting.Repository.MaxCreationLimit <= -1 {
 			return true
 		}
-		return u.NumRepos < setting.Repository.MaxCreationLimit
+		return total < setting.Repository.MaxCreationLimit
 	}
-	return u.NumRepos < u.MaxRepoCreation
+	return total < u.MaxRepoCreation
 }
 
 // CanCreateOrganization returns true if user can create organisation.
@@ -2035,3 +2069,10 @@ func IterateUser(f func(user *User) error) error {
 		}
 	}
 }
+
+// TotalRepoNumInMainOwnedOrgs calculate all repo number in org that
+// user main owned
+func (u *User) TotalRepoNumInMainOwnedOrgs() (int, error) {
+	total, err := x.Where("main_owner_id").SumInt(new(User), "num_repos")
+	return int(total), err
+}

This still be a wired design and I cannot accept that.

[a1012112796]

@lunny That's out of the topic of this pull request. now I think we should make sure when a user can't create repo, should we allow this user to create repo for the org which can create repo? if yes, It's okay to merge this pull request, or it's a big problem and need rewrite many things ...

[Commifreak]

Anything new to that?

[lunny]

Please resolve the conflicts.

[wxiaoguang]

@techknowlogick there is one requested change from you. What should be done to continue?

[lunny]

Why is the first organizaiton?

[a1012112796]

Because curent user can't create repo, and user hasn't choose an organizaiton by query option, so use the the first organizaiton as default.

[lunny]

I think the logic is still confusing.

[wxiaoguang]

Asked by @blackshot , #15504 (comment)

is it possible to improve this PR and get it merged in 1.17?

[a1012112796]

But looks noone think this pull request is right. so I'd like close it.

[wxiaoguang]

Hmm ... I haven't look into the details for it, just saw some discussions above, no idea about what is right.