Refactor "org teams" page and help new users to "add member" to an org (#37051)

* Fix #22054
* Replace #34593, #27800
* And refactor legacy code, fix various problems

---------

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>

options/locale/locale_en-US.json

@@ -2778,9 +2778,9 @@
   "org.settings.labels_desc": "Add labels which can be used on issues for <strong>all repositories</strong> under this organization.",
   "org.members.membership_visibility": "Membership Visibility:",
   "org.members.public": "Visible",
-  "org.members.public_helper": "make hidden",
+  "org.members.public_helper": "Make hidden",
   "org.members.private": "Hidden",
-  "org.members.private_helper": "make visible",
+  "org.members.private_helper": "Make visible",
   "org.members.member_role": "Member Role:",
   "org.members.owner": "Owner",
   "org.members.member": "Member",
@@ -2808,7 +2808,10 @@
   "org.teams.no_desc": "This team has no description",
   "org.teams.settings": "Settings",
   "org.teams.owners_permission_desc": "Owners have full access to <strong>all repositories</strong> and have <strong>administrator access</strong> to the organization.",
+  "org.teams.owners_permission_suggestion": "You can create new teams for members to get fine-grained access control.",
   "org.teams.members": "Team Members",
+  "org.teams.manage_team_member": "Manage teams and members",
+  "org.teams.manage_team_member_prompt": "Members are managed through teams. Add users to a team to invite them to this organization.",
   "org.teams.update_settings": "Update Settings",
   "org.teams.delete_team": "Delete Team",
   "org.teams.add_team_member": "Add Team Member",

routers/web/org/members.go

@@ -5,6 +5,7 @@
 package org
 
 import (
+	"errors"
 	"net/http"
 
 	"code.gitea.io/gitea/models/organization"
@@ -12,6 +13,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/templates"
+	"code.gitea.io/gitea/modules/util"
 	shared_user "code.gitea.io/gitea/routers/web/shared/user"
 	"code.gitea.io/gitea/services/context"
 	org_service "code.gitea.io/gitea/services/org"
@@ -76,11 +78,11 @@ func Members(ctx *context.Context) {
 // MembersAction response for operation to a member of organization
 func MembersAction(ctx *context.Context) {
 	member, err := user_model.GetUserByID(ctx, ctx.FormInt64("uid"))
-	if err != nil {
-		log.Error("GetUserByID: %v", err)
-	}
-	if member == nil {
-		ctx.Redirect(ctx.Org.OrgLink + "/members")
+	if errors.Is(err, util.ErrNotExist) {
+		ctx.HTTPError(http.StatusNotFound)
+		return
+	} else if err != nil {
+		ctx.ServerError("GetUserByID", err)
 		return
 	}
 
@@ -105,40 +107,25 @@ func MembersAction(ctx *context.Context) {
 			return
 		}
 		err = org_service.RemoveOrgUser(ctx, org, member)
-		if organization.IsErrLastOrgOwner(err) {
-			ctx.Flash.Error(ctx.Tr("form.last_org_owner"))
-			ctx.JSONRedirect(ctx.Org.OrgLink + "/members")
-			return
-		}
 	case "leave":
 		err = org_service.RemoveOrgUser(ctx, org, ctx.Doer)
 		if err == nil {
 			ctx.Flash.Success(ctx.Tr("form.organization_leave_success", org.DisplayName()))
-			ctx.JSON(http.StatusOK, map[string]any{
-				"redirect": "", // keep the user stay on current page, in case they want to do other operations.
-			})
-		} else if organization.IsErrLastOrgOwner(err) {
-			ctx.Flash.Error(ctx.Tr("form.last_org_owner"))
-			ctx.JSONRedirect(ctx.Org.OrgLink + "/members")
-		} else {
-			log.Error("RemoveOrgUser(%d,%d): %v", org.ID, ctx.Doer.ID, err)
+			ctx.JSONRedirect(setting.AppSubURL + "/")
+			return
 		}
+	}
+
+	if err == nil {
+		ctx.JSONOK()
 		return
 	}
 
-	if err != nil {
-		log.Error("Action(%s): %v", ctx.PathParam("action"), err)
-		ctx.JSON(http.StatusOK, map[string]any{
-			"ok":  false,
-			"err": err.Error(),
-		})
+	if organization.IsErrLastOrgOwner(err) {
+		ctx.JSONError(ctx.Tr("form.last_org_owner"))
 		return
 	}
 
-	redirect := ctx.Org.OrgLink + "/members"
-	if ctx.PathParam("action") == "leave" {
-		redirect = setting.AppSubURL + "/"
-	}
-
-	ctx.JSONRedirect(redirect)
+	log.Error("Action(%s): %v", ctx.PathParam("action"), err)
+	ctx.JSONError(err.Error()) // FIXME: legacy logic, errors are handled together, it's not right, need to distinguish between different errors
 }

services/org/team.go

@@ -5,6 +5,7 @@ package org
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -306,19 +307,19 @@ func removeTeamMember(ctx context.Context, team *organization.Team, user *user_m
 		return err
 	}
 
-	// Delete access to team repositories.
+	// Delete access to team repositories. If any user or repo is missing, we can continue.
 	for _, repo := range repos {
-		if err := access_model.RecalculateUserAccess(ctx, repo, user.ID); err != nil {
+		if err := access_model.RecalculateUserAccess(ctx, repo, user.ID); err != nil && !errors.Is(err, util.ErrNotExist) {
 			return err
 		}
 
 		// Remove watches from now inaccessible
-		if err := repo_service.ReconsiderWatches(ctx, repo, user); err != nil {
+		if err := repo_service.ReconsiderWatches(ctx, repo, user); err != nil && !errors.Is(err, util.ErrNotExist) {
 			return err
 		}
 
 		// Remove issue assignments from now inaccessible
-		if err := repo_service.ReconsiderRepoIssuesAssignee(ctx, repo, user); err != nil {
+		if err := repo_service.ReconsiderRepoIssuesAssignee(ctx, repo, user); err != nil && !errors.Is(err, util.ErrNotExist) {
 			return err
 		}
 	}

templates/org/member/members.tmpl

@@ -4,6 +4,13 @@
 	<div class="ui container">
 		{{template "base/alert" .}}
 
+		{{if .IsOrganizationOwner}}
+			<div class="flex-text-block">
+				<div class="tw-flex-1">{{ctx.Locale.Tr "org.teams.manage_team_member_prompt"}}</div>
+				<a class="ui primary button" href="./teams">{{ctx.Locale.Tr "org.teams.manage_team_member"}}</a>
+			</div>
+			<div class="divider"></div>
+		{{end}}
 		<div class="flex-list">
 			{{range .Members}}
 				{{$isPublic := index $.MembersIsPublicMember .ID}}
@@ -15,27 +22,27 @@
 						<div class="flex-item-title">
 							{{template "shared/user/name" .}}
 							{{if not $isPublic}}
-								<span class="ui basic tiny label">{{ctx.Locale.Tr "org.members.private"}}</span>
+								<span class="ui basic small label">{{ctx.Locale.Tr "org.members.private"}}</span>
 							{{end}}
 						</div>
-						{{if not $.PublicOnly}}
-							<div class="flex-item-body">
+						<div class="tw-flex tw-flex-col tw-gap-1">
+							{{if not $.PublicOnly}}
+							<div>
 								{{ctx.Locale.Tr "org.members.member_role"}}
 								<strong class="flex-text-inline">{{if index $.MembersIsUserOrgOwner .ID}}{{svg "octicon-shield-lock"}} {{ctx.Locale.Tr "org.members.owner"}}{{else}}{{ctx.Locale.Tr "org.members.member"}}{{end}}</strong>
 							</div>
+							{{end}}
 							{{if $.IsOrganizationOwner}}
-							<div class="flex-item-body">
-								{{ctx.Locale.Tr "admin.users.2fa"}}
-								<strong>
-									{{if index $.MembersTwoFaStatus .ID}}
-										<span class="tw-text-green">{{svg "octicon-check"}}</span>
-									{{else}}
-										{{svg "octicon-x"}}
-									{{end}}
-								</strong>
+							<div>
+								{{ctx.Locale.Tr "admin.users.2fa"}}:
+								{{if index $.MembersTwoFaStatus .ID}}
+									<span class="tw-text-green tw-flex">{{svg "octicon-check"}}</span>
+								{{else}}
+									{{svg "octicon-x"}}
+								{{end}}
 							</div>
 							{{end}}
-						{{end}}
+						</div>
 					</div>
 					<div class="flex-item-trailing">
 						{{if or (eq $.SignedUser.ID .ID) $.IsOrganizationOwner}}
@@ -46,45 +53,23 @@
 							{{end}}
 						{{end}}
 						{{if eq $.SignedUser.ID .ID}}
-							<form>
-								<button class="ui red tiny button delete-button" data-modal-id="leave-organization"
-									data-url="{{$.OrgLink}}/members/action/leave" data-datauid="{{.ID}}"
-									data-name="{{.DisplayName}}"
-									data-data-organization-name="{{$.Org.DisplayName}}">{{ctx.Locale.Tr "org.members.leave"}}</button>
-							</form>
+							<button class="ui red tiny button link-action"
+								data-url="{{$.OrgLink}}/members/action/leave?uid={{.ID}}"
+								data-modal-confirm-header="{{ctx.Locale.Tr "org.members.leave"}}"
+								data-modal-confirm-content="{{ctx.Locale.Tr "org.members.leave.detail" $.Org.DisplayName}}"
+								>{{ctx.Locale.Tr "org.members.leave"}}</button>
 						{{else if $.IsOrganizationOwner}}
-							<form>
-								<button class="ui red tiny button delete-button" data-modal-id="remove-organization-member"
-									data-url="{{$.OrgLink}}/members/action/remove" data-datauid="{{.ID}}"
-									data-name="{{.DisplayName}}"
-									data-data-organization-name="{{$.Org.DisplayName}}">{{ctx.Locale.Tr "org.members.remove"}}</button>
-							</form>
+							<button class="ui red tiny button link-action"
+								data-url="{{$.OrgLink}}/members/action/remove?uid={{.ID}}"
+								data-modal-confirm-header="{{ctx.Locale.Tr "org.members.remove"}}"
+								data-modal-confirm-content="{{ctx.Locale.Tr "org.members.remove.detail" .DisplayName $.Org.DisplayName}}"
+							>{{ctx.Locale.Tr "org.members.remove"}}</button>
 						{{end}}
 					</div>
 				</div>
 			{{end}}
 		</div>
-
 		{{template "base/paginate" .}}
 	</div>
 </div>
-<div class="ui g-modal-confirm delete modal" id="leave-organization">
-	<div class="header">
-		{{ctx.Locale.Tr "org.members.leave"}}
-	</div>
-	<div class="content">
-		<p>{{ctx.Locale.Tr "org.members.leave.detail" (HTMLFormat `<span class="%s"></span>` "dataOrganizationName")}}</p>
-	</div>
-	{{template "base/modal_actions_confirm" .}}
-</div>
-<div class="ui g-modal-confirm delete modal" id="remove-organization-member">
-	<div class="header">
-		{{ctx.Locale.Tr "org.members.remove"}}
-	</div>
-	<div class="content">
-		<p>{{ctx.Locale.Tr "org.members.remove.detail" (HTMLFormat `<span class="%s"></span>` "name") (HTMLFormat `<span class="%s"></span>` "dataOrganizationName")}}</p>
-	</div>
-	{{template "base/modal_actions_confirm" .}}
-</div>
-
 {{template "base/footer" .}}

templates/org/team/repositories.tmpl

@@ -7,9 +7,11 @@
 			{{template "org/team/sidebar" .}}
 			<div class="ui ten wide column">
 				{{template "org/team/navbar" .}}
+				{{$hasTopAttachedSegment := false}}
 				{{$canAddRemove := and $.IsOrganizationOwner (not $.Team.IncludesAllRepositories)}}
 				{{if $canAddRemove}}
-					<div class="ui top attached segment tw-flex tw-flex-wrap tw-gap-2">
+					{{$hasTopAttachedSegment = true}}
+					<div class="ui top attached segment flex-text-block tw-flex-wrap">
 						<form class="ui form ignore-dirty tw-flex-1 tw-flex" action="{{$.OrgLink}}/teams/{{$.Team.LowerName | PathEscape}}/action/repo/add" method="post">
 							<div data-global-init="initSearchRepoBox" data-uid="{{.Org.ID}}" class="ui search">
 								<div class="ui input">
@@ -24,7 +26,11 @@
 						</div>
 					</div>
 				{{end}}
-				<div class="ui{{if not $canAddRemove}} top{{end}} attached segment">
+				{{if $.Team.IncludesAllRepositories}}
+					{{$hasTopAttachedSegment = true}}
+					<div class="ui top attached segment">{{ctx.Locale.Tr "org.teams.all_repositories"}}</div>
+				{{end}}
+				<div class="ui {{if not $hasTopAttachedSegment}}top{{end}} attached segment">
 					<div class="flex-list">
 						{{range $.TeamRepos}}
 							<div class="flex-item tw-items-center">

templates/org/team/sidebar.tmpl

@@ -26,7 +26,8 @@
 		</div>
 		{{if eq .Team.LowerName "owners"}}
 			<div class="item">
-				{{ctx.Locale.Tr "org.teams.owners_permission_desc"}}
+				<p>{{ctx.Locale.Tr "org.teams.owners_permission_desc"}}</p>
+				<p>{{ctx.Locale.Tr "org.teams.owners_permission_suggestion"}}</p>
 			</div>
 		{{else}}
 			<div class="item">

templates/org/team/teams.tmpl

@@ -4,7 +4,8 @@
 	<div class="ui container">
 		{{template "base/alert" .}}
 		{{if .IsOrganizationOwner}}
-			<div class="flex-text-block tw-justify-end">
+			<div class="flex-text-block">
+				<div class="tw-flex-1">{{ctx.Locale.Tr "org.teams.manage_team_member_prompt"}}</div>
 				<a class="ui primary button" href="{{.OrgLink}}/teams/new">{{svg "octicon-plus"}} {{ctx.Locale.Tr "org.create_new_team"}}</a>
 			</div>
 			<div class="divider"></div>

web_src/js/features/comp/ConfirmModal.ts

@@ -2,6 +2,7 @@ import {svg} from '../../svg.ts';
 import {html, htmlRaw} from '../../utils/html.ts';
 import {createElementFromHTML} from '../../utils/dom.ts';
 import {fomanticQuery} from '../../modules/fomantic/base.ts';
+import {hideToastsAll} from '../../modules/toast.ts';
 
 const {i18n} = window.config;
 
@@ -27,6 +28,9 @@ export function createConfirmModal({header = '', content = '', confirmButtonColo
 
 export function confirmModal(modal: HTMLElement | ConfirmModalOptions): Promise<boolean> {
   if (!(modal instanceof HTMLElement)) modal = createConfirmModal(modal);
+  // hide existing toasts when we need to show a new modal, otherwise the toasts only interfere the UI
+  // it's fine to do so because the modal is triggered by user's explicit action, so the user should already have read the toast messages
+  hideToastsAll();
   return new Promise((resolve) => {
     const $modal = fomanticQuery(modal);
     $modal.modal({