diff options
author | Lauris BH <lauris@nix.lv> | 2018-08-06 07:43:22 +0300 |
---|---|---|
committer | Jonas Franz <info@jonasfranz.software> | 2018-08-06 06:43:21 +0200 |
commit | 6e64f9db8eb889f9cc7e8c9576b2f9c89750927e (patch) | |
tree | d76b927295a1d2f69ea73a78e83d9264ce1510f2 /models | |
parent | 9c354a539ab498ebfdebf7395cf17f95f8b24ac8 (diff) | |
download | gitea-6e64f9db8eb889f9cc7e8c9576b2f9c89750927e.tar.gz gitea-6e64f9db8eb889f9cc7e8c9576b2f9c89750927e.zip |
Pull request review/approval and comment on code (#3748)
* Initial ui components for pull request review
* Add Review
Add IssueComment types
Signed-off-by: Jonas Franz <info@jonasfranz.software>
(cherry picked from commit 2b4daab)
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Replace ReviewComment with Content
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add load functions
Add ReviewID to findComments
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add create review comment implementation
Add migration for review
Other small changes
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Simplified create and find functions for review
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Moved "Pending" to first position
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add GetCurrentReview to simplify fetching current review
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Preview for listing comments
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Move new comment form to its own file
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Implement Review form
Show Review comments on comment stream
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add support for single comments
Showing buttons in context
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add pending tag to pending review comments
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add unit tests for Review
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Fetch all review ids at once
Add unit tests
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* gofmt
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Improved comment rendering in "Files" view by adding Comments to DiffLine
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add support for invalidating comments
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Switched back to code.gitea.io/git
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Moved review migration from v64 to v65
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Rebuild css
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* gofmt
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Improve translations
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Fix unit tests by updating fixtures and updating outdated test
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Comments will be shown at the right place now
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add support for deleting CodeComments
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Fix problems caused by files in subdirectories
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add support for showing code comments of reviews in conversation
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add support for "Show/Hide outdated"
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Update code.gitea.io/git
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add support for new webhooks
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Update comparison
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Resolve conflicts
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Minor UI improvements
* update code.gitea.io/git
* Fix ui bug reported by @lunny causing wrong position of add button
Add functionality to "Cancel" button
Add scale effects to add button
Hide "Cancel" button for existing comments
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Prepare solving conflicts
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Show add button only if no comments already exist for the line
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add missing vendor files
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Check if reviewer is nil
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Show forms only to users who are logged in
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Revert "Show forms only to users who are logged in"
This reverts commit c083682
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Save patch in comment
Render patch for code comments
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add link to comment in code
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add reply form to comment list
Show forms only to signed in users
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add 'Reply' as translatable
Add CODE_COMMENT_LINES setting
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* gofmt
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Fix problems introduced by checking for singed in user
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add v70
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Update generated stylesheet
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Fix preview
Beginn with new review comment patch system
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add new algo to generate diff for line range
Remove old algo used for cutting big diffs (it was very buggy)
* Add documentation and example for CutDiffAroundLine
* Fix example of CutDiffAroundLine
* Fix some comment UI rendering bugs
* Add code comment edit mode
* Send notifications / actions to users until review gets published
Fix diff generation bug
Fix wrong hashtag
* Fix vet errors
* Send notifications also for single comments
* Fix some notification bugs, fix link
* Fix: add comment icon is only shown on code lines
* Add lint comment
* Add unit tests for git diff
* Add more error messages
* Regenerated css
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* fmt
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Regenerated CSS with latest less version
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Fix test by updating comment type to new ID
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Introducing CodeComments as type for map[string]map[int64][]*Comment
Other minor code improvements
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Fix data-tab issues
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Remove unnecessary change
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* refactored checkForInvalidation
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Append comments instead of setting
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Use HeadRepo instead of BaseRepo
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Update migration
Signed-off-by: Jonas Franz <info@jonasfranz.de>
* Regenerated CSS
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Add copyright
Signed-off-by: Jonas Franz <info@jonasfranz.software>
* Update index.css
Signed-off-by: Jonas Franz <info@jonasfranz.software>
Diffstat (limited to 'models')
-rw-r--r-- | models/error.go | 22 | ||||
-rw-r--r-- | models/fixtures/comment.yml | 21 | ||||
-rw-r--r-- | models/fixtures/review.yml | 32 | ||||
-rw-r--r-- | models/git_diff.go | 206 | ||||
-rw-r--r-- | models/git_diff_test.go | 105 | ||||
-rw-r--r-- | models/issue_comment.go | 299 | ||||
-rw-r--r-- | models/issue_comment_test.go | 18 | ||||
-rw-r--r-- | models/migrations/migrations.go | 4 | ||||
-rw-r--r-- | models/migrations/v72.go | 31 | ||||
-rw-r--r-- | models/models.go | 1 | ||||
-rw-r--r-- | models/pull.go | 64 | ||||
-rw-r--r-- | models/review.go | 256 | ||||
-rw-r--r-- | models/review_test.go | 107 |
13 files changed, 1131 insertions, 35 deletions
diff --git a/models/error.go b/models/error.go index 029c33aba4..b079f06d84 100644 --- a/models/error.go +++ b/models/error.go @@ -1344,3 +1344,25 @@ func IsErrUnknownDependencyType(err error) bool { func (err ErrUnknownDependencyType) Error() string { return fmt.Sprintf("unknown dependency type [type: %d]", err.Type) } + +// __________ .__ +// \______ \ _______ _|__| ______ _ __ +// | _// __ \ \/ / |/ __ \ \/ \/ / +// | | \ ___/\ /| \ ___/\ / +// |____|_ /\___ >\_/ |__|\___ >\/\_/ +// \/ \/ \/ + +// ErrReviewNotExist represents a "ReviewNotExist" kind of error. +type ErrReviewNotExist struct { + ID int64 +} + +// IsErrReviewNotExist checks if an error is a ErrReviewNotExist. +func IsErrReviewNotExist(err error) bool { + _, ok := err.(ErrReviewNotExist) + return ok +} + +func (err ErrReviewNotExist) Error() string { + return fmt.Sprintf("review does not exist [id: %d]", err.ID) +} diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml index 34df02d28c..6d4812f096 100644 --- a/models/fixtures/comment.yml +++ b/models/fixtures/comment.yml @@ -20,3 +20,24 @@ issue_id: 1 # in repo_id 1 content: "meh..." created_unix: 946684812 +- + id: 4 + type: 21 # code comment + poster_id: 1 + issue_id: 2 + content: "meh..." + review_id: 4 + line: 4 + tree_path: "README.md" + created_unix: 946684812 + invalidated: false +- + id: 5 + type: 21 # code comment + poster_id: 1 + issue_id: 2 + content: "meh..." + line: -4 + tree_path: "README.md" + created_unix: 946684812 + invalidated: false diff --git a/models/fixtures/review.yml b/models/fixtures/review.yml new file mode 100644 index 0000000000..17defd1b3a --- /dev/null +++ b/models/fixtures/review.yml @@ -0,0 +1,32 @@ +- + id: 1 + type: 1 + reviewer_id: 1 + issue_id: 2 + content: "Demo Review" + updated_unix: 946684810 + created_unix: 946684810 +- + id: 2 + type: 1 + reviewer_id: 534543 + issue_id: 534543 + content: "Invalid Review #1" + updated_unix: 946684810 + created_unix: 946684810 +- + id: 3 + type: 1 + reviewer_id: 1 + issue_id: 343545 + content: "Invalid Review #2" + updated_unix: 946684810 + created_unix: 946684810 +- + id: 4 + type: 0 # Pending review + reviewer_id: 1 + issue_id: 2 + content: "Pending Review" + updated_unix: 946684810 + created_unix: 946684810 diff --git a/models/git_diff.go b/models/git_diff.go index 7b0b672ff1..006238cd06 100644 --- a/models/git_diff.go +++ b/models/git_diff.go @@ -14,6 +14,8 @@ import ( "io/ioutil" "os" "os/exec" + "regexp" + "sort" "strconv" "strings" @@ -57,6 +59,7 @@ type DiffLine struct { RightIdx int Type DiffLineType Content string + Comments []*Comment } // GetType returns the type of a DiffLine. @@ -64,6 +67,19 @@ func (d *DiffLine) GetType() int { return int(d.Type) } +// CanComment returns whether or not a line can get commented +func (d *DiffLine) CanComment() bool { + return len(d.Comments) == 0 && d.Type != DiffLineSection +} + +// GetCommentSide returns the comment side of the first comment, if not set returns empty string +func (d *DiffLine) GetCommentSide() string { + if len(d.Comments) == 0 { + return "" + } + return d.Comments[0].DiffSide() +} + // DiffSection represents a section of a DiffFile. type DiffSection struct { Name string @@ -225,11 +241,167 @@ type Diff struct { IsIncomplete bool } +// LoadComments loads comments into each line +func (diff *Diff) LoadComments(issue *Issue, currentUser *User) error { + allComments, err := FetchCodeComments(issue, currentUser) + if err != nil { + return err + } + for _, file := range diff.Files { + if lineCommits, ok := allComments[file.Name]; ok { + for _, section := range file.Sections { + for _, line := range section.Lines { + if comments, ok := lineCommits[int64(line.LeftIdx*-1)]; ok { + line.Comments = append(line.Comments, comments...) + } + if comments, ok := lineCommits[int64(line.RightIdx)]; ok { + line.Comments = append(line.Comments, comments...) + } + sort.SliceStable(line.Comments, func(i, j int) bool { + return line.Comments[i].CreatedUnix < line.Comments[j].CreatedUnix + }) + } + } + } + } + return nil +} + // NumFiles returns number of files changes in a diff. func (diff *Diff) NumFiles() int { return len(diff.Files) } +// Example: @@ -1,8 +1,9 @@ => [..., 1, 8, 1, 9] +var hunkRegex = regexp.MustCompile(`^@@ -([0-9]+),([0-9]+) \+([0-9]+),([0-9]+) @@`) + +func isHeader(lof string) bool { + return strings.HasPrefix(lof, cmdDiffHead) || strings.HasPrefix(lof, "---") || strings.HasPrefix(lof, "+++") +} + +// CutDiffAroundLine cuts a diff of a file in way that only the given line + numberOfLine above it will be shown +// it also recalculates hunks and adds the appropriate headers to the new diff. +// Warning: Only one-file diffs are allowed. +func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLine int) string { + if line == 0 || numbersOfLine == 0 { + // no line or num of lines => no diff + return "" + } + scanner := bufio.NewScanner(originalDiff) + hunk := make([]string, 0) + // begin is the start of the hunk containing searched line + // end is the end of the hunk ... + // currentLine is the line number on the side of the searched line (differentiated by old) + // otherLine is the line number on the opposite side of the searched line (differentiated by old) + var begin, end, currentLine, otherLine int64 + var headerLines int + for scanner.Scan() { + lof := scanner.Text() + // Add header to enable parsing + if isHeader(lof) { + hunk = append(hunk, lof) + headerLines++ + } + if currentLine > line { + break + } + // Detect "hunk" with contains commented lof + if strings.HasPrefix(lof, "@@") { + // Already got our hunk. End of hunk detected! + if len(hunk) > headerLines { + break + } + groups := hunkRegex.FindStringSubmatch(lof) + if old { + begin = com.StrTo(groups[1]).MustInt64() + end = com.StrTo(groups[2]).MustInt64() + // init otherLine with begin of opposite side + otherLine = com.StrTo(groups[3]).MustInt64() + } else { + begin = com.StrTo(groups[3]).MustInt64() + end = com.StrTo(groups[4]).MustInt64() + // init otherLine with begin of opposite side + otherLine = com.StrTo(groups[1]).MustInt64() + } + end += begin // end is for real only the number of lines in hunk + // lof is between begin and end + if begin <= line && end >= line { + hunk = append(hunk, lof) + currentLine = begin + continue + } + } else if len(hunk) > headerLines { + hunk = append(hunk, lof) + // Count lines in context + switch lof[0] { + case '+': + if !old { + currentLine++ + } else { + otherLine++ + } + case '-': + if old { + currentLine++ + } else { + otherLine++ + } + default: + currentLine++ + otherLine++ + } + } + } + + // No hunk found + if currentLine == 0 { + return "" + } + // headerLines + hunkLine (1) = totalNonCodeLines + if len(hunk)-headerLines-1 <= numbersOfLine { + // No need to cut the hunk => return existing hunk + return strings.Join(hunk, "\n") + } + var oldBegin, oldNumOfLines, newBegin, newNumOfLines int64 + if old { + oldBegin = currentLine + newBegin = otherLine + } else { + oldBegin = otherLine + newBegin = currentLine + } + // headers + hunk header + newHunk := make([]string, headerLines) + // transfer existing headers + for idx, lof := range hunk[:headerLines] { + newHunk[idx] = lof + } + // transfer last n lines + for _, lof := range hunk[len(hunk)-numbersOfLine-1:] { + newHunk = append(newHunk, lof) + } + // calculate newBegin, ... by counting lines + for i := len(hunk) - 1; i >= len(hunk)-numbersOfLine; i-- { + switch hunk[i][0] { + case '+': + newBegin-- + newNumOfLines++ + case '-': + oldBegin-- + oldNumOfLines++ + default: + oldBegin-- + newBegin-- + newNumOfLines++ + oldNumOfLines++ + } + } + // construct the new hunk header + newHunk[headerLines] = fmt.Sprintf("@@ -%d,%d +%d,%d @@", + oldBegin, oldNumOfLines, newBegin, newNumOfLines) + return strings.Join(newHunk, "\n") +} + const cmdDiffHead = "diff --git " // ParsePatch builds a Diff object from a io.Reader and some @@ -307,7 +479,6 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D if curFileLinesCount >= maxLines { curFile.IsIncomplete = true } - switch { case line[0] == ' ': diffLine := &DiffLine{Type: DiffLinePlain, Content: line, LeftIdx: leftLine, RightIdx: rightLine} @@ -524,32 +695,46 @@ const ( // GetRawDiff dumps diff results of repository in given commit ID to io.Writer. // TODO: move this function to gogits/git-module func GetRawDiff(repoPath, commitID string, diffType RawDiffType, writer io.Writer) error { + return GetRawDiffForFile(repoPath, "", commitID, diffType, "", writer) +} + +// GetRawDiffForFile dumps diff results of file in given commit ID to io.Writer. +// TODO: move this function to gogits/git-module +func GetRawDiffForFile(repoPath, startCommit, endCommit string, diffType RawDiffType, file string, writer io.Writer) error { repo, err := git.OpenRepository(repoPath) if err != nil { return fmt.Errorf("OpenRepository: %v", err) } - commit, err := repo.GetCommit(commitID) + commit, err := repo.GetCommit(endCommit) if err != nil { return fmt.Errorf("GetCommit: %v", err) } - + fileArgs := make([]string, 0) + if len(file) > 0 { + fileArgs = append(fileArgs, "--", file) + } var cmd *exec.Cmd switch diffType { case RawDiffNormal: - if commit.ParentCount() == 0 { - cmd = exec.Command("git", "show", commitID) + if len(startCommit) != 0 { + cmd = exec.Command("git", append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...)...) + } else if commit.ParentCount() == 0 { + cmd = exec.Command("git", append([]string{"show", endCommit}, fileArgs...)...) } else { c, _ := commit.Parent(0) - cmd = exec.Command("git", "diff", "-M", c.ID.String(), commitID) + cmd = exec.Command("git", append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...)...) } case RawDiffPatch: - if commit.ParentCount() == 0 { - cmd = exec.Command("git", "format-patch", "--no-signature", "--stdout", "--root", commitID) + if len(startCommit) != 0 { + query := fmt.Sprintf("%s...%s", endCommit, startCommit) + cmd = exec.Command("git", append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...)...) + } else if commit.ParentCount() == 0 { + cmd = exec.Command("git", append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...)...) } else { c, _ := commit.Parent(0) - query := fmt.Sprintf("%s...%s", commitID, c.ID.String()) - cmd = exec.Command("git", "format-patch", "--no-signature", "--stdout", query) + query := fmt.Sprintf("%s...%s", endCommit, c.ID.String()) + cmd = exec.Command("git", append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...)...) } default: return fmt.Errorf("invalid diffType: %s", diffType) @@ -560,7 +745,6 @@ func GetRawDiff(repoPath, commitID string, diffType RawDiffType, writer io.Write cmd.Dir = repoPath cmd.Stdout = writer cmd.Stderr = stderr - if err = cmd.Run(); err != nil { return fmt.Errorf("Run: %v - %s", err, stderr) } diff --git a/models/git_diff_test.go b/models/git_diff_test.go index a29830c7fa..ac64771226 100644 --- a/models/git_diff_test.go +++ b/models/git_diff_test.go @@ -2,9 +2,11 @@ package models import ( "html/template" + "strings" "testing" dmp "github.com/sergi/go-diff/diffmatchpatch" + "github.com/stretchr/testify/assert" ) func assertEqual(t *testing.T, s1 string, s2 template.HTML) { @@ -34,3 +36,106 @@ func TestDiffToHTML(t *testing.T) { {Type: dmp.DiffEqual, Text: " biz"}, }, DiffLineDel)) } + +const exampleDiff = `diff --git a/README.md b/README.md +--- a/README.md ++++ b/README.md +@@ -1,3 +1,6 @@ + # gitea-github-migrator ++ ++ Build Status +- Latest Release + Docker Pulls ++ cut off ++ cut off` + +func TestCutDiffAroundLine(t *testing.T) { + result := CutDiffAroundLine(strings.NewReader(exampleDiff), 4, false, 3) + resultByLine := strings.Split(result, "\n") + assert.Len(t, resultByLine, 7) + // Check if headers got transferred + assert.Equal(t, "diff --git a/README.md b/README.md", resultByLine[0]) + assert.Equal(t, "--- a/README.md", resultByLine[1]) + assert.Equal(t, "+++ b/README.md", resultByLine[2]) + // Check if hunk header is calculated correctly + assert.Equal(t, "@@ -2,2 +3,2 @@", resultByLine[3]) + // Check if line got transferred + assert.Equal(t, "+ Build Status", resultByLine[4]) + + // Must be same result as before since old line 3 == new line 5 + newResult := CutDiffAroundLine(strings.NewReader(exampleDiff), 3, true, 3) + assert.Equal(t, result, newResult, "Must be same result as before since old line 3 == new line 5") + + newResult = CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 300) + assert.Equal(t, exampleDiff, newResult) + + emptyResult := CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 0) + assert.Empty(t, emptyResult) + + // Line is out of scope + emptyResult = CutDiffAroundLine(strings.NewReader(exampleDiff), 434, false, 0) + assert.Empty(t, emptyResult) +} + +func BenchmarkCutDiffAroundLine(b *testing.B) { + for n := 0; n < b.N; n++ { + CutDiffAroundLine(strings.NewReader(exampleDiff), 3, true, 3) + } +} + +func ExampleCutDiffAroundLine() { + const diff = `diff --git a/README.md b/README.md +--- a/README.md ++++ b/README.md +@@ -1,3 +1,6 @@ + # gitea-github-migrator ++ ++ Build Status +- Latest Release + Docker Pulls ++ cut off ++ cut off` + result := CutDiffAroundLine(strings.NewReader(diff), 4, false, 3) + println(result) +} + +func setupDefaultDiff() *Diff { + return &Diff{ + Files: []*DiffFile{ + { + Name: "README.md", + Sections: []*DiffSection{ + { + Lines: []*DiffLine{ + { + LeftIdx: 4, + RightIdx: 4, + }, + }, + }, + }, + }, + }, + } +} +func TestDiff_LoadComments(t *testing.T) { + issue := AssertExistsAndLoadBean(t, &Issue{ID: 2}).(*Issue) + user := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User) + diff := setupDefaultDiff() + assert.NoError(t, PrepareTestDatabase()) + assert.NoError(t, diff.LoadComments(issue, user)) + assert.Len(t, diff.Files[0].Sections[0].Lines[0].Comments, 2) +} + +func TestDiffLine_CanComment(t *testing.T) { + assert.False(t, (&DiffLine{Type: DiffLineSection}).CanComment()) + assert.False(t, (&DiffLine{Type: DiffLineAdd, Comments: []*Comment{{Content: "bla"}}}).CanComment()) + assert.True(t, (&DiffLine{Type: DiffLineAdd}).CanComment()) + assert.True(t, (&DiffLine{Type: DiffLineDel}).CanComment()) + assert.True(t, (&DiffLine{Type: DiffLinePlain}).CanComment()) +} + +func TestDiffLine_GetCommentSide(t *testing.T) { + assert.Equal(t, "previous", (&DiffLine{Comments: []*Comment{{Line: -3}}}).GetCommentSide()) + assert.Equal(t, "proposed", (&DiffLine{Comments: []*Comment{{Line: 3}}}).GetCommentSide()) +} diff --git a/models/issue_comment.go b/models/issue_comment.go index ad276e61f9..8cbd9613a0 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1,13 +1,19 @@ -// Copyright 2016 The Gogs Authors. All rights reserved. +// Copyright 2018 The Gitea Authors. +// Copyright 2016 The Gogs Authors. +// All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. package models import ( + "bytes" "fmt" "strings" + "code.gitea.io/git" + "code.gitea.io/gitea/modules/markup/markdown" + "code.gitea.io/gitea/modules/setting" "github.com/Unknwon/com" "github.com/go-xorm/builder" "github.com/go-xorm/xorm" @@ -70,6 +76,10 @@ const ( CommentTypeAddDependency //Dependency removed CommentTypeRemoveDependency + // Comment a line of code + CommentTypeCode + // Reviews a pull request by giving general feedback + CommentTypeReview ) // CommentTag defines comment tag type @@ -106,10 +116,14 @@ type Comment struct { DependentIssue *Issue `xorm:"-"` CommitID int64 - Line int64 + Line int64 // - previous line / + proposed line + TreePath string Content string `xorm:"TEXT"` RenderedContent string `xorm:"-"` + // Path represents the 4 lines of code cemented by this comment + Patch string `xorm:"TEXT"` + CreatedUnix util.TimeStamp `xorm:"INDEX created"` UpdatedUnix util.TimeStamp `xorm:"INDEX updated"` @@ -121,6 +135,10 @@ type Comment struct { // For view issue page. ShowTag CommentTag `xorm:"-"` + + Review *Review `xorm:"-"` + ReviewID int64 + Invalidated bool } // LoadIssue loads issue from database @@ -171,6 +189,20 @@ func (c *Comment) HTMLURL() string { log.Error(4, "LoadIssue(%d): %v", c.IssueID, err) return "" } + if c.Type == CommentTypeCode { + if c.ReviewID == 0 { + return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag()) + } + if c.Review == nil { + if err := c.LoadReview(); err != nil { + log.Warn("LoadReview(%d): %v", c.ReviewID, err) + return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag()) + } + } + if c.Review.Type <= ReviewTypePending { + return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag()) + } + } return fmt.Sprintf("%s#%s", c.Issue.HTMLURL(), c.HashTag()) } @@ -342,6 +374,89 @@ func (c *Comment) LoadReactions() error { return c.loadReactions(x) } +func (c *Comment) loadReview(e Engine) (err error) { + if c.Review, err = getReviewByID(e, c.ReviewID); err != nil { + return err + } + return nil +} + +// LoadReview loads the associated review +func (c *Comment) LoadReview() error { + return c.loadReview(x) +} + +func (c *Comment) checkInvalidation(e Engine, doer *User, repo *git.Repository, branch string) error { + // FIXME differentiate between previous and proposed line + commit, err := repo.LineBlame(branch, repo.Path, c.TreePath, uint(c.UnsignedLine())) + if err != nil { + return err + } + if c.CommitSHA != commit.ID.String() { + c.Invalidated = true + return UpdateComment(doer, c, "") + } + return nil +} + +// CheckInvalidation checks if the line of code comment got changed by another commit. +// If the line got changed the comment is going to be invalidated. +func (c *Comment) CheckInvalidation(repo *git.Repository, doer *User, branch string) error { + return c.checkInvalidation(x, doer, repo, branch) +} + +// DiffSide returns "previous" if Comment.Line is a LOC of the previous changes and "proposed" if it is a LOC of the proposed changes. +func (c *Comment) DiffSide() string { + if c.Line < 0 { + return "previous" + } + return "proposed" +} + +// UnsignedLine returns the LOC of the code comment without + or - +func (c *Comment) UnsignedLine() uint64 { + if c.Line < 0 { + return uint64(c.Line * -1) + } + return uint64(c.Line) +} + +// AsDiff returns c.Patch as *Diff +func (c *Comment) AsDiff() (*Diff, error) { + diff, err := ParsePatch(setting.Git.MaxGitDiffLines, + setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(c.Patch)) + if err != nil { + return nil, err + } + if len(diff.Files) == 0 { + return nil, fmt.Errorf("no file found for comment ID: %d", c.ID) + } + secs := diff.Files[0].Sections + if len(secs) == 0 { + return nil, fmt.Errorf("no sections found for comment ID: %d", c.ID) + } + return diff, nil +} + +// MustAsDiff executes AsDiff and logs the error instead of returning +func (c *Comment) MustAsDiff() *Diff { + diff, err := c.AsDiff() + if err != nil { + log.Warn("MustAsDiff: %v", err) + } + return diff +} + +// CodeCommentURL returns the url to a comment in code +func (c *Comment) CodeCommentURL() string { + err := c.LoadIssue() + if err != nil { // Silently dropping errors :unamused: + log.Error(4, "LoadIssue(%d): %v", c.IssueID, err) + return "" + } + return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag()) +} + func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err error) { var LabelID int64 if opts.Label != nil { @@ -365,6 +480,9 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err OldTitle: opts.OldTitle, NewTitle: opts.NewTitle, DependentIssueID: opts.DependentIssueID, + TreePath: opts.TreePath, + ReviewID: opts.ReviewID, + Patch: opts.Patch, } if _, err = e.Insert(comment); err != nil { return nil, err @@ -374,6 +492,14 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err return nil, err } + if err = sendCreateCommentAction(e, opts, comment); err != nil { + return nil, err + } + + return comment, nil +} + +func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, comment *Comment) (err error) { // Compose comment action, could be plain comment, close or reopen issue/pull request. // This object will be used to notify watchers in the end of function. act := &Action{ @@ -386,14 +512,25 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err CommentID: comment.ID, IsPrivate: opts.Repo.IsPrivate, } - // Check comment type. switch opts.Type { + case CommentTypeCode: + if comment.ReviewID != 0 { + if comment.Review == nil { + if err := comment.LoadReview(); err != nil { + return err + } + } + if comment.Review.Type <= ReviewTypePending { + return nil + } + } + fallthrough case CommentTypeComment: act.OpType = ActionCommentIssue if _, err = e.Exec("UPDATE `issue` SET num_comments=num_comments+1 WHERE id=?", opts.Issue.ID); err != nil { - return nil, err + return err } // Check attachments @@ -404,7 +541,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err if IsErrAttachmentNotExist(err) { continue } - return nil, fmt.Errorf("getAttachmentByUUID [%s]: %v", uuid, err) + return fmt.Errorf("getAttachmentByUUID [%s]: %v", uuid, err) } attachments = append(attachments, attach) } @@ -414,7 +551,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err attachments[i].CommentID = comment.ID // No assign value could be 0, so ignore AllCols(). if _, err = e.ID(attachments[i].ID).Update(attachments[i]); err != nil { - return nil, fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err) + return fmt.Errorf("update attachment [%d]: %v", attachments[i].ID, err) } } @@ -430,7 +567,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err _, err = e.Exec("UPDATE `repository` SET num_closed_issues=num_closed_issues-1 WHERE id=?", opts.Repo.ID) } if err != nil { - return nil, err + return err } case CommentTypeClose: @@ -445,15 +582,13 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err _, err = e.Exec("UPDATE `repository` SET num_closed_issues=num_closed_issues+1 WHERE id=?", opts.Repo.ID) } if err != nil { - return nil, err + return err } } - // update the issue's updated_unix column if err = updateIssueCols(e, opts.Issue, "updated_unix"); err != nil { - return nil, err + return err } - // Notify watchers for whatever action comes in, ignore if no action type. if act.OpType > 0 { if err = notifyWatchers(e, act); err != nil { @@ -463,8 +598,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err log.Error(4, "MailParticipants: %v", err) } } - - return comment, nil + return nil } func createStatusComment(e *xorm.Session, doer *User, repo *Repository, issue *Issue) (*Comment, error) { @@ -616,7 +750,10 @@ type CreateCommentOptions struct { NewTitle string CommitID int64 CommitSHA string + Patch string LineNum int64 + TreePath string + ReviewID int64 Content string Attachments []string // UUIDs of attachments } @@ -673,6 +810,58 @@ func CreateIssueComment(doer *User, repo *Repository, issue *Issue, content stri return comment, nil } +// CreateCodeComment creates a plain code comment at the specified line / path +func CreateCodeComment(doer *User, repo *Repository, issue *Issue, content, treePath string, line, reviewID int64) (*Comment, error) { + var commitID, patch string + pr, err := GetPullRequestByIssueID(issue.ID) + if err != nil { + return nil, fmt.Errorf("GetPullRequestByIssueID: %v", err) + } + if err := pr.GetBaseRepo(); err != nil { + return nil, fmt.Errorf("GetHeadRepo: %v", err) + } + gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) + if err != nil { + return nil, fmt.Errorf("OpenRepository: %v", err) + } + // FIXME differentiate between previous and proposed line + var gitLine = line + if gitLine < 0 { + gitLine *= -1 + } + // FIXME validate treePath + // Get latest commit referencing the commented line + commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(gitLine)) + if err != nil { + return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, gitLine, err) + } + // Only fetch diff if comment is review comment + if reviewID != 0 { + headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName()) + if err != nil { + return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err) + } + patchBuf := new(bytes.Buffer) + if err := GetRawDiffForFile(gitRepo.Path, pr.MergeBase, headCommitID, RawDiffNormal, treePath, patchBuf); err != nil { + return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath) + } + patch = CutDiffAroundLine(strings.NewReader(patchBuf.String()), int64((&Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) + commitID = commit.ID.String() + } + return CreateComment(&CreateCommentOptions{ + Type: CommentTypeCode, + Doer: doer, + Repo: repo, + Issue: issue, + Content: content, + LineNum: line, + TreePath: treePath, + CommitSHA: commitID, + ReviewID: reviewID, + Patch: patch, + }) +} + // CreateRefComment creates a commit reference comment to issue. func CreateRefComment(doer *User, repo *Repository, issue *Issue, content, commitSHA string) error { if len(commitSHA) == 0 { @@ -716,10 +905,11 @@ func GetCommentByID(id int64) (*Comment, error) { // FindCommentsOptions describes the conditions to Find comments type FindCommentsOptions struct { - RepoID int64 - IssueID int64 - Since int64 - Type CommentType + RepoID int64 + IssueID int64 + ReviewID int64 + Since int64 + Type CommentType } func (opts *FindCommentsOptions) toConds() builder.Cond { @@ -730,6 +920,9 @@ func (opts *FindCommentsOptions) toConds() builder.Cond { if opts.IssueID > 0 { cond = cond.And(builder.Eq{"comment.issue_id": opts.IssueID}) } + if opts.ReviewID > 0 { + cond = cond.And(builder.Eq{"comment.review_id": opts.ReviewID}) + } if opts.Since > 0 { cond = cond.And(builder.Gte{"comment.updated_unix": opts.Since}) } @@ -870,3 +1063,75 @@ func DeleteComment(doer *User, comment *Comment) error { return nil } + +// CodeComments represents comments on code by using this structure: FILENAME -> LINE (+ == proposed; - == previous) -> COMMENTS +type CodeComments map[string]map[int64][]*Comment + +func fetchCodeComments(e Engine, issue *Issue, currentUser *User) (CodeComments, error) { + return fetchCodeCommentsByReview(e, issue, currentUser, nil) +} + +func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review *Review) (CodeComments, error) { + pathToLineToComment := make(CodeComments) + if review == nil { + review = &Review{ID: 0} + } + //Find comments + opts := FindCommentsOptions{ + Type: CommentTypeCode, + IssueID: issue.ID, + ReviewID: review.ID, + } + conds := opts.toConds() + if review.ID == 0 { + conds.And(builder.Eq{"invalidated": false}) + } + + var comments []*Comment + if err := e.Where(conds). + Asc("comment.created_unix"). + Asc("comment.id"). + Find(&comments); err != nil { + return nil, err + } + + if err := issue.loadRepo(e); err != nil { + return nil, err + } + // Find all reviews by ReviewID + reviews := make(map[int64]*Review) + var ids = make([]int64, 0, len(comments)) + for _, comment := range comments { + if comment.ReviewID != 0 { + ids = append(ids, comment.ReviewID) + } + } + if err := e.In("id", ids).Find(&reviews); err != nil { + return nil, err + } + for _, comment := range comments { + if re, ok := reviews[comment.ReviewID]; ok && re != nil { + // If the review is pending only the author can see the comments (except the review is set) + if review.ID == 0 { + if re.Type == ReviewTypePending && + (currentUser == nil || currentUser.ID != re.ReviewerID) { + continue + } + } + comment.Review = re + } + + comment.RenderedContent = string(markdown.Render([]byte(comment.Content), issue.Repo.Link(), + issue.Repo.ComposeMetas())) + if pathToLineToComment[comment.TreePath] == nil { + pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment) + } + pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment) + } + return pathToLineToComment, nil +} + +// FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line +func FetchCodeComments(issue *Issue, currentUser *User) (CodeComments, error) { + return fetchCodeComments(x, issue, currentUser) +} diff --git a/models/issue_comment_test.go b/models/issue_comment_test.go index f6a6fbce9b..91dd5c1732 100644 --- a/models/issue_comment_test.go +++ b/models/issue_comment_test.go @@ -39,3 +39,21 @@ func TestCreateComment(t *testing.T) { updatedIssue := AssertExistsAndLoadBean(t, &Issue{ID: issue.ID}).(*Issue) AssertInt64InRange(t, now, then, int64(updatedIssue.UpdatedUnix)) } + +func TestFetchCodeComments(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + issue := AssertExistsAndLoadBean(t, &Issue{ID: 2}).(*Issue) + user := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User) + res, err := FetchCodeComments(issue, user) + assert.NoError(t, err) + assert.Contains(t, res, "README.md") + assert.Contains(t, res["README.md"], int64(4)) + assert.Len(t, res["README.md"][4], 1) + assert.Equal(t, int64(4), res["README.md"][4][0].ID) + + user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + res, err = FetchCodeComments(issue, user2) + assert.NoError(t, err) + assert.Len(t, res, 1) +} diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index b4e1a67187..15bb0723c0 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -194,8 +194,10 @@ var migrations = []Migration{ NewMigration("move team units to team_unit table", moveTeamUnitsToTeamUnitTable), // v70 -> v71 NewMigration("add issue_dependencies", addIssueDependencies), - // v70 -> v71 + // v71 -> v72 NewMigration("protect each scratch token", addScratchHash), + // v72 -> v73 + NewMigration("add review", addReview), } // Migrate database to current version diff --git a/models/migrations/v72.go b/models/migrations/v72.go new file mode 100644 index 0000000000..4924b41ceb --- /dev/null +++ b/models/migrations/v72.go @@ -0,0 +1,31 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "fmt" + + "code.gitea.io/gitea/modules/util" + + "github.com/go-xorm/xorm" +) + +func addReview(x *xorm.Engine) error { + // Review see models/review.go + type Review struct { + ID int64 `xorm:"pk autoincr"` + Type string + ReviewerID int64 `xorm:"index"` + IssueID int64 `xorm:"index"` + Content string + CreatedUnix util.TimeStamp `xorm:"INDEX created"` + UpdatedUnix util.TimeStamp `xorm:"INDEX updated"` + } + + if err := x.Sync2(new(Review)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + return nil +} diff --git a/models/models.go b/models/models.go index 9477b6950a..878e27e996 100644 --- a/models/models.go +++ b/models/models.go @@ -124,6 +124,7 @@ func init() { new(IssueAssignees), new(U2FRegistration), new(TeamUnit), + new(Review), ) gonicNames := []string{"SSL", "UID"} diff --git a/models/pull.go b/models/pull.go index 90b341907d..7faf1f1172 100644 --- a/models/pull.go +++ b/models/pull.go @@ -1075,10 +1075,7 @@ func (prs PullRequestList) loadAttributes(e Engine) error { } // Load issues. - issueIDs := make([]int64, 0, len(prs)) - for i := range prs { - issueIDs = append(issueIDs, prs[i].IssueID) - } + issueIDs := prs.getIssueIDs() issues := make([]*Issue, 0, len(issueIDs)) if err := e. Where("id > 0"). @@ -1097,11 +1094,44 @@ func (prs PullRequestList) loadAttributes(e Engine) error { return nil } +func (prs PullRequestList) getIssueIDs() []int64 { + issueIDs := make([]int64, 0, len(prs)) + for i := range prs { + issueIDs = append(issueIDs, prs[i].IssueID) + } + return issueIDs +} + // LoadAttributes load all the prs attributes func (prs PullRequestList) LoadAttributes() error { return prs.loadAttributes(x) } +func (prs PullRequestList) invalidateCodeComments(e Engine, doer *User, repo *git.Repository, branch string) error { + if len(prs) == 0 { + return nil + } + issueIDs := prs.getIssueIDs() + var codeComments []*Comment + if err := e. + Where("type = ? and invalidated = ?", CommentTypeCode, false). + In("issue_id", issueIDs). + Find(&codeComments); err != nil { + return fmt.Errorf("find code comments: %v", err) + } + for _, comment := range codeComments { + if err := comment.CheckInvalidation(repo, doer, branch); err != nil { + return err + } + } + return nil +} + +// InvalidateCodeComments will lookup the prs for code comments which got invalidated by change +func (prs PullRequestList) InvalidateCodeComments(doer *User, repo *git.Repository, branch string) error { + return prs.invalidateCodeComments(x, doer, repo, branch) +} + func addHeadRepoTasks(prs []*PullRequest) { for _, pr := range prs { log.Trace("addHeadRepoTasks[%d]: composing new test task", pr.ID) @@ -1128,10 +1158,13 @@ func AddTestPullRequestTask(doer *User, repoID int64, branch string, isSync bool } if isSync { - if err = PullRequestList(prs).LoadAttributes(); err != nil { + requests := PullRequestList(prs) + if err = requests.LoadAttributes(); err != nil { log.Error(4, "PullRequestList.LoadAttributes: %v", err) } - + if invalidationErr := checkForInvalidation(requests, repoID, doer, branch); invalidationErr != nil { + log.Error(4, "checkForInvalidation: %v", invalidationErr) + } if err == nil { for _, pr := range prs { pr.Issue.PullRequest = pr @@ -1152,6 +1185,7 @@ func AddTestPullRequestTask(doer *User, repoID int64, branch string, isSync bool go HookQueue.Add(pr.Issue.Repo.ID) } } + } addHeadRepoTasks(prs) @@ -1167,6 +1201,24 @@ func AddTestPullRequestTask(doer *User, repoID int64, branch string, isSync bool } } +func checkForInvalidation(requests PullRequestList, repoID int64, doer *User, branch string) error { + repo, err := GetRepositoryByID(repoID) + if err != nil { + return fmt.Errorf("GetRepositoryByID: %v", err) + } + gitRepo, err := git.OpenRepository(repo.RepoPath()) + if err != nil { + return fmt.Errorf("git.OpenRepository: %v", err) + } + go func() { + err := requests.InvalidateCodeComments(doer, gitRepo, branch) + if err != nil { + log.Error(4, "PullRequestList.InvalidateCodeComments: %v", err) + } + }() + return nil +} + // ChangeUsernameInPullRequests changes the name of head_user_name func ChangeUsernameInPullRequests(oldUserName, newUserName string) error { pr := PullRequest{ diff --git a/models/review.go b/models/review.go new file mode 100644 index 0000000000..3326ea0549 --- /dev/null +++ b/models/review.go @@ -0,0 +1,256 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "fmt" + + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" + "github.com/go-xorm/xorm" + + "github.com/go-xorm/builder" +) + +// ReviewType defines the sort of feedback a review gives +type ReviewType int + +// ReviewTypeUnknown unknown review type +const ReviewTypeUnknown ReviewType = -1 + +const ( + // ReviewTypePending is a review which is not published yet + ReviewTypePending ReviewType = iota + // ReviewTypeApprove approves changes + ReviewTypeApprove + // ReviewTypeComment gives general feedback + ReviewTypeComment + // ReviewTypeReject gives feedback blocking merge + ReviewTypeReject +) + +// Icon returns the corresponding icon for the review type +func (rt ReviewType) Icon() string { + switch rt { + case ReviewTypeApprove: + return "eye" + case ReviewTypeReject: + return "x" + default: + case ReviewTypeComment: + case ReviewTypeUnknown: + return "comment" + } + return "comment" +} + +// Review represents collection of code comments giving feedback for a PR +type Review struct { + ID int64 `xorm:"pk autoincr"` + Type ReviewType + Reviewer *User `xorm:"-"` + ReviewerID int64 `xorm:"index"` + Issue *Issue `xorm:"-"` + IssueID int64 `xorm:"index"` + Content string + + CreatedUnix util.TimeStamp `xorm:"INDEX created"` + UpdatedUnix util.TimeStamp `xorm:"INDEX updated"` + + // CodeComments are the initial code comments of the review + CodeComments CodeComments `xorm:"-"` +} + +func (r *Review) loadCodeComments(e Engine) (err error) { + r.CodeComments, err = fetchCodeCommentsByReview(e, r.Issue, nil, r) + return +} + +// LoadCodeComments loads CodeComments +func (r *Review) LoadCodeComments() error { + return r.loadCodeComments(x) +} + +func (r *Review) loadIssue(e Engine) (err error) { + r.Issue, err = getIssueByID(e, r.IssueID) + return +} + +func (r *Review) loadReviewer(e Engine) (err error) { + if r.ReviewerID == 0 { + return nil + } + r.Reviewer, err = getUserByID(e, r.ReviewerID) + return +} + +func (r *Review) loadAttributes(e Engine) (err error) { + if err = r.loadReviewer(e); err != nil { + return + } + if err = r.loadIssue(e); err != nil { + return + } + return +} + +// LoadAttributes loads all attributes except CodeComments +func (r *Review) LoadAttributes() error { + return r.loadAttributes(x) +} + +// Publish will send notifications / actions to participants for all code comments; parts are concurrent +func (r *Review) Publish() error { + return r.publish(x) +} + +func (r *Review) publish(e *xorm.Engine) error { + if r.Type == ReviewTypePending || r.Type == ReviewTypeUnknown { + return fmt.Errorf("review cannot be published if type is pending or unknown") + } + if r.Issue == nil { + if err := r.loadIssue(e); err != nil { + return err + } + } + if err := r.Issue.loadRepo(e); err != nil { + return err + } + if len(r.CodeComments) == 0 { + if err := r.loadCodeComments(e); err != nil { + return err + } + } + for _, lines := range r.CodeComments { + for _, comments := range lines { + for _, comment := range comments { + go func(en *xorm.Engine, review *Review, comm *Comment) { + sess := en.NewSession() + defer sess.Close() + if err := sendCreateCommentAction(sess, &CreateCommentOptions{ + Doer: comm.Poster, + Issue: review.Issue, + Repo: review.Issue.Repo, + Type: comm.Type, + Content: comm.Content, + }, comm); err != nil { + log.Warn("sendCreateCommentAction: %v", err) + } + }(e, r, comment) + } + } + } + return nil +} + +func getReviewByID(e Engine, id int64) (*Review, error) { + review := new(Review) + if has, err := e.ID(id).Get(review); err != nil { + return nil, err + } else if !has { + return nil, ErrReviewNotExist{ID: id} + } else { + return review, nil + } +} + +// GetReviewByID returns the review by the given ID +func GetReviewByID(id int64) (*Review, error) { + return getReviewByID(x, id) +} + +// FindReviewOptions represent possible filters to find reviews +type FindReviewOptions struct { + Type ReviewType + IssueID int64 + ReviewerID int64 +} + +func (opts *FindReviewOptions) toCond() builder.Cond { + var cond = builder.NewCond() + if opts.IssueID > 0 { + cond = cond.And(builder.Eq{"issue_id": opts.IssueID}) + } + if opts.ReviewerID > 0 { + cond = cond.And(builder.Eq{"reviewer_id": opts.ReviewerID}) + } + if opts.Type != ReviewTypeUnknown { + cond = cond.And(builder.Eq{"type": opts.Type}) + } + return cond +} + +func findReviews(e Engine, opts FindReviewOptions) ([]*Review, error) { + reviews := make([]*Review, 0, 10) + sess := e.Where(opts.toCond()) + return reviews, sess. + Asc("created_unix"). + Asc("id"). + Find(&reviews) +} + +// FindReviews returns reviews passing FindReviewOptions +func FindReviews(opts FindReviewOptions) ([]*Review, error) { + return findReviews(x, opts) +} + +// CreateReviewOptions represent the options to create a review. Type, Issue and Reviewer are required. +type CreateReviewOptions struct { + Content string + Type ReviewType + Issue *Issue + Reviewer *User +} + +func createReview(e Engine, opts CreateReviewOptions) (*Review, error) { + review := &Review{ + Type: opts.Type, + Issue: opts.Issue, + IssueID: opts.Issue.ID, + Reviewer: opts.Reviewer, + ReviewerID: opts.Reviewer.ID, + Content: opts.Content, + } + if _, err := e.Insert(review); err != nil { + return nil, err + } + return review, nil +} + +// CreateReview creates a new review based on opts +func CreateReview(opts CreateReviewOptions) (*Review, error) { + return createReview(x, opts) +} + +func getCurrentReview(e Engine, reviewer *User, issue *Issue) (*Review, error) { + if reviewer == nil { + return nil, nil + } + reviews, err := findReviews(e, FindReviewOptions{ + Type: ReviewTypePending, + IssueID: issue.ID, + ReviewerID: reviewer.ID, + }) + if err != nil { + return nil, err + } + if len(reviews) == 0 { + return nil, ErrReviewNotExist{} + } + return reviews[0], nil +} + +// GetCurrentReview returns the current pending review of reviewer for given issue +func GetCurrentReview(reviewer *User, issue *Issue) (*Review, error) { + return getCurrentReview(x, reviewer, issue) +} + +// UpdateReview will update all cols of the given review in db +func UpdateReview(r *Review) error { + if _, err := x.ID(r.ID).AllCols().Update(r); err != nil { + return err + } + return nil +} diff --git a/models/review_test.go b/models/review_test.go new file mode 100644 index 0000000000..3c0444e7a3 --- /dev/null +++ b/models/review_test.go @@ -0,0 +1,107 @@ +package models + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetReviewByID(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + review, err := GetReviewByID(1) + assert.NoError(t, err) + assert.Equal(t, "Demo Review", review.Content) + assert.Equal(t, ReviewTypeApprove, review.Type) + + _, err = GetReviewByID(23892) + assert.Error(t, err) + assert.True(t, IsErrReviewNotExist(err), "IsErrReviewNotExist") +} + +func TestReview_LoadAttributes(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + review := AssertExistsAndLoadBean(t, &Review{ID: 1}).(*Review) + assert.NoError(t, review.LoadAttributes()) + assert.NotNil(t, review.Issue) + assert.NotNil(t, review.Reviewer) + + invalidReview1 := AssertExistsAndLoadBean(t, &Review{ID: 2}).(*Review) + assert.Error(t, invalidReview1.LoadAttributes()) + + invalidReview2 := AssertExistsAndLoadBean(t, &Review{ID: 3}).(*Review) + assert.Error(t, invalidReview2.LoadAttributes()) + +} + +func TestReview_LoadCodeComments(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + review := AssertExistsAndLoadBean(t, &Review{ID: 4}).(*Review) + assert.NoError(t, review.LoadAttributes()) + assert.NoError(t, review.LoadCodeComments()) + assert.Len(t, review.CodeComments, 1) + assert.Equal(t, int64(4), review.CodeComments["README.md"][int64(4)][0].Line) +} + +func TestReviewType_Icon(t *testing.T) { + assert.Equal(t, "eye", ReviewTypeApprove.Icon()) + assert.Equal(t, "x", ReviewTypeReject.Icon()) + assert.Equal(t, "comment", ReviewTypeComment.Icon()) + assert.Equal(t, "comment", ReviewTypeUnknown.Icon()) + assert.Equal(t, "comment", ReviewType(4).Icon()) +} + +func TestFindReviews(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + reviews, err := FindReviews(FindReviewOptions{ + Type: ReviewTypeApprove, + IssueID: 2, + ReviewerID: 1, + }) + assert.NoError(t, err) + assert.Len(t, reviews, 1) + assert.Equal(t, "Demo Review", reviews[0].Content) +} + +func TestGetCurrentReview(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + issue := AssertExistsAndLoadBean(t, &Issue{ID: 2}).(*Issue) + user := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User) + + review, err := GetCurrentReview(user, issue) + assert.NoError(t, err) + assert.NotNil(t, review) + assert.Equal(t, ReviewTypePending, review.Type) + assert.Equal(t, "Pending Review", review.Content) + + user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + review2, err := GetCurrentReview(user2, issue) + assert.Error(t, err) + assert.True(t, IsErrReviewNotExist(err)) + assert.Nil(t, review2) +} + +func TestCreateReview(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + issue := AssertExistsAndLoadBean(t, &Issue{ID: 2}).(*Issue) + user := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User) + + review, err := CreateReview(CreateReviewOptions{ + Content: "New Review", + Type: ReviewTypePending, + Issue: issue, + Reviewer: user, + }) + assert.NoError(t, err) + assert.Equal(t, "New Review", review.Content) + AssertExistsAndLoadBean(t, &Review{Content: "New Review"}) +} + +func TestUpdateReview(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + review := AssertExistsAndLoadBean(t, &Review{ID: 1}).(*Review) + review.Content = "Updated Review" + assert.NoError(t, UpdateReview(review)) + AssertExistsAndLoadBean(t, &Review{ID: 1, Content: "Updated Review"}) +} |