diff --git a/models/issue.go b/models/issue.go index 6de7c3204..7aab8b939 100644 --- a/models/issue.go +++ b/models/issue.go @@ -824,14 +824,25 @@ func (issue *Issue) UpdateAttachments(uuids []string) (err error) { // ChangeContent changes issue content, as the given user. func (issue *Issue) ChangeContent(doer *User, content string) (err error) { - issue.Content = content - ctx, committer, err := db.TxContext() if err != nil { return err } defer committer.Close() + hasContentHistory, err := issues.HasIssueContentHistory(ctx, issue.ID, 0) + if err != nil { + return fmt.Errorf("HasIssueContentHistory: %v", err) + } + if !hasContentHistory { + if err = issues.SaveIssueContentHistory(db.GetEngine(ctx), issue.PosterID, issue.ID, 0, + issue.CreatedUnix, issue.Content, true); err != nil { + return fmt.Errorf("SaveIssueContentHistory: %v", err) + } + } + + issue.Content = content + if err = updateIssueCols(db.GetEngine(ctx), issue, "content"); err != nil { return fmt.Errorf("UpdateIssueCols: %v", err) } @@ -1012,11 +1023,6 @@ func newIssue(ctx context.Context, doer *User, opts NewIssueOptions) (err error) return err } - if err = issues.SaveIssueContentHistory(e, doer.ID, opts.Issue.ID, 0, - timeutil.TimeStampNow(), opts.Issue.Content, true); err != nil { - return err - } - return opts.Issue.addCrossReferences(ctx, doer, false) } diff --git a/models/issues/content_history.go b/models/issues/content_history.go index 04c30fde9..b423fb8fc 100644 --- a/models/issues/content_history.go +++ b/models/issues/content_history.go @@ -75,7 +75,7 @@ func keepLimitedContentHistory(e db.Engine, issueID, commentID int64, limit int) log.Error("can not query content history for deletion, err=%v", err) return } - if len(res) <= 1 { + if len(res) <= 2 { return } @@ -83,8 +83,8 @@ func keepLimitedContentHistory(e db.Engine, issueID, commentID int64, limit int) for outDatedCount > 0 { var indexToDelete int minEditedInterval := -1 - // find a history revision with minimal edited interval to delete - for i := 1; i < len(res); i++ { + // find a history revision with minimal edited interval to delete, the first and the last should never be deleted + for i := 1; i < len(res)-1; i++ { editedInterval := int(res[i].EditedUnix - res[i-1].EditedUnix) if minEditedInterval == -1 || editedInterval < minEditedInterval { minEditedInterval = editedInterval @@ -167,7 +167,20 @@ func FetchIssueContentHistoryList(dbCtx context.Context, issueID int64, commentI return res, nil } -//SoftDeleteIssueContentHistory soft delete +// HasIssueContentHistory check if a ContentHistory entry exists +func HasIssueContentHistory(dbCtx context.Context, issueID int64, commentID int64) (bool, error) { + exists, err := db.GetEngine(dbCtx).Cols("id").Exist(&ContentHistory{ + IssueID: issueID, + CommentID: commentID, + }) + if err != nil { + log.Error("can not fetch issue content history. err=%v", err) + return false, err + } + return exists, err +} + +// SoftDeleteIssueContentHistory soft delete func SoftDeleteIssueContentHistory(dbCtx context.Context, historyID int64) error { if _, err := db.GetEngine(dbCtx).ID(historyID).Cols("is_deleted", "content_text").Update(&ContentHistory{ IsDeleted: true, diff --git a/models/issues/content_history_test.go b/models/issues/content_history_test.go index f040a7dc5..cc9a7c510 100644 --- a/models/issues/content_history_test.go +++ b/models/issues/content_history_test.go @@ -53,6 +53,11 @@ func TestContentHistory(t *testing.T) { list2, _ := FetchIssueContentHistoryList(dbCtx, 10, 100) assert.Len(t, list2, 5) + hasHistory1, _ := HasIssueContentHistory(dbCtx, 10, 0) + assert.True(t, hasHistory1) + hasHistory2, _ := HasIssueContentHistory(dbCtx, 10, 1) + assert.False(t, hasHistory2) + h6, h6Prev, _ := GetIssueContentHistoryAndPrev(dbCtx, 6) assert.EqualValues(t, 6, h6.ID) assert.EqualValues(t, 5, h6Prev.ID) @@ -63,13 +68,13 @@ func TestContentHistory(t *testing.T) { assert.EqualValues(t, 6, h6.ID) assert.EqualValues(t, 4, h6Prev.ID) - // only keep 3 history revisions for comment_id=100 + // only keep 3 history revisions for comment_id=100, the first and the last should never be deleted keepLimitedContentHistory(dbEngine, 10, 100, 3) list1, _ = FetchIssueContentHistoryList(dbCtx, 10, 0) assert.Len(t, list1, 3) list2, _ = FetchIssueContentHistoryList(dbCtx, 10, 100) assert.Len(t, list2, 3) - assert.EqualValues(t, 7, list2[0].HistoryID) - assert.EqualValues(t, 6, list2[1].HistoryID) + assert.EqualValues(t, 8, list2[0].HistoryID) + assert.EqualValues(t, 7, list2[1].HistoryID) assert.EqualValues(t, 4, list2[2].HistoryID) } diff --git a/services/comments/comments.go b/services/comments/comments.go index d1e5ea4d8..2477f2d2b 100644 --- a/services/comments/comments.go +++ b/services/comments/comments.go @@ -25,10 +25,6 @@ func CreateIssueComment(doer *models.User, repo *models.Repository, issue *model if err != nil { return nil, err } - err = issues.SaveIssueContentHistory(db.GetEngine(db.DefaultContext), doer.ID, issue.ID, comment.ID, timeutil.TimeStampNow(), comment.Content, true) - if err != nil { - return nil, err - } mentions, err := issue.FindAndUpdateIssueMentions(db.DefaultContext, doer, comment.Content) if err != nil { @@ -42,11 +38,26 @@ func CreateIssueComment(doer *models.User, repo *models.Repository, issue *model // UpdateComment updates information of comment. func UpdateComment(c *models.Comment, doer *models.User, oldContent string) error { + var needsContentHistory = c.Content != oldContent && + (c.Type == models.CommentTypeComment || c.Type == models.CommentTypeReview || c.Type == models.CommentTypeCode) + if needsContentHistory { + hasContentHistory, err := issues.HasIssueContentHistory(db.DefaultContext, c.IssueID, c.ID) + if err != nil { + return err + } + if !hasContentHistory { + if err = issues.SaveIssueContentHistory(db.GetEngine(db.DefaultContext), c.PosterID, c.IssueID, c.ID, + c.CreatedUnix, oldContent, true); err != nil { + return err + } + } + } + if err := models.UpdateComment(c, doer); err != nil { return err } - if c.Type == models.CommentTypeComment && c.Content != oldContent { + if needsContentHistory { err := issues.SaveIssueContentHistory(db.GetEngine(db.DefaultContext), doer.ID, c.IssueID, c.ID, timeutil.TimeStampNow(), c.Content, false) if err != nil { return err diff --git a/templates/repo/pulls/files.tmpl b/templates/repo/pulls/files.tmpl index 786669762..ede487d31 100644 --- a/templates/repo/pulls/files.tmpl +++ b/templates/repo/pulls/files.tmpl @@ -1,4 +1,8 @@ {{template "base/head" .}} + + + +