feat: "Add member" button in org members list (#11848)

Fixes #1529.

This adds an "Add member" button to the list of members of an organization, offering a more intuitive way to add a user to an organization (instead of going through the list of teams).
This follows the design proposed in #1529. This PR can already be reviewed as such, but I plan to work on follow-up improvements:
- adding a confirmation dialog when adding the new member to the "Owners" team, since they get absolute rights on the org
- adding a text input to filter the list of teams, making it easier to select the desired teams when there are many of them
- potentially, improving the team creation link so that it brings the user back to the modal dialog once the team is created (but I'm not sure there's a ton of value behind this added complexity, since currently, creating a team will lead the user to the team page, which is a good place to add the member to the team)

This new way of adding members does not support issuing email invites, since we decided in #9884 that the invite feature hasn't got good enough of a UX to advertise it yet. Following [this discussion](https://codeberg.org/forgejo/discussions/issues/441), I am planning to work on enabling invites everywhere (potentially even making it the default).

## Checklist

### Tests for Go changes

- I added test coverage for Go changes...
  - [ ] in their respective `*_test.go` for unit tests.
  - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
  - [x] `make pr-go` before pushing

### Tests for JavaScript changes

- I added test coverage for JavaScript changes...
  - [ ] in `web_src/js/*.test.js` if it can be unit tested.
  - [x] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).

### Documentation

I plan to update https://docs.codeberg.org/collaborating/create-organization/#people once we are ready to take final screenshots of the feature.

### Release notes

- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.

### Screenshots

<!--start release-notes-assistant-->

## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Features
  - [PR](https://codeberg.org/forgejo/forgejo/pulls/11848): <!--number 11848 --><!--line 0 --><!--description IkFkZCBtZW1iZXIiIGJ1dHRvbiBpbiBvcmcgbWVtYmVycyBsaXN0-->"Add member" button in org members list<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11848
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: Antonin Delpeuch <antonin@delpeuch.eu>
Co-committed-by: Antonin Delpeuch <antonin@delpeuch.eu>

options/locale_next/locale_en-US.json

@@ -563,6 +563,10 @@
 	},
 	"teams.add_all_repos.modal.header": "Add all repositories",
 	"teams.remove_all_repos.modal.header": "Remove all repositories",
+	"members.add_member": "Add member",
+	"members.user": "User",
+	"members.user_already_member": "This user is already a member of the organization.",
+	"members.no_team_selected": "Organization members must belong to at least one team.",
 	"pulls.manual_merge.helper": "Manual merge helper",
 	"pulls.manual_merge.helpder.description": "Use this merge commit message when completing the merge manually.",
 	"pulls.manual_merge.commit.title": "Merge commit title",

routers/web/org/members.go

@@ -5,10 +5,12 @@
 package org
 
 import (
+	"fmt"
 	"net/http"
 
 	"forgejo.org/models"
 	"forgejo.org/models/organization"
+	user_model "forgejo.org/models/user"
 	"forgejo.org/modules/base"
 	"forgejo.org/modules/log"
 	"forgejo.org/modules/setting"
@@ -67,8 +69,14 @@ func Members(ctx *context.Context) {
 		ctx.ServerError("GetMembers", err)
 		return
 	}
+	teams, err := organization.FindOrgTeams(ctx, org.ID)
+	if err != nil {
+		ctx.ServerError("GetTeams", err)
+		return
+	}
 	ctx.Data["Page"] = pager
 	ctx.Data["Members"] = members
+	ctx.Data["Teams"] = teams
 	ctx.Data["MembersIsPublicMember"] = membersIsPublic
 	ctx.Data["MembersIsUserOrgOwner"] = organization.IsUserOrgOwner(ctx, members, org.ID)
 	ctx.Data["MembersTwoFaStatus"] = members.GetTwoFaStatus(ctx)
@@ -110,6 +118,59 @@ func MembersAction(ctx *context.Context) {
 			ctx.JSONRedirect(ctx.Org.OrgLink + "/members")
 			return
 		}
+	case "add":
+		if !ctx.Org.IsOwner {
+			ctx.Error(http.StatusNotFound)
+			return
+		}
+		uname := ctx.FormString("uname")
+		var u *user_model.User
+		u, err = user_model.GetUserByName(ctx, uname)
+		if err != nil {
+			ctx.ServerError("GetUserByName", err)
+			return
+		}
+
+		if u.IsOrganization() {
+			ctx.Flash.Error(ctx.Tr("form.cannot_add_org_to_team"))
+			ctx.Redirect(ctx.Org.OrgLink + "/members")
+			return
+		}
+
+		alreadyMember, err := ctx.Org.Organization.IsOrgMember(ctx, u.ID)
+		if err != nil {
+			ctx.ServerError("IsOrgMember", err)
+			return
+		}
+		if alreadyMember {
+			ctx.Flash.Error(ctx.Tr("members.user_already_member"))
+			ctx.Redirect(ctx.Org.OrgLink + "/members")
+			return
+		}
+
+		teams, err := organization.FindOrgTeams(ctx, org.ID)
+		if err != nil {
+			ctx.ServerError("GetTeams", err)
+			return
+		}
+		addedToTeam := false
+		for _, team := range teams {
+			addToTeam := ctx.FormBool(fmt.Sprintf("team_%d", team.ID))
+			if addToTeam {
+				err = models.AddTeamMember(ctx, team, u.ID)
+				if err != nil {
+					ctx.ServerError("AddTeamMember", err)
+					return
+				}
+				addedToTeam = true
+			}
+		}
+
+		if !addedToTeam {
+			ctx.Flash.Error(ctx.Tr("members.no_team_selected"))
+		}
+		ctx.Redirect(ctx.Org.OrgLink + "/members")
+		return
 	case "leave":
 		err = models.RemoveOrgUser(ctx, org.ID, ctx.Doer.ID)
 		if err == nil {

templates/org/member/add_org_member.tmpl

@@ -0,0 +1,55 @@
+<div class="ui g-modal-confirm delete modal" id="delete-label">
+	<div class="header">
+		{{svg "octicon-trash"}}
+		{{ctx.Locale.Tr "repo.issues.label_deletion"}}
+	</div>
+	<div class="content">
+		<p>{{ctx.Locale.Tr "repo.issues.label_deletion_desc"}}</p>
+	</div>
+	{{template "base/modal_actions_confirm" .}}
+</div>
+
+<dialog id="add-member-modal">
+	<article>
+		<header>{{ctx.Locale.Tr "members.add_member"}}</header>
+		<form class="ui add-member form ignore-dirty" action="{{$.OrgLink}}/members/action/add" method="post">
+			<input type="hidden" name="uid" value="{{.SignedUser.ID}}">
+			<div class="content">
+				<div class="field">
+					<label>{{ctx.Locale.Tr "members.user"}}</label>
+					<div id="search-user-box" class="ui search tw-mr-2"{{if .IsEmailInviteEnabled}} data-allow-email="true" data-allow-email-description="{{ctx.Locale.Tr "org.teams.invite_team_member" $.Team.Name}}"{{end}}>
+						<div class="ui input">
+							<input class="prompt" name="uname" placeholder="{{ctx.Locale.Tr "search.user_kind"}}" autocomplete="off" required>
+						</div>
+					</div>
+				</div>
+
+				<div class="field">
+					<label>{{ctx.Locale.Tr "org.teams"}}</label>
+					<div class="ui negative message flash-message flash-error" id="no-team-selected-message" hidden>{{ctx.Locale.Tr "members.no_team_selected"}}</div>
+					<div class="team-list">
+						{{range .Teams}}
+						<div class="inline field">
+							<div class="ui checkbox">
+								<input name="team_{{.ID}}" id="add-member-team_{{.ID}}" type="checkbox">
+								<label for="add-member-team_{{.ID}}">{{.Name}}</label>
+							</div>
+						</div>
+						{{end}}
+						<div class="inline">
+							{{svg "octicon-plus"}} <a href="{{$.OrgLink}}/teams/new">{{ctx.Locale.Tr "org.create_new_team"}}</a>
+						</div>
+					</div>
+				</div>
+
+			</div>
+		</form>
+		<div class="actions">
+			<button class="secondary cancel button">
+				{{svg "octicon-x"}}
+				{{ctx.Locale.Tr "cancel"}}
+			</button>
+			<button class="primary ok button" type="submit">{{ctx.Locale.Tr "members.add_member"}}</button>
+		</div>
+	</article>
+</dialog>

templates/org/member/members.tmpl

@@ -4,6 +4,14 @@
 	<div class="ui container">
 		{{template "base/alert" .}}
 
+		{{if $.IsOrganizationOwner}}
+			<div class="text right">
+				<button class="primary button" id="add-org-member-button">
+				{{svg "octicon-plus"}} {{ctx.Locale.Tr "members.add_member"}}
+				</button>
+			</div>
+			<div class="divider"></div>
+		{{end}}
 		<div class="list">
 			{{range $idx, $_ := .Members}}
 				{{if ne $idx 0}}
@@ -87,5 +95,6 @@
 	</div>
 	{{template "base/modal_actions_confirm" .}}
 </div>
+{{template "org/member/add_org_member" .}}
 
 {{template "base/footer" .}}

tests/e2e/org-members.test.e2e.ts

@@ -49,3 +49,29 @@ test('Leave org', async ({page}) => {
   // Getting error is enough to know that the correct request went though
   await expect(page.locator('.flash-error').getByText('You cannot remove the last user from the "owners" team.')).toBeVisible();
 });
+
+test('Add a new member to the org', async ({page}) => {
+  page.goto('/org/org3/members');
+
+  // Click the "Add member" button
+  const newMemberButton = page.locator('#add-org-member-button');
+  await newMemberButton.click();
+
+  // A modal dialog appears
+  await expect(page.locator('#add-member-modal')).toBeVisible();
+
+  // Fill in the name of the user to add
+  await page.locator('#search-user-box input').fill('user5');
+  // Pick the auto-complete suggestion
+  await page.locator('#search-user-box .results a.result').click();
+
+  // Choose some teams
+  await page.locator('#add-member-team_2').click();
+  await page.locator('#add-member-team_12').click();
+
+  // Click the button
+  await page.locator('#add-member-modal .actions button.ok').click();
+
+  // Getting error is enough to know that the correct request went though
+  await expect(page.locator('.organization.members .list a').getByText('user5 (User Five)')).toBeVisible();
+});

tests/integration/org_members_test.go

@@ -4,12 +4,19 @@
 package integration
 
 import (
+	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 
+	"forgejo.org/models/db"
+	"forgejo.org/models/organization"
+	"forgejo.org/models/unittest"
+	user_model "forgejo.org/models/user"
 	"forgejo.org/tests"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestOrgMembersPage(t *testing.T) {
@@ -24,6 +31,8 @@ func TestOrgMembersPage(t *testing.T) {
 		/* No interactive buttons - though such evaluation is easy to break in rename */
 		assert.Equal(t, 0, doc.Find(".members .list .link-action").Length())
 		assert.Equal(t, 0, doc.Find(".members .list .delete-button").Length())
+		/* Guests cannot add members to organizations */
+		doc.AssertElement(t, "#add-org-member-button", false)
 	})
 
 	t.Run("Member PoV", func(t *testing.T) {
@@ -34,6 +43,8 @@ func TestOrgMembersPage(t *testing.T) {
 		/* Interactive buttons are only available for own entry in the list */
 		assert.Equal(t, 1, doc.Find(".members .list .link-action").Length())
 		assert.Equal(t, 1, doc.Find(".members .list .delete-button").Length())
+		/* Adding new members is not possible, as the member isn't an owner */
+		doc.AssertElement(t, "#add-org-member-button", false)
 	})
 
 	t.Run("Owner PoV", func(t *testing.T) {
@@ -44,5 +55,169 @@ func TestOrgMembersPage(t *testing.T) {
 		/* Interactive buttons are available for all entries in the list (> 2) */
 		assert.Less(t, 2, doc.Find(".members .list .link-action").Length())
 		assert.Less(t, 2, doc.Find(".members .list .delete-button").Length())
+		/* Adding new members is possible */
+		doc.AssertElement(t, "#add-org-member-button", true)
 	})
 }
+
+func TestOrgAddMember(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
+	team1 := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 1})
+	team2 := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2})
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
+
+	isMember, err := organization.IsTeamMember(db.DefaultContext, team1.OrgID, team1.ID, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isMember)
+	isMember, err = organization.IsTeamMember(db.DefaultContext, team2.OrgID, team2.ID, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isMember)
+
+	session := loginUser(t, "user2")
+
+	teamURL := fmt.Sprintf("/org/%s/members", org.Name)
+	req := NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{
+		"uid":    "2",
+		"uname":  user.LoginName,
+		"team_1": "on",
+		"team_2": "on",
+	})
+	resp := session.MakeRequest(t, req, http.StatusSeeOther)
+	assert.Equal(t, teamURL, resp.Header().Get("Location"))
+
+	user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
+	isMember, err = organization.IsTeamMember(db.DefaultContext, team1.OrgID, team1.ID, user.ID)
+	require.NoError(t, err)
+	assert.True(t, isMember)
+	isMember, err = organization.IsTeamMember(db.DefaultContext, team2.OrgID, team2.ID, user.ID)
+	require.NoError(t, err)
+	assert.True(t, isMember)
+}
+
+func TestOrgAddMemberToNoTeam(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
+
+	isOrgMember, err := org.IsOrgMember(db.DefaultContext, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isOrgMember)
+
+	session := loginUser(t, "user2")
+
+	teamURL := fmt.Sprintf("/org/%s/members", org.Name)
+	resp := session.MakeRequest(t, NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{
+		"uid":   "2",
+		"uname": user.LoginName,
+	}), http.StatusSeeOther)
+	assert.Equal(t, teamURL, resp.Header().Get("Location"))
+
+	doc := NewHTMLParser(t, session.MakeRequest(t, NewRequest(t, "GET", teamURL), http.StatusOK).Body)
+	assert.Contains(t, strings.TrimSpace(doc.Find(".flash-error").Text()), "Organization members must belong to at least one team.")
+
+	isOrgMember, err = org.IsOrgMember(db.DefaultContext, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isOrgMember)
+}
+
+func TestOrgAddMemberToForeignTeam(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
+	foreignTeam := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 5})
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
+
+	isOrgMember, err := org.IsOrgMember(db.DefaultContext, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isOrgMember)
+
+	isForeignTeamMember, err := organization.IsTeamMember(db.DefaultContext, foreignTeam.OrgID, foreignTeam.ID, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isForeignTeamMember)
+
+	session := loginUser(t, "user2")
+
+	teamURL := fmt.Sprintf("/org/%s/members", org.Name)
+	// This test scenario is here for security purposes as the UI will not offer adding the user to
+	// a foreign team in the first place (but a malicious request could be performed in other ways).
+	resp := session.MakeRequest(t, NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{
+		"uid":    "2",
+		"uname":  user.LoginName,
+		"team_5": "on",
+	}), http.StatusSeeOther)
+	assert.Equal(t, teamURL, resp.Header().Get("Location"))
+
+	doc := NewHTMLParser(t, session.MakeRequest(t, NewRequest(t, "GET", teamURL), http.StatusOK).Body)
+	// This error message isn't specific to this "exploit" attempt, but shows that no action was taken.
+	assert.Contains(t, strings.TrimSpace(doc.Find(".flash-error").Text()), "Organization members must belong to at least one team.")
+
+	isOrgMember, err = org.IsOrgMember(db.DefaultContext, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isOrgMember)
+
+	isForeignTeamMember, err = organization.IsTeamMember(db.DefaultContext, foreignTeam.OrgID, foreignTeam.ID, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isForeignTeamMember)
+}
+
+func TestOrgAddMemberWithoutProperRights(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
+
+	isOrgMember, err := org.IsOrgMember(db.DefaultContext, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isOrgMember)
+
+	session := loginUser(t, user.Name) // user5 is not a member of this org, so it cannot add anyone to it
+
+	teamURL := fmt.Sprintf("/org/%s/members", org.Name)
+	req := NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{
+		"uid":    "5",
+		"uname":  user.LoginName,
+		"team_2": "on",
+	})
+	session.MakeRequest(t, req, http.StatusNotFound)
+}
+
+func TestOrgAddExistingMemberFails(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
+	team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2})
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28})
+
+	members, _, err := organization.FindOrgMembers(db.DefaultContext, &organization.FindOrgMembersOpts{
+		OrgID: org.ID,
+	})
+	require.NoError(t, err)
+	assert.Len(t, members, 2)
+	isOrgMember, err := org.IsOrgMember(db.DefaultContext, user.ID)
+	require.NoError(t, err)
+	assert.True(t, isOrgMember)
+	isTeamMember, err := organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isTeamMember)
+
+	session := loginUser(t, "user2")
+
+	teamURL := fmt.Sprintf("/org/%s/members", org.Name)
+	req := NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{
+		"uid":    "2",
+		"uname":  user.LoginName,
+		"team_2": "on",
+	})
+	resp := session.MakeRequest(t, req, http.StatusSeeOther)
+	assert.Equal(t, teamURL, resp.Header().Get("Location"))
+	doc := NewHTMLParser(t, session.MakeRequest(t, NewRequest(t, "GET", teamURL), http.StatusOK).Body)
+	assert.Contains(t, strings.TrimSpace(doc.Find(".flash-error").Text()), "This user is already a member of the organization.")
+
+	user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28})
+	isTeamMember, err = organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, user.ID)
+	require.NoError(t, err)
+	assert.False(t, isTeamMember)
+}

web_src/css/base.css

@@ -361,6 +361,11 @@ a.label,
   background: var(--color-hover);
 }
 
+.ui.search > .results .content {
+  padding: 0;
+  background: none;
+}
+
 .inline-code-block {
   padding: 2px 4px;
   border-radius: .24em;

web_src/css/org.css

@@ -202,3 +202,8 @@
 .org-team-navbar .active.item {
   background: var(--color-box-body) !important;
 }
+
+#add-member-modal .team-list {
+  max-height: 300px;
+  overflow-y: auto;
+}

web_src/js/features/add-org-member.ts

@@ -0,0 +1,24 @@
+// Copyright 2026 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+import {showModal} from '../modules/modal.ts';
+
+export function initAddOrgMemberButton() {
+  const addMemberButton = document.querySelector('#add-org-member-button');
+  addMemberButton?.addEventListener('click', () => {
+    showModal('add-member-modal', () => {
+      const form: HTMLFormElement = document.querySelector('.add-member.form');
+      if (!form.checkValidity()) {
+        form.reportValidity();
+        return false;
+      }
+      const selectedTeamCount = document.querySelectorAll('.add-member.form input[type=checkbox]:checked').length;
+      const noTeamSelectedMessage : HTMLElement = document.querySelector('#no-team-selected-message');
+      noTeamSelectedMessage.hidden = (selectedTeamCount !== 0);
+      if (selectedTeamCount === 0) {
+        return false;
+      }
+      form.requestSubmit();
+    });
+    return false;
+  });
+}

web_src/js/index.js

@@ -64,6 +64,7 @@ import {initOrgTeamSearchRepoBox} from './features/org-team.js';
 import {initUserAuthWebAuthn, initUserAuthWebAuthnRegister} from './features/user-auth-webauthn.js';
 import {initRepoRelease, initRepoReleaseNew} from './features/repo-release.js';
 import {initRepoEditor} from './features/repo-editor.js';
+import {initAddOrgMemberButton} from './features/add-org-member.ts';
 import {initCompSearchUserBox} from './features/comp/SearchUserBox.js';
 import {initInstall} from './features/install.js';
 import {initCompWebHookEditor} from './features/comp/WebHookEditor.js';
@@ -152,6 +153,7 @@ onDomReady(() => {
   initRepoEllipsisButton();
   initRepoDiffCommitBranchesAndTags();
   initRepoEditor();
+  initAddOrgMemberButton();
   initRepoGraphGit();
   initRepoIssueContentHistory();
   initRepoIssueDue();