loading up the forgejo repo on tangled to test page performance
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

fix: Allow Organisations to remove the Email Address (#5517)

It is possible to set a Email for a Organization. This Email is optional and only used to be displayed on the profile page. However, once you set an EMail, you can no longer remove it. This PR fixes that.

While working on the tests, I found out, that the API returns a 500 when trying to set an invalid EMail. I fixed that too. It returns a 422 now.

Fixes #4567

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5517
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Otto <otto@codeberg.org>
Co-authored-by: JakobDev <jakobdev@gmx.de>
Co-committed-by: JakobDev <jakobdev@gmx.de>

authored by

JakobDev
JakobDev
and committed by
Otto
45fa9e5a 1316f4d3

+228 -10
+32
models/user/email_address.go
··· 139 139 return ea, nil 140 140 } 141 141 142 + // Deletes the primary email address of the user 143 + // This is only allowed if the user is a organization 144 + func DeletePrimaryEmailAddressOfUser(ctx context.Context, uid int64) error { 145 + user, err := GetUserByID(ctx, uid) 146 + if err != nil { 147 + return err 148 + } 149 + 150 + if user.Type != UserTypeOrganization { 151 + return fmt.Errorf("%s is not a organization", user.Name) 152 + } 153 + 154 + ctx, committer, err := db.TxContext(ctx) 155 + if err != nil { 156 + return err 157 + } 158 + defer committer.Close() 159 + 160 + _, err = db.GetEngine(ctx).Exec("DELETE FROM email_address WHERE uid = ? AND is_primary = true", uid) 161 + if err != nil { 162 + return err 163 + } 164 + 165 + user.Email = "" 166 + err = UpdateUserCols(ctx, user, "email") 167 + if err != nil { 168 + return err 169 + } 170 + 171 + return committer.Commit() 172 + } 173 + 142 174 // GetEmailAddresses returns all email addresses belongs to given user. 143 175 func GetEmailAddresses(ctx context.Context, uid int64) ([]*EmailAddress, error) { 144 176 emails := make([]*EmailAddress, 0, 5)
+18
models/user/email_address_test.go
··· 163 163 }) 164 164 } 165 165 } 166 + 167 + func TestDeletePrimaryEmailAddressOfUser(t *testing.T) { 168 + require.NoError(t, unittest.PrepareTestDatabase()) 169 + 170 + user, err := user_model.GetUserByName(db.DefaultContext, "org3") 171 + require.NoError(t, err) 172 + assert.Equal(t, "org3@example.com", user.Email) 173 + 174 + require.NoError(t, user_model.DeletePrimaryEmailAddressOfUser(db.DefaultContext, user.ID)) 175 + 176 + user, err = user_model.GetUserByName(db.DefaultContext, "org3") 177 + require.NoError(t, err) 178 + assert.Empty(t, user.Email) 179 + 180 + email, err := user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) 181 + assert.True(t, user_model.IsErrEmailAddressNotExist(err)) 182 + assert.Nil(t, email) 183 + }
+5 -5
modules/structs/org.go
··· 47 47 48 48 // EditOrgOption options for editing an organization 49 49 type EditOrgOption struct { 50 - FullName string `json:"full_name" binding:"MaxSize(100)"` 51 - Email string `json:"email" binding:"MaxSize(255)"` 52 - Description string `json:"description" binding:"MaxSize(255)"` 53 - Website string `json:"website" binding:"ValidUrl;MaxSize(255)"` 54 - Location string `json:"location" binding:"MaxSize(50)"` 50 + FullName string `json:"full_name" binding:"MaxSize(100)"` 51 + Email *string `json:"email" binding:"MaxSize(255)"` 52 + Description string `json:"description" binding:"MaxSize(255)"` 53 + Website string `json:"website" binding:"ValidUrl;MaxSize(255)"` 54 + Location string `json:"location" binding:"MaxSize(50)"` 55 55 // possible values are `public`, `limited` or `private` 56 56 // enum: ["public", "limited", "private"] 57 57 Visibility string `json:"visibility" binding:"In(,public,limited,private)"`
+20 -4
routers/api/v1/org/org.go
··· 15 15 user_model "code.gitea.io/gitea/models/user" 16 16 "code.gitea.io/gitea/modules/optional" 17 17 api "code.gitea.io/gitea/modules/structs" 18 + "code.gitea.io/gitea/modules/validation" 18 19 "code.gitea.io/gitea/modules/web" 19 20 "code.gitea.io/gitea/routers/api/v1/user" 20 21 "code.gitea.io/gitea/routers/api/v1/utils" ··· 340 341 // "$ref": "#/responses/Organization" 341 342 // "404": 342 343 // "$ref": "#/responses/notFound" 344 + // "422": 345 + // "$ref": "#/responses/error" 343 346 344 347 form := web.GetForm(ctx).(*api.EditOrgOption) 345 348 346 - if form.Email != "" { 347 - if err := user_service.ReplacePrimaryEmailAddress(ctx, ctx.Org.Organization.AsUser(), form.Email); err != nil { 348 - ctx.Error(http.StatusInternalServerError, "ReplacePrimaryEmailAddress", err) 349 - return 349 + if form.Email != nil { 350 + if *form.Email == "" { 351 + err := user_model.DeletePrimaryEmailAddressOfUser(ctx, ctx.Org.Organization.ID) 352 + if err != nil { 353 + ctx.Error(http.StatusInternalServerError, "DeletePrimaryEmailAddressOfUser", err) 354 + return 355 + } 356 + ctx.Org.Organization.Email = "" 357 + } else { 358 + if err := user_service.ReplacePrimaryEmailAddress(ctx, ctx.Org.Organization.AsUser(), *form.Email); err != nil { 359 + if validation.IsErrEmailInvalid(err) || validation.IsErrEmailCharIsNotSupported(err) { 360 + ctx.Error(http.StatusUnprocessableEntity, "ReplacePrimaryEmailAddress", err) 361 + } else { 362 + ctx.Error(http.StatusInternalServerError, "ReplacePrimaryEmailAddress", err) 363 + } 364 + return 365 + } 350 366 } 351 367 } 352 368
+7 -1
routers/web/org/setting.go
··· 93 93 ctx.Org.OrgLink = setting.AppSubURL + "/org/" + url.PathEscape(org.Name) 94 94 } 95 95 96 - if form.Email != "" { 96 + if form.Email == "" { 97 + err := user_model.DeletePrimaryEmailAddressOfUser(ctx, org.ID) 98 + if err != nil { 99 + ctx.ServerError("DeletePrimaryEmailAddressOfUser", err) 100 + return 101 + } 102 + } else { 97 103 if err := user_service.ReplacePrimaryEmailAddress(ctx, org.AsUser(), form.Email); err != nil { 98 104 ctx.Data["Err_Email"] = true 99 105 ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSettingsOptions, &form)
+3
templates/swagger/v1_json.tmpl
··· 2263 2263 }, 2264 2264 "404": { 2265 2265 "$ref": "#/responses/notFound" 2266 + }, 2267 + "422": { 2268 + "$ref": "#/responses/error" 2266 2269 } 2267 2270 } 2268 2271 }
+54
tests/integration/api_org_test.go
··· 218 218 assert.EqualValues(t, "Empty", data.Data[0].Name) 219 219 } 220 220 } 221 + 222 + func TestAPIOrgChangeEmail(t *testing.T) { 223 + defer tests.PrepareTestEnv(t)() 224 + 225 + session := loginUser(t, "user1") 226 + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization) 227 + 228 + t.Run("Invalid", func(t *testing.T) { 229 + newMail := "invalid" 230 + settings := api.EditOrgOption{Email: &newMail} 231 + 232 + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", "/api/v1/orgs/org3", &settings).AddTokenAuth(token), http.StatusUnprocessableEntity) 233 + 234 + var org *api.Organization 235 + DecodeJSON(t, resp, &org) 236 + 237 + assert.Empty(t, org.Email) 238 + }) 239 + 240 + t.Run("Valid", func(t *testing.T) { 241 + newMail := "example@example.com" 242 + settings := api.EditOrgOption{Email: &newMail} 243 + 244 + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", "/api/v1/orgs/org3", &settings).AddTokenAuth(token), http.StatusOK) 245 + 246 + var org *api.Organization 247 + DecodeJSON(t, resp, &org) 248 + 249 + assert.Equal(t, "example@example.com", org.Email) 250 + }) 251 + 252 + t.Run("NoChange", func(t *testing.T) { 253 + settings := api.EditOrgOption{} 254 + 255 + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", "/api/v1/orgs/org3", &settings).AddTokenAuth(token), http.StatusOK) 256 + 257 + var org *api.Organization 258 + DecodeJSON(t, resp, &org) 259 + 260 + assert.Equal(t, "example@example.com", org.Email) 261 + }) 262 + 263 + t.Run("Empty", func(t *testing.T) { 264 + newMail := "" 265 + settings := api.EditOrgOption{Email: &newMail} 266 + 267 + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", "/api/v1/orgs/org3", &settings).AddTokenAuth(token), http.StatusOK) 268 + 269 + var org *api.Organization 270 + DecodeJSON(t, resp, &org) 271 + 272 + assert.Empty(t, org.Email) 273 + }) 274 + }
+89
tests/integration/org_settings_test.go
··· 1 + // Copyright 2024 The Forgejo Authors. All rights reserved. 2 + // SPDX-License-Identifier: MIT 3 + 4 + package integration 5 + 6 + import ( 7 + "fmt" 8 + "net/http" 9 + "testing" 10 + 11 + auth_model "code.gitea.io/gitea/models/auth" 12 + api "code.gitea.io/gitea/modules/structs" 13 + "code.gitea.io/gitea/tests" 14 + 15 + "github.com/stretchr/testify/assert" 16 + ) 17 + 18 + func getOrgSettingsFormData(t *testing.T, session *TestSession, orgName string) map[string]string { 19 + return map[string]string{ 20 + "_csrf": GetCSRF(t, session, fmt.Sprintf("/org/%s/settings", orgName)), 21 + "name": orgName, 22 + "full_name": "", 23 + "email": "", 24 + "description": "", 25 + "website": "", 26 + "location": "", 27 + "visibility": "0", 28 + "repo_admin_change_team_access": "on", 29 + "max_repo_creation": "-1", 30 + } 31 + } 32 + 33 + func getOrgSettings(t *testing.T, token, orgName string) *api.Organization { 34 + t.Helper() 35 + 36 + req := NewRequestf(t, "GET", "/api/v1/orgs/%s", orgName).AddTokenAuth(token) 37 + resp := MakeRequest(t, req, http.StatusOK) 38 + 39 + var org *api.Organization 40 + DecodeJSON(t, resp, &org) 41 + 42 + return org 43 + } 44 + 45 + func TestOrgSettingsChangeEmail(t *testing.T) { 46 + defer tests.PrepareTestEnv(t)() 47 + 48 + const orgName = "org3" 49 + settingsURL := fmt.Sprintf("/org/%s/settings", orgName) 50 + 51 + session := loginUser(t, "user1") 52 + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadOrganization) 53 + 54 + t.Run("Invalid", func(t *testing.T) { 55 + defer tests.PrintCurrentTest(t)() 56 + 57 + settings := getOrgSettingsFormData(t, session, orgName) 58 + 59 + settings["email"] = "invalid" 60 + session.MakeRequest(t, NewRequestWithValues(t, "POST", settingsURL, settings), http.StatusOK) 61 + 62 + org := getOrgSettings(t, token, orgName) 63 + assert.Equal(t, "org3@example.com", org.Email) 64 + }) 65 + 66 + t.Run("Valid", func(t *testing.T) { 67 + defer tests.PrintCurrentTest(t)() 68 + 69 + settings := getOrgSettingsFormData(t, session, orgName) 70 + 71 + settings["email"] = "example@example.com" 72 + session.MakeRequest(t, NewRequestWithValues(t, "POST", settingsURL, settings), http.StatusSeeOther) 73 + 74 + org := getOrgSettings(t, token, orgName) 75 + assert.Equal(t, "example@example.com", org.Email) 76 + }) 77 + 78 + t.Run("Empty", func(t *testing.T) { 79 + defer tests.PrintCurrentTest(t)() 80 + 81 + settings := getOrgSettingsFormData(t, session, orgName) 82 + 83 + settings["email"] = "" 84 + session.MakeRequest(t, NewRequestWithValues(t, "POST", settingsURL, settings), http.StatusSeeOther) 85 + 86 + org := getOrgSettings(t, token, orgName) 87 + assert.Empty(t, org.Email) 88 + }) 89 + }