[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 commit564e701f40
) (cherry picked from commitde8f2e03cc
) (cherry picked from commit0182cff12e
) (cherry picked from commit9c74254d46
)
This commit is contained in:
parent
b18c2e8d65
commit
2f0b68f821
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)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1244,6 +1244,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) {
|
||||
|
|
|
@ -767,6 +767,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