Merge remote-tracking branch 'giteaofficial/main'

* giteaofficial/main:
  Move org functions (go-gitea#19753)
  [doctor] pq: syntax error at or near "." quote user table name (go-gitea#19765)
  [doctor] update the help with fix capabilities (go-gitea#19762)
  Remove fomantic progress module (go-gitea#19760)
  Make Ctrl+Enter (quick submit) work for issue comment and wiki editor (go-gitea#19729)
  Enable packages by default again (as described by docs) (go-gitea#19746)
  Replace blue button and label classes with primary (go-gitea#19763)
  Fix org package owner permissions (go-gitea#19742)

Author: zjjhot

cmd/doctor.go

@@ -24,8 +24,8 @@ import (
 // CmdDoctor represents the available doctor sub-command.
 var CmdDoctor = cli.Command{
 	Name:        "doctor",
-	Usage:       "Diagnose problems",
-	Description: "A command to diagnose problems with the current Gitea instance according to the given configuration.",
+	Usage:       "Diagnose and optionally fix problems",
+	Description: "A command to diagnose problems with the current Gitea instance according to the given configuration. Some problems can optionally be fixed by modifying the database or data storage.",
 	Action:      runDoctor,
 	Flags: []cli.Flag{
 		cli.BoolFlag{

models/organization/org_user.go

@@ -6,6 +6,7 @@ package organization
 
 import (
 	"context"
+	"fmt"
 
 	"code.gitea.io/gitea/models/db"
 	user_model "code.gitea.io/gitea/models/user"
@@ -81,3 +82,43 @@ func CanCreateOrgRepo(orgID, uid int64) (bool, error) {
 		And("team_user.org_id = ?", orgID).
 		Exist(new(Team))
 }
+
+// IsUserOrgOwner returns true if user is in the owner team of given organization.
+func IsUserOrgOwner(users user_model.UserList, orgID int64) map[int64]bool {
+	results := make(map[int64]bool, len(users))
+	for _, user := range users {
+		results[user.ID] = false // Set default to false
+	}
+	ownerMaps, err := loadOrganizationOwners(db.DefaultContext, users, orgID)
+	if err == nil {
+		for _, owner := range ownerMaps {
+			results[owner.UID] = true
+		}
+	}
+	return results
+}
+
+func loadOrganizationOwners(ctx context.Context, users user_model.UserList, orgID int64) (map[int64]*TeamUser, error) {
+	if len(users) == 0 {
+		return nil, nil
+	}
+	ownerTeam, err := GetOwnerTeam(ctx, orgID)
+	if err != nil {
+		if IsErrTeamNotExist(err) {
+			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 = db.GetEngine(ctx).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
+}

models/organization/org_user_test.go

@@ -70,3 +70,57 @@ func testIsUserOrgOwner(t *testing.T, uid, orgID int64, expected bool) {
 	assert.NoError(t, err)
 	assert.Equal(t, expected, is)
 }
+
+func TestUserListIsPublicMember(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	tt := []struct {
+		orgid    int64
+		expected map[int64]bool
+	}{
+		{3, map[int64]bool{2: true, 4: false, 28: true}},
+		{6, map[int64]bool{5: true, 28: 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) {
+			testUserListIsPublicMember(t, v.orgid, v.expected)
+		})
+	}
+}
+
+func testUserListIsPublicMember(t *testing.T, orgID int64, expected map[int64]bool) {
+	org, err := GetOrgByID(orgID)
+	assert.NoError(t, err)
+	_, membersIsPublic, err := org.GetMembers()
+	assert.NoError(t, err)
+	assert.Equal(t, expected, membersIsPublic)
+}
+
+func TestUserListIsUserOrgOwner(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	tt := []struct {
+		orgid    int64
+		expected map[int64]bool
+	}{
+		{3, map[int64]bool{2: true, 4: false, 28: false}},
+		{6, map[int64]bool{5: true, 28: false}},
+		{7, map[int64]bool{5: true}},
+		{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) {
+			testUserListIsUserOrgOwner(t, v.orgid, v.expected)
+		})
+	}
+}
+
+func testUserListIsUserOrgOwner(t *testing.T, orgID int64, expected map[int64]bool) {
+	org, err := GetOrgByID(orgID)
+	assert.NoError(t, err)
+	members, _, err := org.GetMembers()
+	assert.NoError(t, err)
+	assert.Equal(t, expected, IsUserOrgOwner(members, orgID))
+}

models/userlist.go

@@ -12,6 +12,7 @@ import (
 	packages_model "code.gitea.io/gitea/models/packages"
 	"code.gitea.io/gitea/models/perm"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/structs"
 )
 
 // Package contains owner, access mode and optional the package descriptor
@@ -50,22 +51,29 @@ func packageAssignment(ctx *Context, errCb func(int, string, interface{})) {
 		Owner: ctx.ContextUser,
 	}
 
-	if ctx.Doer != nil && ctx.Doer.ID == ctx.ContextUser.ID {
-		ctx.Package.AccessMode = perm.AccessModeOwner
+	if ctx.Package.Owner.IsOrganization() {
+		// 1. Get user max authorize level for the org (may be none, if user is not member of the org)
+		if ctx.Doer != nil {
+			var err error
+			ctx.Package.AccessMode, err = organization.OrgFromUser(ctx.Package.Owner).GetOrgUserMaxAuthorizeLevel(ctx.Doer.ID)
+			if err != nil {
+				errCb(http.StatusInternalServerError, "GetOrgUserMaxAuthorizeLevel", err)
+				return
+			}
+		}
+		// 2. If authorize level is none, check if org is visible to user
+		if ctx.Package.AccessMode == perm.AccessModeNone && organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) {
+			ctx.Package.AccessMode = perm.AccessModeRead
+		}
 	} else {
-		if ctx.Package.Owner.IsOrganization() {
-			if organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) {
+		if ctx.Doer != nil && !ctx.Doer.IsGhost() {
+			// 1. Check if user is package owner
+			if ctx.Doer.ID == ctx.Package.Owner.ID {
+				ctx.Package.AccessMode = perm.AccessModeOwner
+			} else if ctx.Package.Owner.Visibility == structs.VisibleTypePublic || ctx.Package.Owner.Visibility == structs.VisibleTypeLimited { // 2. Check if package owner is public or limited
 				ctx.Package.AccessMode = perm.AccessModeRead
-				if ctx.Doer != nil {
-					var err error
-					ctx.Package.AccessMode, err = organization.OrgFromUser(ctx.Package.Owner).GetOrgUserMaxAuthorizeLevel(ctx.Doer.ID)
-					if err != nil {
-						errCb(http.StatusInternalServerError, "GetOrgUserMaxAuthorizeLevel", err)
-						return
-					}
-				}
 			}
-		} else {
+		} else if ctx.Package.Owner.Visibility == structs.VisibleTypePublic { // 3. Check if package owner is public
 			ctx.Package.AccessMode = perm.AccessModeRead
 		}
 	}

models/userlist_test.go

@@ -191,10 +191,10 @@ func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) er
 			"action", "repository", "action.repo_id=repository.id"),
 		// find OAuth2Grant without existing user
 		genericOrphanCheck("Orphaned OAuth2Grant without existing User",
-			"oauth2_grant", "user", "oauth2_grant.user_id=user.id"),
+			"oauth2_grant", "user", "oauth2_grant.user_id=`user`.id"),
 		// find OAuth2Application without existing user
 		genericOrphanCheck("Orphaned OAuth2Application without existing User",
-			"oauth2_application", "user", "oauth2_application.uid=user.id"),
+			"oauth2_application", "user", "oauth2_application.uid=`user`.id"),
 		// find OAuth2AuthorizationCode without existing OAuth2Grant
 		genericOrphanCheck("Orphaned OAuth2AuthorizationCode without existing OAuth2Grant",
 			"oauth2_authorization_code", "oauth2_grant", "oauth2_authorization_code.grant_id=oauth2_grant.id"),

modules/context/package.go

@@ -295,7 +295,7 @@ func newRepository() {
 		log.Fatal("Failed to map Repository.PullRequest settings: %v", err)
 	}
 
-	if !Cfg.Section("packages").Key("ENABLED").MustBool(false) {
+	if !Cfg.Section("packages").Key("ENABLED").MustBool(true) {
 		Repository.DisabledRepoUnits = append(Repository.DisabledRepoUnits, "repo.packages")
 	}
 

modules/doctor/dbconsistency.go

@@ -63,7 +63,7 @@ func Members(ctx *context.Context) {
 	ctx.Data["Page"] = pager
 	ctx.Data["Members"] = members
 	ctx.Data["MembersIsPublicMember"] = membersIsPublic
-	ctx.Data["MembersIsUserOrgOwner"] = models.IsUserOrgOwner(members, org.ID)
+	ctx.Data["MembersIsUserOrgOwner"] = organization.IsUserOrgOwner(members, org.ID)
 	ctx.Data["MembersTwoFaStatus"] = members.GetTwoFaStatus()
 
 	ctx.HTML(http.StatusOK, tplMembers)

modules/setting/repository.go

@@ -623,7 +623,7 @@ func (f *CodeCommentForm) Validate(req *http.Request, errs binding.Errors) bindi
 // SubmitReviewForm for submitting a finished code review
 type SubmitReviewForm struct {
 	Content  string
-	Type     string `binding:"Required;In(approve,comment,reject)"`
+	Type     string
 	CommitID string
 	Files    []string
 }
@@ -634,7 +634,7 @@ func (f *SubmitReviewForm) Validate(req *http.Request, errs binding.Errors) bind
 	return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
 }
 
-// ReviewType will return the corresponding reviewtype for type
+// ReviewType will return the corresponding ReviewType for type
 func (f SubmitReviewForm) ReviewType() models.ReviewType {
 	switch f.Type {
 	case "approve":
@@ -643,6 +643,8 @@ func (f SubmitReviewForm) ReviewType() models.ReviewType {
 		return models.ReviewTypeComment
 	case "reject":
 		return models.ReviewTypeReject
+	case "":
+		return models.ReviewTypeComment // default to comment when doing quick-submit (Ctrl+Enter) on the review form
 	default:
 		return models.ReviewTypeUnknown
 	}