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.

[GITEA] Allow user to select email for file operations in Web UI

- Add a dropdown to the web interface for changing files to select which
Email should be used for the commit. It only shows (and verifies) that a
activated mail can be used, while this isn't necessary, it's better to
have this already in place.
- Added integration testing.
- Resolves https://codeberg.org/forgejo/forgejo/issues/281

(cherry picked from commit 564e701f407c0e110f3c7a4102bf7ed7902b815f)
(cherry picked from commit de8f2e03cc7d274049dd6a849b3d226968782644)
(cherry picked from commit 0182cff12ed4b68bd49ebc2b9951d9a29f7a36ca)
(cherry picked from commit 9c74254d4606febd702315c670db4fb6b14040a1)
(cherry picked from commit 2f0b68f821ae53dd12b496cc660353d5bf7cd143)
(cherry picked from commit 079b995d49ba7a625035fe9ec53741f6b0112007)
(cherry picked from commit 6952ea6ee3de8157d056c4381de7529de6eaef7b)
(cherry picked from commit 6c7d5a5d140152be80ec38a979a2a7b704ce653a)
(cherry picked from commit 49c39f0ed5a011b26f2e33f35811bb31fab3cf64)
(cherry picked from commit a8f9727388192c6c22b2f8cbbae15a96203ec3b6)

authored by

Gusted and committed by
Earl Warren
75ce1e2a 51fab301

+302 -25
+19
models/user/email_address.go
··· 231 231 return emails, nil 232 232 } 233 233 234 + type ActivatedEmailAddress struct { 235 + ID int64 236 + Email string 237 + } 238 + 239 + func GetActivatedEmailAddresses(ctx context.Context, uid int64) ([]*ActivatedEmailAddress, error) { 240 + emails := make([]*ActivatedEmailAddress, 0, 8) 241 + if err := db.GetEngine(ctx). 242 + Table("email_address"). 243 + Select("id, email"). 244 + Where("uid=?", uid). 245 + And("is_activated=?", true). 246 + Asc("id"). 247 + Find(&emails); err != nil { 248 + return nil, err 249 + } 250 + return emails, nil 251 + } 252 + 234 253 // GetEmailAddressByID gets a user's email address by ID 235 254 func GetEmailAddressByID(ctx context.Context, uid, id int64) (*EmailAddress, error) { 236 255 // User ID is required for security reasons
+35
models/user/email_address_test.go
··· 4 4 package user_test 5 5 6 6 import ( 7 + "fmt" 7 8 "testing" 8 9 9 10 "code.gitea.io/gitea/models/db" ··· 219 220 }) 220 221 } 221 222 } 223 + 224 + func TestGetActivatedEmailAddresses(t *testing.T) { 225 + assert.NoError(t, unittest.PrepareTestDatabase()) 226 + 227 + testCases := []struct { 228 + UID int64 229 + expected []*user_model.ActivatedEmailAddress 230 + }{ 231 + { 232 + UID: 1, 233 + expected: []*user_model.ActivatedEmailAddress{{ID: 9, Email: "user1@example.com"}, {ID: 33, Email: "user1-2@example.com"}, {ID: 34, Email: "user1-3@example.com"}}, 234 + }, 235 + { 236 + UID: 2, 237 + expected: []*user_model.ActivatedEmailAddress{{ID: 3, Email: "user2@example.com"}}, 238 + }, 239 + { 240 + UID: 4, 241 + expected: []*user_model.ActivatedEmailAddress{{ID: 11, Email: "user4@example.com"}}, 242 + }, 243 + { 244 + UID: 11, 245 + expected: []*user_model.ActivatedEmailAddress{}, 246 + }, 247 + } 248 + 249 + for _, testCase := range testCases { 250 + t.Run(fmt.Sprintf("User %d", testCase.UID), func(t *testing.T) { 251 + emails, err := user_model.GetActivatedEmailAddresses(db.DefaultContext, testCase.UID) 252 + assert.NoError(t, err) 253 + assert.Equal(t, testCase.expected, emails) 254 + }) 255 + } 256 + }
+65 -1
routers/web/repo/editor.go
··· 14 14 git_model "code.gitea.io/gitea/models/git" 15 15 repo_model "code.gitea.io/gitea/models/repo" 16 16 "code.gitea.io/gitea/models/unit" 17 + user_model "code.gitea.io/gitea/models/user" 17 18 "code.gitea.io/gitea/modules/base" 18 19 "code.gitea.io/gitea/modules/charset" 19 20 "code.gitea.io/gitea/modules/context" ··· 99 100 return treeNames, treePaths 100 101 } 101 102 103 + // getSelectableEmailAddresses returns which emails can be used by the user as 104 + // email for a Git commiter. 105 + func getSelectableEmailAddresses(ctx *context.Context) ([]*user_model.ActivatedEmailAddress, error) { 106 + // Retrieve emails that the user could use for commiter identity. 107 + commitEmails, err := user_model.GetActivatedEmailAddresses(ctx, ctx.Doer.ID) 108 + if err != nil { 109 + return nil, fmt.Errorf("GetActivatedEmailAddresses: %w", err) 110 + } 111 + 112 + // Allow for the placeholder mail to be used. Use -1 as ID to identify 113 + // this entry to be the placerholder mail of the user. 114 + placeholderMail := &user_model.ActivatedEmailAddress{ID: -1, Email: ctx.Doer.GetPlaceholderEmail()} 115 + if ctx.Doer.KeepEmailPrivate { 116 + commitEmails = append([]*user_model.ActivatedEmailAddress{placeholderMail}, commitEmails...) 117 + } else { 118 + commitEmails = append(commitEmails, placeholderMail) 119 + } 120 + 121 + return commitEmails, nil 122 + } 123 + 102 124 func editFile(ctx *context.Context, isNewFile bool) { 103 125 ctx.Data["PageIsEdit"] = true 104 126 ctx.Data["IsNewFile"] = isNewFile ··· 177 199 treeNames = append(treeNames, fileName) 178 200 } 179 201 202 + commitEmails, err := getSelectableEmailAddresses(ctx) 203 + if err != nil { 204 + ctx.ServerError("getSelectableEmailAddresses", err) 205 + return 206 + } 207 + 180 208 ctx.Data["TreeNames"] = treeNames 181 209 ctx.Data["TreePaths"] = treePaths 182 210 ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() ··· 192 220 ctx.Data["PreviewableExtensions"] = strings.Join(markup.PreviewableExtensions(), ",") 193 221 ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",") 194 222 ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, treePath) 223 + ctx.Data["CommitMails"] = commitEmails 224 + ctx.Data["DefaultCommitMail"] = ctx.Doer.GetEmail() 195 225 196 226 ctx.HTML(http.StatusOK, tplEditFile) 197 227 } ··· 227 257 branchName = form.NewBranchName 228 258 } 229 259 260 + commitEmails, err := getSelectableEmailAddresses(ctx) 261 + if err != nil { 262 + ctx.ServerError("getSelectableEmailAddresses", err) 263 + return 264 + } 265 + 230 266 ctx.Data["PageIsEdit"] = true 231 267 ctx.Data["PageHasPosted"] = true 232 268 ctx.Data["IsNewFile"] = isNewFile ··· 243 279 ctx.Data["PreviewableExtensions"] = strings.Join(markup.PreviewableExtensions(), ",") 244 280 ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",") 245 281 ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, form.TreePath) 282 + ctx.Data["CommitMails"] = commitEmails 283 + ctx.Data["DefaultCommitMail"] = ctx.Doer.GetEmail() 246 284 247 285 if ctx.HasError() { 248 286 ctx.HTML(http.StatusOK, tplEditFile) ··· 277 315 operation = "create" 278 316 } 279 317 318 + gitIdentity := &files_service.IdentityOptions{ 319 + Name: ctx.Doer.Name, 320 + } 321 + 322 + // -1 is defined as placeholder email. 323 + if form.CommitMailID == -1 { 324 + gitIdentity.Email = ctx.Doer.GetPlaceholderEmail() 325 + } else { 326 + // Check if the given email is activated. 327 + email, err := user_model.GetEmailAddressByID(ctx, ctx.Doer.ID, form.CommitMailID) 328 + if err != nil { 329 + ctx.ServerError("GetEmailAddressByID", err) 330 + return 331 + } 332 + 333 + if email == nil || !email.IsActivated { 334 + ctx.Data["Err_CommitMailID"] = true 335 + ctx.RenderWithErr(ctx.Tr("repo.editor.invalid_commit_mail"), tplEditFile, &form) 336 + return 337 + } 338 + 339 + gitIdentity.Email = email.Email 340 + } 341 + 280 342 if _, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.ChangeRepoFilesOptions{ 281 343 LastCommitID: form.LastCommit, 282 344 OldBranch: ctx.Repo.BranchName, ··· 290 352 ContentReader: strings.NewReader(strings.ReplaceAll(form.Content, "\r", "")), 291 353 }, 292 354 }, 293 - Signoff: form.Signoff, 355 + Signoff: form.Signoff, 356 + Author: gitIdentity, 357 + Committer: gitIdentity, 294 358 }); err != nil { 295 359 // This is where we handle all the errors thrown by files_service.ChangeRepoFiles 296 360 if git.IsErrNotExist(err) {
+1
services/forms/repo_form.go
··· 760 760 CommitChoice string `binding:"Required;MaxSize(50)"` 761 761 NewBranchName string `binding:"GitRefName;MaxSize(100)"` 762 762 LastCommit string 763 + CommitMailID int64 `binding:"Required"` 763 764 Signoff bool 764 765 } 765 766
+4
services/repository/files/file.go
··· 123 123 if committer.Name != "" { 124 124 committerUser.FullName = committer.Name 125 125 } 126 + // Use the provided email and not revert to placeholder mail. 127 + committerUser.KeepEmailPrivate = false 126 128 } else { 127 129 committerUser = &user_model.User{ 128 130 FullName: committer.Name, ··· 136 138 if authorUser.Name != "" { 137 139 authorUser.FullName = author.Name 138 140 } 141 + // Use the provided email and not revert to placeholder mail. 142 + authorUser.KeepEmailPrivate = false 139 143 } else { 140 144 authorUser = &user_model.User{ 141 145 FullName: author.Name,
+8
templates/repo/editor/commit_form.tmpl
··· 66 66 </div> 67 67 {{end}} 68 68 </div> 69 + <div class="field {{if .Err_CommitMailID}}error{{end}}"> 70 + <label for="commit_mail_id">Commit email</label> 71 + <select class="ui selection dropdown" name="commit_mail_id"> 72 + {{range .CommitMails}} 73 + <option{{if eq $.DefaultCommitMail .Email}} selected="selected"{{end}} value="{{.ID}}">{{.Email}}</option> 74 + {{end}} 75 + </select> 76 + </div> 69 77 </div> 70 78 <button id="commit-button" type="submit" class="ui primary button"> 71 79 {{if eq .commit_choice "commit-to-new-branch"}}{{ctx.Locale.Tr "repo.editor.propose_file_change"}}{{else}}{{ctx.Locale.Tr "repo.editor.commit_changes"}}{{end}}
+6 -5
tests/integration/api_repo_languages_test.go
··· 26 26 27 27 // Save new file to master branch 28 28 req = NewRequestWithValues(t, "POST", "/user2/repo1/_new/master/", map[string]string{ 29 - "_csrf": doc.GetCSRF(), 30 - "last_commit": lastCommit, 31 - "tree_path": "test.go", 32 - "content": "package main", 33 - "commit_choice": "direct", 29 + "_csrf": doc.GetCSRF(), 30 + "last_commit": lastCommit, 31 + "tree_path": "test.go", 32 + "content": "package main", 33 + "commit_choice": "direct", 34 + "commit_mail_id": "3", 34 35 }) 35 36 session.MakeRequest(t, req, http.StatusSeeOther) 36 37
+159 -15
tests/integration/editor_test.go
··· 4 4 package integration 5 5 6 6 import ( 7 + "fmt" 7 8 "net/http" 8 9 "net/http/httptest" 9 10 "net/url" 10 11 "path" 11 12 "testing" 12 13 14 + repo_model "code.gitea.io/gitea/models/repo" 15 + "code.gitea.io/gitea/models/unittest" 16 + user_model "code.gitea.io/gitea/models/user" 13 17 gitea_context "code.gitea.io/gitea/modules/context" 18 + "code.gitea.io/gitea/modules/git" 14 19 "code.gitea.io/gitea/modules/json" 20 + "code.gitea.io/gitea/modules/translation" 21 + "code.gitea.io/gitea/tests" 15 22 16 23 "github.com/stretchr/testify/assert" 17 24 ) ··· 30 37 31 38 // Save new file to master branch 32 39 req = NewRequestWithValues(t, "POST", "/user2/repo1/_new/master/", map[string]string{ 33 - "_csrf": doc.GetCSRF(), 34 - "last_commit": lastCommit, 35 - "tree_path": "test.txt", 36 - "content": "Content", 37 - "commit_choice": "direct", 40 + "_csrf": doc.GetCSRF(), 41 + "last_commit": lastCommit, 42 + "tree_path": "test.txt", 43 + "content": "Content", 44 + "commit_choice": "direct", 45 + "commit_mail_id": "3", 38 46 }) 39 47 session.MakeRequest(t, req, http.StatusSeeOther) 40 48 }) ··· 67 75 68 76 // Save new file to master branch 69 77 req = NewRequestWithValues(t, "POST", "/user2/repo1/_new/master/", map[string]string{ 70 - "_csrf": doc.GetCSRF(), 71 - "last_commit": lastCommit, 72 - "tree_path": "test.txt", 73 - "content": "Content", 74 - "commit_choice": "direct", 78 + "_csrf": doc.GetCSRF(), 79 + "last_commit": lastCommit, 80 + "tree_path": "test.txt", 81 + "content": "Content", 82 + "commit_choice": "direct", 83 + "commit_mail_id": "3", 75 84 }) 76 85 77 86 resp = session.MakeRequest(t, req, http.StatusOK) ··· 111 120 // Submit the edits 112 121 req = NewRequestWithValues(t, "POST", path.Join(user, repo, "_edit", branch, filePath), 113 122 map[string]string{ 114 - "_csrf": htmlDoc.GetCSRF(), 115 - "last_commit": lastCommit, 116 - "tree_path": filePath, 117 - "content": newContent, 118 - "commit_choice": "direct", 123 + "_csrf": htmlDoc.GetCSRF(), 124 + "last_commit": lastCommit, 125 + "tree_path": filePath, 126 + "content": newContent, 127 + "commit_choice": "direct", 128 + "commit_mail_id": "-1", 119 129 }, 120 130 ) 121 131 session.MakeRequest(t, req, http.StatusSeeOther) ··· 146 156 "content": newContent, 147 157 "commit_choice": "commit-to-new-branch", 148 158 "new_branch_name": targetBranch, 159 + "commit_mail_id": "-1", 149 160 }, 150 161 ) 151 162 session.MakeRequest(t, req, http.StatusSeeOther) ··· 171 182 testEditFileToNewBranch(t, session, "user2", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited)\n") 172 183 }) 173 184 } 185 + 186 + func TestEditFileCommitMail(t *testing.T) { 187 + onGiteaRun(t, func(t *testing.T, _ *url.URL) { 188 + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) 189 + 190 + session := loginUser(t, user.Name) 191 + link := path.Join("user2", "repo1", "_edit", "master", "README.md") 192 + 193 + lastCommitAndCSRF := func() (string, string) { 194 + req := NewRequest(t, "GET", link) 195 + resp := session.MakeRequest(t, req, http.StatusOK) 196 + 197 + htmlDoc := NewHTMLParser(t, resp.Body) 198 + lastCommit := htmlDoc.GetInputValueByName("last_commit") 199 + assert.NotEmpty(t, lastCommit) 200 + 201 + return lastCommit, htmlDoc.GetCSRF() 202 + } 203 + 204 + t.Run("Not activated", func(t *testing.T) { 205 + defer tests.PrintCurrentTest(t)() 206 + 207 + email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 35, UID: user.ID}) 208 + assert.False(t, email.IsActivated) 209 + 210 + lastCommit, csrf := lastCommitAndCSRF() 211 + req := NewRequestWithValues(t, "POST", link, map[string]string{ 212 + "_csrf": csrf, 213 + "last_commit": lastCommit, 214 + "tree_path": "README.md", 215 + "content": "new_content", 216 + "commit_choice": "direct", 217 + "commit_mail_id": fmt.Sprintf("%d", email.ID), 218 + }) 219 + resp := session.MakeRequest(t, req, http.StatusOK) 220 + 221 + htmlDoc := NewHTMLParser(t, resp.Body) 222 + assert.Contains(t, 223 + htmlDoc.doc.Find(".ui.negative.message").Text(), 224 + translation.NewLocale("en-US").Tr("repo.editor.invalid_commit_mail"), 225 + ) 226 + }) 227 + 228 + t.Run("Not belong to user", func(t *testing.T) { 229 + defer tests.PrintCurrentTest(t)() 230 + 231 + email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 1, IsActivated: true}) 232 + assert.NotEqualValues(t, email.UID, user.ID) 233 + 234 + lastCommit, csrf := lastCommitAndCSRF() 235 + req := NewRequestWithValues(t, "POST", link, map[string]string{ 236 + "_csrf": csrf, 237 + "last_commit": lastCommit, 238 + "tree_path": "README.md", 239 + "content": "new_content", 240 + "commit_choice": "direct", 241 + "commit_mail_id": fmt.Sprintf("%d", email.ID), 242 + }) 243 + resp := session.MakeRequest(t, req, http.StatusOK) 244 + 245 + htmlDoc := NewHTMLParser(t, resp.Body) 246 + assert.Contains(t, 247 + htmlDoc.doc.Find(".ui.negative.message").Text(), 248 + translation.NewLocale("en-US").Tr("repo.editor.invalid_commit_mail"), 249 + ) 250 + }) 251 + 252 + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) 253 + gitRepo, _ := git.OpenRepository(git.DefaultContext, repo1.RepoPath()) 254 + defer gitRepo.Close() 255 + 256 + t.Run("Placeholder mail", func(t *testing.T) { 257 + defer tests.PrintCurrentTest(t)() 258 + 259 + lastCommit, csrf := lastCommitAndCSRF() 260 + req := NewRequestWithValues(t, "POST", link, map[string]string{ 261 + "_csrf": csrf, 262 + "last_commit": lastCommit, 263 + "tree_path": "README.md", 264 + "content": "authored by placeholder mail", 265 + "commit_choice": "direct", 266 + "commit_mail_id": "-1", 267 + }) 268 + session.MakeRequest(t, req, http.StatusSeeOther) 269 + 270 + commit, err := gitRepo.GetCommitByPath("README.md") 271 + assert.NoError(t, err) 272 + 273 + fileContent, err := commit.GetFileContent("README.md", 64) 274 + assert.NoError(t, err) 275 + assert.EqualValues(t, "authored by placeholder mail", fileContent) 276 + 277 + assert.EqualValues(t, "user2", commit.Author.Name) 278 + assert.EqualValues(t, "user2@noreply.example.org", commit.Author.Email) 279 + assert.EqualValues(t, "user2", commit.Committer.Name) 280 + assert.EqualValues(t, "user2@noreply.example.org", commit.Committer.Email) 281 + }) 282 + 283 + t.Run("Normal", func(t *testing.T) { 284 + defer tests.PrintCurrentTest(t)() 285 + 286 + // Require that the user has KeepEmailPrivate enabled, because it needs 287 + // to be tested that even with this setting enabled, it will use the 288 + // provided mail and not revert to the placeholder one. 289 + assert.True(t, user.KeepEmailPrivate) 290 + 291 + email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 3, UID: user.ID, IsActivated: true}) 292 + 293 + lastCommit, csrf := lastCommitAndCSRF() 294 + req := NewRequestWithValues(t, "POST", link, map[string]string{ 295 + "_csrf": csrf, 296 + "last_commit": lastCommit, 297 + "tree_path": "README.md", 298 + "content": "authored by activated mail", 299 + "commit_choice": "direct", 300 + "commit_mail_id": fmt.Sprintf("%d", email.ID), 301 + }) 302 + session.MakeRequest(t, req, http.StatusSeeOther) 303 + 304 + commit, err := gitRepo.GetCommitByPath("README.md") 305 + assert.NoError(t, err) 306 + 307 + fileContent, err := commit.GetFileContent("README.md", 64) 308 + assert.NoError(t, err) 309 + assert.EqualValues(t, "authored by activated mail", fileContent) 310 + 311 + assert.EqualValues(t, "user2", commit.Author.Name) 312 + assert.EqualValues(t, email.Email, commit.Author.Email) 313 + assert.EqualValues(t, "user2", commit.Committer.Name) 314 + assert.EqualValues(t, email.Email, commit.Committer.Email) 315 + }) 316 + }) 317 + }
+5 -4
tests/integration/empty_repo_test.go
··· 50 50 doc := NewHTMLParser(t, resp.Body).Find(`input[name="commit_choice"]`) 51 51 assert.Empty(t, doc.AttrOr("checked", "_no_")) 52 52 req = NewRequestWithValues(t, "POST", "/user30/empty/_new/"+setting.Repository.DefaultBranch, map[string]string{ 53 - "_csrf": GetCSRF(t, session, "/user/settings"), 54 - "commit_choice": "direct", 55 - "tree_path": "test-file.md", 56 - "content": "newly-added-test-file", 53 + "_csrf": GetCSRF(t, session, "/user/settings"), 54 + "commit_choice": "direct", 55 + "tree_path": "test-file.md", 56 + "content": "newly-added-test-file", 57 + "commit_mail_id": "32", 57 58 }) 58 59 59 60 resp = session.MakeRequest(t, req, http.StatusSeeOther)