From 7361ade2e5ddc310f78308d1b18e90073cb8b05e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 29 Oct 2026 15:35:11 -0700 Subject: [PATCH] fix(user): answer the follow of a bot user with the profile card (#39980) Signed-off-by: wxiaoguang Co-authored-by: wxiaoguang --- models/user/follow.go | 4 ++++ models/user/user.go | 14 ++++++++++++- routers/api/v1/user/follower.go | 11 ++--------- routers/web/user/profile.go | 22 +++++++++------------ services/context/context_response.go | 5 +++++ tests/integration/api_user_follow_test.go | 9 +++++++++ tests/integration/issue_test.go | 2 +- tests/integration/user_test.go | 24 +++++++++++++++++++++++ 8 files changed, 67 insertions(+), 24 deletions(-) diff --git a/models/user/follow.go b/models/user/follow.go index 11d293bc5e8..ae750b4e2fb 100644 --- a/models/user/follow.go +++ b/models/user/follow.go @@ -8,6 +8,7 @@ import ( "gitea.dev/models/db" "gitea.dev/modules/timeutil" + "gitea.dev/modules/util" "xorm.io/builder" ) @@ -32,6 +33,9 @@ func IsFollowing(ctx context.Context, userID, followID int64) bool { // FollowUser marks someone be another's follower. func FollowUser(ctx context.Context, user, follow *User) (err error) { + if !follow.IsFollowable() { + return util.NewInvalidArgumentErrorf("user %d can not be followed", follow.ID) + } if user.ID == follow.ID || IsFollowing(ctx, user.ID, follow.ID) { return nil } diff --git a/models/user/user.go b/models/user/user.go index 66eff59ef42..c4e366e667c 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -441,6 +441,18 @@ func (u *User) IsIndividual() bool { return u.ID > 0 && u.Type == UserTypeIndividual } +// IsFollowable returns whether other users can follow this user (individuals, bots and organizations) +func (u *User) IsFollowable() bool { + if u.ID <= 0 { + return false + } + switch u.Type { + case UserTypeIndividual, UserTypeBot, UserTypeOrganization: + return true + } + return false +} + // IsTypeBot returns whether the user is of type bot func (u *User) IsTypeBot() bool { return u.Type == UserTypeBot @@ -1053,7 +1065,7 @@ func GetUserByIDs(ctx context.Context, ids []int64) ([]*User, error) { return users, err } -// Glossaries for XxxUser: +// HINT: USER-TYPE-GLOSSARY: Glossaries for XxxUser: // * Possible: The user may or may not exist in the database, but we can still return a valid user object (e.g. Ghost user) for the given ID. // * Individual: The user is a real person, not an organization or a bot. // * Doer: The user is the one performing an action (individual, bot or system) diff --git a/routers/api/v1/user/follower.go b/routers/api/v1/user/follower.go index e198463083b..7420b6bebcb 100644 --- a/routers/api/v1/user/follower.go +++ b/routers/api/v1/user/follower.go @@ -5,7 +5,6 @@ package user import ( - "errors" "net/http" user_model "gitea.dev/models/user" @@ -212,7 +211,6 @@ func CheckFollowing(ctx *context.APIContext) { checkUserFollowing(ctx, ctx.ContextUser, target.ID) } -// Follow follow a user func Follow(ctx *context.APIContext) { // swagger:operation PUT /user/following/{username} user userCurrentPutFollow // --- @@ -230,17 +228,12 @@ func Follow(ctx *context.APIContext) { // "$ref": "#/responses/error" if err := user_model.FollowUser(ctx, ctx.Doer, ctx.ContextUser); err != nil { - if errors.Is(err, user_model.ErrBlockedUser) { - ctx.APIError(http.StatusForbidden, err.Error()) - } else { - ctx.APIErrorInternal(err) - } + ctx.APIErrorAuto(err) return } ctx.Status(http.StatusNoContent) } -// Unfollow unfollow a user func Unfollow(ctx *context.APIContext) { // swagger:operation DELETE /user/following/{username} user userCurrentDeleteFollow // --- @@ -258,7 +251,7 @@ func Unfollow(ctx *context.APIContext) { // "$ref": "#/responses/error" if err := user_model.UnfollowUser(ctx, ctx.Doer.ID, ctx.ContextUser.ID); err != nil { - ctx.APIErrorInternal(err) + ctx.APIErrorAuto(err) return } ctx.Status(http.StatusNoContent) diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 53ce18d773f..c6e6b4063d9 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -5,7 +5,6 @@ package user import ( - "fmt" "net/http" activities_model "gitea.dev/models/activities" @@ -33,7 +32,7 @@ const ( // OwnerProfile render profile page for a user or a organization (aka, repo owner) func OwnerProfile(ctx *context.Context) { - ctx.Data["EnableFeed"] = setting.Other.EnableFeed + // HINT: USER-TYPE-GLOSSARY: in the future it needs to handle more user types correctly if ctx.ContextUser.IsOrganization() { org.Home(ctx) } else { @@ -313,25 +312,22 @@ func ActionUserFollow(ctx *context.Context) { case "unfollow": err = user_model.UnfollowUser(ctx, ctx.Doer.ID, ctx.ContextUser.ID) } - if err != nil { - log.Error("Failed to apply action %q: %v", ctx.FormString("action"), err) - ctx.HTTPError(http.StatusBadRequest, fmt.Sprintf("Action %q failed", ctx.FormString("action"))) + ctx.ServerError("ActionUserFollow", err) return } + if _, err := shared_user.RenderUserOrgHeader(ctx); err != nil { ctx.ServerError("RenderUserOrgHeader", err) return } - if ctx.ContextUser.IsIndividual() { - ctx.HTML(http.StatusOK, tplProfileBigAvatar) - return - } else if ctx.ContextUser.IsOrganization() { + + // HINT: USER-TYPE-GLOSSARY: follow the logic in OwnerProfile + if ctx.ContextUser.IsOrganization() { ctx.Data["Org"] = ctx.ContextUser - ctx.Data["IsFollowing"] = ctx.Doer != nil && user_model.IsFollowing(ctx, ctx.Doer.ID, ctx.ContextUser.ID) + ctx.Data["IsFollowing"] = user_model.IsFollowing(ctx, ctx.Doer.ID, ctx.ContextUser.ID) ctx.HTML(http.StatusOK, tplFollowUnfollow) - return + } else { + ctx.HTML(http.StatusOK, tplProfileBigAvatar) } - log.Error("Failed to apply action %q: unsupported context user type: %s", ctx.FormString("action"), ctx.ContextUser.Type) - ctx.HTTPError(http.StatusBadRequest, fmt.Sprintf("Action %q failed", ctx.FormString("action"))) } diff --git a/services/context/context_response.go b/services/context/context_response.go index 55df78d58ba..a6fd75a5314 100644 --- a/services/context/context_response.go +++ b/services/context/context_response.go @@ -166,11 +166,16 @@ func (ctx *Context) buildUserErrorMessage(msg string, err error) (userErrorMsg s // ServerError displays a 500 (Internal Server Error) page and prints the given error, if any. // If the error is controlled by our error system, a related 404 page can be displayed instead. +// TODO: refactor this function like APIErrorAuto in the future, and handle more error types and http codes. func (ctx *Context) ServerError(logMsg string, logErr error) { if errors.Is(logErr, util.ErrNotExist) { ctx.notFoundInternal(1, logMsg, logErr) return } + if errMsg, httpCode := util.ErrorUnwrapForUser(logErr); errMsg != "" { + ctx.HTTPError(httpCode, errMsg) + return + } ctx.serverErrorInternal(1, logMsg, logErr) } diff --git a/tests/integration/api_user_follow_test.go b/tests/integration/api_user_follow_test.go index 7aeec13a864..86dc4b12e6b 100644 --- a/tests/integration/api_user_follow_test.go +++ b/tests/integration/api_user_follow_test.go @@ -9,12 +9,14 @@ import ( "testing" auth_model "gitea.dev/models/auth" + "gitea.dev/models/db" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" api "gitea.dev/modules/structs" "gitea.dev/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAPIFollow(t *testing.T) { @@ -41,6 +43,13 @@ func TestAPIFollow(t *testing.T) { req = NewRequest(t, "PUT", "/api/v1/user/following/user2"). AddTokenAuth(getUserToken(t, user34.Name, auth_model.AccessTokenScopeWriteUser)) MakeRequest(t, req, http.StatusForbidden) + + // reserved users can't be followed + _, err := db.GetEngine(t.Context()).Insert(&user_model.User{Name: "reserved-user", LowerName: "reserved-user", Type: user_model.UserTypeUserReserved}) + require.NoError(t, err) + req = NewRequest(t, "PUT", "/api/v1/user/following/reserved-user"). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusBadRequest) }) for _, list := range []struct { diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index f6dd99edb52..2d685892931 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -602,7 +602,7 @@ func TestIssueReaction(t *testing.T) { req := NewRequestWithValues(t, "POST", path.Join(issueURL, "/reactions/react"), map[string]string{ "content": "8ball", }) - session.MakeRequest(t, req, http.StatusInternalServerError) + session.MakeRequest(t, req, http.StatusForbidden) req = NewRequestWithValues(t, "POST", path.Join(issueURL, "/reactions/react"), map[string]string{ "content": "eyes", }) diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 1e8a79767dd..3d48a3c9b72 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -22,11 +22,13 @@ import ( "gitea.dev/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestUser(t *testing.T) { defer tests.PrepareTestEnv(t)() t.Run("ViewUser", testViewUser) + t.Run("FollowUser", testUserFollow) t.Run("RenameInvalidUsername", testRenameInvalidUsername) t.Run("RenameReservedUsername", testRenameReservedUsername) t.Run("ViewLimitedAndPrivateUserAndRename", testViewLimitedAndPrivateUserAndRename) @@ -385,3 +387,25 @@ func testUserLocationMapLink(t *testing.T) { htmlDoc := NewHTMLParser(t, resp.Body) AssertHTMLElement(t, htmlDoc, `a[href="https://example/foo/A%2Fb"]`, true) } + +func testUserFollow(t *testing.T) { + bot := &user_model.User{ + Name: "follow-bot", + Email: "follow-bot@example.com", + Type: user_model.UserTypeBot, + } + require.NoError(t, user_model.CreateUser(t.Context(), bot, &user_model.Meta{})) + + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"}) + session := loginUser(t, doer.Name) + + req := NewRequestWithValues(t, "POST", "/"+bot.Name, map[string]string{"action": "follow"}) + resp := session.MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), `id="profile-avatar-card"`) + assert.True(t, user_model.IsFollowing(t.Context(), doer.ID, bot.ID)) + + req = NewRequestWithValues(t, "POST", "/"+bot.Name, map[string]string{"action": "unfollow"}) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), `id="profile-avatar-card"`) + assert.False(t, user_model.IsFollowing(t.Context(), doer.ID, bot.ID)) +}