forked from NYANDEV/forgejo
[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)
This commit is contained in:
parent
5f528dd85f
commit
6c7d5a5d14
10 changed files with 303 additions and 25 deletions
|
@ -189,6 +189,25 @@ func GetEmailAddresses(ctx context.Context, uid int64) ([]*EmailAddress, error)
|
|||
return emails, nil
|
||||
}
|
||||
|
||||
type ActivatedEmailAddress struct {
|
||||
ID int64
|
||||
Email string
|
||||
}
|
||||
|
||||
func GetActivatedEmailAddresses(ctx context.Context, uid int64) ([]*ActivatedEmailAddress, error) {
|
||||
emails := make([]*ActivatedEmailAddress, 0, 8)
|
||||
if err := db.GetEngine(ctx).
|
||||
Table("email_address").
|
||||
Select("id, email").
|
||||
Where("uid=?", uid).
|
||||
And("is_activated=?", true).
|
||||
Asc("id").
|
||||
Find(&emails); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return emails, nil
|
||||
}
|
||||
|
||||
// GetEmailAddressByID gets a user's email address by ID
|
||||
func GetEmailAddressByID(ctx context.Context, uid, id int64) (*EmailAddress, error) {
|
||||
// User ID is required for security reasons
|
||||
|
|
|
@ -4,6 +4,7 @@
|
|||
package user_test
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
|
@ -309,3 +310,37 @@ func TestEmailAddressValidate(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetActivatedEmailAddresses(t *testing.T) {
|
||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||
|
||||
testCases := []struct {
|
||||
UID int64
|
||||
expected []*user_model.ActivatedEmailAddress
|
||||
}{
|
||||
{
|
||||
UID: 1,
|
||||
expected: []*user_model.ActivatedEmailAddress{{ID: 9, Email: "user1@example.com"}, {ID: 33, Email: "user1-2@example.com"}, {ID: 34, Email: "user1-3@example.com"}},
|
||||
},
|
||||
{
|
||||
UID: 2,
|
||||
expected: []*user_model.ActivatedEmailAddress{{ID: 3, Email: "user2@example.com"}},
|
||||
},
|
||||
{
|
||||
UID: 4,
|
||||
expected: []*user_model.ActivatedEmailAddress{{ID: 11, Email: "user4@example.com"}},
|
||||
},
|
||||
{
|
||||
UID: 11,
|
||||
expected: []*user_model.ActivatedEmailAddress{},
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range testCases {
|
||||
t.Run(fmt.Sprintf("User %d", testCase.UID), func(t *testing.T) {
|
||||
emails, err := user_model.GetActivatedEmailAddresses(db.DefaultContext, testCase.UID)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, testCase.expected, emails)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1247,6 +1247,7 @@ editor.new_branch_name_desc = New branch name…
|
|||
editor.cancel = Cancel
|
||||
editor.filename_cannot_be_empty = The filename cannot be empty.
|
||||
editor.filename_is_invalid = The filename is invalid: "%s".
|
||||
editor.invalid_commit_mail = Invalid mail for creating a commit.
|
||||
editor.branch_does_not_exist = Branch "%s" does not exist in this repository.
|
||||
editor.branch_already_exists = Branch "%s" already exists in this repository.
|
||||
editor.directory_is_a_file = Directory name "%s" is already used as a filename in this repository.
|
||||
|
|
|
@ -14,6 +14,7 @@ import (
|
|||
git_model "code.gitea.io/gitea/models/git"
|
||||
repo_model "code.gitea.io/gitea/models/repo"
|
||||
"code.gitea.io/gitea/models/unit"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
"code.gitea.io/gitea/modules/base"
|
||||
"code.gitea.io/gitea/modules/charset"
|
||||
"code.gitea.io/gitea/modules/context"
|
||||
|
@ -99,6 +100,27 @@ func getParentTreeFields(treePath string) (treeNames, treePaths []string) {
|
|||
return treeNames, treePaths
|
||||
}
|
||||
|
||||
// getSelectableEmailAddresses returns which emails can be used by the user as
|
||||
// email for a Git commiter.
|
||||
func getSelectableEmailAddresses(ctx *context.Context) ([]*user_model.ActivatedEmailAddress, error) {
|
||||
// Retrieve emails that the user could use for commiter identity.
|
||||
commitEmails, err := user_model.GetActivatedEmailAddresses(ctx, ctx.Doer.ID)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("GetActivatedEmailAddresses: %w", err)
|
||||
}
|
||||
|
||||
// Allow for the placeholder mail to be used. Use -1 as ID to identify
|
||||
// this entry to be the placerholder mail of the user.
|
||||
placeholderMail := &user_model.ActivatedEmailAddress{ID: -1, Email: ctx.Doer.GetPlaceholderEmail()}
|
||||
if ctx.Doer.KeepEmailPrivate {
|
||||
commitEmails = append([]*user_model.ActivatedEmailAddress{placeholderMail}, commitEmails...)
|
||||
} else {
|
||||
commitEmails = append(commitEmails, placeholderMail)
|
||||
}
|
||||
|
||||
return commitEmails, nil
|
||||
}
|
||||
|
||||
func editFile(ctx *context.Context, isNewFile bool) {
|
||||
ctx.Data["PageIsEdit"] = true
|
||||
ctx.Data["IsNewFile"] = isNewFile
|
||||
|
@ -177,6 +199,12 @@ func editFile(ctx *context.Context, isNewFile bool) {
|
|||
treeNames = append(treeNames, fileName)
|
||||
}
|
||||
|
||||
commitEmails, err := getSelectableEmailAddresses(ctx)
|
||||
if err != nil {
|
||||
ctx.ServerError("getSelectableEmailAddresses", err)
|
||||
return
|
||||
}
|
||||
|
||||
ctx.Data["TreeNames"] = treeNames
|
||||
ctx.Data["TreePaths"] = treePaths
|
||||
ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL()
|
||||
|
@ -192,6 +220,8 @@ func editFile(ctx *context.Context, isNewFile bool) {
|
|||
ctx.Data["PreviewableExtensions"] = strings.Join(markup.PreviewableExtensions(), ",")
|
||||
ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",")
|
||||
ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, treePath)
|
||||
ctx.Data["CommitMails"] = commitEmails
|
||||
ctx.Data["DefaultCommitMail"] = ctx.Doer.GetEmail()
|
||||
|
||||
ctx.HTML(http.StatusOK, tplEditFile)
|
||||
}
|
||||
|
@ -227,6 +257,12 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
|
|||
branchName = form.NewBranchName
|
||||
}
|
||||
|
||||
commitEmails, err := getSelectableEmailAddresses(ctx)
|
||||
if err != nil {
|
||||
ctx.ServerError("getSelectableEmailAddresses", err)
|
||||
return
|
||||
}
|
||||
|
||||
ctx.Data["PageIsEdit"] = true
|
||||
ctx.Data["PageHasPosted"] = true
|
||||
ctx.Data["IsNewFile"] = isNewFile
|
||||
|
@ -243,6 +279,8 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
|
|||
ctx.Data["PreviewableExtensions"] = strings.Join(markup.PreviewableExtensions(), ",")
|
||||
ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",")
|
||||
ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, form.TreePath)
|
||||
ctx.Data["CommitMails"] = commitEmails
|
||||
ctx.Data["DefaultCommitMail"] = ctx.Doer.GetEmail()
|
||||
|
||||
if ctx.HasError() {
|
||||
ctx.HTML(http.StatusOK, tplEditFile)
|
||||
|
@ -277,6 +315,30 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
|
|||
operation = "create"
|
||||
}
|
||||
|
||||
gitIdentity := &files_service.IdentityOptions{
|
||||
Name: ctx.Doer.Name,
|
||||
}
|
||||
|
||||
// -1 is defined as placeholder email.
|
||||
if form.CommitMailID == -1 {
|
||||
gitIdentity.Email = ctx.Doer.GetPlaceholderEmail()
|
||||
} else {
|
||||
// Check if the given email is activated.
|
||||
email, err := user_model.GetEmailAddressByID(ctx, ctx.Doer.ID, form.CommitMailID)
|
||||
if err != nil {
|
||||
ctx.ServerError("GetEmailAddressByID", err)
|
||||
return
|
||||
}
|
||||
|
||||
if email == nil || !email.IsActivated {
|
||||
ctx.Data["Err_CommitMailID"] = true
|
||||
ctx.RenderWithErr(ctx.Tr("repo.editor.invalid_commit_mail"), tplEditFile, &form)
|
||||
return
|
||||
}
|
||||
|
||||
gitIdentity.Email = email.Email
|
||||
}
|
||||
|
||||
if _, err := files_service.ChangeRepoFiles(ctx, ctx.Repo.Repository, ctx.Doer, &files_service.ChangeRepoFilesOptions{
|
||||
LastCommitID: form.LastCommit,
|
||||
OldBranch: ctx.Repo.BranchName,
|
||||
|
@ -291,6 +353,8 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
|
|||
},
|
||||
},
|
||||
Signoff: form.Signoff,
|
||||
Author: gitIdentity,
|
||||
Committer: gitIdentity,
|
||||
}); err != nil {
|
||||
// This is where we handle all the errors thrown by files_service.ChangeRepoFiles
|
||||
if git.IsErrNotExist(err) {
|
||||
|
|
|
@ -768,6 +768,7 @@ type EditRepoFileForm struct {
|
|||
CommitChoice string `binding:"Required;MaxSize(50)"`
|
||||
NewBranchName string `binding:"GitRefName;MaxSize(100)"`
|
||||
LastCommit string
|
||||
CommitMailID int64 `binding:"Required"`
|
||||
Signoff bool
|
||||
}
|
||||
|
||||
|
|
|
@ -123,6 +123,8 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m
|
|||
if committer.Name != "" {
|
||||
committerUser.FullName = committer.Name
|
||||
}
|
||||
// Use the provided email and not revert to placeholder mail.
|
||||
committerUser.KeepEmailPrivate = false
|
||||
} else {
|
||||
committerUser = &user_model.User{
|
||||
FullName: committer.Name,
|
||||
|
@ -136,6 +138,8 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m
|
|||
if authorUser.Name != "" {
|
||||
authorUser.FullName = author.Name
|
||||
}
|
||||
// Use the provided email and not revert to placeholder mail.
|
||||
authorUser.KeepEmailPrivate = false
|
||||
} else {
|
||||
authorUser = &user_model.User{
|
||||
FullName: author.Name,
|
||||
|
|
|
@ -66,6 +66,14 @@
|
|||
</div>
|
||||
{{end}}
|
||||
</div>
|
||||
<div class="field {{if .Err_CommitMailID}}error{{end}}">
|
||||
<label for="commit_mail_id">Commit email</label>
|
||||
<select class="ui selection dropdown" name="commit_mail_id">
|
||||
{{range .CommitMails}}
|
||||
<option{{if eq $.DefaultCommitMail .Email}} selected="selected"{{end}} value="{{.ID}}">{{.Email}}</option>
|
||||
{{end}}
|
||||
</select>
|
||||
</div>
|
||||
</div>
|
||||
<button id="commit-button" type="submit" class="ui primary button">
|
||||
{{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}}
|
||||
|
|
|
@ -31,6 +31,7 @@ func TestRepoLanguages(t *testing.T) {
|
|||
"tree_path": "test.go",
|
||||
"content": "package main",
|
||||
"commit_choice": "direct",
|
||||
"commit_mail_id": "3",
|
||||
})
|
||||
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
|
||||
|
|
|
@ -4,14 +4,21 @@
|
|||
package integration
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"path"
|
||||
"testing"
|
||||
|
||||
repo_model "code.gitea.io/gitea/models/repo"
|
||||
"code.gitea.io/gitea/models/unittest"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
gitea_context "code.gitea.io/gitea/modules/context"
|
||||
"code.gitea.io/gitea/modules/git"
|
||||
"code.gitea.io/gitea/modules/json"
|
||||
"code.gitea.io/gitea/modules/translation"
|
||||
"code.gitea.io/gitea/tests"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
@ -35,6 +42,7 @@ func TestCreateFile(t *testing.T) {
|
|||
"tree_path": "test.txt",
|
||||
"content": "Content",
|
||||
"commit_choice": "direct",
|
||||
"commit_mail_id": "3",
|
||||
})
|
||||
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
})
|
||||
|
@ -72,6 +80,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) {
|
|||
"tree_path": "test.txt",
|
||||
"content": "Content",
|
||||
"commit_choice": "direct",
|
||||
"commit_mail_id": "3",
|
||||
})
|
||||
|
||||
resp = session.MakeRequest(t, req, http.StatusOK)
|
||||
|
@ -116,6 +125,7 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa
|
|||
"tree_path": filePath,
|
||||
"content": newContent,
|
||||
"commit_choice": "direct",
|
||||
"commit_mail_id": "-1",
|
||||
},
|
||||
)
|
||||
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
|
@ -146,6 +156,7 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra
|
|||
"content": newContent,
|
||||
"commit_choice": "commit-to-new-branch",
|
||||
"new_branch_name": targetBranch,
|
||||
"commit_mail_id": "-1",
|
||||
},
|
||||
)
|
||||
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
|
@ -171,3 +182,136 @@ func TestEditFileToNewBranch(t *testing.T) {
|
|||
testEditFileToNewBranch(t, session, "user2", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited)\n")
|
||||
})
|
||||
}
|
||||
|
||||
func TestEditFileCommitMail(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, _ *url.URL) {
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||
|
||||
session := loginUser(t, user.Name)
|
||||
link := path.Join("user2", "repo1", "_edit", "master", "README.md")
|
||||
|
||||
lastCommitAndCSRF := func() (string, string) {
|
||||
req := NewRequest(t, "GET", link)
|
||||
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||
lastCommit := htmlDoc.GetInputValueByName("last_commit")
|
||||
assert.NotEmpty(t, lastCommit)
|
||||
|
||||
return lastCommit, htmlDoc.GetCSRF()
|
||||
}
|
||||
|
||||
t.Run("Not activated", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 35, UID: user.ID})
|
||||
assert.False(t, email.IsActivated)
|
||||
|
||||
lastCommit, csrf := lastCommitAndCSRF()
|
||||
req := NewRequestWithValues(t, "POST", link, map[string]string{
|
||||
"_csrf": csrf,
|
||||
"last_commit": lastCommit,
|
||||
"tree_path": "README.md",
|
||||
"content": "new_content",
|
||||
"commit_choice": "direct",
|
||||
"commit_mail_id": fmt.Sprintf("%d", email.ID),
|
||||
})
|
||||
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||
assert.Contains(t,
|
||||
htmlDoc.doc.Find(".ui.negative.message").Text(),
|
||||
translation.NewLocale("en-US").Tr("repo.editor.invalid_commit_mail"),
|
||||
)
|
||||
})
|
||||
|
||||
t.Run("Not belong to user", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 1, IsActivated: true})
|
||||
assert.NotEqualValues(t, email.UID, user.ID)
|
||||
|
||||
lastCommit, csrf := lastCommitAndCSRF()
|
||||
req := NewRequestWithValues(t, "POST", link, map[string]string{
|
||||
"_csrf": csrf,
|
||||
"last_commit": lastCommit,
|
||||
"tree_path": "README.md",
|
||||
"content": "new_content",
|
||||
"commit_choice": "direct",
|
||||
"commit_mail_id": fmt.Sprintf("%d", email.ID),
|
||||
})
|
||||
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||
assert.Contains(t,
|
||||
htmlDoc.doc.Find(".ui.negative.message").Text(),
|
||||
translation.NewLocale("en-US").Tr("repo.editor.invalid_commit_mail"),
|
||||
)
|
||||
})
|
||||
|
||||
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
|
||||
gitRepo, _ := git.OpenRepository(git.DefaultContext, repo1.RepoPath())
|
||||
defer gitRepo.Close()
|
||||
|
||||
t.Run("Placeholder mail", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
lastCommit, csrf := lastCommitAndCSRF()
|
||||
req := NewRequestWithValues(t, "POST", link, map[string]string{
|
||||
"_csrf": csrf,
|
||||
"last_commit": lastCommit,
|
||||
"tree_path": "README.md",
|
||||
"content": "authored by placeholder mail",
|
||||
"commit_choice": "direct",
|
||||
"commit_mail_id": "-1",
|
||||
})
|
||||
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
|
||||
commit, err := gitRepo.GetCommitByPath("README.md")
|
||||
assert.NoError(t, err)
|
||||
|
||||
fileContent, err := commit.GetFileContent("README.md", 64)
|
||||
assert.NoError(t, err)
|
||||
assert.EqualValues(t, "authored by placeholder mail", fileContent)
|
||||
|
||||
assert.EqualValues(t, "user2", commit.Author.Name)
|
||||
assert.EqualValues(t, "user2@noreply.example.org", commit.Author.Email)
|
||||
assert.EqualValues(t, "user2", commit.Committer.Name)
|
||||
assert.EqualValues(t, "user2@noreply.example.org", commit.Committer.Email)
|
||||
})
|
||||
|
||||
t.Run("Normal", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
// Require that the user has KeepEmailPrivate enabled, because it needs
|
||||
// to be tested that even with this setting enabled, it will use the
|
||||
// provided mail and not revert to the placeholder one.
|
||||
assert.True(t, user.KeepEmailPrivate)
|
||||
|
||||
email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{ID: 3, UID: user.ID, IsActivated: true})
|
||||
|
||||
lastCommit, csrf := lastCommitAndCSRF()
|
||||
req := NewRequestWithValues(t, "POST", link, map[string]string{
|
||||
"_csrf": csrf,
|
||||
"last_commit": lastCommit,
|
||||
"tree_path": "README.md",
|
||||
"content": "authored by activated mail",
|
||||
"commit_choice": "direct",
|
||||
"commit_mail_id": fmt.Sprintf("%d", email.ID),
|
||||
})
|
||||
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
|
||||
commit, err := gitRepo.GetCommitByPath("README.md")
|
||||
assert.NoError(t, err)
|
||||
|
||||
fileContent, err := commit.GetFileContent("README.md", 64)
|
||||
assert.NoError(t, err)
|
||||
assert.EqualValues(t, "authored by activated mail", fileContent)
|
||||
|
||||
assert.EqualValues(t, "user2", commit.Author.Name)
|
||||
assert.EqualValues(t, email.Email, commit.Author.Email)
|
||||
assert.EqualValues(t, "user2", commit.Committer.Name)
|
||||
assert.EqualValues(t, email.Email, commit.Committer.Email)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
|
|
@ -58,6 +58,7 @@ func TestEmptyRepoAddFile(t *testing.T) {
|
|||
"commit_choice": "direct",
|
||||
"tree_path": "test-file.md",
|
||||
"content": "newly-added-test-file",
|
||||
"commit_mail_id": "32",
|
||||
})
|
||||
|
||||
resp = session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
|
|
Loading…
Reference in a new issue