Move organization's visibility change to danger zone. (#34814)

<img width="1185" alt="image"
src="https://github.com/user-attachments/assets/d48c4db2-df7c-43b3-986c-62e042190a51"
/>

<img width="829" alt="image"
src="https://github.com/user-attachments/assets/772b8a14-6837-41f2-9d78-9f0489ef1c7d"
/>

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/user/user.go

@@ -955,6 +955,16 @@ func UpdateUserCols(ctx context.Context, u *User, cols ...string) error {
 	return err
 }
 
+// UpdateUserColsNoAutoTime update user according special columns
+func UpdateUserColsNoAutoTime(ctx context.Context, u *User, cols ...string) error {
+	if err := ValidateUser(u, cols...); err != nil {
+		return err
+	}
+
+	_, err := db.GetEngine(ctx).ID(u.ID).Cols(cols...).NoAutoTime().Update(u)
+	return err
+}
+
 // GetInactiveUsers gets all inactive users
 func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, error) {
 	cond := builder.And(

modules/indexer/issues/util.go

@@ -97,10 +97,14 @@ func getIssueIndexerData(ctx context.Context, issueID int64) (*internal.IndexerD
 		return nil, false, err
 	}
 
+	if err := issue.Repo.LoadOwner(ctx); err != nil {
+		return nil, false, fmt.Errorf("issue.Repo.LoadOwner: %w", err)
+	}
+
 	return &internal.IndexerData{
 		ID:                 issue.ID,
 		RepoID:             issue.RepoID,
-		IsPublic:           !issue.Repo.IsPrivate,
+		IsPublic:           !issue.Repo.IsPrivate && issue.Repo.Owner.Visibility.IsPublic(),
 		Title:              issue.Title,
 		Content:            issue.Content,
 		Comments:           comments,

options/locale/locale_en-US.ini

@@ -120,6 +120,7 @@ error404 = The page you are trying to reach either <strong>does not exist</stron
 error503 = The server could not complete your request. Please try again later.
 go_back = Go Back
 invalid_data = Invalid data: %v
+nothing_has_been_changed = Nothing has been changed.
 
 never = Never
 unknown = Unknown
@@ -2843,6 +2844,11 @@ settings.location = Location
 settings.permission = Permissions
 settings.repoadminchangeteam = Repository admin can add and remove access for teams
 settings.visibility = Visibility
+settings.change_visibility = Change Visibility
+settings.change_visibility_notices_1 = If the organization is converted to private, the repository stars will be removed and cannot be restored.
+settings.change_visibility_notices_2 = Non-members will lose access to the organization’s repositories if visibility is changed to private.
+settings.change_visibility_success = The visibility of organization %s has been successfully changed.
+settings.visibility_desc = Change who can view the organization and its repositories.
 settings.visibility.public = Public
 settings.visibility.limited = Limited (Visible to authenticated users only)
 settings.visibility.limited_shortname = Limited

routers/web/org/setting.go

@@ -17,6 +17,7 @@ import (
 	"code.gitea.io/gitea/modules/optional"
 	repo_module "code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/templates"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
@@ -25,7 +26,6 @@ import (
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/forms"
 	org_service "code.gitea.io/gitea/services/org"
-	repo_service "code.gitea.io/gitea/services/repository"
 	user_service "code.gitea.io/gitea/services/user"
 )
 
@@ -83,38 +83,17 @@ func SettingsPost(ctx *context.Context) {
 		Description:               optional.Some(form.Description),
 		Website:                   optional.Some(form.Website),
 		Location:                  optional.Some(form.Location),
-		Visibility:                optional.Some(form.Visibility),
 		RepoAdminChangeTeamAccess: optional.Some(form.RepoAdminChangeTeamAccess),
 	}
 	if ctx.Doer.IsAdmin {
 		opts.MaxRepoCreation = optional.Some(form.MaxRepoCreation)
 	}
 
-	visibilityChanged := org.Visibility != form.Visibility
-
 	if err := user_service.UpdateUser(ctx, org.AsUser(), opts); err != nil {
 		ctx.ServerError("UpdateUser", err)
 		return
 	}
 
-	// update forks visibility
-	if visibilityChanged {
-		repos, _, err := repo_model.GetUserRepositories(ctx, repo_model.SearchRepoOptions{
-			Actor: org.AsUser(), Private: true, ListOptions: db.ListOptions{Page: 1, PageSize: org.NumRepos},
-		})
-		if err != nil {
-			ctx.ServerError("GetRepositories", err)
-			return
-		}
-		for _, repo := range repos {
-			repo.OwnerName = org.Name
-			if err := repo_service.UpdateRepository(ctx, repo, true); err != nil {
-				ctx.ServerError("UpdateRepository", err)
-				return
-			}
-		}
-	}
-
 	log.Trace("Organization setting updated: %s", org.Name)
 	ctx.Flash.Success(ctx.Tr("org.settings.update_setting_success"))
 	ctx.Redirect(ctx.Org.OrgLink + "/settings")
@@ -251,3 +230,28 @@ func SettingsRenamePost(ctx *context.Context) {
 	ctx.Flash.Success(ctx.Tr("org.settings.rename_success", oldOrgName, newOrgName))
 	ctx.JSONRedirect(setting.AppSubURL + "/org/" + url.PathEscape(newOrgName) + "/settings")
 }
+
+// SettingsChangeVisibilityPost response for change organization visibility
+func SettingsChangeVisibilityPost(ctx *context.Context) {
+	visibility, ok := structs.VisibilityModes[ctx.FormString("visibility")]
+	if !ok {
+		ctx.Flash.Error(ctx.Tr("invalid_data", visibility))
+		ctx.JSONRedirect(setting.AppSubURL + "/org/" + url.PathEscape(ctx.Org.Organization.Name) + "/settings")
+		return
+	}
+
+	if ctx.Org.Organization.Visibility == visibility {
+		ctx.Flash.Info(ctx.Tr("nothing_has_been_changed"))
+		ctx.JSONRedirect(setting.AppSubURL + "/org/" + url.PathEscape(ctx.Org.Organization.Name) + "/settings")
+		return
+	}
+
+	if err := org_service.ChangeOrganizationVisibility(ctx, ctx.Org.Organization, visibility); err != nil {
+		log.Error("ChangeOrganizationVisibility: %v", err)
+		ctx.JSONError(ctx.Tr("error.occurred"))
+		return
+	}
+
+	ctx.Flash.Success(ctx.Tr("org.settings.change_visibility_success", ctx.Org.Organization.Name))
+	ctx.JSONRedirect(setting.AppSubURL + "/org/" + url.PathEscape(ctx.Org.Organization.Name) + "/settings")
+}

routers/web/web.go

@@ -971,6 +971,7 @@ func registerWebRoutes(m *web.Router) {
 
 				m.Post("/rename", web.Bind(forms.RenameOrgForm{}), org.SettingsRenamePost)
 				m.Post("/delete", org.SettingsDeleteOrgPost)
+				m.Post("/visibility", org.SettingsChangeVisibilityPost)
 
 				m.Group("/packages", func() {
 					m.Get("", org.Packages)

services/forms/org.go

@@ -41,7 +41,6 @@ type UpdateOrgSettingForm struct {
 	Description               string `binding:"MaxSize(255)"`
 	Website                   string `binding:"ValidUrl;MaxSize(255)"`
 	Location                  string `binding:"MaxSize(50)"`
-	Visibility                structs.VisibleType
 	MaxRepoCreation           int
 	RepoAdminChangeTeamAccess bool
 }

services/org/org.go

@@ -8,13 +8,17 @@ import (
 	"fmt"
 
 	actions_model "code.gitea.io/gitea/models/actions"
+	activities_model "code.gitea.io/gitea/models/activities"
 	"code.gitea.io/gitea/models/db"
 	org_model "code.gitea.io/gitea/models/organization"
 	packages_model "code.gitea.io/gitea/models/packages"
+	access_model "code.gitea.io/gitea/models/perm/access"
 	repo_model "code.gitea.io/gitea/models/repo"
 	secret_model "code.gitea.io/gitea/models/secret"
 	user_model "code.gitea.io/gitea/models/user"
+	issue_indexer "code.gitea.io/gitea/modules/indexer/issues"
 	"code.gitea.io/gitea/modules/storage"
+	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
 	repo_service "code.gitea.io/gitea/services/repository"
 )
@@ -102,3 +106,70 @@ func DeleteOrganization(ctx context.Context, org *org_model.Organization, purge
 
 	return nil
 }
+
+func updateOrgRepoForVisibilityChanged(ctx context.Context, repo *repo_model.Repository, makePrivate bool) error {
+	// Organization repository need to recalculate access table when visibility is changed.
+	if err := access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil {
+		return fmt.Errorf("recalculateTeamAccesses: %w", err)
+	}
+
+	if makePrivate {
+		if _, err := db.GetEngine(ctx).Where("repo_id = ?", repo.ID).Cols("is_private").Update(&activities_model.Action{
+			IsPrivate: true,
+		}); err != nil {
+			return err
+		}
+
+		if err := repo_model.ClearRepoStars(ctx, repo.ID); err != nil {
+			return err
+		}
+	}
+
+	// Create/Remove git-daemon-export-ok for git-daemon...
+	if err := repo_service.CheckDaemonExportOK(ctx, repo); err != nil {
+		return err
+	}
+
+	// If visibility is changed, we need to update the issue indexer.
+	// Since the data in the issue indexer have field to indicate if the repo is public or not.
+	// FIXME: it should check organization visibility instead of repository visibility only.
+	issue_indexer.UpdateRepoIndexer(ctx, repo.ID)
+
+	forkRepos, err := repo_model.GetRepositoriesByForkID(ctx, repo.ID)
+	if err != nil {
+		return fmt.Errorf("getRepositoriesByForkID: %w", err)
+	}
+	for i := range forkRepos {
+		if err := updateOrgRepoForVisibilityChanged(ctx, forkRepos[i], makePrivate); err != nil {
+			return fmt.Errorf("updateRepoForVisibilityChanged[%s]: %w", forkRepos[i].FullName(), err)
+		}
+	}
+	return nil
+}
+
+func ChangeOrganizationVisibility(ctx context.Context, org *org_model.Organization, visibility structs.VisibleType) error {
+	if org.Visibility == visibility {
+		return nil
+	}
+
+	org.Visibility = visibility
+	// FIXME: If it's a big forks network(forks and sub forks), the database transaction will be too long to fail.
+	return db.WithTx(ctx, func(ctx context.Context) error {
+		if err := user_model.UpdateUserColsNoAutoTime(ctx, org.AsUser(), "visibility"); err != nil {
+			return err
+		}
+
+		repos, _, err := repo_model.GetUserRepositories(ctx, repo_model.SearchRepoOptions{
+			Actor: org.AsUser(), Private: true, ListOptions: db.ListOptionsAll,
+		})
+		if err != nil {
+			return err
+		}
+		for _, repo := range repos {
+			if err := updateOrgRepoForVisibilityChanged(ctx, repo, visibility == structs.VisibleTypePrivate); err != nil {
+				return fmt.Errorf("updateOrgRepoForVisibilityChanged: %w", err)
+			}
+		}
+		return nil
+	})
+}

services/repository/create.go

@@ -469,7 +469,7 @@ func cleanupRepository(repoID int64) {
 }
 
 func updateGitRepoAfterCreate(ctx context.Context, repo *repo_model.Repository) error {
-	if err := checkDaemonExportOK(ctx, repo); err != nil {
+	if err := CheckDaemonExportOK(ctx, repo); err != nil {
 		return fmt.Errorf("checkDaemonExportOK: %w", err)
 	}
 

services/repository/repository.go

@@ -142,7 +142,7 @@ func MakeRepoPublic(ctx context.Context, repo *repo_model.Repository) (err error
 		}
 
 		// Create/Remove git-daemon-export-ok for git-daemon...
-		if err := checkDaemonExportOK(ctx, repo); err != nil {
+		if err := CheckDaemonExportOK(ctx, repo); err != nil {
 			return err
 		}
 
@@ -197,7 +197,7 @@ func MakeRepoPrivate(ctx context.Context, repo *repo_model.Repository) (err erro
 		}
 
 		// Create/Remove git-daemon-export-ok for git-daemon...
-		if err := checkDaemonExportOK(ctx, repo); err != nil {
+		if err := CheckDaemonExportOK(ctx, repo); err != nil {
 			return err
 		}
 
@@ -243,8 +243,8 @@ func LinkedRepository(ctx context.Context, a *repo_model.Attachment) (*repo_mode
 	return nil, -1, nil
 }
 
-// checkDaemonExportOK creates/removes git-daemon-export-ok for git-daemon...
-func checkDaemonExportOK(ctx context.Context, repo *repo_model.Repository) error {
+// CheckDaemonExportOK creates/removes git-daemon-export-ok for git-daemon...
+func CheckDaemonExportOK(ctx context.Context, repo *repo_model.Repository) error {
 	if err := repo.LoadOwner(ctx); err != nil {
 		return err
 	}
@@ -314,7 +314,7 @@ func updateRepository(ctx context.Context, repo *repo_model.Repository, visibili
 		}
 
 		// Create/Remove git-daemon-export-ok for git-daemon...
-		if err := checkDaemonExportOK(ctx, repo); err != nil {
+		if err := CheckDaemonExportOK(ctx, repo); err != nil {
 			return err
 		}
 

templates/org/settings/options.tmpl

@@ -29,29 +29,6 @@
 				<input id="location" name="location"  value="{{.Org.Location}}" maxlength="50">
 			</div>
 
-			<div class="divider"></div>
-			<div class="field" id="visibility_box">
-				<label for="visibility">{{ctx.Locale.Tr "org.settings.visibility"}}</label>
-				<div class="field">
-					<div class="ui radio checkbox">
-						<input class="enable-system-radio" name="visibility" type="radio" value="0" {{if eq .CurrentVisibility 0}}checked{{end}}>
-						<label>{{ctx.Locale.Tr "org.settings.visibility.public"}}</label>
-					</div>
-				</div>
-				<div class="field">
-					<div class="ui radio checkbox">
-						<input class="enable-system-radio" name="visibility" type="radio" value="1" {{if eq .CurrentVisibility 1}}checked{{end}}>
-						<label>{{ctx.Locale.Tr "org.settings.visibility.limited"}}</label>
-					</div>
-				</div>
-				<div class="field">
-					<div class="ui radio checkbox">
-						<input class="enable-system-radio" name="visibility" type="radio" value="2" {{if eq .CurrentVisibility 2}}checked{{end}}>
-						<label>{{ctx.Locale.Tr "org.settings.visibility.private"}}</label>
-					</div>
-				</div>
-			</div>
-
 			<div class="field" id="permission_box">
 				<label>{{ctx.Locale.Tr "org.settings.permission"}}</label>
 				<div class="field">

templates/org/settings/options_dangerzone.tmpl

@@ -3,6 +3,16 @@
 </h4>
 <div class="ui attached error danger segment">
 	<div class="flex-list">
+		<div class="flex-item tw-items-center">
+			<div class="flex-item-main">
+				<div class="flex-item-title">{{ctx.Locale.Tr "org.settings.visibility"}}</div>
+				<div class="flex-item-body">{{ctx.Locale.Tr "org.settings.visibility_desc"}}</div>
+			</div>
+			<div class="flex-item-trailing">
+				<button class="ui basic red show-modal button" data-modal="#change-visibility-org-modal">{{ctx.Locale.Tr "org.settings.change_visibility"}}</button>
+			</div>
+		</div>
+
 		<div class="flex-item tw-items-center">
 			<div class="flex-item-main">
 				<div class="flex-item-title">{{ctx.Locale.Tr "org.settings.rename"}}</div>
@@ -25,15 +35,55 @@
 	</div>
 </div>
 
+<div class="ui small modal" id="change-visibility-org-modal">
+	<div class="header">
+		{{ctx.Locale.Tr "org.settings.change_visibility"}}
+	</div>
+	<div class="content">
+		<div class="ui warning message">
+			<ul>
+				<li>{{ctx.Locale.Tr "org.settings.change_visibility_notices_1"}}</li>
+				<li>{{ctx.Locale.Tr "org.settings.change_visibility_notices_2"}}</li>
+			</ul>
+		</div>
+		<form class="ui form form-fetch-action" action="{{.Link}}/visibility" method="post">
+			{{.CsrfTokenHtml}}
+			<input type="hidden" name="current_visibility" value="{{.CurrentVisibility}}">
+			<div class="tw-flex tw-flex-col tw-gap-3">
+				<label>{{ctx.Locale.Tr "org.settings.visibility"}}</label>
+				<div class="ui radio checkbox">
+					<input name="visibility" type="radio" value="public" {{if .CurrentVisibility.IsPublic}}checked{{end}}>
+					<label>{{ctx.Locale.Tr "org.settings.visibility.public"}}</label>
+				</div>
+				<div class="ui radio checkbox">
+					<input name="visibility" type="radio" value="limited" {{if .CurrentVisibility.IsLimited}}checked{{end}}>
+					<label>{{ctx.Locale.Tr "org.settings.visibility.limited"}}</label>
+				</div>
+				<div class="ui radio checkbox">
+					<input name="visibility" type="radio" value="private" {{if .CurrentVisibility.IsPrivate}}checked{{end}}>
+					<label>{{ctx.Locale.Tr "org.settings.visibility.private"}}</label>
+				</div>
+			</div>
+
+			<div class="actions">
+				<button class="ui cancel button">{{ctx.Locale.Tr "settings.cancel"}}</button>
+				<button class="ui red button">{{ctx.Locale.Tr "org.settings.change_visibility"}}</button>
+			</div>
+		</form>
+	</div>
+</div>
+
 <div class="ui small modal" id="rename-org-modal">
 	<div class="header">
 		{{ctx.Locale.Tr "org.settings.rename"}}
 	</div>
 	<div class="content">
-		<ul class="ui warning message">
-			<li>{{ctx.Locale.Tr "org.settings.rename_notices_1"}}</li>
-			<li>{{ctx.Locale.Tr "org.settings.rename_notices_2"}}</li>
-		</ul>
+		<div class="ui warning message">
+			<ul>
+				<li>{{ctx.Locale.Tr "org.settings.rename_notices_1"}}</li>
+				<li>{{ctx.Locale.Tr "org.settings.rename_notices_2"}}</li>
+			</ul>
+		</div>
 		<form class="ui form form-fetch-action" action="{{.Link}}/rename" method="post">
 			{{.CsrfTokenHtml}}
 			<div class="field">
@@ -65,12 +115,14 @@
 		{{ctx.Locale.Tr "org.settings.delete_account"}}
 	</div>
 	<div class="content">
-		<ul class="ui warning message">
-			<li>{{ctx.Locale.Tr "org.settings.delete_notices_1"}}</li>
-			<li>{{ctx.Locale.Tr "org.settings.delete_notices_2" .Org.Name}}</li>
-			<li>{{ctx.Locale.Tr "org.settings.delete_notices_3" .Org.Name}}</li>
-			<li>{{ctx.Locale.Tr "org.settings.delete_notices_4" .Org.Name}}</li>
-		</ul>
+		<div class="ui warning message">
+			<ul>
+				<li>{{ctx.Locale.Tr "org.settings.delete_notices_1"}}</li>
+				<li>{{ctx.Locale.Tr "org.settings.delete_notices_2" .Org.Name}}</li>
+				<li>{{ctx.Locale.Tr "org.settings.delete_notices_3" .Org.Name}}</li>
+				<li>{{ctx.Locale.Tr "org.settings.delete_notices_4" .Org.Name}}</li>
+			</ul>
+		</div>
 		<form class="ui form form-fetch-action" action="{{.Link}}/delete" method="post">
 			{{.CsrfTokenHtml}}
 			<div class="field">

web_src/css/base.css

@@ -604,10 +604,8 @@ img.ui.avatar,
 }
 
 .ui .message > ul {
-  margin-left: auto;
-  margin-right: auto;
-  display: table;
-  text-align: left;
+  margin: 0;
+  padding: 0 1em;
 }
 
 .ui .header > i + .content {